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=... ============================================================================== --- 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/o... ============================================================================== --- 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=254... ============================================================================== --- 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=... ============================================================================== --- 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=25... ============================================================================== --- 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=25... ============================================================================== --- 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=25... ============================================================================== --- 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=254... ============================================================================== --- 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))