Author: fireball Date: Sun Dec 16 01:06:10 2007 New Revision: 31262
URL: http://svn.reactos.org/svn/reactos?rev=31262&view=rev Log: - Use CmpLockRegistry instead of manual registry locking -- this also makes the registry lock shared instead of exclusive. - Start using KCB locking/unlocking, usually shared first. - Support KCB lock promotion from shared to exclusive when value cache functions require it (SearchNeedExclusiveLock -- not yet used). - Add multiple checks for deleted KCBs so that operations on them fail. - Protect symlinks from being erroneously modified. - Remove a ReactOS symlink hack. - Add code to setup the KCB Value Cache (right now we use the node list since nobody was setting the value cache up). - Detect and assert on CM_KCB_SYM_LINK_FOUND, this is used by caching which we don't support. - Protect KEY_NO_DELETE nodes from deletion. - Lock the registry during hive flushing. - Respect CmpNoWrite if it's set. - Don't ignore Allocate flag sent to CmpLinkhivetoMaster during hive loading.
Modified: trunk/reactos/ntoskrnl/config/cmapi.c
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 Sun Dec 16 01:06:10 2007 @@ -266,10 +266,56 @@ NTSTATUS Status; BOOLEAN Found, Result; ULONG Count, ChildIndex, SmallData, Storage; - - /* Acquire hive lock exclusively */ - KeEnterCriticalRegion(); - ExAcquireResourceExclusiveLite(&CmpRegistryLock, TRUE); + VALUE_SEARCH_RETURN_TYPE SearchResult; + + /* Acquire hive lock */ + CmpLockRegistry(); + CmpAcquireKcbLockShared(Kcb); + + /* Sanity check */ + ASSERT(sizeof(ULONG) == CM_KEY_VALUE_SMALL); + + /* Don't touch deleted KCBs */ +DoAgain: + if (Kcb->Delete) + { + /* Fail */ + Status = STATUS_KEY_DELETED; + goto Quickie; + } + + /* Don't let anyone mess with symlinks */ + if ((Kcb->Flags & KEY_SYM_LINK) && + ((Type != REG_LINK) || + !(ValueName) || + !(RtlEqualUnicodeString(&CmSymbolicLinkValueName, ValueName, TRUE)))) + { + /* Invalid modification of a symlink key */ + Status = STATUS_ACCESS_DENIED; + goto Quickie; + } + + /* Search for the value */ + SearchResult = SearchFail; + if (SearchResult == SearchNeedExclusiveLock) + { + /* Try again with the exclusive lock */ + CmpConvertKcbSharedToExclusive(Kcb); + goto DoAgain; + } + else if (SearchResult == SearchSuccess) + { + /* We don't actually need to do anything! */ + Status = STATUS_SUCCESS; + goto Quickie; + } + + /* We need the exclusive KCB lock now */ + if (!(CmpIsKcbLockedExclusive(Kcb)) && !(CmpTryToConvertKcbSharedToExclusive(Kcb))) + { + /* Acquire exclusive lock */ + CmpConvertKcbSharedToExclusive(Kcb); + }
/* Get pointer to key cell */ Hive = Kcb->KeyHive; @@ -315,7 +361,11 @@ /* No child list, we'll need to add it */ ChildIndex = 0; } - + + /* The KCB must be locked exclusive at this point */ + ASSERT((CmpIsKcbLockedExclusive(Kcb) == TRUE) || + (CmpTestRegistryLockExclusive() == TRUE)); + /* Mark the cell dirty */ HvMarkCellDirty(Hive, Cell, FALSE);
@@ -357,27 +407,23 @@ SmallData); }
- /* Mark link key */ - if ((Type == REG_LINK) && - (_wcsicmp(ValueName->Buffer, L"SymbolicLinkValue") == 0)) - { - Parent->Flags |= KEY_SYM_LINK; - } - /* Check for success */ -Quickie: if (NT_SUCCESS(Status)) { + /* Check if the maximum value name length changed */ ASSERT(Parent->MaxValueNameLen == Kcb->KcbMaxValueNameLen); if (Parent->MaxValueNameLen < ValueName->Length) { + /* Set the new values */ Parent->MaxValueNameLen = ValueName->Length; Kcb->KcbMaxValueNameLen = ValueName->Length; } - + + /* Check if the maximum data length changed */ ASSERT(Parent->MaxValueDataLen == Kcb->KcbMaxValueDataLen); if (Parent->MaxValueDataLen < DataLength) { + /* Update it */ Parent->MaxValueDataLen = DataLength; Kcb->KcbMaxValueDataLen = Parent->MaxValueDataLen; } @@ -385,11 +431,32 @@ /* Save the write time */ KeQuerySystemTime(&Parent->LastWriteTime); KeQuerySystemTime(&Kcb->KcbLastWriteTime); - } - - /* Release the lock */ - ExReleaseResourceLite(&CmpRegistryLock); - KeLeaveCriticalRegion(); + + /* Check if the cell is cached */ + if ((Found) && (CMP_IS_CELL_CACHED(Kcb->ValueCache.ValueList))) + { + /* Shouldn't happen */ + ASSERT(FALSE); + } + else + { + /* Cleanup the value cache */ + CmpCleanUpKcbValueCache(Kcb); + + /* Sanity checks */ + ASSERT(!(CMP_IS_CELL_CACHED(Kcb->ValueCache.ValueList))); + ASSERT(!(Kcb->ExtFlags & CM_KCB_SYM_LINK_FOUND)); + + /* Set the value cache */ + Kcb->ValueCache.Count = Parent->ValueList.Count; + Kcb->ValueCache.ValueList = Parent->ValueList.List; + } + } + +Quickie: + /* Release the locks */ + CmpReleaseKcbLock(Kcb); + CmpUnlockRegistry(); return Status; }
@@ -408,8 +475,19 @@ BOOLEAN Result;
/* Acquire hive lock */ - KeEnterCriticalRegion(); - ExAcquireResourceExclusiveLite(&CmpRegistryLock, TRUE); + CmpLockRegistry(); + + /* Lock KCB exclusively */ + CmpAcquireKcbLockExclusive(Kcb); + + /* Don't touch deleted keys */ + if (Kcb->Delete) + { + /* Undo everything */ + CmpReleaseKcbLock(Kcb); + CmpUnlockRegistry(); + return STATUS_KEY_DELETED; + }
/* Get the hive and the cell index */ Hive = Kcb->KeyHive; @@ -490,6 +568,17 @@ Kcb->KcbMaxValueNameLen = 0; Kcb->KcbMaxValueDataLen = 0; } + + /* Cleanup the value cache */ + CmpCleanUpKcbValueCache(Kcb); + + /* Sanity checks */ + ASSERT(!(CMP_IS_CELL_CACHED(Kcb->ValueCache.ValueList))); + ASSERT(!(Kcb->ExtFlags & CM_KCB_SYM_LINK_FOUND)); + + /* Set the value cache */ + Kcb->ValueCache.Count = ChildList->Count; + Kcb->ValueCache.ValueList = ChildList->List;
/* Change default Status to success */ Status = STATUS_SUCCESS; @@ -507,9 +596,9 @@ HvReleaseCell(Hive, ChildCell); }
- /* Release hive lock */ - ExReleaseResourceLite(&CmpRegistryLock); - KeLeaveCriticalRegion(); + /* Release locks */ + CmpReleaseKcbLock(Kcb); + CmpUnlockRegistry(); return Status; }
@@ -533,8 +622,27 @@ PAGED_CODE();
/* Acquire hive lock */ - KeEnterCriticalRegion(); - ExAcquireResourceExclusiveLite(&CmpRegistryLock, TRUE); + CmpLockRegistry(); + + /* Lock the KCB shared */ + CmpAcquireKcbLockShared(Kcb); + + /* Don't touch deleted keys */ +DoAgain: + if (Kcb->Delete) + { + /* Undo everything */ + CmpReleaseKcbLock(Kcb); + CmpUnlockRegistry(); + return STATUS_KEY_DELETED; + } + + /* We don't deal with this yet */ + if (Kcb->ExtFlags & CM_KCB_SYM_LINK_FOUND) + { + /* Shouldn't happen */ + ASSERT(FALSE); + }
/* Get the hive */ Hive = Kcb->KeyHive; @@ -547,6 +655,17 @@ &ValueData, &ValueCached, &CellToRelease); + if (Result == SearchNeedExclusiveLock) + { + /* Check if we need an exclusive lock */ + ASSERT(CellToRelease == HCELL_NIL); + ASSERT(ValueData == NULL); + + /* Try with exclusive KCB lock */ + CmpConvertKcbSharedToExclusive(Kcb); + goto DoAgain; + } + if (Result == SearchSuccess) { /* Sanity check */ @@ -562,6 +681,12 @@ Length, ResultLength, &Status); + if (Result == SearchNeedExclusiveLock) + { + /* Try with exclusive KCB lock */ + CmpConvertKcbSharedToExclusive(Kcb); + goto DoAgain; + } } else { @@ -572,9 +697,9 @@ /* If we have a cell to release, do so */ if (CellToRelease != HCELL_NIL) HvReleaseCell(Hive, CellToRelease);
- /* Release hive lock */ - ExReleaseResourceLite(&CmpRegistryLock); - KeLeaveCriticalRegion(); + /* Release locks */ + CmpReleaseKcbLock(Kcb); + CmpUnlockRegistry(); return Status; }
@@ -599,8 +724,20 @@ PAGED_CODE();
/* Acquire hive lock */ - KeEnterCriticalRegion(); - ExAcquireResourceExclusiveLite(&CmpRegistryLock, TRUE); + CmpLockRegistry(); + + /* Lock the KCB shared */ + CmpAcquireKcbLockShared(Kcb); + + /* Don't touch deleted keys */ +DoAgain: + if (Kcb->Delete) + { + /* Undo everything */ + CmpReleaseKcbLock(Kcb); + CmpUnlockRegistry(); + return STATUS_KEY_DELETED; + }
/* Get the hive and parent */ Hive = Kcb->KeyHive; @@ -620,6 +757,13 @@ HvReleaseCell(Hive, Kcb->KeyCell); Status = STATUS_NO_MORE_ENTRIES; goto Quickie; + } + + /* We don't deal with this yet */ + if (Kcb->ExtFlags & CM_KCB_SYM_LINK_FOUND) + { + /* Shouldn't happen */ + ASSERT(FALSE); }
/* Find the value list */ @@ -627,7 +771,17 @@ &CellData, &IndexIsCached, &CellToRelease); - if (Result != SearchSuccess) + if (Result == SearchNeedExclusiveLock) + { + /* Check if we need an exclusive lock */ + ASSERT(CellToRelease == HCELL_NIL); + ASSERT(ValueData == NULL); + + /* Try with exclusive KCB lock */ + CmpConvertKcbSharedToExclusive(Kcb); + goto DoAgain; + } + else if (Result != SearchSuccess) { /* Sanity check */ ASSERT(CellData == NULL); @@ -646,10 +800,16 @@ IndexIsCached, &ValueIsCached, &CellToRelease2); - if (Result != SearchSuccess) + if (Result == SearchNeedExclusiveLock) + { + /* Try with exclusive KCB lock */ + CmpConvertKcbSharedToExclusive(Kcb); + goto DoAgain; + } + else if (Result != SearchSuccess) { /* Sanity check */ - ASSERT(CellToRelease2 == HCELL_NIL); + ASSERT(ValueData == NULL);
/* Release the cells and fail */ Status = STATUS_INSUFFICIENT_RESOURCES; @@ -666,6 +826,12 @@ Length, ResultLength, &Status); + if (Result == SearchNeedExclusiveLock) + { + /* Try with exclusive KCB lock */ + CmpConvertKcbSharedToExclusive(Kcb); + goto DoAgain; + }
Quickie: /* If we have a cell to release, do so */ @@ -677,9 +843,9 @@ /* If we have a cell to release, do so */ if (CellToRelease2 != HCELL_NIL) HvReleaseCell(Hive, CellToRelease2);
- /* Release hive lock */ - ExReleaseResourceLite(&CmpRegistryLock); - KeLeaveCriticalRegion(); + /* Release locks */ + CmpReleaseKcbLock(Kcb); + CmpUnlockRegistry(); return Status; }
@@ -911,8 +1077,10 @@ PCM_KEY_NODE Parent;
/* Acquire hive lock */ - KeEnterCriticalRegion(); - ExAcquireResourceExclusiveLite(&CmpRegistryLock, TRUE); + CmpLockRegistry(); + + /* Lock KCB shared */ + CmpAcquireKcbLockShared(Kcb);
/* Get the hive and parent */ Hive = Kcb->KeyHive; @@ -921,6 +1089,14 @@ { /* Fail */ Status = STATUS_INSUFFICIENT_RESOURCES; + goto Quickie; + } + + /* Don't touch deleted keys */ + if (Kcb->Delete) + { + /* Fail */ + Status = STATUS_KEY_DELETED; goto Quickie; }
@@ -961,9 +1137,9 @@ }
Quickie: - /* Release hive lock */ - ExReleaseResourceLite(&CmpRegistryLock); - KeLeaveCriticalRegion(); + /* Release locks */ + CmpReleaseKcbLock(Kcb); + CmpUnlockRegistry(); return Status; }
@@ -982,8 +1158,19 @@ HCELL_INDEX ChildCell;
/* Acquire hive lock */ - KeEnterCriticalRegion(); - ExAcquireResourceExclusiveLite(&CmpRegistryLock, TRUE); + CmpLockRegistry(); + + /* Lock the KCB shared */ + CmpAcquireKcbLockShared(Kcb); + + /* Don't touch deleted keys */ + if (Kcb->Delete) + { + /* Undo everything */ + CmpReleaseKcbLock(Kcb); + CmpUnlockRegistry(); + return STATUS_KEY_DELETED; + }
/* Get the hive and parent */ Hive = Kcb->KeyHive; @@ -1027,9 +1214,9 @@ ResultLength);
Quickie: - /* Release hive lock */ - ExReleaseResourceLite(&CmpRegistryLock); - KeLeaveCriticalRegion(); + /* Release locks */ + CmpReleaseKcbLock(Kcb); + CmpUnlockRegistry(); return Status; }
@@ -1041,11 +1228,32 @@ PHHIVE Hive; PCM_KEY_NODE Node, Parent; HCELL_INDEX Cell, ParentCell; - PCM_KEY_CONTROL_BLOCK Kcb = KeyBody->KeyControlBlock; + PCM_KEY_CONTROL_BLOCK Kcb;
/* Acquire hive lock */ - KeEnterCriticalRegion(); - ExAcquireResourceExclusiveLite(&CmpRegistryLock, TRUE); + CmpLockRegistry(); + + /* Get the kcb */ + Kcb = KeyBody->KeyControlBlock; + + /* Don't allow deleting the root */ + if (!Kcb->ParentKcb) + { + /* Fail */ + CmpUnlockRegistry(); + return STATUS_CANNOT_DELETE; + } + + /* Lock parent and child */ + CmpAcquireTwoKcbLocksExclusiveByKey(Kcb->ConvKey, Kcb->ParentKcb->ConvKey); + + /* Check if we're already being deleted */ + if (Kcb->Delete) + { + /* Don't do it twice */ + Status = STATUS_SUCCESS; + goto Quickie2; + }
/* Get the hive and node */ Hive = Kcb->KeyHive; @@ -1059,34 +1267,22 @@ Status = STATUS_INSUFFICIENT_RESOURCES; goto Quickie; } - - /* Check if we have no parent */ - if (!Node->Parent) - { - /* This is an attempt to delete \Registry itself! */ - Status = STATUS_CANNOT_DELETE; - goto Quickie; - } - - /* Check if we're already being deleted */ - if (Kcb->Delete) - { - /* Don't do it twice */ - Status = STATUS_SUCCESS; - goto Quickie; - } - + /* Sanity check */ ASSERT(Node->Flags == Kcb->Flags);
/* Check if we don't have any children */ - if (!(Node->SubKeyCounts[Stable] + Node->SubKeyCounts[Volatile])) + if (!(Node->SubKeyCounts[Stable] + Node->SubKeyCounts[Volatile]) && + !(Node->Flags & KEY_NO_DELETE)) { /* Get the parent and free the cell */ ParentCell = Node->Parent; Status = CmpFreeKeyByCell(Hive, Cell, TRUE); if (NT_SUCCESS(Status)) { + /* Clean up information we have on the subkey */ + CmpCleanUpSubKeyInfo(Kcb->ParentKcb); + /* Get the parent node */ Parent = (PCM_KEY_NODE)HvGetCell(Hive, ParentCell); if (Parent) @@ -1119,16 +1315,16 @@ Status = STATUS_CANNOT_DELETE; }
- /* Flush the registry */ - CmpLazyFlush(); - Quickie: /* Release the cell */ HvReleaseCell(Hive, Cell); + + /* Release the KCB locks */ +Quickie2: + CmpReleaseTwoKcbLockByKey(Kcb->ConvKey, Kcb->ParentKcb->ConvKey);
/* Release hive lock */ - ExReleaseResourceLite(&CmpRegistryLock); - KeLeaveCriticalRegion(); + CmpUnlockRegistry(); return Status; }
@@ -1140,6 +1336,9 @@ PCMHIVE CmHive; NTSTATUS Status = STATUS_SUCCESS; PHHIVE Hive; + + /* Ignore flushes until we're ready */ + if (CmpNoWrite) return STATUS_SUCCESS;
/* Get the hives */ Hive = Kcb->KeyHive; @@ -1239,7 +1438,7 @@ }
/* Lock the registry shared */ - //CmpLockRegistry(); + CmpLockRegistry();
/* Lock the hive to this thread */ CmHive->Hive.HiveFlags |= HIVE_IS_UNLOADING; @@ -1252,7 +1451,7 @@ Status = CmpLinkHiveToMaster(TargetKey->ObjectName, TargetKey->RootDirectory, CmHive, - FALSE, + Allocate, TargetKey->SecurityDescriptor); if (NT_SUCCESS(Status)) { @@ -1276,7 +1475,7 @@ }
/* Unlock the registry */ - //CmpUnlockRegistry(); + CmpUnlockRegistry();
/* Close handle and return */ if (KeyHandle) ZwClose(KeyHandle);