- Add a signature field to KCBs, and use it to validate KCBs as well as to mark them dead after they've been freed (to catch invalid reuse).

- Add assertions to make sure key hashes are valid.
- Finish implementing EnlistKeyBodyWithKCB since the new parse routine is now used, and implement analogous DelistKeyBodyFromKCB.
- Fix delay item allocation code which was overwriting pool memory.
- Re-enable allocation-page support for allocating KCBs.
- Implement the CmpDelayDerefKCBWorker now that delay alloc/free works.
- Fix CmpDelayDerefKeyControlBlock which was overwriting KCB flags and incorrectly setting reference counts for the KCB.
- CmpDelayCloseWorkItemActive should be enabled when arming the delayed close timer.
- Fix reference counting bugs in CmpAddToDelayedClose and CmpRemoveFromDelayedClose.
- Fix wrong sizes of CM_KCBS_PER_PAGE and CM_DELAYS_PER_PAGE which caused more pool overwrites.

svn path=/trunk/; revision=31282
This commit is contained in:
Aleksey Bragin 2007-12-16 16:37:15 +00:00
parent db48b40873
commit 8918c7ecb0
6 changed files with 235 additions and 60 deletions

View file

@ -55,7 +55,7 @@ CmpFreeKeyControlBlock(IN PCM_KEY_CONTROL_BLOCK Kcb)
PAGED_CODE();
/* Sanity checks */
ASSERT(IsListEmpty(&(Kcb->KeyBodyListHead)) == TRUE);
ASSERT(IsListEmpty(&Kcb->KeyBodyListHead) == TRUE);
for (i = 0; i < 4; i++) ASSERT(Kcb->KeyBodyArray[i] == NULL);
/* Check if it wasn't privately allocated */
@ -87,12 +87,12 @@ CmpFreeKeyControlBlock(IN PCM_KEY_CONTROL_BLOCK Kcb)
if (++AllocPage->FreeCount == CM_KCBS_PER_PAGE)
{
/* Loop all the entries */
for (i = CM_KCBS_PER_PAGE; i; i--)
for (i = 0; i < CM_KCBS_PER_PAGE; i++)
{
/* Get the KCB */
Kcb = (PVOID)((ULONG_PTR)AllocPage +
FIELD_OFFSET(CM_ALLOC_PAGE, AllocPage) +
(i - 1) * sizeof(CM_KEY_CONTROL_BLOCK));
i * sizeof(CM_KEY_CONTROL_BLOCK));
/* Remove the entry */
RemoveEntryList(&Kcb->FreeListEntry);
@ -117,7 +117,7 @@ CmpAllocateKeyControlBlock(VOID)
PAGED_CODE();
/* Check if private allocations are initialized */
if (FALSE)
if (CmpAllocInited)
{
/* They are, acquire the bucket lock */
KeAcquireGuardedMutex(&CmpAllocBucketLock);
@ -208,28 +208,28 @@ CmpAllocateDelayItem(VOID)
/* Look for an item on the free list */
SearchList:
if (!IsListEmpty(&CmpFreeDelayItemsListHead))
{
/* Get the current entry in the list */
NextEntry = RemoveHeadList(&CmpFreeDelayItemsListHead);
/* Grab the item */
Entry = CONTAINING_RECORD(NextEntry, CM_DELAY_ALLOC, ListEntry);
/* Clear the list */
Entry->ListEntry.Flink = Entry->ListEntry.Blink = NULL;
/* Grab the alloc page */
AllocPage = (PCM_ALLOC_PAGE)((ULONG_PTR)Entry & 0xFFFFF000);
/* Decrease free entries */
ASSERT(AllocPage->FreeCount != 0);
AllocPage->FreeCount--;
/* Release the lock */
KeReleaseGuardedMutex(&CmpDelayAllocBucketLock);
return Entry;
}
if (!IsListEmpty(&CmpFreeDelayItemsListHead))
{
/* Get the current entry in the list */
NextEntry = RemoveHeadList(&CmpFreeDelayItemsListHead);
/* Grab the item */
Entry = CONTAINING_RECORD(NextEntry, CM_DELAY_ALLOC, ListEntry);
/* Clear the list */
Entry->ListEntry.Flink = Entry->ListEntry.Blink = NULL;
/* Grab the alloc page */
AllocPage = (PCM_ALLOC_PAGE)((ULONG_PTR)Entry & 0xFFFFF000);
/* Decrease free entries */
ASSERT(AllocPage->FreeCount != 0);
AllocPage->FreeCount--;
/* Release the lock */
KeReleaseGuardedMutex(&CmpDelayAllocBucketLock);
return Entry;
}
/* Allocate an allocation page */
AllocPage = ExAllocatePoolWithTag(PagedPool, PAGE_SIZE, TAG_CM);

View file

@ -100,15 +100,41 @@ VOID
NTAPI
CmpDelayDerefKCBWorker(IN PVOID Context)
{
PCM_DELAY_DEREF_KCB_ITEM Entry;
PAGED_CODE();
/* Sanity check */
ASSERT(CmpDelayDerefKCBWorkItemActive);
/* FIXME: TODO */
DPRINT1("CmpDelayDerefKCBWorker has work to do!\n");
return;
ASSERT(FALSE);
/* Lock the registry and and list lock */
CmpLockRegistry();
KeAcquireGuardedMutex(&CmpDelayDerefKCBLock);
/* Check if the list is empty */
while (!IsListEmpty(&CmpDelayDerefKCBListHead))
{
/* Grab an entry */
Entry = (PVOID)RemoveHeadList(&CmpDelayDerefKCBListHead);
/* We can release the lock now */
KeReleaseGuardedMutex(&CmpDelayDerefKCBLock);
/* Now grab the actual entry */
Entry = CONTAINING_RECORD(Entry, CM_DELAY_DEREF_KCB_ITEM, ListEntry);
Entry->ListEntry.Flink = Entry->ListEntry.Blink = NULL;
/* Dereference and free */
CmpDereferenceKeyControlBlock(Entry->Kcb);
CmpFreeDelayItem(Entry);
/* Lock the list again */
KeAcquireGuardedMutex(&CmpDelayDerefKCBLock);
}
/* We're done */
CmpDelayDerefKCBWorkItemActive = FALSE;
KeReleaseGuardedMutex(&CmpDelayDerefKCBLock);
CmpUnlockRegistry();
}
VOID
@ -133,19 +159,22 @@ VOID
NTAPI
CmpDelayDerefKeyControlBlock(IN PCM_KEY_CONTROL_BLOCK Kcb)
{
USHORT OldRefCount, NewRefCount;
LONG OldRefCount, NewRefCount;
LARGE_INTEGER Timeout;
PCM_DELAY_DEREF_KCB_ITEM Entry;
PAGED_CODE();
/* Get the previous reference count */
OldRefCount = Kcb->RefCount;
/* Write the new one */
NewRefCount = (USHORT)InterlockedCompareExchange((PLONG)&Kcb->RefCount,
OldRefCount - 1,
OldRefCount);
if (NewRefCount != OldRefCount) return;
OldRefCount = *(PLONG)&Kcb->RefCount;
NewRefCount = OldRefCount - 1;
if (((NewRefCount & 0xFFFF) > 0) &&
(InterlockedCompareExchange((PLONG)&Kcb->RefCount,
NewRefCount,
OldRefCount) == OldRefCount))
{
/* KCB still had references, so we're done */
return;
}
/* Allocate a delay item */
Entry = CmpAllocateDelayItem();
@ -179,6 +208,9 @@ CmpArmDelayedCloseTimer(VOID)
{
LARGE_INTEGER Timeout;
PAGED_CODE();
/* Set the worker active */
CmpDelayCloseWorkItemActive = TRUE;
/* Setup the interval */
Timeout.QuadPart = CmpDelayCloseIntervalInSeconds * -10000000;
@ -220,14 +252,18 @@ CmpAddToDelayedClose(IN PCM_KEY_CONTROL_BLOCK Kcb,
if (Kcb->InDelayClose) ASSERT(FALSE);
/* Get the previous reference count */
OldRefCount = Kcb->InDelayClose;
OldRefCount = *(PLONG)&Kcb->InDelayClose;
ASSERT(OldRefCount == 0);
/* Write the new one */
NewRefCount = InterlockedCompareExchange((PLONG)&Kcb->InDelayClose,
1,
OldRefCount);
if (NewRefCount != OldRefCount) ASSERT(FALSE);
NewRefCount = 1;
if (InterlockedCompareExchange((PLONG)&Kcb->InDelayClose,
NewRefCount,
OldRefCount) != OldRefCount)
{
/* Sanity check */
ASSERT(FALSE);
}
/* Reset the delayed close index */
Kcb->DelayedCloseIndex = 0;
@ -290,15 +326,19 @@ CmpRemoveFromDelayedClose(IN PCM_KEY_CONTROL_BLOCK Kcb)
/* Sanity check */
if (!Kcb->InDelayClose) ASSERT(FALSE);
/* Get the old reference count */
OldRefCount = Kcb->InDelayClose;
/* Get the previous reference count */
OldRefCount = *(PLONG)&Kcb->InDelayClose;
ASSERT(OldRefCount == 1);
/* Set it to 0 */
NewRefCount = InterlockedCompareExchange((PLONG)&Kcb->InDelayClose,
0,
OldRefCount);
if (NewRefCount != OldRefCount) ASSERT(FALSE);
/* Write the new one */
NewRefCount = 0;
if (InterlockedCompareExchange((PLONG)&Kcb->InDelayClose,
NewRefCount,
OldRefCount) != OldRefCount)
{
/* Sanity check */
ASSERT(FALSE);
}
/* Remove the link to the entry */
Kcb->DelayCloseEntry = NULL;
@ -307,5 +347,3 @@ CmpRemoveFromDelayedClose(IN PCM_KEY_CONTROL_BLOCK Kcb)
Kcb->DelayedCloseIndex = CmpDelayedCloseSize;
}

View file

@ -80,6 +80,7 @@ CmpRemoveKeyHash(IN PCM_KEY_HASH KeyHash)
{
PCM_KEY_HASH *Prev;
PCM_KEY_HASH Current;
ASSERT_VALID_HASH(KeyHash);
/* Lookup all the keys in this index entry */
Prev = &GET_HASH_ENTRY(CmpCacheTable, KeyHash->ConvKey).Entry;
@ -88,12 +89,14 @@ CmpRemoveKeyHash(IN PCM_KEY_HASH KeyHash)
/* Save the current one and make sure it's valid */
Current = *Prev;
ASSERT(Current != NULL);
ASSERT_VALID_HASH(Current);
/* Check if it matches */
if (Current == KeyHash)
{
/* Then write the previous one */
*Prev = Current->NextHash;
if (*Prev) ASSERT_VALID_HASH(*Prev);
break;
}
@ -109,6 +112,7 @@ CmpInsertKeyHash(IN PCM_KEY_HASH KeyHash,
{
ULONG i;
PCM_KEY_HASH Entry;
ASSERT_VALID_HASH(KeyHash);
/* Get the hash index */
i = GET_HASH_INDEX(KeyHash->ConvKey);
@ -121,6 +125,7 @@ CmpInsertKeyHash(IN PCM_KEY_HASH KeyHash,
while (Entry)
{
/* Check if this matches */
ASSERT_VALID_HASH(Entry);
if ((KeyHash->ConvKey == Entry->ConvKey) &&
(KeyHash->KeyCell == Entry->KeyCell) &&
(KeyHash->KeyHive == Entry->KeyHive))
@ -488,6 +493,9 @@ CmpCleanUpKcbCacheWithLock(IN PCM_KEY_CONTROL_BLOCK Kcb,
Parent = Kcb->ParentKcb;
if (!Kcb->Delete) CmpRemoveKeyControlBlock(Kcb);
/* Set invalid KCB signature */
Kcb->Signature = CM_KCB_INVALID_SIGNATURE;
/* Free the KCB as well */
CmpFreeKeyControlBlock(Kcb);
@ -595,12 +603,15 @@ CmpDereferenceKeyControlBlockWithLock(IN PCM_KEY_CONTROL_BLOCK Kcb,
IN BOOLEAN LockHeldExclusively)
{
/* Sanity check */
ASSERT((CmpIsKcbLockedExclusive(Kcb) == TRUE) ||
(CmpTestRegistryLockExclusive() == TRUE));
ASSERT_KCB_VALID(Kcb);
/* Check if this is the last reference */
if ((InterlockedDecrement((PLONG)&Kcb->RefCount) & 0xFFFF) == 0)
{
/* Sanity check */
ASSERT((CmpIsKcbLockedExclusive(Kcb) == TRUE) ||
(CmpTestRegistryLockExclusive() == TRUE));
/* Check if we should do a direct delete */
if (((CmpHoldLazyFlush) &&
!(Kcb->ExtFlags & CM_KCB_SYM_LINK_FOUND) &&
@ -699,6 +710,7 @@ CmpCreateKeyControlBlock(IN PHHIVE Hive,
InitializeKCBKeyBodyList(Kcb);
/* Set it up */
Kcb->Signature = CM_KCB_SIGNATURE;
Kcb->Delete = FALSE;
Kcb->RefCount = 1;
Kcb->KeyHive = Hive;
@ -706,6 +718,7 @@ CmpCreateKeyControlBlock(IN PHHIVE Hive,
Kcb->ConvKey = ConvKey;
Kcb->DelayedCloseIndex = CmpDelayedCloseSize;
Kcb->InDelayClose = 0;
ASSERT_KCB_VALID(Kcb);
/* Check if we have two hash entires */
HashLock = Flags & CMP_LOCK_HASHES_FOR_KCB ? TRUE : FALSE;
@ -730,9 +743,11 @@ CmpCreateKeyControlBlock(IN PHHIVE Hive,
{
/* Sanity check */
ASSERT(!FoundKcb->Delete);
Kcb->Signature = CM_KCB_INVALID_SIGNATURE;
/* Free the one we allocated and reference this one */
CmpFreeKeyControlBlock(Kcb);
ASSERT_KCB_VALID(FoundKcb);
Kcb = FoundKcb;
if (!CmpReferenceKeyControlBlock(Kcb))
{
@ -790,6 +805,7 @@ CmpCreateKeyControlBlock(IN PHHIVE Hive,
{
/* Remove the KCB and free it */
CmpRemoveKeyControlBlock(Kcb);
Kcb->Signature = CM_KCB_INVALID_SIGNATURE;
CmpFreeKeyControlBlock(Kcb);
Kcb = NULL;
}
@ -833,12 +849,20 @@ CmpCreateKeyControlBlock(IN PHHIVE Hive,
/* Remove the KCB and free it */
CmpRemoveKeyControlBlock(Kcb);
Kcb->Signature = CM_KCB_INVALID_SIGNATURE;
CmpFreeKeyControlBlock(Kcb);
Kcb = NULL;
}
}
}
/* Check if this is a KCB inside a frozen hive */
if ((Kcb) && (((PCMHIVE)Hive)->Frozen) && (!(Kcb->Flags & KEY_SYM_LINK)))
{
/* Don't add these to the delay close */
Kcb->ExtFlags |= CM_KCB_NO_DELAY_CLOSE;
}
/* Sanity check */
ASSERT((!Kcb) || (Kcb->Delete == FALSE));
@ -867,12 +891,98 @@ NTAPI
EnlistKeyBodyWithKCB(IN PCM_KEY_BODY KeyBody,
IN ULONG Flags)
{
ULONG i;
/* Sanity check */
ASSERT(KeyBody->KeyControlBlock != NULL);
/* Initialize the list entry */
InitializeListHead(&KeyBody->KeyBodyList);
/* FIXME: Implement once we don't link parents to children anymore */
/* Check if we can use the parent KCB array */
for (i = 0; i < 4; i++)
{
/* Add it into the list */
if (!InterlockedCompareExchangePointer(&KeyBody->KeyControlBlock->
KeyBodyArray[i],
KeyBody,
NULL))
{
/* Added */
return;
}
}
/* Array full, check if we need to unlock the KCB */
if (Flags & CMP_ENLIST_KCB_LOCKED_SHARED)
{
/* It's shared, so release the KCB shared lock */
CmpReleaseKcbLock(KeyBody->KeyControlBlock);
ASSERT(!(Flags & CMP_ENLIST_KCB_LOCKED_EXCLUSIVE));
}
/* Check if we need to lock the KCB */
if (!(Flags & CMP_ENLIST_KCB_LOCKED_EXCLUSIVE))
{
/* Acquire the lock */
CmpAcquireKcbLockExclusive(KeyBody->KeyControlBlock);
}
/* Make sure we have the exclusive lock */
ASSERT((CmpIsKcbLockedExclusive(KeyBody->KeyControlBlock) == TRUE) ||
(CmpTestRegistryLockExclusive() == TRUE));
/* do the insert */
InsertTailList(&KeyBody->KeyControlBlock->KeyBodyListHead,
&KeyBody->KeyBodyList);
/* Check if we did a manual lock */
if (!(Flags & (CMP_ENLIST_KCB_LOCKED_SHARED |
CMP_ENLIST_KCB_LOCKED_EXCLUSIVE)))
{
/* Release the lock */
CmpReleaseKcbLock(KeyBody->KeyControlBlock);
}
}
VOID
NTAPI
DelistKeyBodyFromKCB(IN PCM_KEY_BODY KeyBody,
IN BOOLEAN LockHeld)
{
ULONG i;
/* Sanity check */
ASSERT(KeyBody->KeyControlBlock != NULL);
/* Check if we can use the parent KCB array */
for (i = 0; i < 4; i++)
{
/* Add it into the list */
if (InterlockedCompareExchangePointer(&KeyBody->KeyControlBlock->
KeyBodyArray[i],
NULL,
KeyBody) == KeyBody)
{
/* Removed */
return;
}
}
/* Sanity checks */
ASSERT(IsListEmpty(&KeyBody->KeyControlBlock->KeyBodyListHead) == FALSE);
ASSERT(IsListEmpty(&KeyBody->KeyBodyList) == FALSE);
/* Lock the KCB */
if (!LockHeld) CmpAcquireKcbLockExclusive(KeyBody->KeyControlBlock);
ASSERT((CmpIsKcbLockedExclusive(KeyBody->KeyControlBlock) == TRUE) ||
(CmpTestRegistryLockExclusive() == TRUE));
/* Remove the entry */
RemoveEntryList(&KeyBody->KeyBodyList);
/* Unlock it it if we did a manual lock */
if (!LockHeld) CmpReleaseKcbLock(KeyBody->KeyControlBlock);
}

View file

@ -66,8 +66,8 @@ CmpDeleteKeyObject(PVOID DeletedObject)
Kcb = KeyBody->KeyControlBlock;
if (Kcb)
{
/* Delist the key (once new parse routines are used) */
//DelistKeyBodyFromKCB(KeyBody, FALSE);
/* Delist the key */
DelistKeyBodyFromKCB(KeyBody, FALSE);
}
/* Dereference the KCB */

View file

@ -36,13 +36,18 @@
#define CMTRACE(x, ...) DPRINT(__VA_ARGS__)
#endif
//
// Hack since bigkeys are not yet supported
//
#define ASSERT_VALUE_BIG(h, s) \
ASSERTMSG("Big keys not supported!", !CmpIsKeyValueBig(h, s));
//
// CM_KEY_CONTROL_BLOCK Signatures
//
#define CM_KCB_SIGNATURE TAG('C', 'm', 'K', 'b')
#define CM_KCB_INVALID_SIGNATURE TAG('C', 'm', 'F', '4')
//
// CM_KEY_CONTROL_BLOCK Flags
//
@ -80,6 +85,12 @@
#define CMP_CREATE_FAKE_KCB 0x1
#define CMP_LOCK_HASHES_FOR_KCB 0x2
//
// EnlistKeyBodyWithKCB Flags
//
#define CMP_ENLIST_KCB_LOCKED_SHARED 0x1
#define CMP_ENLIST_KCB_LOCKED_EXCLUSIVE 0x2
//
// Maximum size of Value Cache
//
@ -94,9 +105,9 @@
// Number of items that can fit inside an Allocation Page
//
#define CM_KCBS_PER_PAGE \
PAGE_SIZE / sizeof(CM_KEY_CONTROL_BLOCK)
((PAGE_SIZE - FIELD_OFFSET(CM_ALLOC_PAGE, AllocPage)) / sizeof(CM_KEY_CONTROL_BLOCK))
#define CM_DELAYS_PER_PAGE \
PAGE_SIZE / sizeof(CM_DELAYED_CLOSE_ENTRY)
((PAGE_SIZE - FIELD_OFFSET(CM_ALLOC_PAGE, AllocPage)) / sizeof(CM_DELAY_ALLOC))
//
// Value Search Results
@ -229,6 +240,7 @@ typedef struct _CM_NAME_CONTROL_BLOCK
//
typedef struct _CM_KEY_CONTROL_BLOCK
{
ULONG Signature;
USHORT RefCount;
USHORT Flags;
struct
@ -907,6 +919,13 @@ EnlistKeyBodyWithKCB(
IN ULONG Flags
);
VOID
NTAPI
DelistKeyBodyFromKCB(
IN PCM_KEY_BODY KeyBody,
IN BOOLEAN LockHeld
);
NTSTATUS
NTAPI
CmpFreeKeyByCell(

View file

@ -63,6 +63,8 @@ CmpIsKeyValueBig(IN PHHIVE Hive,
GET_HASH_KEY(ConvKey) % CmpHashTableSize
#define GET_HASH_ENTRY(Table, ConvKey) \
(Table[GET_HASH_INDEX(ConvKey)])
#define ASSERT_VALID_HASH(h) \
ASSERT_KCB_VALID(CONTAINING_RECORD((h), CM_KEY_CONTROL_BLOCK, KeyHash))
//
// Returns whether or not the cell is cached
@ -96,6 +98,12 @@ CmpIsKeyValueBig(IN PHHIVE Hive,
ASSERT((CmpSpecialBootCondition == TRUE) || \
(CmpTestRegistryLockExclusive() == TRUE))
//
// Makes sure this is a valid KCB
//
#define ASSERT_KCB_VALID(k) \
ASSERT((k)->Signature == CM_KCB_SIGNATURE)
//
// Checks if a KCB is exclusively locked
//