From 4c63ed5a7af6338cc63f1395d9895b22c0827a47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herm=C3=A8s=20B=C3=A9lusca-Ma=C3=AFto?= Date: Sat, 25 Sep 2021 00:03:56 +0200 Subject: [PATCH] [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); } }