[ros-diffs] [ion] 22267: - Final fixes to ObpCreateHandle that I can think of before splitting Create/Increment (might be lacking some security checks or trace/database functionality, but I haven't found any info on the latter, while the former I will stub, but I don't have the skills to imlement (ObAssignObjectSecurity)): * Honour ObjectType passed to the function and fail if it doesn't match. * Use table-based logic instead of process-based logic for Kernel vs User-mode handles (same change that's been do

Author: ion
Date: Wed Jun  7 10:15:59 2006
New Revision: 22267

URL: http://svn.reactos.ru/svn/reactos?rev=22267&view=rev
Log:
- Final fixes to ObpCreateHandle that I can think of before splitting Create/Increment
(might be lacking some security checks or trace/database functionality, but I haven't
found any info on the latter, while the former I will stub, but I don't have the
skills to imlement (ObAssignObjectSecurity)):
  * Honour ObjectType passed to the function and fail if it doesn't match.
  * Use table-based logic instead of process-based logic for Kernel vs User-mode handles
(same change that's been done all over the place, since it requires only one
dereference of the process object).
  * Do the GENERIC/MAXIMUM_ALLOWED logic directly inside the ACCESS_STATE structure.
  * This is where we should call the OpenProcedure (acc. to Gl00my), but this kills win32k
-- investigate, #ifed out for now.
  * Increase the object type's number of handles as well.
  * Set the handle table entry's ObAttributes correctly; the old code seems to have
been messing that up.
  * Honour the AdditionalReferences parameter and do referencing bias if requested.
  * Honour the ReturnedObject parameter to return the object pointer back to the caller.
  * Add OBTRACEing to the function.
  * If we failed because a handle couldn't be allocated, use the distinguied
STATUS_INSUFFICIENT_RESOURCES error code instead of the generic STATUS_UNSCUCESFFUL, and
backout all the changes we made by calling ObpDecrementHandleCount.

Modified:
    trunk/reactos/ntoskrnl/ob/obhandle.c

Modified: trunk/reactos/ntoskrnl/ob/obhandle.c
URL:
http://svn.reactos.ru/svn/reactos/trunk/reactos/ntoskrnl/ob/obhandle.c?rev=…
==============================================================================
--- trunk/reactos/ntoskrnl/ob/obhandle.c (original)
+++ trunk/reactos/ntoskrnl/ob/obhandle.c Wed Jun  7 10:15:59 2006
@@ -359,7 +359,7 @@
                                               // ObOpenHandle == 1, I'm guessing this
is actually the
                                               // OpenReason. Also makes sense since this
function is shared
                                               // by Duplication, Creation and Opening.
-                IN PVOID ObjectBody,
+                IN PVOID Object,
                 IN POBJECT_TYPE Type OPTIONAL,
                 IN PACCESS_STATE AccessState,
                 IN ULONG AdditionalReferences,
