[ros-diffs] [ion] 22650: - ObpCreateUnnamedHandle/ObpCreateHandle => Reference the object before calling ExCreateHandle. - Fix two critical bugs in ObInsertObject: We were creating a handle for the wrong object (in ObInsertObject) and we were not passing the ReferencedObject parameter to ObpCreateHandle, so that object was never being returned properly to the caller. - ObfDereferenceObject shouldn't check for the OB_FLAG_PERMANENT flag, or else it would never be possible to kill permanent objects while in k

Author: ion
Date: Tue Jun 27 05:16:17 2006
New Revision: 22650

URL: http://svn.reactos.org/svn/reactos?rev=22650&view=rev
Log:
- ObpCreateUnnamedHandle/ObpCreateHandle => Reference the object before calling
ExCreateHandle.
- Fix two critical bugs in ObInsertObject: We were creating a handle for the wrong object
(in ObInsertObject) and we were not passing the ReferencedObject parameter to
ObpCreateHandle, so that object was never being returned properly to the caller.
- ObfDereferenceObject shouldn't check for the OB_FLAG_PERMANENT flag, or else it
would never be possible to kill permanent objects while in kernel mode (permanent objects
only apply to user-mode handles).

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

Modified: trunk/reactos/ntoskrnl/ob/obhandle.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/obhandle.c?rev…
==============================================================================
--- trunk/reactos/ntoskrnl/ob/obhandle.c (original)
+++ trunk/reactos/ntoskrnl/ob/obhandle.c Tue Jun 27 05:16:17 2006
@@ -520,6 +520,7 @@
     BOOLEAN AttachedToProcess = FALSE;
     PVOID HandleTable;
     NTSTATUS Status;
+    ULONG i;
     PAGED_CODE();
 
     /* Get the object header and type */
@@ -579,6 +580,18 @@
 
     /* Save the access mask */
     NewEntry.GrantedAccess = DesiredAccess;
+
+    /* Handle extra references */
+    if (AdditionalReferences)
+    {
+        /* Make a copy in case we fail later below */
+        i = AdditionalReferences;
+        while (i--)
+        {
+            /* Increment the count */
+            InterlockedIncrement(&ObjectHeader->PointerCount);
+        }
+    }
 
     /*
      * Create the actual handle. We'll need to do this *after* calling
@@ -597,13 +610,6 @@
     /* Make sure 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)
         {
@@ -615,12 +621,19 @@
         *ReturnedHandle = Handle;
         if (ReturnedObject) *ReturnedObject = Object;
         OBTRACE(OB_HANDLE_DEBUG,
-                "%s %s - Returning Handle: %lx HC LC %lx %lx\n",
+                "%s - Returning Handle: %lx HC LC %lx %lx\n",
                 __FUNCTION__,
                 Handle,
                 ObjectHeader->HandleCount,
                 ObjectHeader->PointerCount);
         return STATUS_SUCCESS;
+    }
+
+    /* Handle extra references */
+    while (AdditionalReferences--)
+    {
+        /* Decrement the count */
+        InterlockedDecrement(&ObjectHeader->PointerCount);
     }
 
     /* Decrement the handle count and detach */
@@ -692,6 +705,7 @@
     POBJECT_TYPE ObjectType;
     PVOID HandleTable;
     NTSTATUS Status;
+    ULONG i;
     PAGED_CODE();
 
     /* Get the object header and type */
@@ -764,6 +778,18 @@
     NewEntry.GrantedAccess = AccessState->RemainingDesiredAccess |
                              AccessState->PreviouslyGrantedAccess;
 
+    /* Handle extra references */
+    if (AdditionalReferences)
+    {
+        /* Make a copy in case we fail later below */
+        i = AdditionalReferences;
+        while (i--)
+        {
+            /* Increment the count */
+            InterlockedIncrement(&ObjectHeader->PointerCount);
+        }
+    }
+
     /*
      * Create the actual handle. We'll need to do this *after* calling
      * ObpIncrementHandleCount to make sure that Object Security is valid
@@ -781,13 +807,6 @@
     /* Make sure 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)
         {
@@ -805,6 +824,13 @@
                 ObjectHeader->HandleCount,
                 ObjectHeader->PointerCount);
         return STATUS_SUCCESS;
+    }
+
+    /* Handle extra references */
+    while (AdditionalReferences--)
+    {
+        /* Increment the count */
+        InterlockedDecrement(&ObjectHeader->PointerCount);
     }
 
     /* Decrement the handle count and detach */
