- 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.

svn path=/trunk/; revision=31262

reactos/ntoskrnl/config/cmapi.c

@@ -266,10 +266,56 @@ CmSetValueKey(IN PCM_KEY_CONTROL_BLOCK Kcb,
     NTSTATUS Status;
     BOOLEAN Found, Result;
     ULONG Count, ChildIndex, SmallData, Storage;
+    VALUE_SEARCH_RETURN_TYPE SearchResult;
 
-    /* Acquire hive lock exclusively */
+    /* Acquire hive lock */
-    KeEnterCriticalRegion();
+    CmpLockRegistry();
-    ExAcquireResourceExclusiveLite(&CmpRegistryLock, TRUE);
+    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;
@@ -316,6 +362,10 @@ CmSetValueKey(IN PCM_KEY_CONTROL_BLOCK Kcb,
         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 @@ CmSetValueKey(IN PCM_KEY_CONTROL_BLOCK Kcb,
                                         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 @@ Quickie:
         /* Save the write time */
         KeQuerySystemTime(&Parent->LastWriteTime);
         KeQuerySystemTime(&Kcb->KcbLastWriteTime);
+        
+        /* 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;
+        }
     }
     
-    /* Release the lock */
+Quickie:
-    ExReleaseResourceLite(&CmpRegistryLock);
+    /* Release the locks */
-    KeLeaveCriticalRegion();
+    CmpReleaseKcbLock(Kcb);
+    CmpUnlockRegistry();
     return Status;
 }
 
@@ -408,8 +475,19 @@ CmDeleteValueKey(IN PCM_KEY_CONTROL_BLOCK Kcb,
     BOOLEAN Result;
 
     /* Acquire hive lock */
-    KeEnterCriticalRegion();
+    CmpLockRegistry();
-    ExAcquireResourceExclusiveLite(&CmpRegistryLock, TRUE);
+    
+    /* 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;
@@ -491,6 +569,17 @@ CmDeleteValueKey(IN PCM_KEY_CONTROL_BLOCK Kcb,
             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 @@ Quickie:
         HvReleaseCell(Hive, ChildCell);
     }
 
-    /* Release hive lock */
+    /* Release locks */
-    ExReleaseResourceLite(&CmpRegistryLock);
+    CmpReleaseKcbLock(Kcb);
-    KeLeaveCriticalRegion();
+    CmpUnlockRegistry();
     return Status;
 }
 
@@ -533,8 +622,27 @@ CmQueryValueKey(IN PCM_KEY_CONTROL_BLOCK Kcb,
     PAGED_CODE();
 
     /* Acquire hive lock */
-    KeEnterCriticalRegion();
+    CmpLockRegistry();
-    ExAcquireResourceExclusiveLite(&CmpRegistryLock, TRUE);
+    
+    /* 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 @@ CmQueryValueKey(IN PCM_KEY_CONTROL_BLOCK Kcb,
                                          &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 @@ CmQueryValueKey(IN PCM_KEY_CONTROL_BLOCK Kcb,
                                       Length,
                                       ResultLength,
                                       &Status);
+        if (Result == SearchNeedExclusiveLock)
+        {            
+            /* Try with exclusive KCB lock */
+            CmpConvertKcbSharedToExclusive(Kcb);
+            goto DoAgain;
+        }
     }
     else
     {
@@ -572,9 +697,9 @@ CmQueryValueKey(IN PCM_KEY_CONTROL_BLOCK Kcb,
     /* If we have a cell to release, do so */
     if (CellToRelease != HCELL_NIL) HvReleaseCell(Hive, CellToRelease);
 
-    /* Release hive lock */
+    /* Release locks */
-    ExReleaseResourceLite(&CmpRegistryLock);
+    CmpReleaseKcbLock(Kcb);
-    KeLeaveCriticalRegion();
+    CmpUnlockRegistry();
     return Status;
 }
 
@@ -599,8 +724,20 @@ CmEnumerateValueKey(IN PCM_KEY_CONTROL_BLOCK Kcb,
     PAGED_CODE();
 
     /* Acquire hive lock */
-    KeEnterCriticalRegion();
+    CmpLockRegistry();
-    ExAcquireResourceExclusiveLite(&CmpRegistryLock, TRUE);
+    
+    /* 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;
@@ -622,12 +759,29 @@ CmEnumerateValueKey(IN PCM_KEY_CONTROL_BLOCK Kcb,
         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 */
     Result = CmpGetValueListFromCache(Kcb,
                                       &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 @@ CmEnumerateValueKey(IN PCM_KEY_CONTROL_BLOCK Kcb,
                                      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 @@ CmEnumerateValueKey(IN PCM_KEY_CONTROL_BLOCK Kcb,
                                   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 @@ Quickie:
     /* If we have a cell to release, do so */
     if (CellToRelease2 != HCELL_NIL) HvReleaseCell(Hive, CellToRelease2);
 
-    /* Release hive lock */
+    /* Release locks */
-    ExReleaseResourceLite(&CmpRegistryLock);
+    CmpReleaseKcbLock(Kcb);
-    KeLeaveCriticalRegion();
+    CmpUnlockRegistry();
     return Status;
 }
 
@@ -911,8 +1077,10 @@ CmQueryKey(IN PCM_KEY_CONTROL_BLOCK Kcb,
     PCM_KEY_NODE Parent;
 
     /* Acquire hive lock */
-    KeEnterCriticalRegion();
+    CmpLockRegistry();
-    ExAcquireResourceExclusiveLite(&CmpRegistryLock, TRUE);
+    
+    /* Lock KCB shared */
+    CmpAcquireKcbLockShared(Kcb);
 
     /* Get the hive and parent */
     Hive = Kcb->KeyHive;
@@ -924,6 +1092,14 @@ CmQueryKey(IN PCM_KEY_CONTROL_BLOCK Kcb,
         goto Quickie;
     }
     
+    /* Don't touch deleted keys */
+    if (Kcb->Delete)
+    {
+        /* Fail */
+        Status = STATUS_KEY_DELETED;
+        goto Quickie;
+    }
+
     /* Check what class we got */
     switch (KeyInformationClass)
     {
@@ -961,9 +1137,9 @@ CmQueryKey(IN PCM_KEY_CONTROL_BLOCK Kcb,
     }
 
 Quickie:
-    /* Release hive lock */
+    /* Release locks */
-    ExReleaseResourceLite(&CmpRegistryLock);
+    CmpReleaseKcbLock(Kcb);
-    KeLeaveCriticalRegion();
+    CmpUnlockRegistry();
     return Status;
 }
 
@@ -982,8 +1158,19 @@ CmEnumerateKey(IN PCM_KEY_CONTROL_BLOCK Kcb,
     HCELL_INDEX ChildCell;
 
     /* Acquire hive lock */
-    KeEnterCriticalRegion();
+    CmpLockRegistry();
-    ExAcquireResourceExclusiveLite(&CmpRegistryLock, TRUE);
+    
+    /* 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 @@ CmEnumerateKey(IN PCM_KEY_CONTROL_BLOCK Kcb,
                              ResultLength);
 
 Quickie:
-    /* Release hive lock */
+    /* Release locks */
-    ExReleaseResourceLite(&CmpRegistryLock);
+    CmpReleaseKcbLock(Kcb);
-    KeLeaveCriticalRegion();
+    CmpUnlockRegistry();
     return Status;
 }
 
@@ -1041,11 +1228,32 @@ CmDeleteKey(IN PCM_KEY_BODY KeyBody)
     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();
+    CmpLockRegistry();
-    ExAcquireResourceExclusiveLite(&CmpRegistryLock, TRUE);
+    
+    /* 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;
@@ -1060,33 +1268,21 @@ CmDeleteKey(IN PCM_KEY_BODY KeyBody)
         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 @@ CmDeleteKey(IN PCM_KEY_BODY KeyBody)
         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);
+    CmpUnlockRegistry();
-    KeLeaveCriticalRegion();
     return Status;
 }
 
@@ -1141,6 +1337,9 @@ CmFlushKey(IN PCM_KEY_CONTROL_BLOCK Kcb,
     NTSTATUS Status = STATUS_SUCCESS;
     PHHIVE Hive;
 
+    /* Ignore flushes until we're ready */
+    if (CmpNoWrite) return STATUS_SUCCESS;
+    
     /* Get the hives */
     Hive = Kcb->KeyHive;
     CmHive = (PCMHIVE)Hive;
@@ -1239,7 +1438,7 @@ CmLoadKey(IN POBJECT_ATTRIBUTES TargetKey,
     }
     
     /* Lock the registry shared */
-    //CmpLockRegistry();
+    CmpLockRegistry();
     
     /* Lock the hive to this thread */
     CmHive->Hive.HiveFlags |= HIVE_IS_UNLOADING;
@@ -1252,7 +1451,7 @@ CmLoadKey(IN POBJECT_ATTRIBUTES TargetKey,
     Status = CmpLinkHiveToMaster(TargetKey->ObjectName,
                                  TargetKey->RootDirectory,
                                  CmHive,
-                                 FALSE,
+                                 Allocate,
                                  TargetKey->SecurityDescriptor);
     if (NT_SUCCESS(Status))
     {
@@ -1276,7 +1475,7 @@ CmLoadKey(IN POBJECT_ATTRIBUTES TargetKey,
     }
     
     /* Unlock the registry */
-    //CmpUnlockRegistry();
+    CmpUnlockRegistry();
     
     /* Close handle and return */
     if (KeyHandle) ZwClose(KeyHandle);