Author: fireball Date: Sat Dec 15 23:51:40 2007 New Revision: 31257
URL: http://svn.reactos.org/svn/reactos?rev=31257&view=rev Log: - Fix typo in KCB flag. - CmDeleteKey takes a Key Body, not a KCB. - Simplify some code since ValueCache.RealKCB is a KCB now, not a hacked PKEY_OBJECT, so a bunch of typecasts can be removed. - Implement CmpCleanUpSubKeyInfo to remove cached subkey information when a KCB is deleted (subkeys are not cached yet, however). - Add checks in various code paths to make sure we don't touch a read-only KCB. - Don't double-dereference a key object during a delete. - Add checks for invalid key information classes. - For KeyNameInformation, the caller must have some handle access to succeed the call. - Validate alignment of key value names. - Don't accept NULLs at the end of key value names. - Validate key value name lengths. - Validate data lengths. - Remove a couple of forced lazy flushes that aren't required anymore. - Use CmpLockRegistry() in ntapi.c instead of the manual lock release.
Modified: trunk/reactos/ntoskrnl/config/cmapi.c trunk/reactos/ntoskrnl/config/cmkcbncb.c trunk/reactos/ntoskrnl/config/ntapi.c trunk/reactos/ntoskrnl/include/internal/cm.h
Modified: trunk/reactos/ntoskrnl/config/cmapi.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/config/cmapi.c?rev... ============================================================================== --- trunk/reactos/ntoskrnl/config/cmapi.c (original) +++ trunk/reactos/ntoskrnl/config/cmapi.c Sat Dec 15 23:51:40 2007 @@ -1035,12 +1035,13 @@
NTSTATUS NTAPI -CmDeleteKey(IN PCM_KEY_CONTROL_BLOCK Kcb) +CmDeleteKey(IN PCM_KEY_BODY KeyBody) { NTSTATUS Status; PHHIVE Hive; PCM_KEY_NODE Node, Parent; HCELL_INDEX Cell, ParentCell; + PCM_KEY_CONTROL_BLOCK Kcb = KeyBody->KeyControlBlock;
/* Acquire hive lock */ KeEnterCriticalRegion();
Modified: trunk/reactos/ntoskrnl/config/cmkcbncb.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/config/cmkcbncb.c?... ============================================================================== --- trunk/reactos/ntoskrnl/config/cmkcbncb.c (original) +++ trunk/reactos/ntoskrnl/config/cmkcbncb.c Sat Dec 15 23:51:40 2007 @@ -449,15 +449,15 @@ else if (Kcb->ExtFlags & CM_KCB_SYM_LINK_FOUND) { /* This is a sym link, check if there's only one reference left */ - if ((((PCM_KEY_CONTROL_BLOCK)Kcb->ValueCache.RealKcb)->RefCount == 1) && - !(((PCM_KEY_CONTROL_BLOCK)Kcb->ValueCache.RealKcb)->Delete)) + if ((Kcb->ValueCache.RealKcb->RefCount == 1) && + !(Kcb->ValueCache.RealKcb->Delete)) { /* Disable delay close for the KCB */ - ((PCM_KEY_CONTROL_BLOCK)Kcb->ValueCache.RealKcb)->ExtFlags |= CM_KCB_NO_DELAY_CLOSE; + Kcb->ValueCache.RealKcb->ExtFlags |= CM_KCB_NO_DELAY_CLOSE; }
/* Dereference the KCB */ - CmpDelayDerefKeyControlBlock((PCM_KEY_CONTROL_BLOCK)Kcb->ValueCache.RealKcb); + CmpDelayDerefKeyControlBlock(Kcb->ValueCache.RealKcb); Kcb->ExtFlags &= ~CM_KCB_SYM_LINK_FOUND; } } @@ -498,6 +498,61 @@ LockHeldExclusively ? CmpDereferenceKeyControlBlockWithLock(Kcb,LockHeldExclusively) : CmpDelayDerefKeyControlBlock(Kcb); + } +} + +VOID +NTAPI +CmpCleanUpSubKeyInfo(IN PCM_KEY_CONTROL_BLOCK Kcb) +{ + PCM_KEY_NODE KeyNode; + + /* Sanity check */ + ASSERT((CmpIsKcbLockedExclusive(Kcb) == TRUE) || + (CmpTestRegistryLockExclusive() == TRUE)); + + /* Check if there's any cached subkey */ + if (Kcb->ExtFlags & (CM_KCB_NO_SUBKEY | CM_KCB_SUBKEY_ONE | CM_KCB_SUBKEY_HINT)) + { + /* Check if there's a hint */ + if (Kcb->ExtFlags & (CM_KCB_SUBKEY_HINT)) + { + /* Kill it */ + ExFreePool(Kcb->IndexHint); + } + + /* Remove subkey flags */ + Kcb->ExtFlags &= ~(CM_KCB_NO_SUBKEY | CM_KCB_SUBKEY_ONE | CM_KCB_SUBKEY_HINT); + } + + /* Check if there's no linked cell */ + if (Kcb->KeyCell == HCELL_NIL) + { + /* Make sure it's a delete */ + ASSERT(Kcb->Delete); + KeyNode = NULL; + } + else + { + /* Get the key node */ + KeyNode = (PCM_KEY_NODE)HvGetCell(Kcb->KeyHive, Kcb->KeyCell); + } + + /* Check if we got the node */ + if (!KeyNode) + { + /* We didn't, mark the cached data invalid */ + Kcb->ExtFlags |= CM_KCB_INVALID_CACHED_INFO; + } + else + { + /* We have a keynode, update subkey counts */ + Kcb->ExtFlags &= ~CM_KCB_INVALID_CACHED_INFO; + Kcb->SubKeyCount = KeyNode->SubKeyCounts[Stable] + + KeyNode->SubKeyCounts[Volatile]; + + /* Release the cell */ + HvReleaseCell(Kcb->KeyHive, Kcb->KeyCell); } }
@@ -821,4 +876,3 @@ /* FIXME: Implement once we don't link parents to children anymore */ }
-
Modified: trunk/reactos/ntoskrnl/config/ntapi.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/config/ntapi.c?rev... ============================================================================== --- trunk/reactos/ntoskrnl/config/ntapi.c (original) +++ trunk/reactos/ntoskrnl/config/ntapi.c Sat Dec 15 23:51:40 2007 @@ -33,7 +33,7 @@ HANDLE Handle; PAGED_CODE(); DPRINT("NtCreateKey(OB 0x%wZ)\n", ObjectAttributes->ObjectName); - + /* Setup the parse context */ ParseContext.CreateOperation = TRUE; ParseContext.CreateOptions = CreateOptions; @@ -65,7 +65,7 @@ NTSTATUS Status; PAGED_CODE(); DPRINT("NtOpenKey(OB 0x%wZ)\n", ObjectAttributes->ObjectName); - + /* Just let the object manager handle this */ Status = ObOpenObjectByName(ObjectAttributes, CmpKeyObjectType, @@ -90,7 +90,6 @@ REG_DELETE_KEY_INFORMATION DeleteKeyInfo; REG_POST_OPERATION_INFORMATION PostOperationInfo; PAGED_CODE(); - DPRINT("NtDeleteKey(KH 0x%p)\n", KeyHandle);
/* Verify that the handle is valid and is a registry key */ @@ -98,13 +97,9 @@ DELETE, CmpKeyObjectType, ExGetPreviousMode(), - (PVOID *)&KeyObject, + (PVOID*)&KeyObject, NULL); - if (!NT_SUCCESS(Status)) - { - DPRINT("ObReferenceObjectByHandle() failed with Status = 0x%08X\n"); - return Status; - } + if (!NT_SUCCESS(Status)) return Status;
/* Setup the callback */ PostOperationInfo.Object = (PVOID)KeyObject; @@ -112,16 +107,17 @@ Status = CmiCallRegisteredCallbacks(RegNtPreDeleteKey, &DeleteKeyInfo); if (NT_SUCCESS(Status)) { - /* Call the internal API */ - Status = CmDeleteKey(KeyObject->KeyControlBlock); - - /* Remove the keep-alive reference */ - ObDereferenceObject(KeyObject); - if (KeyObject->KeyControlBlock->KeyHive != - KeyObject->KeyControlBlock->ParentKcb->KeyHive) + /* Check if we are read-only */ + if ((KeyObject->KeyControlBlock->ExtFlags & CM_KCB_READ_ONLY_KEY) || + (KeyObject->KeyControlBlock->ParentKcb->ExtFlags & CM_KCB_READ_ONLY_KEY)) { - /* Dereference again */ - ObDereferenceObject(KeyObject); + /* Fail */ + Status = STATUS_ACCESS_DENIED; + } + else + { + /* Call the internal API */ + Status = CmDeleteKey(KeyObject); }
/* Do post callback */ @@ -148,22 +144,26 @@ REG_ENUMERATE_KEY_INFORMATION EnumerateKeyInfo; REG_POST_OPERATION_INFORMATION PostOperationInfo; PAGED_CODE(); - DPRINT("NtEnumerateKey() KH 0x%x, Index 0x%x, KIC %d, Length %d\n", - KeyHandle, Index, KeyInformationClass, Length); + KeyHandle, Index, KeyInformationClass, Length); + + /* Reject classes we don't know about */ + if ((KeyInformationClass != KeyBasicInformation) && + (KeyInformationClass != KeyNodeInformation) && + (KeyInformationClass != KeyFullInformation)) + { + /* Fail */ + return STATUS_INVALID_PARAMETER; + }
/* Verify that the handle is valid and is a registry key */ Status = ObReferenceObjectByHandle(KeyHandle, KEY_ENUMERATE_SUB_KEYS, CmpKeyObjectType, ExGetPreviousMode(), - (PVOID *)&KeyObject, + (PVOID*)&KeyObject, NULL); - if (!NT_SUCCESS(Status)) - { - DPRINT("ObReferenceObjectByHandle() failed with Status = 0x%08X\n"); - return Status; - } + if (!NT_SUCCESS(Status)) return Status;
/* Setup the callback */ PostOperationInfo.Object = (PVOID)KeyObject; @@ -209,22 +209,26 @@ REG_ENUMERATE_VALUE_KEY_INFORMATION EnumerateValueKeyInfo; REG_POST_OPERATION_INFORMATION PostOperationInfo; PAGED_CODE(); - DPRINT("NtEnumerateValueKey() KH 0x%x, Index 0x%x, KVIC %d, Length %d\n", - KeyHandle, Index, KeyValueInformationClass, Length); + KeyHandle, Index, KeyValueInformationClass, Length); + + /* Reject classes we don't know about */ + if ((KeyValueInformationClass != KeyValueBasicInformation) && + (KeyValueInformationClass != KeyValueFullInformation) && + (KeyValueInformationClass != KeyValuePartialInformation)) + { + /* Fail */ + return STATUS_INVALID_PARAMETER; + }
/* Verify that the handle is valid and is a registry key */ Status = ObReferenceObjectByHandle(KeyHandle, KEY_QUERY_VALUE, CmpKeyObjectType, ExGetPreviousMode(), - (PVOID *)&KeyObject, + (PVOID*)&KeyObject, NULL); - if (!NT_SUCCESS(Status)) - { - DPRINT("ObReferenceObjectByHandle() failed with Status = 0x%08X\n"); - return Status; - } + if (!NT_SUCCESS(Status)) return Status;
/* Setup the callback */ PostOperationInfo.Object = (PVOID)KeyObject; @@ -269,25 +273,57 @@ PCM_KEY_BODY KeyObject; REG_QUERY_KEY_INFORMATION QueryKeyInfo; REG_POST_OPERATION_INFORMATION PostOperationInfo; - PAGED_CODE(); - + OBJECT_HANDLE_INFORMATION HandleInfo; + PAGED_CODE(); DPRINT("NtQueryKey() KH 0x%x, KIC %d, Length %d\n", - KeyHandle, KeyInformationClass, Length); - - /* Verify that the handle is valid and is a registry key */ - Status = ObReferenceObjectByHandle(KeyHandle, - (KeyInformationClass != - KeyNameInformation) ? - KEY_QUERY_VALUE : 0, - CmpKeyObjectType, - ExGetPreviousMode(), - (PVOID *)&KeyObject, - NULL); - if (!NT_SUCCESS(Status)) - { - DPRINT("ObReferenceObjectByHandle() failed with Status = 0x%08X\n"); - return Status; - } + KeyHandle, KeyInformationClass, Length); + + /* Reject invalid classes */ + if ((KeyInformationClass != KeyBasicInformation) && + (KeyInformationClass != KeyNodeInformation) && + (KeyInformationClass != KeyFullInformation) && + (KeyInformationClass != KeyNameInformation) && + (KeyInformationClass != KeyCachedInformation) && + (KeyInformationClass != KeyFlagsInformation)) + { + /* Fail */ + return STATUS_INVALID_PARAMETER; + } + + /* Check if just the name is required */ + if (KeyInformationClass == KeyNameInformation) + { + /* Ignore access level */ + Status = ObReferenceObjectByHandle(KeyHandle, + 0, + CmpKeyObjectType, + ExGetPreviousMode(), + (PVOID*)&KeyObject, + &HandleInfo); + if (NT_SUCCESS(Status)) + { + /* At least a single bit of access is required */ + if (!HandleInfo.GrantedAccess) + { + /* No such luck */ + ObDereferenceObject(KeyObject); + Status = STATUS_ACCESS_DENIED; + } + } + } + else + { + /* Get a reference */ + Status = ObReferenceObjectByHandle(KeyHandle, + KEY_QUERY_VALUE, + CmpKeyObjectType, + ExGetPreviousMode(), + (PVOID*)&KeyObject, + NULL); + } + + /* Quit on failure */ + if (!NT_SUCCESS(Status)) return Status;
/* Setup the callback */ PostOperationInfo.Object = (PVOID)KeyObject; @@ -331,8 +367,8 @@ PCM_KEY_BODY KeyObject; REG_QUERY_VALUE_KEY_INFORMATION QueryValueKeyInfo; REG_POST_OPERATION_INFORMATION PostOperationInfo; - PAGED_CODE(); - + UNICODE_STRING ValueNameCopy = *ValueName; + PAGED_CODE(); DPRINT("NtQueryValueKey() KH 0x%x, VN '%wZ', KVIC %d, Length %d\n", KeyHandle, ValueName, KeyValueInformationClass, Length);
@@ -341,18 +377,32 @@ KEY_QUERY_VALUE, CmpKeyObjectType, ExGetPreviousMode(), - (PVOID *)&KeyObject, + (PVOID*)&KeyObject, NULL); - if (!NT_SUCCESS(Status)) - { - DPRINT("ObReferenceObjectByHandle() failed with Status = 0x%08X\n"); - return Status; + if (!NT_SUCCESS(Status)) return Status; + + /* Make sure the name is aligned properly */ + if ((ValueNameCopy.Length & (sizeof(WCHAR) - 1))) + { + /* It isn't, so we'll fail */ + ObDereferenceObject(KeyObject); + return STATUS_INVALID_PARAMETER; + } + else + { + /* Ignore any null characters at the end */ + while ((ValueNameCopy.Length) && + !(ValueNameCopy.Buffer[ValueNameCopy.Length / sizeof(WCHAR) - 1])) + { + /* Skip it */ + ValueNameCopy.Length -= sizeof(WCHAR); + } }
/* Setup the callback */ PostOperationInfo.Object = (PVOID)KeyObject; QueryValueKeyInfo.Object = (PVOID)KeyObject; - QueryValueKeyInfo.ValueName = ValueName; + QueryValueKeyInfo.ValueName = &ValueNameCopy; QueryValueKeyInfo.KeyValueInformationClass = KeyValueInformationClass; QueryValueKeyInfo.Length = Length; QueryValueKeyInfo.ResultLength = ResultLength; @@ -363,7 +413,7 @@ { /* Call the internal API */ Status = CmQueryValueKey(KeyObject->KeyControlBlock, - *ValueName, + ValueNameCopy, KeyValueInformationClass, KeyValueInformation, Length, @@ -373,8 +423,6 @@ PostOperationInfo.Status = Status; CmiCallRegisteredCallbacks(RegNtPostQueryValueKey, &PostOperationInfo); } - - DPRINT("NtQueryValueKey() returning 0x%08X\n", Status);
/* Dereference and return status */ ObDereferenceObject(KeyObject); @@ -394,8 +442,8 @@ PCM_KEY_BODY KeyObject; REG_SET_VALUE_KEY_INFORMATION SetValueKeyInfo; REG_POST_OPERATION_INFORMATION PostOperationInfo; - PAGED_CODE(); - + UNICODE_STRING ValueNameCopy = *ValueName; + PAGED_CODE(); DPRINT("NtSetValueKey() KH 0x%x, VN '%wZ', TI %x, T %d, DS %d\n", KeyHandle, ValueName, TitleIndex, Type, DataSize);
@@ -404,12 +452,34 @@ KEY_SET_VALUE, CmpKeyObjectType, ExGetPreviousMode(), - (PVOID *)&KeyObject, + (PVOID*)&KeyObject, NULL); - if (!NT_SUCCESS(Status)) - { - DPRINT1("ObReferenceObjectByHandle() failed with Status = 0x%08X\n", Status); - return Status; + if (!NT_SUCCESS(Status)) return Status; + + /* Make sure the name is aligned, not too long, and the data under 4GB */ + if ( (ValueNameCopy.Length > 32767) || + ((ValueNameCopy.Length & (sizeof(WCHAR) - 1))) || + (DataSize > 0x80000000)) + { + /* Fail */ + ObDereferenceObject(KeyObject); + return STATUS_INVALID_PARAMETER; + } + + /* Ignore any null characters at the end */ + while ((ValueNameCopy.Length) && + !(ValueNameCopy.Buffer[ValueNameCopy.Length / sizeof(WCHAR) - 1])) + { + /* Skip it */ + ValueNameCopy.Length -= sizeof(WCHAR); + } + + /* Don't touch read-only keys */ + if (KeyObject->KeyControlBlock->ExtFlags & CM_KCB_READ_ONLY_KEY) + { + /* Fail */ + ObDereferenceObject(KeyObject); + return STATUS_ACCESS_DENIED; }
/* Setup callback */ @@ -427,19 +497,18 @@ { /* Call the internal API */ Status = CmSetValueKey(KeyObject->KeyControlBlock, - ValueName, + &ValueNameCopy, Type, Data, DataSize); }
- /* Do the post-callback and de-reference the key object */ + /* Do the post-callback */ PostOperationInfo.Status = Status; CmiCallRegisteredCallbacks(RegNtPostSetValueKey, &PostOperationInfo); + + /* Dereference and return status */ ObDereferenceObject(KeyObject); - - /* Synchronize the hives and return */ - CmpLazyFlush(); return Status; }
@@ -453,6 +522,7 @@ REG_DELETE_VALUE_KEY_INFORMATION DeleteValueKeyInfo; REG_POST_OPERATION_INFORMATION PostOperationInfo; KPROCESSOR_MODE PreviousMode = ExGetPreviousMode(); + UNICODE_STRING ValueNameCopy = *ValueName; PAGED_CODE();
/* Verify that the handle is valid and is a registry key */ @@ -462,10 +532,22 @@ PreviousMode, (PVOID *)&KeyObject, NULL); - if (!NT_SUCCESS(Status)) - { - DPRINT("ObReferenceObjectByHandle() failed with Status = 0x%08X\n"); - return Status; + if (!NT_SUCCESS(Status)) return Status; + + /* Don't touch read-only keys */ + if (KeyObject->KeyControlBlock->ExtFlags & CM_KCB_READ_ONLY_KEY) + { + /* Fail */ + ObDereferenceObject(KeyObject); + return STATUS_ACCESS_DENIED; + } + + /* Make sure the name is aligned properly */ + if ((ValueNameCopy.Length & (sizeof(WCHAR) - 1))) + { + /* It isn't, so we'll fail */ + ObDereferenceObject(KeyObject); + return STATUS_INVALID_PARAMETER; }
/* Do the callback */ @@ -476,7 +558,7 @@ if (NT_SUCCESS(Status)) { /* Call the internal API */ - Status = CmDeleteValueKey(KeyObject->KeyControlBlock, *ValueName); + Status = CmDeleteValueKey(KeyObject->KeyControlBlock, ValueNameCopy);
/* Do the post callback */ PostOperationInfo.Object = (PVOID)KeyObject; @@ -485,9 +567,8 @@ &PostOperationInfo); }
- /* Dereference the key body and synchronize the hives */ + /* Dereference the key body */ ObDereferenceObject(KeyObject); - CmpLazyFlush(); return Status; }
@@ -509,8 +590,10 @@ if (!NT_SUCCESS(Status)) return Status;
/* Lock the registry */ - KeEnterCriticalRegion(); - ExAcquireResourceExclusiveLite(&CmpRegistryLock, TRUE); + CmpLockRegistry(); + + /* Lock the KCB */ + CmpAcquireKcbLockShared(KeyObject->KeyControlBlock);
/* Make sure KCB isn't deleted */ if (KeyObject->KeyControlBlock->Delete) @@ -524,9 +607,9 @@ Status = CmFlushKey(KeyObject->KeyControlBlock, FALSE); }
- /* Release the lock */ - ExReleaseResourceLite(&CmpRegistryLock); - KeLeaveCriticalRegion(); + /* Release the locks */ + CmpReleaseKcbLock(KeyObject->KeyControlBlock); + CmpUnlockRegistry();
/* Dereference the object and return status */ ObDereferenceObject(KeyObject);
Modified: trunk/reactos/ntoskrnl/include/internal/cm.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/c... ============================================================================== --- trunk/reactos/ntoskrnl/include/internal/cm.h (original) +++ trunk/reactos/ntoskrnl/include/internal/cm.h Sat Dec 15 23:51:40 2007 @@ -53,7 +53,7 @@ #define CM_KCB_KEY_NON_EXIST 0x10 #define CM_KCB_NO_DELAY_CLOSE 0x20 #define CM_KCB_INVALID_CACHED_INFO 0x40 -#define CM_KEY_READ_ONLY_KEY 0x80 +#define CM_KCB_READ_ONLY_KEY 0x80
// // CM_KEY_VALUE Types @@ -577,6 +577,16 @@ OUT PHCELL_INDEX CellToRelease );
+VALUE_SEARCH_RETURN_TYPE +NTAPI +CmpCompareNewValueDataAgainstKCBCache( + IN PCM_KEY_CONTROL_BLOCK Kcb, + IN PUNICODE_STRING ValueName, + IN ULONG Type, + IN PVOID Data, + IN ULONG DataSize +); + // // Registry Validation Functions // @@ -860,9 +870,21 @@
VOID NTAPI +CmpCleanUpKcbValueCache( + IN PCM_KEY_CONTROL_BLOCK Kcb +); + +VOID +NTAPI CmpCleanUpKcbCacheWithLock( IN PCM_KEY_CONTROL_BLOCK Kcb, IN BOOLEAN LockHeldExclusively +); + +VOID +NTAPI +CmpCleanUpSubKeyInfo( + IN PCM_KEY_CONTROL_BLOCK Kcb );
VOID @@ -1324,7 +1346,7 @@ NTSTATUS NTAPI CmDeleteKey( - IN PCM_KEY_CONTROL_BLOCK Kcb + IN PCM_KEY_BODY KeyBody );
NTSTATUS