https://git.reactos.org/?p=reactos.git;a=commitdiff;h=4c63ed5a7af6338cc63f13...
commit 4c63ed5a7af6338cc63f1395d9895b22c0827a47 Author: Hermès Bélusca-Maïto hermes.belusca-maito@reactos.org AuthorDate: Sat Sep 25 00:03:56 2021 +0200 Commit: Hermès Bélusca-Maïto hermes.belusca-maito@reactos.org CommitDate: Sat Sep 25 00:47:43 2021 +0200
[NTOS:OB] Clarify and fix the usage of the Obp*DirectoryLock*() and ObpReleaseLookupContextObject() functions.
- Disentangle the usage of ObpAcquireDirectoryLockExclusive() when it's used only for accessing a directory structure, or as part of a lookup operation.
The Obp*DirectoryLock*() -- both shared and exclusive -- functions are only for locking an OB directory, for reading or writing its structure members.
When performing lookup operations (insertions/deletions of entries within a directory), use a ObpAcquireLookupContextLock() function that exclusively locks the directory and saves extra lock state, that can be used by ObpReleaseLookupContextObject() for cleanup.
- Add documentation for these functions. --- ntoskrnl/include/internal/ob_x.h | 96 ++++++++++++++++++++++++++++++++++------ ntoskrnl/ob/obdir.c | 2 +- ntoskrnl/ob/obinit.c | 6 +-- ntoskrnl/ob/oblife.c | 6 +-- ntoskrnl/ob/obname.c | 12 +++-- 5 files changed, 93 insertions(+), 29 deletions(-)
diff --git a/ntoskrnl/include/internal/ob_x.h b/ntoskrnl/include/internal/ob_x.h index 587b82ebc61..0594c08b8f8 100644 --- a/ntoskrnl/include/internal/ob_x.h +++ b/ntoskrnl/include/internal/ob_x.h @@ -169,15 +169,26 @@ ObpDereferenceNameInfo(IN POBJECT_HEADER_NAME_INFO HeaderNameInfo) } }
+/** + * @brief + * Locks a directory for shared access. + * Used for reading members of the directory object. + * + * @param[in] Directory + * The directory to lock. + * + * @param[in] Context + * The lookup lock context. + */ FORCEINLINE VOID ObpAcquireDirectoryLockShared(IN POBJECT_DIRECTORY Directory, - IN POBP_LOOKUP_CONTEXT Context) + IN POBP_LOOKUP_CONTEXT Context) { - /* It's not, set lock flag */ + /* Update lock flag */ Context->LockStateSignature = OBP_LOCK_STATE_PRE_ACQUISITION_SHARED;
- /* Lock it */ + /* Acquire an shared directory lock */ KeEnterCriticalRegion(); ExAcquirePushLockShared(&Directory->Lock);
@@ -185,10 +196,21 @@ ObpAcquireDirectoryLockShared(IN POBJECT_DIRECTORY Directory, Context->LockStateSignature = OBP_LOCK_STATE_POST_ACQUISITION_SHARED; }
+/** + * @brief + * Locks a directory for exclusive access. + * Used for writing/reading members of the directory object. + * + * @param[in] Directory + * The directory to lock. + * + * @param[in] Context + * The lookup lock context. + */ FORCEINLINE VOID ObpAcquireDirectoryLockExclusive(IN POBJECT_DIRECTORY Directory, - IN POBP_LOOKUP_CONTEXT Context) + IN POBP_LOOKUP_CONTEXT Context) { /* Update lock flag */ Context->LockStateSignature = OBP_LOCK_STATE_PRE_ACQUISITION_EXCLUSIVE; @@ -197,18 +219,24 @@ ObpAcquireDirectoryLockExclusive(IN POBJECT_DIRECTORY Directory, KeEnterCriticalRegion(); ExAcquirePushLockExclusive(&Directory->Lock);
- /* Set the directory */ - Context->Directory = Directory; - - /* Update lock settings */ + /* Update lock flag */ Context->LockStateSignature = OBP_LOCK_STATE_POST_ACQUISITION_EXCLUSIVE; - Context->DirectoryLocked = TRUE; }
+/** + * @brief + * Unlocks a previously shared or exclusively locked directory. + * + * @param[in] Directory + * The directory to unlock. + * + * @param[in] Context + * The lookup lock context. + */ FORCEINLINE VOID ObpReleaseDirectoryLock(IN POBJECT_DIRECTORY Directory, - IN POBP_LOOKUP_CONTEXT Context) + IN POBP_LOOKUP_CONTEXT Context) { /* Release the lock */ ExReleasePushLock(&Directory->Lock); @@ -216,6 +244,15 @@ ObpReleaseDirectoryLock(IN POBJECT_DIRECTORY Directory, KeLeaveCriticalRegion(); }
+/** + * @brief + * Initializes a new object directory lookup context. + * Used for lookup operations (insertions/deletions) in a directory. + * Employed in conjunction with the directory locking functions. + * + * @param[in] Context + * The new lookup context to initialize. + */ FORCEINLINE VOID ObpInitializeLookupContext(IN POBP_LOOKUP_CONTEXT Context) @@ -227,6 +264,29 @@ ObpInitializeLookupContext(IN POBP_LOOKUP_CONTEXT Context) Context->LockStateSignature = OBP_LOCK_STATE_INITIALIZED; }
+/** + * @brief + * Locks an object directory lookup context for performing + * lookup operations (insertions/deletions) in a directory. + * The directory is locked for exclusive access. + * + * @param[in] Context + * The lookup context to lock. + * + * @param[in] Directory + * The directory on which the lookup context applies. + */ +FORCEINLINE +VOID +ObpAcquireLookupContextLock(IN POBP_LOOKUP_CONTEXT Context, + IN POBJECT_DIRECTORY Directory) +{ + /* Acquire an exclusive directory lock and save its lock state */ + ObpAcquireDirectoryLockExclusive(Directory, Context); + Context->Directory = Directory; + Context->DirectoryLocked = TRUE; +} + FORCEINLINE VOID ObpReleaseLookupContextObject(IN POBP_LOOKUP_CONTEXT Context) @@ -234,14 +294,14 @@ ObpReleaseLookupContextObject(IN POBP_LOOKUP_CONTEXT Context) POBJECT_HEADER ObjectHeader; POBJECT_HEADER_NAME_INFO HeaderNameInfo;
- /* Check if we had found an object */ + /* Check if we had an object */ if (Context->Object) { /* Get the object name information */ ObjectHeader = OBJECT_TO_OBJECT_HEADER(Context->Object); HeaderNameInfo = OBJECT_HEADER_TO_NAME_INFO(ObjectHeader);
- /* release the name information */ + /* Release the name information */ ObpDereferenceNameInfo(HeaderNameInfo);
/* Dereference the object */ @@ -250,6 +310,14 @@ ObpReleaseLookupContextObject(IN POBP_LOOKUP_CONTEXT Context) } }
+/** + * @brief + * Releases an initialized object directory lookup context. + * Unlocks it if necessary, and dereferences the underlying object. + * + * @param[in] Context + * The lookup context to release. + */ FORCEINLINE VOID ObpReleaseLookupContext(IN POBP_LOOKUP_CONTEXT Context) @@ -257,13 +325,13 @@ ObpReleaseLookupContext(IN POBP_LOOKUP_CONTEXT Context) /* Check if we came back with the directory locked */ if (Context->DirectoryLocked) { - /* Release the lock */ + /* Release the directory lock */ ObpReleaseDirectoryLock(Context->Directory, Context); Context->Directory = NULL; Context->DirectoryLocked = FALSE; }
- /* Clear the context */ + /* Clear the context */ ObpReleaseLookupContextObject(Context); }
diff --git a/ntoskrnl/ob/obdir.c b/ntoskrnl/ob/obdir.c index 1c7b457c726..cb7699ac840 100644 --- a/ntoskrnl/ob/obdir.c +++ b/ntoskrnl/ob/obdir.c @@ -573,7 +573,7 @@ NtQueryDirectoryObject(IN HANDLE DirectoryHandle, return Status; }
- /* Lock directory in shared mode */ + /* Lock the directory in shared mode */ ObpAcquireDirectoryLockShared(Directory, &LookupContext);
/* Start at position 0 */ diff --git a/ntoskrnl/ob/obinit.c b/ntoskrnl/ob/obinit.c index 0b03779d820..8c61db54756 100644 --- a/ntoskrnl/ob/obinit.c +++ b/ntoskrnl/ob/obinit.c @@ -386,11 +386,9 @@ ObPostPhase0: Status = NtClose(Handle); if (!NT_SUCCESS(Status)) return FALSE;
- /* Initialize lookup context */ + /* Initialize the lookup context and lock it */ ObpInitializeLookupContext(&Context); - - /* Lock it */ - ObpAcquireDirectoryLockExclusive(ObpTypeDirectoryObject, &Context); + ObpAcquireLookupContextLock(&Context, ObpTypeDirectoryObject);
/* Loop the object types */ ListHead = &ObpTypeObjectType->TypeList; diff --git a/ntoskrnl/ob/oblife.c b/ntoskrnl/ob/oblife.c index 5fc68b0d6b5..a01f5373bca 100644 --- a/ntoskrnl/ob/oblife.c +++ b/ntoskrnl/ob/oblife.c @@ -1093,8 +1093,8 @@ ObCreateObjectType(IN PUNICODE_STRING TypeName, /* Check if we've already created the directory of types */ if (ObpTypeDirectoryObject) { - /* Acquire the directory lock */ - ObpAcquireDirectoryLockExclusive(ObpTypeDirectoryObject, &Context); + /* Lock the lookup context */ + ObpAcquireLookupContextLock(&Context, ObpTypeDirectoryObject);
/* Do the lookup */ if (ObpLookupEntryDirectory(ObpTypeDirectoryObject, @@ -1853,7 +1853,7 @@ NtSetInformationObject(IN HANDLE ObjectHandle, OBP_LOOKUP_CONTEXT LookupContext; ObpInitializeLookupContext(&LookupContext);
- /* Set its session ID */ + /* Set the directory session ID */ ObpAcquireDirectoryLockExclusive(Directory, &LookupContext); Directory->SessionId = PsGetCurrentProcessSessionId(); ObpReleaseDirectoryLock(Directory, &LookupContext); diff --git a/ntoskrnl/ob/obname.c b/ntoskrnl/ob/obname.c index 586561cd9b0..667815d41dc 100644 --- a/ntoskrnl/ob/obname.c +++ b/ntoskrnl/ob/obname.c @@ -321,11 +321,9 @@ ObpDeleteNameCheck(IN PVOID Object) (ObjectNameInfo->Directory) && !(ObjectHeader->Flags & OB_FLAG_PERMANENT)) { - /* Setup a lookup context */ + /* Setup a lookup context and lock it */ ObpInitializeLookupContext(&Context); - - /* Lock the directory */ - ObpAcquireDirectoryLockExclusive(ObjectNameInfo->Directory, &Context); + ObpAcquireLookupContextLock(&Context, ObjectNameInfo->Directory);
/* Do the lookup */ Object = ObpLookupEntryDirectory(ObjectNameInfo->Directory, @@ -352,7 +350,7 @@ ObpDeleteNameCheck(IN PVOID Object) ObpDeleteSymbolicLinkName(Object); }
- /* Check if the kernel exclusive is set */ + /* Check if the kernel exclusive flag is set */ ObjectNameInfo = OBJECT_HEADER_TO_NAME_INFO(ObjectHeader); if ((ObjectNameInfo) && (ObjectNameInfo->QueryReferences & OB_FLAG_KERNEL_EXCLUSIVE)) @@ -843,8 +841,8 @@ ParseFromRoot: /* Check if we are inserting an object */ if (InsertObject) { - /* Lock the directory */ - ObpAcquireDirectoryLockExclusive(Directory, LookupContext); + /* Lock the lookup context */ + ObpAcquireLookupContextLock(LookupContext, Directory); } }