@@ -1052,7 +1078,7 @@
 
     /* Make sure that the handle is inheritable */
     Ret = (HandleTableEntry->ObAttributes & EX_HANDLE_ENTRY_INHERITABLE) != 0;
-    if(Ret)
+    if (Ret)
     {
         /* Get the object header */
         ObjectHeader = EX_HTE_TO_HDR(HandleTableEntry);
@@ -1519,9 +1545,10 @@
     ObpReleaseCapturedAttributes(&ObjectCreateInfo);
     if (ObjectName.Buffer) ObpReleaseCapturedName(&ObjectName);
     OBTRACE(OB_HANDLE_DEBUG,
-            "%s returning Object with PC S: %lx %lx\n",
+            "%s - returning Object %p with PC S: %lx %lx\n",
             __FUNCTION__,
-            OBJECT_TO_OBJECT_HEADER(Object)->PointerCount,
+            Object,
+            Object ? OBJECT_TO_OBJECT_HEADER(Object)->PointerCount : -1,
             Status);
     return Status;
 }
@@ -1722,7 +1749,7 @@
         Header->ObjectCreateInfo = NULL;
 
         /* Remove the extra keep-alive reference */
-        //ObDereferenceObject(Object); FIXME: Will require massive changes
+        //ObDereferenceObject(Object); // FIXME: Needs sync changes
 
         /* Return */
         return Status;
@@ -1853,6 +1880,7 @@
         if (!NT_SUCCESS(Status))
         {
             /* We failed, dereference the object and delete the access state */
+            KEBUGCHECK(0);
             ObDereferenceObject(Object);
             if (PassedAccessState == &AccessState)
             {
@@ -1875,13 +1903,13 @@
     {
         /* Create the handle */
         Status = ObpCreateHandle(OpenReason,
-                                 &Header->Body,
+                                 FoundObject,
                                  NULL,
                                  PassedAccessState,
                                  AdditionalReferences + 1,
                                  ObjectCreateInfo->Attributes,
                                  ExGetPreviousMode(),
-                                 NULL,
+                                 ReferencedObject,
                                  Handle);
     }
 
@@ -1897,7 +1925,7 @@
     }
 
     /* Remove the extra keep-alive reference */
-    //ObDereferenceObject(Object); FIXME: Will require massive changes
+    //ObDereferenceObject(Object);
 
     /* Check if we created our own access state */
     if (PassedAccessState == &AccessState)

Modified: trunk/reactos/ntoskrnl/ob/obinit.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/obinit.c?rev=2…
==============================================================================
--- trunk/reactos/ntoskrnl/ob/obinit.c (original)
+++ trunk/reactos/ntoskrnl/ob/obinit.c Tue Jun 27 05:16:17 2006
@@ -38,7 +38,7 @@
 };
 
 PDEVICE_MAP ObSystemDeviceMap = NULL;
-ULONG ObpTraceLevel = OB_NAMESPACE_DEBUG;
+ULONG ObpTraceLevel = OB_HANDLE_DEBUG | OB_REFERENCE_DEBUG;
 
 /* PRIVATE FUNCTIONS *********************************************************/
 

Modified: trunk/reactos/ntoskrnl/ob/obref.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/obref.c?rev=22…
==============================================================================
--- trunk/reactos/ntoskrnl/ob/obref.c (original)
+++ trunk/reactos/ntoskrnl/ob/obref.c Tue Jun 27 05:16:17 2006
@@ -89,8 +89,7 @@
     Header = OBJECT_TO_OBJECT_HEADER(Object);
 
     /* Check whether the object can now be deleted. */
-    if (!(InterlockedDecrement(&Header->PointerCount)) &&
-        !(Header->Flags & OB_FLAG_PERMANENT))
+    if (!(InterlockedDecrement(&Header->PointerCount)))
     {
         /* Sanity check */
         ASSERT(!Header->HandleCount);