[reactos] 09/11: [NTOS:CM] Implement cache lookup and cleanup subkey information for cache consistency
1 Oct
2023
1 Oct
'23
6:51 p.m.
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=f4de5ceb9e5338c7346ae…
commit f4de5ceb9e5338c7346ae52705441c069e9e8ea2
Author: George Bișoc <george.bisoc(a)reactos.org>
AuthorDate: Thu Feb 16 21:42:26 2023 +0100
Commit: George Bișoc <george.bisoc(a)reactos.org>
CommitDate: Sun Oct 1 20:06:02 2023 +0200
[NTOS:CM] Implement cache lookup and cleanup subkey information for cache
consistency
During an open or create procedure of a registry key, the registry parser grabs
a key control block (KCB) from the parser object and uses its information to do the
necessary work in order to obtain a pointer to the newly created or opened registry
key.
However, the registry parsers faces several issues. First, we don't do subkey
cache cleaning
information against gathered KCBs so whenever we do a registry parse we end up with
KCBs
that have cache inconsistencies. Moreover we don't do any locking of whatever KCB
we
are grabing during a parse procedure.
=== PROPOSED CHANGES ===
* Implement CmpComputeHashValue and CmpLookInCache functions. With CmpComputeHashValue
we can
compute the convkey hashes of each subkey in the path name of a key so we can lock
them
with CmpBuildAndLockKcbArray. CmpLookInCache is a function that searches for the
suitable
KCB in the cache. The factors that determine if a KCB is "suitable" are:
-- the currently found KCB in the hash list has the same levels as that of the
given KCB from the parse object;
-- The key names from the computed hash values match with the block name of
the KCB;
-- The currently found KCB is not deleted.
The KCB will be changed if the key path name points to a partial match name in
the cache. The KCB from the parse object will be used if we have a full match
of remaining levels.
* Add missing CMP_LOCK_HASHES_FOR_KCB flags on CmpCreateKeyControlBlock calls
that create KCBs during a parse procedure. Such lock has to be preserved until
we're done with the registry parsing.
* On CmpDoCreateChild, preserve the exclusive lock of the KCB when we are
enlisting the key body.
* On CmpDoCreate, make sure that the passed parent KCB is locked exclusively and
lock the hiver flusher as we don't want the flusher to kick in during a key
creation on the given hive. Cleanup the subkey info when we're creating a key
object. Also implement missing cleanup path codes. Furthermore, avoid key
object creation if the parent KCB is protected with a read-only switch.
* Soft rewrite the CmpDoOpen function, namely how we manage a direct open vs
create KCB on open scenario. When a KCB is found in cache avoid touching
the key node. If the symbolic link has been resolved (aka found) then lock
exclusively the symbolic KCB. Otherwise just give the cached KCB to the caller.
If it were for the caller to request a KCB creation, we must check the passed
KCB from the parser object is locked exclusively, unlike on the case above
the caller doesn't want to create a KCB because there's already one in the
cache.
We don't want anybody to touch our KCB while we are still toying with it during
its birth. Furthermore, enlist the key body but mind the kind of lock it's been
used.
* On CmpCreateLinkNode, avoid creating a key object if the parent KCB is protected
with a read-only switch. In addition, add missing hive flusher locks for both
the target hive and its child. Cleanup the subkey information of the KCB when
creating a link node, this ensures our cached KCB data remains consistent.
* Do a direct open on CmpParseKey if no remaining subkey levels have been found
during hash computation and cache lookup, in this case the given KCB is the
block that points to the exact key. This happens when for example someone tried
to call RegOpenKeyExW but submitting NULL to the lpSubKey argument parameter.
CORE-10581
ROSTESTS-198
---
ntoskrnl/config/cmparse.c | 832 +++++++++++++++++++++++++++++++++++++----
ntoskrnl/include/internal/cm.h | 2 +-
2 files changed, 761 insertions(+), 73 deletions(-)
diff --git a/ntoskrnl/config/cmparse.c b/ntoskrnl/config/cmparse.c
index 38346c2501d..dec2fd72cda 100644
--- a/ntoskrnl/config/cmparse.c
+++ b/ntoskrnl/config/cmparse.c
@@ -336,7 +336,7 @@ CmpDoCreateChild(IN PHHIVE Hive,
*KeyCell,
KeyNode,
ParentKcb,
- 0, // CMP_LOCK_HASHES_FOR_KCB,
+ CMP_LOCK_HASHES_FOR_KCB,
Name);
if (!Kcb)
{
@@ -355,7 +355,7 @@ CmpDoCreateChild(IN PHHIVE Hive,
KeyBody->KeyControlBlock = Kcb;
/* Link it with the KCB */
- EnlistKeyBodyWithKCB(KeyBody, 0);
+ EnlistKeyBodyWithKCB(KeyBody, CMP_ENLIST_KCB_LOCKED_EXCLUSIVE);
/* Assign security */
Status = SeAssignSecurity(ParentDescriptor,
@@ -419,6 +419,17 @@ CmpDoCreate(IN PHHIVE Hive,
LARGE_INTEGER TimeStamp;
PCM_KEY_NODE KeyNode;
+ /* Make sure the KCB is locked and lock the flusher */
+ CMP_ASSERT_KCB_LOCK(ParentKcb);
+ CmpLockHiveFlusherShared((PCMHIVE)Hive);
+
+ /* Bail out on read-only KCBs */
+ if (ParentKcb->ExtFlags & CM_KCB_READ_ONLY_KEY)
+ {
+ Status = STATUS_ACCESS_DENIED;
+ goto Exit;
+ }
+
/* Check if the parent is being deleted */
if (ParentKcb->Delete)
{
@@ -493,8 +504,17 @@ CmpDoCreate(IN PHHIVE Hive,
/* Now add the subkey */
if (!CmpAddSubKey(Hive, Cell, KeyCell))
{
- /* Failure! We don't handle this yet! */
- ASSERT(FALSE);
+ /* Free the created child */
+ CmpFreeKeyByCell(Hive, KeyCell, FALSE);
+
+ /* Purge out this KCB */
+ KeyBody->KeyControlBlock->Delete = TRUE;
+ CmpRemoveKeyControlBlock(KeyBody->KeyControlBlock);
+
+ /* And cleanup the key body object */
+ ObDereferenceObjectDeferDelete(*Object);
+ Status = STATUS_INSUFFICIENT_RESOURCES;
+ goto Exit;
}
/* Get the key node */
@@ -502,9 +522,21 @@ CmpDoCreate(IN PHHIVE Hive,
if (!KeyNode)
{
/* Fail, this shouldn't happen */
- ASSERT(FALSE);
+ CmpFreeKeyByCell(Hive, KeyCell, TRUE); // Subkey linked above
+
+ /* Purge out this KCB */
+ KeyBody->KeyControlBlock->Delete = TRUE;
+ CmpRemoveKeyControlBlock(KeyBody->KeyControlBlock);
+
+ /* And cleanup the key body object */
+ ObDereferenceObjectDeferDelete(*Object);
+ Status = STATUS_INSUFFICIENT_RESOURCES;
+ goto Exit;
}
+ /* Clean up information on this subkey */
+ CmpCleanUpSubKeyInfo(KeyBody->KeyControlBlock->ParentKcb);
+
/* Sanity checks */
ASSERT(KeyBody->KeyControlBlock->ParentKcb->KeyCell == Cell);
ASSERT(KeyBody->KeyControlBlock->ParentKcb->KeyHive == Hive);
@@ -539,7 +571,16 @@ CmpDoCreate(IN PHHIVE Hive,
if (!CellData)
{
/* This shouldn't happen */
- ASSERT(FALSE);
+ CmpFreeKeyByCell(Hive, KeyCell, TRUE); // Subkey linked above
+
+ /* Purge out this KCB */
+ KeyBody->KeyControlBlock->Delete = TRUE;
+ CmpRemoveKeyControlBlock(KeyBody->KeyControlBlock);
+
+ /* And cleanup the key body object */
+ ObDereferenceObjectDeferDelete(*Object);
+ Status = STATUS_INSUFFICIENT_RESOURCES;
+ goto Exit;
}
/* Update the flags */
@@ -551,6 +592,7 @@ CmpDoCreate(IN PHHIVE Hive,
Exit:
/* Release the flusher lock and return status */
+ CmpUnlockHiveFlusher((PCMHIVE)Hive);
return Status;
}
@@ -565,10 +607,13 @@ CmpDoOpen(IN PHHIVE Hive,
IN PCM_PARSE_CONTEXT Context OPTIONAL,
IN ULONG ControlFlags,
IN OUT PCM_KEY_CONTROL_BLOCK *CachedKcb,
+ IN PULONG KcbsLocked,
IN PUNICODE_STRING KeyName,
OUT PVOID *Object)
{
NTSTATUS Status;
+ BOOLEAN LockKcb = FALSE;
+ BOOLEAN IsLockShared = FALSE;
PCM_KEY_BODY KeyBody = NULL;
PCM_KEY_CONTROL_BLOCK Kcb = NULL;
@@ -600,37 +645,87 @@ CmpDoOpen(IN PHHIVE Hive,
Context->Disposition = REG_OPENED_EXISTING_KEY;
}
- /* Do this in the registry lock */
- CmpLockRegistry();
-
- /* If we have a KCB, make sure it's locked */
- //ASSERT(CmpIsKcbLockedExclusive(*CachedKcb));
+ /* Lock the KCB on creation if asked */
+ if (ControlFlags & CMP_CREATE_KCB_KCB_LOCKED)
+ {
+ LockKcb = TRUE;
+ }
/* Check if caller doesn't want to create a KCB */
if (ControlFlags & CMP_OPEN_KCB_NO_CREATE)
{
+ /*
+ * The caller doesn't want to create a KCB. This means the KCB
+ * is already in cache and other threads may take use of it
+ * so it has to be locked in share mode.
+ */
+ IsLockShared = TRUE;
+
/* Check if this is a symlink */
- if ((Node->Flags & KEY_SYM_LINK) && !(Attributes &
OBJ_OPENLINK))
+ if (((*CachedKcb)->Flags & KEY_SYM_LINK) && !(Attributes &
OBJ_OPENLINK))
{
- /* This case for a cached KCB is not implemented yet */
- ASSERT(FALSE);
+ /* Is this symlink found? */
+ if ((*CachedKcb)->ExtFlags & CM_KCB_SYM_LINK_FOUND)
+ {
+ /* Get the real KCB, is this deleted? */
+ Kcb = (*CachedKcb)->ValueCache.RealKcb;
+ if (Kcb->Delete)
+ {
+ /*
+ * The real KCB is gone, do a reparse. We used to lock the KCB in
+ * shared mode as others may have taken use of it but since we
+ * must do a reparse of the key the only thing that matter is us.
+ * Lock the KCB exclusively so nobody is going to mess with the
KCB.
+ */
+ DPRINT1("The real KCB is deleted, attempt a reparse\n");
+ CmpUnLockKcbArray(KcbsLocked);
+
CmpAcquireKcbLockExclusiveByIndex(GET_HASH_INDEX((*CachedKcb)->ConvKey));
+ CmpCleanUpKcbValueCache(*CachedKcb);
+ KcbsLocked[0] = 1;
+ KcbsLocked[1] = GET_HASH_INDEX((*CachedKcb)->ConvKey);
+ return STATUS_REPARSE;
+ }
+
+ /*
+ * The symlink has been found. As in the similar case above,
+ * the KCB of the symlink exclusively, we don't want anybody
+ * to mess it up.
+ */
+ CmpUnLockKcbArray(KcbsLocked);
+
CmpAcquireKcbLockExclusiveByIndex(GET_HASH_INDEX((*CachedKcb)->ConvKey));
+ KcbsLocked[0] = 1;
+ KcbsLocked[1] = GET_HASH_INDEX((*CachedKcb)->ConvKey);
+ }
+ else
+ {
+ /* We must do a reparse */
+ DPRINT("The symlink is not found, attempt a reparse\n");
+ return STATUS_REPARSE;
+ }
+ }
+ else
+ {
+ /* This is not a symlink, just give the cached KCB already */
+ Kcb = *CachedKcb;
}
/* The caller wants to open a cached KCB */
- if (!CmpReferenceKeyControlBlock(*CachedKcb))
+ if (!CmpReferenceKeyControlBlock(Kcb))
{
- /* Release the registry lock */
- CmpUnlockRegistry();
-
/* Return failure code */
return STATUS_INSUFFICIENT_RESOURCES;
}
-
- /* Our kcb is that one */
- Kcb = *CachedKcb;
}
else
{
+ /*
+ * The caller wants to create a new KCB. Unlike the code path above, here
+ * we must check if the lock is exclusively held because in the scenario
+ * where the caller doesn't want to create a KCB is because it is already
+ * in the cache and it must have a shared lock instead.
+ */
+ ASSERT(CmpIsKcbLockedExclusive(*CachedKcb));
+
/* Check if this is a symlink */
if ((Node->Flags & KEY_SYM_LINK) && !(Attributes &
OBJ_OPENLINK))
{
@@ -639,47 +734,39 @@ CmpDoOpen(IN PHHIVE Hive,
Cell,
Node,
*CachedKcb,
- 0,
+ LockKcb ? CMP_LOCK_HASHES_FOR_KCB : 0,
KeyName);
if (!Kcb)
{
- /* Release registry lock and return failure */
- CmpUnlockRegistry();
+ /* Return failure */
return STATUS_INSUFFICIENT_RESOURCES;
}
/* Make sure it's also locked, and set the pointer */
- //ASSERT(CmpIsKcbLockedExclusive(Kcb));
+ ASSERT(CmpIsKcbLockedExclusive(Kcb));
*CachedKcb = Kcb;
- /* Release the registry lock */
- CmpUnlockRegistry();
-
/* Return reparse required */
return STATUS_REPARSE;
}
- /* Create the KCB. FIXME: Use lock flag */
+ /* Create the KCB */
Kcb = CmpCreateKeyControlBlock(Hive,
Cell,
Node,
*CachedKcb,
- 0,
+ LockKcb ? CMP_LOCK_HASHES_FOR_KCB : 0,
KeyName);
if (!Kcb)
{
- /* Release registry lock and return failure */
- CmpUnlockRegistry();
+ /* Return failure */
return STATUS_INSUFFICIENT_RESOURCES;
}
- }
- /* Make sure it's also locked, and set the pointer */
- //ASSERT(CmpIsKcbLockedExclusive(Kcb));
- *CachedKcb = Kcb;
-
- /* Release the registry lock */
- CmpUnlockRegistry();
+ /* Make sure it's also locked, and set the pointer */
+ ASSERT(CmpIsKcbLockedExclusive(Kcb));
+ *CachedKcb = Kcb;
+ }
/* Allocate the key object */
Status = ObCreateObject(AccessMode,
@@ -701,7 +788,7 @@ CmpDoOpen(IN PHHIVE Hive,
KeyBody->NotifyBlock = NULL;
/* Link to the KCB */
- EnlistKeyBodyWithKCB(KeyBody, 0);
+ EnlistKeyBodyWithKCB(KeyBody, IsLockShared ? CMP_ENLIST_KCB_LOCKED_SHARED :
CMP_ENLIST_KCB_LOCKED_EXCLUSIVE);
/*
* We are already holding a lock against the KCB that is assigned
@@ -748,6 +835,7 @@ CmpCreateLinkNode(IN PHHIVE Hive,
IN ULONG CreateOptions,
IN PCM_PARSE_CONTEXT Context,
IN PCM_KEY_CONTROL_BLOCK ParentKcb,
+ IN PULONG KcbsLocked,
OUT PVOID *Object)
{
NTSTATUS Status;
@@ -765,6 +853,18 @@ CmpCreateLinkNode(IN PHHIVE Hive,
return STATUS_ACCESS_DENIED;
}
+ /* Make sure the KCB is locked and lock the flusher */
+ CMP_ASSERT_KCB_LOCK(ParentKcb);
+ CmpLockHiveFlusherShared((PCMHIVE)Hive);
+ CmpLockHiveFlusherShared((PCMHIVE)Context->ChildHive.KeyHive);
+
+ /* Bail out on read-only KCBs */
+ if (ParentKcb->ExtFlags & CM_KCB_READ_ONLY_KEY)
+ {
+ Status = STATUS_ACCESS_DENIED;
+ goto Exit;
+ }
+
/* Check if the parent is being deleted */
if (ParentKcb->Delete)
{
@@ -827,8 +927,9 @@ CmpCreateLinkNode(IN PHHIVE Hive,
AccessMode,
CreateOptions,
NULL,
- 0,
+ CMP_CREATE_KCB_KCB_LOCKED,
&Kcb,
+ KcbsLocked,
&Name,
Object);
HvReleaseCell(Context->ChildHive.KeyHive, KeyCell);
@@ -930,6 +1031,9 @@ CmpCreateLinkNode(IN PHHIVE Hive,
/* Get the key body */
KeyBody = (PCM_KEY_BODY)*Object;
+ /* Clean up information on this subkey */
+ CmpCleanUpSubKeyInfo(KeyBody->KeyControlBlock->ParentKcb);
+
/* Sanity checks */
ASSERT(KeyBody->KeyControlBlock->ParentKcb->KeyCell == Cell);
ASSERT(KeyBody->KeyControlBlock->ParentKcb->KeyHive == Hive);
@@ -966,6 +1070,8 @@ CmpCreateLinkNode(IN PHHIVE Hive,
Exit:
/* Release the flusher locks and return status */
+ CmpUnlockHiveFlusher((PCMHIVE)Context->ChildHive.KeyHive);
+ CmpUnlockHiveFlusher((PCMHIVE)Hive);
return Status;
}
@@ -1005,40 +1111,550 @@ CmpHandleExitNode(IN OUT PHHIVE *Hive,
}
}
+static
+ULONG
+CmpComputeHashValue(
+ _In_ PUNICODE_STRING RemainingName,
+ _In_ ULONG ConvKey,
+ _Inout_ PCM_HASH_CACHE_STACK HashCacheStack,
+ _Out_ PULONG TotalSubKeys)
+{
+ ULONG CopyConvKey;
+ ULONG SubkeysInTotal;
+ ULONG RemainingSubkeysInTotal;
+ PWCHAR RemainingNameBuffer;
+ USHORT RemainingNameLength;
+ USHORT KeyNameLength;
+
+ /* Don't compute the hashes on a NULL remaining name */
+ RemainingNameBuffer = RemainingName->Buffer;
+ RemainingNameLength = RemainingName->Length;
+ if (RemainingNameLength == 0)
+ {
+ *TotalSubKeys = 0;
+ return 0;
+ }
+
+ /* Skip any leading separator */
+ while (RemainingNameLength >= sizeof(WCHAR) &&
+ *RemainingNameBuffer == OBJ_NAME_PATH_SEPARATOR)
+ {
+ RemainingNameBuffer++;
+ RemainingNameLength -= sizeof(WCHAR);
+ }
+
+ /* Now set up the hash stack entries and compute the hashes */
+ SubkeysInTotal = 0;
+ RemainingSubkeysInTotal = 0;
+ KeyNameLength = 0;
+ CopyConvKey = ConvKey;
+ HashCacheStack[RemainingSubkeysInTotal].NameOfKey.Buffer = RemainingNameBuffer;
+ while (RemainingNameLength > 0)
+ {
+ /* Is this character a separator? */
+ if (*RemainingNameBuffer != OBJ_NAME_PATH_SEPARATOR)
+ {
+ /* It's not, add it to the hash */
+ CopyConvKey = COMPUTE_HASH_CHAR(CopyConvKey, *RemainingNameBuffer);
+
+ /* Go to the next character (add up the length of the character as well) */
+ RemainingNameBuffer++;
+ KeyNameLength += sizeof(WCHAR);
+ RemainingNameLength -= sizeof(WCHAR);
+
+ /*
+ * We are at the end of the key name path. Take into account
+ * the last character and if we still have space in the hash
+ * stack, add it up in the remaining list.
+ */
+ if (RemainingNameLength == 0)
+ {
+ if (RemainingSubkeysInTotal < CMP_SUBKEY_LEVELS_DEPTH_LIMIT)
+ {
+ HashCacheStack[RemainingSubkeysInTotal].NameOfKey.Length =
KeyNameLength;
+ HashCacheStack[RemainingSubkeysInTotal].NameOfKey.MaximumLength =
KeyNameLength;
+ HashCacheStack[RemainingSubkeysInTotal].ConvKey = CopyConvKey;
+ RemainingSubkeysInTotal++;
+ }
+
+ SubkeysInTotal++;
+ }
+ }
+ else
+ {
+ /* Skip any leading separator */
+ while (RemainingNameLength >= sizeof(WCHAR) &&
+ *RemainingNameBuffer == OBJ_NAME_PATH_SEPARATOR)
+ {
+ RemainingNameBuffer++;
+ RemainingNameLength -= sizeof(WCHAR);
+ }
+
+ /*
+ * It would be possible that a malformed key pathname may be passed
+ * to the registry parser such as a path with only separators like
+ * "\\\\" for example. This would trick the function into
believing
+ * the key path has subkeys albeit that is not the case.
+ */
+ ASSERT(RemainingNameLength != 0);
+
+ /* Take into account this subkey */
+ SubkeysInTotal++;
+
+ /* And add it up to the hash stack */
+ if (RemainingSubkeysInTotal < CMP_SUBKEY_LEVELS_DEPTH_LIMIT)
+ {
+ HashCacheStack[RemainingSubkeysInTotal].NameOfKey.Length =
KeyNameLength;
+ HashCacheStack[RemainingSubkeysInTotal].NameOfKey.MaximumLength =
KeyNameLength;
+ HashCacheStack[RemainingSubkeysInTotal].ConvKey = CopyConvKey;
+
+ RemainingSubkeysInTotal++;
+ KeyNameLength = 0;
+
+ /*
+ * Precaution check -- we have added up a remaining
+ * subkey above but we must ensure we still have space
+ * to hold up the new subkey for which we will compute
+ * the hashes, so that we don't blow up the hash stack.
+ */
+ if (RemainingSubkeysInTotal < CMP_SUBKEY_LEVELS_DEPTH_LIMIT)
+ {
+ HashCacheStack[RemainingSubkeysInTotal].NameOfKey.Buffer =
RemainingNameBuffer;
+ }
+ }
+ }
+ }
+
+ *TotalSubKeys = SubkeysInTotal;
+ return RemainingSubkeysInTotal;
+}
+
+static
+BOOLEAN
+CmpCompareSubkeys(
+ _In_ PCM_HASH_CACHE_STACK HashCacheStack,
+ _In_ PCM_KEY_CONTROL_BLOCK CurrentKcb,
+ _In_ ULONG RemainingSubkeys,
+ _Out_ PCM_KEY_CONTROL_BLOCK *ParentKcb)
+{
+ LONG HashStackIndex;
+ LONG Result;
+ PCM_NAME_CONTROL_BLOCK NameBlock;
+ UNICODE_STRING CurrentNameBlock;
+
+ ASSERT(CurrentKcb != NULL);
+
+ /* Loop each hash and check that they match */
+ HashStackIndex = RemainingSubkeys;
+ while (HashStackIndex >= 0)
+ {
+ /* Does the subkey hash match? */
+ if (CurrentKcb->ConvKey != HashCacheStack[HashStackIndex].ConvKey)
+ {
+ *ParentKcb = CurrentKcb;
+ return FALSE;
+ }
+
+ /* Compare the subkey string, is the name compressed? */
+ NameBlock = CurrentKcb->NameBlock;
+ if (NameBlock->Compressed)
+ {
+ Result =
CmpCompareCompressedName(&HashCacheStack[HashStackIndex].NameOfKey,
+ NameBlock->Name,
+ NameBlock->NameLength);
+ }
+ else
+ {
+ CurrentNameBlock.Buffer = NameBlock->Name;
+ CurrentNameBlock.Length = NameBlock->NameLength;
+ CurrentNameBlock.MaximumLength = NameBlock->NameLength;
+
+ Result =
RtlCompareUnicodeString(&HashCacheStack[HashStackIndex].NameOfKey,
+ &CurrentNameBlock,
+ TRUE);
+ }
+
+ /* Do the subkey names match? */
+ if (Result)
+ {
+ *ParentKcb = CurrentKcb;
+ return FALSE;
+ }
+
+ /* Go to the next subkey hash */
+ HashStackIndex--;
+ }
+
+ /* All the subkeys match */
+ *ParentKcb = CurrentKcb->ParentKcb;
+ return TRUE;
+}
+
+static
+VOID
+CmpRemoveSubkeysInRemainingName(
+ _In_ PCM_HASH_CACHE_STACK HashCacheStack,
+ _In_ ULONG RemainingSubkeys,
+ _Inout_ PUNICODE_STRING RemainingName)
+{
+ ULONG HashStackIndex = 0;
+
+ /* Skip any leading separator on matching name */
+ while (RemainingName->Length >= sizeof(WCHAR) &&
+ RemainingName->Buffer[0] == OBJ_NAME_PATH_SEPARATOR)
+ {
+ RemainingName->Buffer++;
+ RemainingName->Length -= sizeof(WCHAR);
+ }
+
+ /* Skip the subkeys as well */
+ while (HashStackIndex <= RemainingSubkeys)
+ {
+ RemainingName->Buffer += HashCacheStack[HashStackIndex].NameOfKey.Length /
sizeof(WCHAR);
+ RemainingName->Length -= HashCacheStack[HashStackIndex].NameOfKey.Length;
+
+ /* Skip any leading separator */
+ while (RemainingName->Length >= sizeof(WCHAR) &&
+ RemainingName->Buffer[0] == OBJ_NAME_PATH_SEPARATOR)
+ {
+ RemainingName->Buffer++;
+ RemainingName->Length -= sizeof(WCHAR);
+ }
+
+ /* Go to the next hash */
+ HashStackIndex++;
+ }
+}
+
+static
+NTSTATUS
+CmpLookInCache(
+ _In_ PCM_HASH_CACHE_STACK HashCacheStack,
+ _In_ BOOLEAN LockKcbsExclusive,
+ _In_ ULONG TotalRemainingSubkeys,
+ _Inout_ PUNICODE_STRING RemainingName,
+ _Inout_ PULONG OuterStackArray,
+ _Inout_ PCM_KEY_CONTROL_BLOCK *Kcb,
+ _Out_ PHHIVE *Hive,
+ _Out_ PHCELL_INDEX Cell,
+ _Out_ PULONG MatchRemainSubkeyLevel)
+{
+ LONG RemainingSubkeys;
+ ULONG TotalLevels;
+ BOOLEAN SubkeysMatch;
+ PCM_KEY_CONTROL_BLOCK CurrentKcb, ParentKcb;
+ PCM_KEY_HASH HashEntry = NULL;
+ BOOLEAN KeyFoundInCache = FALSE;
+ PULONG LockedKcbs = NULL;
+
+ /* Reference the KCB */
+ if (!CmpReferenceKeyControlBlock(*Kcb))
+ {
+ /* This key is opened too many times, bail out */
+ DPRINT1("Could not reference the KCB, too many references (KCB
0x%p)\n", Kcb);
+ return STATUS_UNSUCCESSFUL;
+ }
+
+ /* Prepare to lock the KCBs */
+ LockedKcbs = CmpBuildAndLockKcbArray(HashCacheStack,
+ LockKcbsExclusive ? CMP_LOCK_KCB_ARRAY_EXCLUSIVE
: CMP_LOCK_KCB_ARRAY_SHARED,
+ *Kcb,
+ OuterStackArray,
+ TotalRemainingSubkeys,
+ 0);
+ NT_ASSERT(LockedKcbs);
+
+ /* Lookup in the cache */
+ RemainingSubkeys = TotalRemainingSubkeys - 1;
+ TotalLevels = TotalRemainingSubkeys + (*Kcb)->TotalLevels + 1;
+ while (RemainingSubkeys >= 0)
+ {
+ /* Get the hash entry from the cache */
+ HashEntry = GET_HASH_ENTRY(CmpCacheTable,
HashCacheStack[RemainingSubkeys].ConvKey)->Entry;
+
+ /* Take one level down as we are processing this hash entry */
+ TotalLevels--;
+
+ while (HashEntry != NULL)
+ {
+ /* Validate this hash and obtain the current KCB */
+ ASSERT_VALID_HASH(HashEntry);
+ CurrentKcb = CONTAINING_RECORD(HashEntry, CM_KEY_CONTROL_BLOCK, KeyHash);
+
+ /* Does this KCB have matching levels? */
+ if (TotalLevels == CurrentKcb->TotalLevels)
+ {
+ /*
+ * We have matching subkey levels but don't directly assume we have
+ * a matching key path in cache. Start comparing each subkey.
+ */
+ SubkeysMatch = CmpCompareSubkeys(HashCacheStack,
+ CurrentKcb,
+ RemainingSubkeys,
+ &ParentKcb);
+ if (SubkeysMatch)
+ {
+ /* All subkeys match, now check if the base KCB matches with parent
*/
+ if (*Kcb == ParentKcb)
+ {
+ /* Is the KCB marked as deleted? */
+ if (CurrentKcb->Delete ||
+ CurrentKcb->ParentKcb->Delete)
+ {
+ /*
+ * Either the current or its parent KCB is marked
+ * but we had a shared lock so probably a naughty
+ * thread was deleting it. Retry doing a cache
+ * lookup again with exclusive lock.
+ */
+ if (!LockKcbsExclusive)
+ {
+ CmpUnLockKcbArray(LockedKcbs);
+ CmpDereferenceKeyControlBlock(*Kcb);
+ DPRINT1("The current KCB or its parent is deleted,
retrying looking in the cache\n");
+ return STATUS_RETRY;
+ }
+
+ /* We're under an exclusive lock, is the KCB deleted yet?
*/
+ if (CurrentKcb->Delete)
+ {
+ /* The KCB is gone, the key should no longer belong in
the cache */
+ CmpRemoveKeyControlBlock(CurrentKcb);
+ CmpUnLockKcbArray(LockedKcbs);
+ CmpDereferenceKeyControlBlock(*Kcb);
+ DPRINT1("The current KCB is deleted (KCB
0x%p)\n", CurrentKcb);
+ return STATUS_OBJECT_NAME_NOT_FOUND;
+ }
+
+ /*
+ * The parent is deleted so it must be that somebody
created
+ * a fake key. Assert ourselves that is the case.
+ */
+ ASSERT(CurrentKcb->ExtFlags & CM_KCB_KEY_NON_EXIST);
+
+ /* Remove this KCB out of cache if someone still uses it */
+ if (CurrentKcb->RefCount != 0)
+ {
+ CurrentKcb->Delete = TRUE;
+ CmpRemoveKeyControlBlock(CurrentKcb);
+ }
+ else
+ {
+ /* Otherwise expunge it */
+ CmpRemoveFromDelayedClose(CurrentKcb);
+ CmpCleanUpKcbCacheWithLock(CurrentKcb, FALSE);
+ }
+
+ /* Stop looking for next hashes as the KCB is kaput */
+ break;
+ }
+
+ /* We finally found the key in cache, acknowledge it */
+ KeyFoundInCache = TRUE;
+
+ /* Remove the subkeys in the remaining name and stop looking in
the cache */
+ CmpRemoveSubkeysInRemainingName(HashCacheStack, RemainingSubkeys,
RemainingName);
+ break;
+ }
+ }
+ }
+
+ /* Go to the next hash */
+ HashEntry = HashEntry->NextHash;
+ }
+
+ /* Stop looking in cache if we found the matching key */
+ if (KeyFoundInCache)
+ {
+ DPRINT("Key found in cache, stop looking\n");
+ break;
+ }
+
+ /* Keep looking in the cache until we run out of remaining subkeys */
+ RemainingSubkeys--;
+ }
+
+ /* Return the matching subkey levels */
+ *MatchRemainSubkeyLevel = RemainingSubkeys + 1;
+
+ /* We have to update the KCB if the key was found in cache */
+ if (KeyFoundInCache)
+ {
+ /*
+ * Before we change the KCB we must dereference the prior
+ * KCB that we no longer need it.
+ */
+ CmpDereferenceKeyControlBlock(*Kcb);
+ *Kcb = CurrentKcb;
+
+ /* Reference the new KCB now */
+ if (!CmpReferenceKeyControlBlock(*Kcb))
+ {
+ /* This key is opened too many times, bail out */
+ DPRINT1("Could not reference the KCB, too many references (KCB
0x%p)\n", Kcb);
+ return STATUS_UNSUCCESSFUL;
+ }
+
+ /* Update hive and cell data from current KCB */
+ *Hive = CurrentKcb->KeyHive;
+ *Cell = CurrentKcb->KeyCell;
+ }
+
+ /* Unlock the KCBs */
+ CmpUnLockKcbArray(LockedKcbs);
+ return STATUS_SUCCESS;
+}
+
NTSTATUS
NTAPI
-CmpBuildHashStackAndLookupCache(IN PCM_KEY_BODY ParseObject,
- IN OUT PCM_KEY_CONTROL_BLOCK *Kcb,
- IN PUNICODE_STRING Current,
- OUT PHHIVE *Hive,
- OUT HCELL_INDEX *Cell,
- OUT PULONG TotalRemainingSubkeys,
- OUT PULONG MatchRemainSubkeyLevel,
- OUT PULONG TotalSubkeys,
- OUT PULONG OuterStackArray,
- OUT PULONG *LockedKcbs)
+CmpBuildHashStackAndLookupCache(
+ _In_ PCM_KEY_BODY ParseObject,
+ _Inout_ PCM_KEY_CONTROL_BLOCK *Kcb,
+ _In_ PUNICODE_STRING Current,
+ _Out_ PHHIVE *Hive,
+ _Out_ PHCELL_INDEX Cell,
+ _Out_ PULONG TotalRemainingSubkeys,
+ _Out_ PULONG MatchRemainSubkeyLevel,
+ _Out_ PULONG TotalSubkeys,
+ _Inout_ PULONG OuterStackArray,
+ _Out_ PULONG *LockedKcbs)
{
- /* We don't lock anything for now */
- *LockedKcbs = NULL;
+ NTSTATUS Status;
+ ULONG ConvKey;
+ ULONG SubkeysInTotal, RemainingSubkeysInTotal, MatchRemainingSubkeys;
+ CM_HASH_CACHE_STACK HashCacheStack[CMP_SUBKEY_LEVELS_DEPTH_LIMIT];
- /* Calculate hash values */
- *TotalRemainingSubkeys = 0xBAADF00D;
+ /* Make sure it's not a dead KCB */
+ ASSERT((*Kcb)->RefCount > 0);
/* Lock the registry */
CmpLockRegistry();
+ /* Calculate hash values for every subkey this key path has */
+ ConvKey = (*Kcb)->ConvKey;
+ RemainingSubkeysInTotal = CmpComputeHashValue(Current,
+ ConvKey,
+ HashCacheStack,
+ &SubkeysInTotal);
+
+ /* This key path has too many subkeys */
+ if (SubkeysInTotal > CMP_SUBKEY_LEVELS_DEPTH_LIMIT)
+ {
+ DPRINT1("The key path has too many subkeys - %lu\n", SubkeysInTotal);
+ *LockedKcbs = NULL;
+ return STATUS_NAME_TOO_LONG;
+ }
+
/* Return hive and cell data */
*Hive = (*Kcb)->KeyHive;
*Cell = (*Kcb)->KeyCell;
- /* Make sure it's not a dead KCB */
- ASSERT((*Kcb)->RefCount > 0);
+ /* Do we have any subkeys? */
+ if (!RemainingSubkeysInTotal && !SubkeysInTotal)
+ {
+ /*
+ * We don't have any subkeys nor remaining levels, the
+ * KCB points to the actual key. Lock it.
+ */
+ if (!CmpReferenceKeyControlBlock(*Kcb))
+ {
+ /* This key is opened too many times, bail out */
+ DPRINT1("Could not reference the KCB, too many references (KCB
0x%p)\n", Kcb);
+ return STATUS_UNSUCCESSFUL;
+ }
- /* Reference it */
- (VOID)CmpReferenceKeyControlBlock(*Kcb);
+ CmpAcquireKcbLockSharedByIndex(GET_HASH_INDEX(ConvKey));
- /* Return success for now */
- return STATUS_SUCCESS;
+ /* Add this KCB in the array of locked KCBs */
+ OuterStackArray[0] = 1;
+ OuterStackArray[1] = GET_HASH_INDEX(ConvKey);
+ *LockedKcbs = OuterStackArray;
+
+ /* And return all the subkey level counters */
+ *TotalRemainingSubkeys = RemainingSubkeysInTotal;
+ *MatchRemainSubkeyLevel = 0;
+ *TotalSubkeys = SubkeysInTotal;
+ return STATUS_SUCCESS;
+ }
+
+ /* Lookup in the cache */
+ Status = CmpLookInCache(HashCacheStack,
+ FALSE,
+ RemainingSubkeysInTotal,
+ Current,
+ OuterStackArray,
+ Kcb,
+ Hive,
+ Cell,
+ &MatchRemainingSubkeys);
+ if (!NT_SUCCESS(Status))
+ {
+ /* Bail out if cache lookup failed for other reasons */
+ if (Status != STATUS_RETRY)
+ {
+ DPRINT1("CmpLookInCache() failed (Status 0x%lx)\n", Status);
+ *LockedKcbs = NULL;
+ return Status;
+ }
+
+ /* Retry looking in the cache but with KCBs locked exclusively */
+ Status = CmpLookInCache(HashCacheStack,
+ TRUE,
+ RemainingSubkeysInTotal,
+ Current,
+ OuterStackArray,
+ Kcb,
+ Hive,
+ Cell,
+ &MatchRemainingSubkeys);
+ if (!NT_SUCCESS(Status))
+ {
+ DPRINT1("CmpLookInCache() failed after retry (Status 0x%lx)\n",
Status);
+ *LockedKcbs = NULL;
+ return Status;
+ }
+ }
+
+ /*
+ * Check if we have a full match of remaining levels.
+ *
+ * FIXME: It is possible we can catch a fake key from the cache
+ * when we did the lookup, in such case we should not do any
+ * locking as such KCB does not point to any real information.
+ * Currently ReactOS doesn't create fake KCBs so we are good
+ * for now.
+ */
+ if (RemainingSubkeysInTotal == MatchRemainingSubkeys)
+ {
+ /*
+ * Just simply lock this KCB as it points to the full
+ * subkey levels in cache.
+ */
+ CmpAcquireKcbLockSharedByIndex(GET_HASH_INDEX((*Kcb)->ConvKey));
+ OuterStackArray[0] = 1;
+ OuterStackArray[1] = GET_HASH_INDEX((*Kcb)->ConvKey);
+ *LockedKcbs = OuterStackArray;
+ }
+ else
+ {
+ /*
+ * We only have a partial match so other subkey levels
+ * have each KCB. Simply just lock them.
+ */
+ *LockedKcbs = CmpBuildAndLockKcbArray(HashCacheStack,
+ CMP_LOCK_KCB_ARRAY_EXCLUSIVE,
+ *Kcb,
+ OuterStackArray,
+ RemainingSubkeysInTotal,
+ MatchRemainingSubkeys);
+ NT_ASSERT(*LockedKcbs);
+ }
+
+ /* Return all the subkey level counters */
+ *TotalRemainingSubkeys = RemainingSubkeysInTotal;
+ *MatchRemainSubkeyLevel = MatchRemainingSubkeys;
+ *TotalSubkeys = SubkeysInTotal;
+ return Status;
}
NTSTATUS
@@ -1064,7 +1680,9 @@ CmpParseKey(IN PVOID ParseObject,
UNICODE_STRING Current, NextName;
PCM_PARSE_CONTEXT ParseContext = Context;
ULONG TotalRemainingSubkeys = 0, MatchRemainSubkeyLevel = 0, TotalSubkeys = 0;
- PULONG LockedKcbs = NULL;
+ ULONG LockedKcbArray[CMP_KCBS_IN_ARRAY_LIMIT];
+ PULONG LockedKcbs;
+ BOOLEAN IsKeyCached = FALSE;
BOOLEAN Result, Last;
PAGED_CODE();
@@ -1109,8 +1727,15 @@ CmpParseKey(IN PVOID ParseObject,
&TotalRemainingSubkeys,
&MatchRemainSubkeyLevel,
&TotalSubkeys,
- NULL,
+ LockedKcbArray,
&LockedKcbs);
+ CMP_ASSERT_REGISTRY_LOCK();
+ if (!NT_SUCCESS(Status))
+ {
+ DPRINT1("Failed to look in cache, stop parsing (Status 0x%lx)\n",
Status);
+ ParentKcb = NULL;
+ goto Quickie;
+ }
/* This is now the parent */
ParentKcb = Kcb;
@@ -1118,10 +1743,6 @@ CmpParseKey(IN PVOID ParseObject,
/* Sanity check */
ASSERT(ParentKcb != NULL);
- /* Check if everything was found cached */
- if (!TotalRemainingSubkeys)
- ASSERTMSG("Caching not implemented\n", FALSE);
-
/* Don't do anything if we're being deleted */
if (Kcb->Delete)
{
@@ -1129,6 +1750,51 @@ CmpParseKey(IN PVOID ParseObject,
goto Quickie;
}
+ /* Check if everything was found cached */
+ if (!TotalRemainingSubkeys)
+ {
+ /*
+ * We don't have any remaining subkey levels so we're good
+ * that we have an already perfect candidate for a KCB, just
+ * do the open directly.
+ */
+ DPRINT("No remaining subkeys, the KCB points to the actual key\n");
+ IsKeyCached = TRUE;
+ goto KeyCachedOpenNow;
+ }
+
+ /* Check if we have a matching level */
+ if (MatchRemainSubkeyLevel)
+ {
+ /*
+ * We have a matching level, check if that matches
+ * with the total levels of subkeys. Do the open directly
+ * if that is the case, because the whole subkeys levels
+ * is cached.
+ */
+ if (MatchRemainSubkeyLevel == TotalSubkeys)
+ {
+ DPRINT("We have a full matching level, open the key now\n");
+ IsKeyCached = TRUE;
+ goto KeyCachedOpenNow;
+ }
+
+ /*
+ * We only have a partial match, make sure we did not
+ * get mismatched hive data.
+ */
+ ASSERT(Hive == Kcb->KeyHive);
+ ASSERT(Cell == Kcb->KeyCell);
+ }
+
+ /*
+ * FIXME: Currently the registry parser doesn't check for fake
+ * KCBs. CmpCreateKeyControlBlock does have the necessary implementation
+ * to create such fake keys but we don't create these fake keys anywhere.
+ * When we will do, we must improve the registry parser routine to handle
+ * fake keys a bit differently here.
+ */
+
/* Check if this is a symlink */
if (Kcb->Flags & KEY_SYM_LINK)
{
@@ -1154,6 +1820,10 @@ CmpParseKey(IN PVOID ParseObject,
}
Current.MaximumLength += NextName.MaximumLength;
+ /* CmpGetSymbolicLink doesn't want a lock */
+ CmpUnLockKcbArray(LockedKcbs);
+ LockedKcbs = NULL;
+
/* Parse the symlink */
if (CmpGetSymbolicLink(Hive,
CompleteName,
@@ -1221,6 +1891,7 @@ CmpParseKey(IN PVOID ParseObject,
}
}
+KeyCachedOpenNow:
/* Do the open */
Status = CmpDoOpen(Hive,
Cell,
@@ -1229,12 +1900,17 @@ CmpParseKey(IN PVOID ParseObject,
AccessMode,
Attributes,
ParseContext,
- 0,
+ IsKeyCached ? CMP_OPEN_KCB_NO_CREATE :
CMP_CREATE_KCB_KCB_LOCKED,
&Kcb,
+ LockedKcbs,
&NextName,
Object);
if (Status == STATUS_REPARSE)
{
+ /* CmpGetSymbolicLink doesn't want a lock */
+ CmpUnLockKcbArray(LockedKcbs);
+ LockedKcbs = NULL;
+
/* Parse the symlink */
if (!CmpGetSymbolicLink(Hive,
CompleteName,
@@ -1272,7 +1948,7 @@ CmpParseKey(IN PVOID ParseObject,
Cell,
Node,
ParentKcb,
- 0,
+ CMP_LOCK_HASHES_FOR_KCB,
&NextName);
if (!Kcb)
{
@@ -1282,7 +1958,7 @@ CmpParseKey(IN PVOID ParseObject,
}
/* Dereference the parent and set the new one */
- CmpDereferenceKeyControlBlock(ParentKcb);
+ CmpDereferenceKeyControlBlockWithLock(ParentKcb, FALSE);
ParentKcb = Kcb;
}
else
@@ -1302,6 +1978,7 @@ CmpParseKey(IN PVOID ParseObject,
Attributes,
ParseContext,
ParentKcb,
+ LockedKcbs,
Object);
}
else if (Hive == &CmiVolatileHive->Hive &&
CmpNoVolatileCreates)
@@ -1360,6 +2037,10 @@ CmpParseKey(IN PVOID ParseObject,
}
Current.MaximumLength += NextName.MaximumLength;
+ /* CmpGetSymbolicLink doesn't want a lock */
+ CmpUnLockKcbArray(LockedKcbs);
+ LockedKcbs = NULL;
+
/* Parse the symlink */
if (CmpGetSymbolicLink(Hive,
CompleteName,
@@ -1406,8 +2087,9 @@ CmpParseKey(IN PVOID ParseObject,
AccessMode,
Attributes,
ParseContext,
- CMP_OPEN_KCB_NO_CREATE /* | CMP_CREATE_KCB_KCB_LOCKED
*/,
+ CMP_OPEN_KCB_NO_CREATE,
&Kcb,
+ LockedKcbs,
&NextName,
Object);
if (Status == STATUS_REPARSE)
@@ -1426,8 +2108,14 @@ CmpParseKey(IN PVOID ParseObject,
}
}
- /* Dereference the parent if it exists */
Quickie:
+ /* Unlock all the KCBs */
+ if (LockedKcbs != NULL)
+ {
+ CmpUnLockKcbArray(LockedKcbs);
+ }
+
+ /* Dereference the parent if it exists */
if (ParentKcb)
CmpDereferenceKeyControlBlock(ParentKcb);
diff --git a/ntoskrnl/include/internal/cm.h b/ntoskrnl/include/internal/cm.h
index eab9a1c9b61..db702bd8b28 100644
--- a/ntoskrnl/include/internal/cm.h
+++ b/ntoskrnl/include/internal/cm.h
@@ -1133,12 +1133,12 @@ CmpCreateLinkNode(
IN PHHIVE Hive,
IN HCELL_INDEX Cell,
IN PACCESS_STATE AccessState,
- IN PULONG KcbsLocked,
IN UNICODE_STRING Name,
IN KPROCESSOR_MODE AccessMode,
IN ULONG CreateOptions,
IN PCM_PARSE_CONTEXT Context,
IN PCM_KEY_CONTROL_BLOCK ParentKcb,
+ IN PULONG KcbsLocked,
OUT PVOID *Object
);
405
days inactive
405
days old
0 comments
1 participants
participants (1)
-
George Bișoc