@@ -369,92 +369,162 @@
                 OUT PHANDLE ReturnedHandle)
 {
     HANDLE_TABLE_ENTRY NewEntry;
-    PEPROCESS Process, CurrentProcess;
+    PVOID HandleTable;
     POBJECT_HEADER ObjectHeader;
+    POBJECT_TYPE ObjectType;
     HANDLE Handle;
     KAPC_STATE ApcState;
     BOOLEAN AttachedToProcess = FALSE;
-    ACCESS_MASK GrantedAccess;
-
-    PAGED_CODE();
-
-    DPRINT("ObpCreateHandle(obj %p)\n",ObjectBody);
-
-    ASSERT(ObjectBody);
-
-    CurrentProcess = PsGetCurrentProcess();
-
-    ObjectHeader = OBJECT_TO_OBJECT_HEADER(ObjectBody);
-
-    /* check that this is a valid kernel pointer */
+
+    /* Get the object header and type */
+    ObjectHeader = OBJECT_TO_OBJECT_HEADER(Object);
+    ObjectType = ObjectHeader->Type;
+    OBTRACE("OBTRACE - %s - Creating handle for: %p. Reason: %lx. HC LC %lx
%lx\n",
+            __FUNCTION__,
+            Object,
+            OpenReason,
+            ObjectHeader->HandleCount,
+            ObjectHeader->PointerCount);
+
+    /* Check if the types match */
+    if ((Type) && (ObjectType != Type))
+    {
+        /* They don't; fail */
+        DPRINT1("Type mismatch: %wZ, %wZ\n", &ObjectType->Name,
&Type->Name);
+        return STATUS_OBJECT_TYPE_MISMATCH;
+    }
+
+    /* Check if this is a kernel handle */
+    if ((HandleAttributes & OBJ_KERNEL_HANDLE) && (AccessMode ==
KernelMode))
+    {
+        /* Set the handle table */
+        HandleTable = ObpKernelHandleTable;
+
+        /* Check if we're not in the system process */
+        if (PsGetCurrentProcess() != PsInitialSystemProcess)
+        {
+            /* Attach to the system process */
+            KeStackAttachProcess(&PsInitialSystemProcess->Pcb, &ApcState);
+            AttachedToProcess = TRUE;
+        }
+    }
+    else
+    {
+        /* Get the current handle table */
+        HandleTable = PsGetCurrentProcess()->ObjectTable;
+    }
+
+    /* Convert MAXIMUM_ALLOWED to GENERIC_ALL */
+    if (AccessState->RemainingDesiredAccess & MAXIMUM_ALLOWED)
+    {
+        /* Mask out MAXIMUM_ALLOWED and stick GENERIC_ALL instead */
+        AccessState->RemainingDesiredAccess &= ~MAXIMUM_ALLOWED;
+        AccessState->RemainingDesiredAccess |= GENERIC_ALL;
+    }
+
+    /* Check if we have to map the GENERIC mask */
+    if (AccessState->RemainingDesiredAccess & GENERIC_ACCESS)
+    {
+        /* Map it to the correct access masks */
+        RtlMapGenericMask(&AccessState->RemainingDesiredAccess,
+                          &ObjectType->TypeInfo.GenericMapping);
+    }
+
+    /* Increase the handle count */
+    if(InterlockedIncrement(&ObjectHeader->HandleCount) == 1)
+    {
+        /*
+         * FIXME: Is really needed? Perhaps we should instead take
+         * advantage of the AddtionalReferences parameter to add the
+         * bias when required. This might be the source of the mysterious
+         * ReactOS bug where ObInsertObject *requires* an immediate dereference
+         * even in a success case.
+         * Whill have to think more about this when doing the Increment/Create
+         * split later.
+         */
+        ObReferenceObject(Object);
+    }
+
+    /* Check if we have an open procedure */
+#if 0
+    if (ObjectType->TypeInfo.OpenProcedure)
+    {
+        /* Call it */
+        ObjectType->TypeInfo.OpenProcedure(OpenReason,
+                                           PsGetCurrentProcess(),
+                                           Object,
+                                           AccessState->
+                                           PreviouslyGrantedAccess,
+                                           0);
+    }
+#endif
+
+    /* Increase total number of handles */
+    ObjectType->TotalNumberOfHandles++;
+
+    /* Save the object header (assert its validity too) */
     ASSERT((ULONG_PTR)ObjectHeader & EX_HANDLE_ENTRY_LOCKED);
-
-    GrantedAccess = AccessState->RemainingDesiredAccess |
-                    AccessState->PreviouslyGrantedAccess;
-    if (GrantedAccess & MAXIMUM_ALLOWED)
-    {
-        GrantedAccess &= ~MAXIMUM_ALLOWED;
-        GrantedAccess |= GENERIC_ALL;
-    }
-
-    if (GrantedAccess & GENERIC_ACCESS)
-    {
-        RtlMapGenericMask(&GrantedAccess,
-            &ObjectHeader->Type->TypeInfo.GenericMapping);
-    }
-
     NewEntry.Object = ObjectHeader;
-    if(HandleAttributes & OBJ_INHERIT)
-        NewEntry.ObAttributes |= EX_HANDLE_ENTRY_INHERITABLE;
-    else
-        NewEntry.ObAttributes &= ~EX_HANDLE_ENTRY_INHERITABLE;
-    NewEntry.GrantedAccess = GrantedAccess;
-
-    if ((HandleAttributes & OBJ_KERNEL_HANDLE) &&
-        ExGetPreviousMode == KernelMode)
-    {
-        Process = PsInitialSystemProcess;
-        if (Process != CurrentProcess)
-        {
-            KeStackAttachProcess(&Process->Pcb,
-                &ApcState);
-            AttachedToProcess = TRUE;
-        }
-    }
-    else
-    {
-        Process = CurrentProcess;
-        /* mask out the OBJ_KERNEL_HANDLE attribute */
-        HandleAttributes &= ~OBJ_KERNEL_HANDLE;
-    }
-
-    Handle = ExCreateHandle(Process->ObjectTable,
-        &NewEntry);
-
-    if (AttachedToProcess)
-    {
-        KeUnstackDetachProcess(&ApcState);
-    }
-
-    if(Handle != NULL)
-    {
+
+    /* Mask out the internal attributes */
+    NewEntry.ObAttributes |= HandleAttributes &
+                             (EX_HANDLE_ENTRY_PROTECTFROMCLOSE |
+                              EX_HANDLE_ENTRY_INHERITABLE |
+                              EX_HANDLE_ENTRY_AUDITONCLOSE);
+
+     /* Save the access mask */
+    NewEntry.GrantedAccess = AccessState->RemainingDesiredAccess |
+                             AccessState->PreviouslyGrantedAccess;
+
+    /*
+     * Create the actual handle. We'll need to do this *after* calling
+     * ObpIncrementHandleCount to make sure that Object Security is valid
+     * (specified in Gl00my documentation on Ob)
+     */
+    OBTRACE("OBTRACE - %s - Handle Properties: [%p-%lx-%lx]\n",
+            __FUNCTION__,
+            NewEntry.Object, NewEntry.ObAttributes & 3, NewEntry.GrantedAccess);
+    Handle = ExCreateHandle(HandleTable, &NewEntry);
+
+    /* Detach it needed */
+    if (AttachedToProcess) KeUnstackDetachProcess(&ApcState);
+
+    /* Check if we got a handle */
+    if(Handle)
+    {
+        /* Handle extra references */
+        while (AdditionalReferences--)
+        {
+            /* Increment the count */
+            InterlockedIncrement(&ObjectHeader->PointerCount);
+        }
+
+        /* Check if this was a kernel handle */
         if (HandleAttributes & OBJ_KERNEL_HANDLE)
         {
-            /* mark the handle value */
+            /* Set the kernel handle bit */
             Handle = ObMarkHandleAsKernelHandle(Handle);
         }
 
-        if(InterlockedIncrement(&ObjectHeader->HandleCount) == 1)
-        {
-            ObReferenceObject(ObjectBody);
-        }
-
+        /* Return handle and object */
         *ReturnedHandle = Handle;
-
+        if (ReturnedObject) *ReturnedObject = Object;
+
+        /* Return success */
+        OBTRACE("OBTRACE - %s - Incremented count for: %p. Reason: %lx HC LC %lx
%lx\n",
+                __FUNCTION__,
+                Object,
+                OpenReason,
+                ObjectHeader->HandleCount,
+                ObjectHeader->PointerCount);
         return STATUS_SUCCESS;
     }
 
-    return STATUS_UNSUCCESSFUL;
+    /* Decrement the handle count and fail */
+    ObpDecrementHandleCount(&ObjectHeader->Body,
+                            PsGetCurrentProcess(),
+                            NewEntry.GrantedAccess);
+    return STATUS_INSUFFICIENT_RESOURCES;
 }
 
 /*++