[ros-diffs] [ion] 25407: - Fix a bug in ExfWakePushLock. - Implement object directory locking to avoid race conditions in Ob and enable most of the query referencing code.

ion at svn.reactos.org ion at svn.reactos.org
Wed Jan 10 02:00:46 CET 2007


Author: ion
Date: Wed Jan 10 04:00:46 2007
New Revision: 25407

URL: http://svn.reactos.org/svn/reactos?rev=25407&view=rev
Log:
- Fix a bug in ExfWakePushLock.
- Implement object directory locking to avoid race conditions in Ob and enable most of the query referencing code.

Modified:
    trunk/reactos/ntoskrnl/ex/pushlock.c
    trunk/reactos/ntoskrnl/include/internal/ob_x.h
    trunk/reactos/ntoskrnl/ob/obdir.c
    trunk/reactos/ntoskrnl/ob/obhandle.c
    trunk/reactos/ntoskrnl/ob/obinit.c
    trunk/reactos/ntoskrnl/ob/oblife.c
    trunk/reactos/ntoskrnl/ob/obname.c
    trunk/reactos/ntoskrnl/ob/obref.c

Modified: trunk/reactos/ntoskrnl/ex/pushlock.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ex/pushlock.c?rev=25407&r1=25406&r2=25407&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/ex/pushlock.c (original)
+++ trunk/reactos/ntoskrnl/ex/pushlock.c Wed Jan 10 04:00:46 2007
@@ -97,7 +97,7 @@
                 OldValue = NewValue;
 
                 /* Check if it's still locked */
-                if (OldValue.Locked) continue;
+                if (!OldValue.Locked) break;
             }
         }
 

Modified: trunk/reactos/ntoskrnl/include/internal/ob_x.h
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/ob_x.h?rev=25407&r1=25406&r2=25407&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/include/internal/ob_x.h (original)
+++ trunk/reactos/ntoskrnl/include/internal/ob_x.h Wed Jan 10 04:00:46 2007
@@ -51,7 +51,7 @@
 
 VOID
 FORCEINLINE
