[NTOS:CM] Do not acquire the lock twice when the Object Manager calls CmpSecurityMethod

Whenever a security request is invoked into a key object, such as when requesting
information from its security descriptor, the Object Manager will execute
the CmpSecurityMethod method to do the job.

The problem is that CmpSecurityMethod is not aware if the key control block
of the key body already has a lock acquired which means the function will attempt
to acquire a lock again, leading to a deadlock. This happens if the same
calling thread locks the KCB but it also wants to acquire security information
with ObCheckObjectAccess in CmpDoOpen.

Windows has a hack in CmpSecurityMethod where the passed KCB pointer is ORed
with a bitfield mask to avoid locking in all cases. This is ugly because it negates
every thread to acquire a lock if at least one has it.

ntoskrnl/config/cmparse.c

@@ -284,6 +284,7 @@ CmpDoCreateChild(IN PHHIVE Hive,
     KeyBody = (PCM_KEY_BODY)(*Object);
     KeyBody->Type = CM_KEY_BODY_TYPE;
     KeyBody->KeyControlBlock = NULL;
+    KeyBody->KcbLocked = FALSE;
 
     /* Check if we had a class */
     if (ParseContext->Class.Length > 0)
@@ -702,6 +703,15 @@ CmpDoOpen(IN PHHIVE Hive,
         /* Link to the KCB */
         EnlistKeyBodyWithKCB(KeyBody, 0);
 
+        /*
+         * We are already holding a lock against the KCB that is assigned
+         * to this key body. This is to prevent a potential deadlock on
+         * CmpSecurityMethod as ObCheckObjectAccess will invoke the Object
+         * Manager to call that method, of which CmpSecurityMethod would
+         * attempt to acquire a lock again.
+         */
+        KeyBody->KcbLocked = TRUE;
+
         if (!ObCheckObjectAccess(*Object,
                                  AccessState,
                                  FALSE,
@@ -711,6 +721,12 @@ CmpDoOpen(IN PHHIVE Hive,
             /* Access check failed */
             ObDereferenceObject(*Object);
         }
+
+        /*
+         * We are done, the lock we are holding will be released
+         * once the registry parsing is done.
+         */
+        KeyBody->KcbLocked = FALSE;
     }
     else
     {

ntoskrnl/config/cmse.c

@@ -281,10 +281,15 @@ CmpSecurityMethod(IN PVOID ObjectBody,
     /* Acquire the KCB lock */
     if (OperationCode == QuerySecurityDescriptor)
     {
-        CmpAcquireKcbLockShared(Kcb);
+        /* Avoid recursive locking if somebody already holds it */
+        if (!((PCM_KEY_BODY)ObjectBody)->KcbLocked)
+        {
+            CmpAcquireKcbLockShared(Kcb);
+        }
     }
     else
     {
+        ASSERT(!((PCM_KEY_BODY)ObjectBody)->KcbLocked);
         CmpAcquireKcbLockExclusive(Kcb);
     }
 
@@ -334,8 +339,14 @@ CmpSecurityMethod(IN PVOID ObjectBody,
             KeBugCheckEx(SECURITY_SYSTEM, 0, STATUS_INVALID_PARAMETER, 0, 0);
     }
 
-    /* Release the KCB lock */
-    CmpReleaseKcbLock(Kcb);
+    /*
+     * Release the KCB lock, but only if we locked it ourselves and
+     * nobody else was locking it by themselves.
+     */
+    if (!((PCM_KEY_BODY)ObjectBody)->KcbLocked)
+    {
+        CmpReleaseKcbLock(Kcb);
+    }
 
     /* Release the hive lock */
     CmpUnlockRegistry();

ntoskrnl/config/cmsysini.c

@@ -1126,6 +1126,7 @@ CmpCreateRegistryRoot(VOID)
     RootKey->Type = CM_KEY_BODY_TYPE;
     RootKey->NotifyBlock = NULL;
     RootKey->ProcessID = PsGetCurrentProcessId();
+    RootKey->KcbLocked = FALSE;
 
     /* Link with KCB */
     EnlistKeyBodyWithKCB(RootKey, 0);

ntoskrnl/include/internal/cm.h

@@ -235,6 +235,9 @@ typedef struct _CM_KEY_BODY
     struct _CM_NOTIFY_BLOCK *NotifyBlock;
     HANDLE ProcessID;
     LIST_ENTRY KeyBodyList;
+
+    /* ReactOS specific -- boolean flag to avoid recursive locking of the KCB */
+    BOOLEAN KcbLocked;
 } CM_KEY_BODY, *PCM_KEY_BODY;
 
 //