From d7eb720f13d02e04cb86757f0e173c3e5898f77a Mon Sep 17 00:00:00 2001 From: Aleksey Bragin Date: Tue, 4 Dec 2007 18:44:51 +0000 Subject: [PATCH] Do a full clean rebuild after this commit! - Begin fixing CM_KEY_NODE. - More proper use of registry lock in create operations. - Use ObOpenObjectByName (as a test only, ignore the results) in CmpLinkHiveToMaster. - Implement routines to acquire/release two KCB locks atomically. - Implement KCB dereference without lock. - Fix locking in CmpCreateKeyControlBlock so that it now properly locks KCBs when requested. - Implement rudimentary new-style parse routine that only walks the registry path until it's time to create the link node (since this is the only scenario where it's called from). Prints out the status of the operation but doesn't actually do anything. svn path=/trunk/; revision=31004 --- reactos/lib/cmlib/cmdata.h | 18 ++ reactos/ntoskrnl/cm/ntfunc.c | 6 + reactos/ntoskrnl/cm/regobj.c | 47 ++++-- reactos/ntoskrnl/config/cm.h | 35 +++- reactos/ntoskrnl/config/cmkcbncb.c | 64 +++++++- reactos/ntoskrnl/config/cmparse.c | 255 ++++++++++++++++++++++++++++- reactos/ntoskrnl/config/cmsysini.c | 79 +++++++++ 7 files changed, 476 insertions(+), 28 deletions(-) diff --git a/reactos/lib/cmlib/cmdata.h b/reactos/lib/cmlib/cmdata.h index cc83e69d0f8..74cb8dbf3d8 100644 --- a/reactos/lib/cmlib/cmdata.h +++ b/reactos/lib/cmlib/cmdata.h @@ -76,6 +76,15 @@ typedef struct _CHILD_LIST HCELL_INDEX List; } CHILD_LIST, *PCHILD_LIST; +// +// Node Key Reference to Parents +// +typedef struct _CM_KEY_REFERENCE +{ + HCELL_INDEX KeyCell; + PHHIVE KeyHive; +} CM_KEY_REFERENCE, *PCM_KEY_REFERENCE; + // // Node Key // @@ -87,6 +96,15 @@ typedef struct _CM_KEY_NODE ULONG Spare; HCELL_INDEX Parent; ULONG SubKeyCounts[HTYPE_COUNT]; + union + { + struct + { + HCELL_INDEX SubKeyLists[HTYPE_COUNT]; + CHILD_LIST ValueList; + }; + //CM_KEY_REFERENCE ChildHiveReference; + }; HCELL_INDEX SubKeyLists[HTYPE_COUNT]; CHILD_LIST ValueList; HCELL_INDEX Security; diff --git a/reactos/ntoskrnl/cm/ntfunc.c b/reactos/ntoskrnl/cm/ntfunc.c index c29ba0c6bf0..6c2681dd39a 100644 --- a/reactos/ntoskrnl/cm/ntfunc.c +++ b/reactos/ntoskrnl/cm/ntfunc.c @@ -136,6 +136,9 @@ NtCreateKey(OUT PHANDLE KeyHandle, Status = STATUS_OBJECT_NAME_NOT_FOUND; goto Cleanup; } + + /* Lock the registry */ + CmpLockRegistry(); /* Create the key */ Status = CmpDoCreate(Parent->KeyControlBlock->KeyHive, @@ -187,6 +190,9 @@ NtCreateKey(OUT PHANDLE KeyHandle, /* Add the keep-alive reference */ ObReferenceObject(KeyObject); + + /* Unlock registry */ + CmpUnlockRegistry(); /* Force a lazy flush */ CmpLazyFlush(); diff --git a/reactos/ntoskrnl/cm/regobj.c b/reactos/ntoskrnl/cm/regobj.c index 44bf9466401..7e5a5123da9 100644 --- a/reactos/ntoskrnl/cm/regobj.c +++ b/reactos/ntoskrnl/cm/regobj.c @@ -493,20 +493,31 @@ CmiScanKeyList(PKEY_OBJECT Parent, return STATUS_SUCCESS; } - +NTSTATUS +NTAPI +CmpParseKey2(IN PVOID ParseObject, + IN PVOID ObjectType, + IN OUT PACCESS_STATE AccessState, + IN KPROCESSOR_MODE AccessMode, + IN ULONG Attributes, + IN OUT PUNICODE_STRING CompleteName, + IN OUT PUNICODE_STRING RemainingName, + IN OUT PVOID Context OPTIONAL, + IN PSECURITY_QUALITY_OF_SERVICE SecurityQos OPTIONAL, + OUT PVOID *Object); NTSTATUS NTAPI CmpParseKey(IN PVOID ParsedObject, - IN PVOID ObjectType, - IN OUT PACCESS_STATE AccessState, - IN KPROCESSOR_MODE AccessMode, - IN ULONG Attributes, - IN OUT PUNICODE_STRING FullPath, - IN OUT PUNICODE_STRING RemainingName, - IN OUT PVOID Context OPTIONAL, - IN PSECURITY_QUALITY_OF_SERVICE SecurityQos OPTIONAL, - OUT PVOID *NextObject) + IN PVOID ObjectType, + IN OUT PACCESS_STATE AccessState, + IN KPROCESSOR_MODE AccessMode, + IN ULONG Attributes, + IN OUT PUNICODE_STRING FullPath, + IN OUT PUNICODE_STRING RemainingName, + IN OUT PVOID Context OPTIONAL, + IN PSECURITY_QUALITY_OF_SERVICE SecurityQos OPTIONAL, + OUT PVOID *NextObject) { HCELL_INDEX BlockOffset; PKEY_OBJECT FoundObject; @@ -523,6 +534,22 @@ CmpParseKey(IN PVOID ParsedObject, PCM_KEY_CONTROL_BLOCK ParentKcb = NULL, Kcb; PCM_KEY_NODE Node; + /* Detect new-style parse */ + if (Context) + { + /* Call proper parse routine */ + return CmpParseKey2(ParsedObject, + ObjectType, + AccessState, + AccessMode, + Attributes, + FullPath, + RemainingName, + Context, + SecurityQos, + NextObject); + } + ParsedKey = ParsedObject; VERIFY_KEY_OBJECT(ParsedKey); diff --git a/reactos/ntoskrnl/config/cm.h b/reactos/ntoskrnl/config/cm.h index 850837f99ba..787935ed2e2 100644 --- a/reactos/ntoskrnl/config/cm.h +++ b/reactos/ntoskrnl/config/cm.h @@ -193,15 +193,6 @@ typedef struct _CM_INDEX_HINT_BLOCK ULONG HashKey[ANYSIZE_ARRAY]; } CM_INDEX_HINT_BLOCK, *PCM_INDEX_HINT_BLOCK; -// -// Key Reference -// -typedef struct _CM_KEY_REFERENCE -{ - HCELL_INDEX KeyCell; - PHHIVE KeyHive; -} CM_KEY_REFERENCE, *PCM_KEY_REFERENCE; - // // Key Body // @@ -782,6 +773,12 @@ CmpTestRegistryLockExclusive( VOID ); +BOOLEAN +NTAPI +CmpTestRegistryLock( + VOID +); + VOID NTAPI CmpLockRegistryExclusive( @@ -884,6 +881,12 @@ CmpDereferenceKeyControlBlockWithLock( IN BOOLEAN LockHeldExclusively ); +VOID +NTAPI +CmpDereferenceKeyControlBlock( + IN PCM_KEY_CONTROL_BLOCK Kcb +); + VOID NTAPI EnlistKeyBodyWithKCB( @@ -906,6 +909,20 @@ CmpFreeKeyByCell( IN BOOLEAN Unlink ); +VOID +NTAPI +CmpAcquireTwoKcbLocksExcusiveByKey( + IN ULONG ConvKey1, + IN ULONG ConvKey2 +); + +VOID +NTAPI +CmpReleaseTwoKcbLockByKey( + IN ULONG ConvKey1, + IN ULONG ConvKey2 +); + // // Name Functions // diff --git a/reactos/ntoskrnl/config/cmkcbncb.c b/reactos/ntoskrnl/config/cmkcbncb.c index e768fc92279..07f9ed030c1 100644 --- a/reactos/ntoskrnl/config/cmkcbncb.c +++ b/reactos/ntoskrnl/config/cmkcbncb.c @@ -501,6 +501,39 @@ CmpCleanUpKcbCacheWithLock(IN PCM_KEY_CONTROL_BLOCK Kcb, } } +VOID +NTAPI +CmpDereferenceKeyControlBlock(IN PCM_KEY_CONTROL_BLOCK Kcb) +{ + LONG OldRefCount, NewRefCount; + ULONG ConvKey; + + /* Get the ref count and update it */ + OldRefCount = *(PLONG)&Kcb->RefCount; + NewRefCount = OldRefCount - 1; + + /* Check if we still have refenreces */ + if( (NewRefCount & 0xffff) > 0) + { + /* Do the dereference */ + if (InterlockedCompareExchange((PLONG)&Kcb->RefCount, + NewRefCount, + OldRefCount) == OldRefCount) + { + /* We'de done */ + return; + } + } + + /* Save the key */ + ConvKey = Kcb->ConvKey; + + /* Do the dereference inside the lock */ + CmpAcquireKcbLockExclusive(Kcb); + CmpDereferenceKeyControlBlockWithLock(Kcb, FALSE); + CmpReleaseKcbLockByKey(ConvKey); +} + VOID NTAPI CmpDereferenceKeyControlBlockWithLock(IN PCM_KEY_CONTROL_BLOCK Kcb, @@ -621,10 +654,19 @@ CmpCreateKeyControlBlock(IN PHHIVE Hive, /* Check if we have two hash entires */ HashLock = Flags & CMP_LOCK_HASHES_FOR_KCB ? TRUE : FALSE; - if (HashLock) + if (!HashLock) { - /* Not yet implemented */ - KeBugCheck(0); + /* It's not locked, do we have a parent? */ + if (Parent) + { + /* Lock the parent KCB and ourselves */ + CmpAcquireTwoKcbLocksExcusiveByKey(ConvKey, Parent->ConvKey); + } + else + { + /* Lock only ourselves */ + CmpAcquireKcbLockExclusive(Kcb); + } } /* Check if we already have a KCB */ @@ -746,10 +788,19 @@ CmpCreateKeyControlBlock(IN PHHIVE Hive, ASSERT((!Kcb) || (Kcb->Delete == FALSE)); /* Check if we had locked the hashes */ - if (HashLock) + if (!HashLock) { - /* Not yet implemented: Unlock hashes */ - KeBugCheck(0); + /* We locked them manually, do we have a parent? */ + if (Parent) + { + /* Unlock the parent KCB and ourselves */ + CmpReleaseTwoKcbLockByKey(ConvKey, Parent->ConvKey); + } + else + { + /* Unlock only ourselves */ + CmpReleaseKcbLockByKey(ConvKey); + } } /* Return the KCB */ @@ -764,3 +815,4 @@ EnlistKeyBodyWithKCB(IN PCM_KEY_BODY KeyBody, ASSERT(FALSE); } + diff --git a/reactos/ntoskrnl/config/cmparse.c b/reactos/ntoskrnl/config/cmparse.c index 6de5784550a..6f28f773302 100644 --- a/reactos/ntoskrnl/config/cmparse.c +++ b/reactos/ntoskrnl/config/cmparse.c @@ -215,7 +215,7 @@ CmpDoCreateChild(IN PHHIVE Hive, *KeyCell, KeyNode, ParentKcb, - 0, + CMP_LOCK_HASHES_FOR_KCB, Name); if (!Kcb) { @@ -281,6 +281,12 @@ CmpDoCreate(IN PHHIVE Hive, UNICODE_STRING LocalClass = {0}; if (!Class) Class = &LocalClass; + /* Sanity check */ +#if 0 + ASSERT((CmpIsKcbLockedExclusive(ParentKcb) == TRUE) || + (CmpTestRegistryLockExclusive() == TRUE)); +#endif + /* Acquire the flusher lock */ ExAcquirePushLockShared((PVOID)&((PCMHIVE)Hive)->FlusherLock); @@ -447,7 +453,12 @@ CmpDoOpen(IN PHHIVE Hive, //ASSERT(CmpIsKcbLockedExclusive(*CachedKcb)); /* Create the KCB. FIXME: Use lock flag */ - Kcb = CmpCreateKeyControlBlock(Hive, Cell, Node, *CachedKcb, 0, KeyName); + Kcb = CmpCreateKeyControlBlock(Hive, + Cell, + Node, + *CachedKcb, + CMP_LOCK_HASHES_FOR_KCB, + KeyName); if (!Kcb) return STATUS_INSUFFICIENT_RESOURCES; /* Make sure it's also locked, and set the pointer */ @@ -503,7 +514,10 @@ CmpCreateLinkNode(IN PHHIVE Hive, LARGE_INTEGER TimeStamp; PCM_KEY_NODE KeyNode; PCM_KEY_CONTROL_BLOCK Kcb = ParentKcb; - +#if 0 + CMP_ASSERT_REGISTRY_LOCK(); +#endif + /* Link nodes only allowed on the master */ if (Hive != &CmiVolatileHive->Hive) { @@ -712,3 +726,238 @@ Exit: ExReleasePushLock((PVOID)&((PCMHIVE)Hive)->FlusherLock); return Status; } + +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) +{ + ULONG HashKeyCopy; + + /* We don't lock anything for now */ + *LockedKcbs = NULL; + + /* Make a copy of the hash key */ + HashKeyCopy = (*Kcb)->ConvKey; + + /* Calculate hash values */ + *TotalRemainingSubkeys = 0xBAADF00D; + + /* Lock the registry */ + CmpLockRegistry(); + + /* Return hive and cell data */ + *Hive = (*Kcb)->KeyHive; + *Cell = (*Kcb)->KeyCell; + + /* Return success for now */ + return STATUS_SUCCESS; +} + +NTSTATUS +NTAPI +CmpParseKey2(IN PVOID ParseObject, + IN PVOID ObjectType, + IN OUT PACCESS_STATE AccessState, + IN KPROCESSOR_MODE AccessMode, + IN ULONG Attributes, + IN OUT PUNICODE_STRING CompleteName, + IN OUT PUNICODE_STRING RemainingName, + IN OUT PVOID Context OPTIONAL, + IN PSECURITY_QUALITY_OF_SERVICE SecurityQos OPTIONAL, + OUT PVOID *Object) +{ + NTSTATUS Status = STATUS_UNSUCCESSFUL; + PCM_KEY_CONTROL_BLOCK Kcb, ParentKcb; + PHHIVE Hive = NULL; + PCM_KEY_NODE Node = NULL; + HCELL_INDEX Cell = HCELL_NIL, NextCell; + UNICODE_STRING Current, NextName; + PCM_PARSE_CONTEXT ParseContext = Context; + ULONG TotalRemainingSubkeys = 0, MatchRemainSubkeyLevel = 0, TotalSubkeys = 0; + PULONG LockedKcbs = NULL; + BOOLEAN Result, Last; + PAGED_CODE(); + DPRINT1("New style parse routine called: %wZ %wZ!\n", CompleteName, RemainingName); + + /* Loop path separators at the end */ + while ((RemainingName->Length) && + (RemainingName->Buffer[(RemainingName->Length / sizeof(WCHAR)) - 1] == + OBJ_NAME_PATH_SEPARATOR)) + { + /* Remove path separator */ + RemainingName->Length -= sizeof(WCHAR); + } + + /* Fail if this isn't a key object */ + if (ObjectType != CmpKeyObjectType) return STATUS_OBJECT_TYPE_MISMATCH; + + /* Copy the remaining name */ + Current = *RemainingName; + + /* Check if this is a create */ + if (!(ParseContext) || !(ParseContext->CreateOperation)) + { + /* It isn't, so no context */ + ParseContext = NULL; + } + + /* Grab the KCB */ + Kcb = ((PKEY_OBJECT)ParseObject)->KeyControlBlock; + DPRINT1("KCB Parse: %p\n", Kcb); + + /* Lookup in the cache */ + Status = CmpBuildHashStackAndLookupCache(ParseObject, + &Kcb, + &Current, + &Hive, + &Cell, + &TotalRemainingSubkeys, + &MatchRemainSubkeyLevel, + &TotalSubkeys, + NULL, + &LockedKcbs); + + /* This is now the parent */ + ParentKcb = Kcb; + DPRINT1("ParentKcb Parse: %p\n", ParentKcb); + DPRINT1("Hive Parse: %p\n", Hive); + DPRINT1("Cell Parse: %p\n", Cell); + + /* Check if everything was found cached */ + if (!TotalRemainingSubkeys) ASSERTMSG("Caching not implemented", FALSE); + + /* Don't do anything if we're being deleted */ + if (Kcb->Delete) return STATUS_OBJECT_NAME_NOT_FOUND; + + /* Check if this is a symlink */ + if (Kcb->Flags & KEY_SYM_LINK) + { + /* Not implemented */ + DPRINT1("Parsing sym link\n"); + while (TRUE); + } + + /* Get the key node */ + Node = (PCM_KEY_NODE)HvGetCell(Hive, Cell); + if (!Node) return STATUS_INSUFFICIENT_RESOURCES; + DPRINT1("Node Parse: %p\n", Node); + + /* Start parsing */ + Status = STATUS_SUCCESS; + while (TRUE) + { + /* Get the next component */ + Result = CmpGetNextName(&Current, &NextName, &Last); + DPRINT1("Result Parse: %p\n", Result); + if ((Result) && (NextName.Length)) + { + /* See if this is a sym link */ + if (!(Kcb->Flags & KEY_SYM_LINK)) + { + /* Find the subkey */ + NextCell = CmpFindSubKeyByName(Hive, Node, &NextName); + DPRINT1("NextCell Parse: %lx\n", NextCell); + if (NextCell != HCELL_NIL) + { + /* Get the new node */ + Cell = NextCell; + Node = (PCM_KEY_NODE)HvGetCell(Hive, Cell); + DPRINT1("Node Parse: %p\n", Node); + if (!Node) ASSERT(FALSE); + + /* Check if this was the last key */ + if (Last) + { + /* Shouldn't happen yet */ + DPRINT1("Unhandled: Open of last key\n"); + break; + } + + /* Get hive and cell from reference */ + //Hive = Node->ChildHiveReference.KeyHive; + //Cell = Node->ChildHiveReference.KeyCell; + Node = (PCM_KEY_NODE)HvGetCell(Hive, Cell); + DPRINT1("Node Parse: %p\n", Node); + if (!Node) ASSERT(FALSE); + + /* Create a KCB for this key */ + Kcb = CmpCreateKeyControlBlock(Hive, + Cell, + Node, + ParentKcb, + 0, + &NextName); + if (!Kcb) ASSERT(FALSE); + DPRINT1("Kcb Parse: %p\n", Kcb); + + /* Dereference the parent and set the new one */ + CmpDereferenceKeyControlBlock(ParentKcb); + ParentKcb = Kcb; + } + else + { + /* Check if this was the last key for a create */ + if ((Last) && (ParseContext)) + { + /* Check if we're doing a link node */ + if (ParseContext->CreateLink) + { + /* The only thing we should see */ + DPRINT1("Expected: Creating new link\n"); + } + else + { + /* Create: should not see this (yet) */ + DPRINT1("Unexpected: Creating new child\n"); + while (TRUE); + } + + /* Update disposition */ + ParseContext->Disposition = REG_CREATED_NEW_KEY; + break; + } + else + { + /* Key not found */ + Status = STATUS_OBJECT_NAME_NOT_FOUND; + break; + } + } + } + else + { + /* Not implemented */ + DPRINT1("Parsing sym link\n"); + while (TRUE); + } + } + else if ((Result) && (Last)) + { + /* Opening root: unexpected */ + DPRINT1("Unexpected: Opening root\n"); + while (TRUE); + } + else + { + /* Bogus */ + Status = STATUS_INVALID_PARAMETER; + break; + } + } + + /* Dereference the parent if it exists */ + if (ParentKcb) CmpDereferenceKeyControlBlock(ParentKcb); + + /* Unlock the registry */ + CmpUnlockRegistry(); + return STATUS_NOT_IMPLEMENTED; +} diff --git a/reactos/ntoskrnl/config/cmsysini.c b/reactos/ntoskrnl/config/cmsysini.c index 48ca62963c2..edae3295999 100644 --- a/reactos/ntoskrnl/config/cmsysini.c +++ b/reactos/ntoskrnl/config/cmsysini.c @@ -504,6 +504,7 @@ CmpLinkHiveToMaster(IN PUNICODE_STRING LinkName, UNICODE_STRING ObjectName; OBJECT_CREATE_INFORMATION ObjectCreateInfo; CM_PARSE_CONTEXT ParseContext = {0}; + HANDLE KeyHandle; PAGED_CODE(); /* Setup the object attributes */ @@ -530,6 +531,17 @@ CmpLinkHiveToMaster(IN PUNICODE_STRING LinkName, ParseContext.ChildHive.KeyCell = RegistryHive->Hive.BaseBlock->RootCell; } + DPRINT1("Ready to parse\n"); + Status = ObOpenObjectByName(&ObjectAttributes, + CmpKeyObjectType, + KernelMode, + NULL, + KEY_READ | KEY_WRITE, + (PVOID)&ParseContext, + &KeyHandle); + DPRINT1("Parse done: %lx\n", Status); + //while (TRUE); + /* Capture all the info */ Status = ObpCaptureObjectAttributes(&ObjectAttributes, KernelMode, @@ -1602,6 +1614,73 @@ CmpUnlockRegistry(VOID) KeLeaveCriticalRegion(); } +VOID +NTAPI +CmpAcquireTwoKcbLocksExcusiveByKey(IN ULONG ConvKey1, + IN ULONG ConvKey2) +{ + ULONG Index1, Index2; + + /* Sanity check */ + CMP_ASSERT_REGISTRY_LOCK(); + + /* Get hash indexes */ + Index1 = GET_HASH_INDEX(ConvKey1); + Index2 = GET_HASH_INDEX(ConvKey2); + + /* See which one is highest */ + if (Index1 < Index2) + { + /* Grab them in the proper order */ + CmpAcquireKcbLockExclusiveByKey(ConvKey1); + CmpAcquireKcbLockExclusiveByKey(ConvKey2); + } + else + { + /* Grab the second one first, then the first */ + CmpAcquireKcbLockExclusiveByKey(ConvKey2); + if (Index1 != Index2) CmpAcquireKcbLockExclusiveByKey(ConvKey1); + } +} + +VOID +NTAPI +CmpReleaseTwoKcbLockByKey(IN ULONG ConvKey1, + IN ULONG ConvKey2) +{ + ULONG Index1, Index2; + + /* Sanity check */ + CMP_ASSERT_REGISTRY_LOCK(); + + /* Get hash indexes */ + Index1 = GET_HASH_INDEX(ConvKey1); + Index2 = GET_HASH_INDEX(ConvKey2); + ASSERT((GET_HASH_ENTRY(CmpCacheTable, ConvKey2).Owner == KeGetCurrentThread()) || + (CmpTestRegistryLockExclusive())); + + /* See which one is highest */ + if (Index1 < Index2) + { + /* Grab them in the proper order */ + ASSERT((GET_HASH_ENTRY(CmpCacheTable, ConvKey1).Owner == KeGetCurrentThread()) || + (CmpTestRegistryLockExclusive())); + CmpReleaseKcbLockByKey(ConvKey2); + CmpReleaseKcbLockByKey(ConvKey1); + } + else + { + /* Release the first one first, then the second */ + if (Index1 != Index2) + { + ASSERT((GET_HASH_ENTRY(CmpCacheTable, ConvKey1).Owner == KeGetCurrentThread()) || + (CmpTestRegistryLockExclusive())); + CmpReleaseKcbLockByKey(ConvKey1); + } + CmpReleaseKcbLockByKey(ConvKey2); + } +} + VOID NTAPI CmShutdownSystem(VOID)