-_ObpDecrementQueryReference(IN POBJECT_HEADER_NAME_INFO HeaderNameInfo)
+ObpDecrementQueryReference(IN POBJECT_HEADER_NAME_INFO HeaderNameInfo)
 {
     POBJECT_DIRECTORY Directory;
 
@@ -79,7 +79,7 @@
 
 VOID
 FORCEINLINE
-_ObpAcquireDirectoryLockShared(IN POBJECT_DIRECTORY Directory,
+ObpAcquireDirectoryLockShared(IN POBJECT_DIRECTORY Directory,
                                IN POBP_LOOKUP_CONTEXT Context)
 {
     /* It's not, set lock flag */
@@ -95,7 +95,7 @@
 
 VOID
 FORCEINLINE
-_ObpAcquireDirectoryLockExclusive(IN POBJECT_DIRECTORY Directory,
+ObpAcquireDirectoryLockExclusive(IN POBJECT_DIRECTORY Directory,
                                   IN POBP_LOOKUP_CONTEXT Context)
 {
     /* Update lock flag */
@@ -115,7 +115,7 @@
 
 VOID
 FORCEINLINE
-_ObpReleaseDirectoryLock(IN POBJECT_DIRECTORY Directory,
+ObpReleaseDirectoryLock(IN POBJECT_DIRECTORY Directory,
                          IN POBP_LOOKUP_CONTEXT Context)
 {
     /* Release the lock */
@@ -126,7 +126,7 @@
 
 VOID
 FORCEINLINE
-_ObpInitializeDirectoryLookup(IN POBP_LOOKUP_CONTEXT Context)
+ObpInitializeDirectoryLookup(IN POBP_LOOKUP_CONTEXT Context)
 {
     /* Initialize a null context */
     Context->Object = NULL;
@@ -137,7 +137,7 @@
 
 VOID
 FORCEINLINE
-_ObpReleaseLookupContextObject(IN POBP_LOOKUP_CONTEXT Context)
+ObpReleaseLookupContextObject(IN POBP_LOOKUP_CONTEXT Context)
 {
     POBJECT_HEADER ObjectHeader;
     POBJECT_HEADER_NAME_INFO HeaderNameInfo;
@@ -150,7 +150,7 @@
         HeaderNameInfo = OBJECT_HEADER_TO_NAME_INFO(ObjectHeader);
 
         /* Check if we do have name information */
-        if (HeaderNameInfo) _ObpDecrementQueryReference(HeaderNameInfo);
+        if (HeaderNameInfo) ObpDecrementQueryReference(HeaderNameInfo);
 
         /* Dereference the object */
         ObDereferenceObject(Context->Object);
@@ -160,60 +160,20 @@
 
 VOID
 FORCEINLINE
-_ObpCleanupDirectoryLookup(IN POBP_LOOKUP_CONTEXT Context)
+ObpCleanupDirectoryLookup(IN POBP_LOOKUP_CONTEXT Context)
 {
     /* Check if we came back with the directory locked */
     if (Context->DirectoryLocked)
     {
         /* Release the lock */
-        _ObpReleaseDirectoryLock(Context->Directory, Context);
+        ObpReleaseDirectoryLock(Context->Directory, Context);
     }
 
     /* Clear the context  */
     Context->Directory = NULL;
     Context->DirectoryLocked = FALSE;
-    _ObpReleaseLookupContextObject(Context);
-}
-
-#if _OB_DEBUG_
-#define ObpAcquireDirectoryLockShared(a, b)                                 \
-{                                                                           \
-    DbgPrint("OB QUERY: Acquiring lock at %s %d\n", __FUNCTION__, __LINE__);\
-    _ObpAcquireDirectoryLockShared(a, b);                                   \
-}
-#define ObpAcquireDirectoryLockExclusive(a, b)                              \
-{                                                                           \
-    DbgPrint("OB QUERY: Acquiring lock at %s %d\n", __FUNCTION__, __LINE__);\
-    _ObpAcquireDirectoryLockExclusive(a, b);                                \
-}
-#define ObpReleaseDirectoryLock(a, b)                                       \
-{                                                                           \
-    DbgPrint("OB QUERY: Releasing lock at %s %d\n", __FUNCTION__, __LINE__);\
-    _ObpReleaseDirectoryLock(a, b);                                         \
-}
-#define ObpInitializeDirectoryLookup(a)                                     \
-{                                                                           \
-    DbgPrint("OB QUERY: Initialization at %s %d\n", __FUNCTION__, __LINE__);\
-    _ObpInitializeDirectoryLookup(a);                                       \
-}
-#define ObpCleanupDirectoryLookup(a, b)                                     \
-{                                                                           \
-    DbgPrint("OB QUERY: Cleanup at %s %d\n", __FUNCTION__, __LINE__);       \
-    _ObpCleanupDirectoryLookup(a, b);                                       \
-}
-#define ObpDecrementQueryReference(a)                                       \
-{                                                                           \
-    DbgPrint("OB QUERY: Decrement at %s %d\n", __FUNCTION__, __LINE__);     \
-    _ObpDecrementQueryReference(a);                                         \
-}
-#else
-#define ObpDecrementQueryReference          _ObpDecrementQueryReference
-#define ObpAcquireDirectoryLockExclusive    _ObpAcquireDirectoryLockExclusive
-#define ObpAcquireDirectoryLockShared       _ObpAcquireDirectoryLockShared
-#define ObpReleaseDirectoryLock             _ObpReleaseDirectoryLock
-#define ObpInitializeDirectoryLookup        _ObpInitializeDirectoryLookup
-#define ObpCleanupDirectoryLookup           _ObpCleanupDirectoryLookup
-#endif
+    ObpReleaseLookupContextObject(Context);
+}
 
 VOID
 FORCEINLINE

Modified: trunk/reactos/ntoskrnl/ob/obdir.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/obdir.c?rev=25407&r1=25406&r2=25407&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/ob/obdir.c (original)
+++ trunk/reactos/ntoskrnl/ob/obdir.c Wed Jan 10 04:00:46 2007
@@ -247,11 +247,11 @@
         if (HeaderNameInfo)
         {
             /* Add a query reference */
-            //ObpIncrementQueryReference(ObjectHeader, HeaderNameInfo);
+            ObpIncrementQueryReference(ObjectHeader, HeaderNameInfo);
         }
 
         /* Reference the object being looked up */
-        //ObReferenceObject(FoundObject);
+        ObReferenceObject(FoundObject);
 
         /* Check if the directory was locked */
         if (!Context->DirectoryLocked)
@@ -282,7 +282,7 @@
     if (Context->Object)
     {
         /* We already did a lookup, so remove this object's query reference */
-        //ObpRemoveQueryReference(Context->Object);
+        //ObpDecrementQueryReference(Context->Object);
     }
 
     /* Return the object we found */

Modified: trunk/reactos/ntoskrnl/ob/obhandle.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/obhandle.c?rev=25407&r1=25406&r2=25407&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/ob/obhandle.c (original)
+++ trunk/reactos/ntoskrnl/ob/obhandle.c Wed Jan 10 04:00:46 2007
@@ -1249,7 +1249,7 @@
     if ((Type) && (ObjectType != Type))
     {
         /* They don't, cleanup */
-        //if (Context) ObpCleanupDirectoryLookup(Context);
+        if (Context) ObpCleanupDirectoryLookup(Context);
         return STATUS_OBJECT_TYPE_MISMATCH;
     }
 
@@ -1287,7 +1287,7 @@
          * We failed (meaning security failure, according to NT Internals)
          * detach and return
          */
-        //if (Context) ObpCleanupDirectoryLookup(Context);
+        if (Context) ObpCleanupDirectoryLookup(Context);
         if (AttachedToProcess) KeUnstackDetachProcess(&ApcState);
         return Status;
     }
@@ -1327,8 +1327,7 @@
     }
 
     /* Now we can release the object */
-    //if (Context) ObpCleanupDirectoryLookup(Context);
-    if (Context) Context->Object = NULL;
+    if (Context) ObpCleanupDirectoryLookup(Context);
 
     /* Save the object header */
     NewEntry.Object = ObjectHeader;
@@ -2117,8 +2116,7 @@
     if (!NT_SUCCESS(Status))
     {
         /* Cleanup after lookup */
-        //ObpCleanupDirectoryLookup(&TempBuffer->LookupContext);
-        TempBuffer->LookupContext.Object = NULL;
+        ObpCleanupDirectoryLookup(&TempBuffer->LookupContext);
         goto Cleanup;
     }
 
@@ -2152,8 +2150,7 @@
         Status = STATUS_INVALID_PARAMETER;
 
         /* Cleanup after lookup */
-        //ObpCleanupDirectoryLookup(&TempBuffer->LookupContext);
-        TempBuffer->LookupContext.Object = NULL;
+        ObpCleanupDirectoryLookup(&TempBuffer->LookupContext);
     }
     else
     {
@@ -2589,6 +2586,9 @@
         /* Check if anything until now failed */
         if (!NT_SUCCESS(Status))
         {
+            /* Cleanup after lookup */
+            ObpCleanupDirectoryLookup(&Context);
+
             /* Remove query reference that we added */
             if (ObjectNameInfo) ObpDecrementQueryReference(ObjectNameInfo);
 
@@ -2657,6 +2657,9 @@
         /* Check if anything until now failed */
         if (!NT_SUCCESS(Status))
         {
+            /* Cleanup lookup context */
+            ObpCleanupDirectoryLookup(&Context);
+
             /* Remove query reference that we added */
             if (ObjectNameInfo) ObpDecrementQueryReference(ObjectNameInfo);
 

Modified: trunk/reactos/ntoskrnl/ob/obinit.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/obinit.c?rev=25407&r1=25406&r2=25407&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/ob/obinit.c (original)
+++ trunk/reactos/ntoskrnl/ob/obinit.c Wed Jan 10 04:00:46 2007
@@ -282,10 +282,7 @@
     ObpInitializeDirectoryLookup(&Context);
 
     /* Lock it */
-    //ObpAcquireDirectoryLockExclusive(ObpTypeDirectoryObject, &Context);
-    Context.Directory = ObpTypeDirectoryObject;
-    Context.DirectoryLocked = TRUE;
-    Context.LockStateSignature = 0xCCCC1234;
+    ObpAcquireDirectoryLockExclusive(ObpTypeDirectoryObject, &Context);
 
     /* Loop the object types */
     ListHead = &ObTypeObjectType->TypeList;
@@ -323,8 +320,7 @@
     }
 
     /* Cleanup after lookup */
-    //ObpCleanupDirectoryLookup(&Context);
-    Context.Object = NULL;
+    ObpCleanupDirectoryLookup(&Context);
 
     /* Initialize DOS Devices Directory and related Symbolic Links */
     Status = ObpCreateDosDevicesDirectory();

Modified: trunk/reactos/ntoskrnl/ob/oblife.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/oblife.c?rev=25407&r1=25406&r2=25407&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/ob/oblife.c (original)
+++ trunk/reactos/ntoskrnl/ob/oblife.c Wed Jan 10 04:00:46 2007
@@ -965,10 +965,7 @@
     if (ObpTypeDirectoryObject)
     {
         /* Acquire the directory lock */
-        //ObpAcquireDirectoryLockExclusive(ObpTypeDirectoryObject, &Context);
-        Context.Directory = ObpTypeDirectoryObject;
-        Context.DirectoryLocked = TRUE;
-        Context.LockStateSignature = 0xCCCC1234;
+        ObpAcquireDirectoryLockExclusive(ObpTypeDirectoryObject, &Context);
 
         /* Do the lookup */
         if (ObpLookupEntryDirectory(ObpTypeDirectoryObject,
@@ -978,7 +975,7 @@
                                     &Context))
         {
             /* We have already created it, so fail */
-            Context.Object = NULL;
+            ObpCleanupDirectoryLookup(&Context);
             return STATUS_OBJECT_NAME_COLLISION;
         }
     }
@@ -990,7 +987,7 @@
     if (!ObjectName.Buffer)
     {
         /* Out of memory, fail */
-        Context.Object = NULL;
+        ObpCleanupDirectoryLookup(&Context);
         return STATUS_INSUFFICIENT_RESOURCES;
     }
 
@@ -1007,9 +1004,8 @@
                                (POBJECT_HEADER*)&Header);
     if (!NT_SUCCESS(Status))
     {
-        Context.Object = NULL;
-
         /* Free the name and fail */
+        ObpCleanupDirectoryLookup(&Context);
         ExFreePool(ObjectName.Buffer);
         return Status;
     }
@@ -1138,7 +1134,8 @@
             ObReferenceObject(ObpTypeDirectoryObject);
         }
 
-        Context.Object = NULL;
+        /* Cleanup the lookup context */
+        ObpCleanupDirectoryLookup(&Context);
 
         /* Return the object type and success */
         *ObjectType = LocalObjectType;
@@ -1146,7 +1143,7 @@
     }
 
     /* If we got here, then we failed */
-    Context.Object = NULL;
+    ObpCleanupDirectoryLookup(&Context);
     return STATUS_INSUFFICIENT_RESOURCES;
 }
 

Modified: trunk/reactos/ntoskrnl/ob/obname.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/obname.c?rev=25407&r1=25406&r2=25407&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/ob/obname.c (original)
+++ trunk/reactos/ntoskrnl/ob/obname.c Wed Jan 10 04:00:46 2007
@@ -191,16 +191,14 @@
     if (!(ObjectHeader->HandleCount) &&
          (ObjectNameInfo) &&
          (ObjectNameInfo->Name.Length) &&
+         (ObjectNameInfo->Directory) &&
          !(ObjectHeader->Flags & OB_FLAG_PERMANENT))
     {
         /* Setup a lookup context */
         ObpInitializeDirectoryLookup(&Context);
 
         /* Lock the directory */
-        //ObpAcquireDirectoryLockExclusive(ObjectNameInfo->Directory, &Context);
-        Context.Directory = ObjectNameInfo->Directory;
-        Context.DirectoryLocked = TRUE;
-        Context.LockStateSignature = 0xCCCC1234;
+        ObpAcquireDirectoryLockExclusive(ObjectNameInfo->Directory, &Context);
 
         /* Do the lookup */
         Object = ObpLookupEntryDirectory(ObjectNameInfo->Directory,
@@ -253,8 +251,7 @@
         }
 
         /* Cleanup after lookup */
-        //ObpCleanupDirectoryLookup(&Context);
-        Context.Object = NULL;
+        ObpCleanupDirectoryLookup(&Context);
 
         /* Remove another query reference since we added one on top */
         ObpDecrementQueryReference(ObjectNameInfo);
@@ -573,10 +570,7 @@
             if (InsertObject)
             {
                 /* Lock the directory */
-                //ObpAcquireDirectoryLockExclusive(Directory, LookupContext);
-                LookupContext->Directory = Directory;
-                LookupContext->DirectoryLocked = TRUE;
-                LookupContext->LockStateSignature = 0xCCCC1234;
+                ObpAcquireDirectoryLockExclusive(Directory, LookupContext);
             }
         }
 
@@ -696,8 +690,7 @@
             InterlockedExchangeAdd(&ObjectHeader->PointerCount, 1);
 
             /* Cleanup from the first lookup */
-            //ObpCleanupDirectoryLookup(LookupContext);
-            LookupContext->Object = NULL;
+            ObpCleanupDirectoryLookup(LookupContext);
 
             /* Check if we have a referenced directory */
             if (ReferencedDirectory)
@@ -863,8 +856,7 @@
     if (!NT_SUCCESS(Status))
     {
         /* Cleanup after lookup */
-        //ObpCleanupDirectoryLookup(LookupContext);
-        LookupContext->Object = NULL;
+        ObpCleanupDirectoryLookup(LookupContext);
     }
 
     /* Check if we have a device map and dereference it if so */

Modified: trunk/reactos/ntoskrnl/ob/obref.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/obref.c?rev=25407&r1=25406&r2=25407&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/ob/obref.c (original)
+++ trunk/reactos/ntoskrnl/ob/obref.c Wed Jan 10 04:00:46 2007
@@ -430,8 +430,7 @@
                                  &Object);
 
     /* Cleanup after lookup */
-    //ObpCleanupDirectoryLookup(&Context);
-    Context.Object = NULL;
+    ObpCleanupDirectoryLookup(&Context);
 
     /* Check if the lookup succeeded */
     if (NT_SUCCESS(Status))




More information about the Ros-diffs mailing list