[hbelusca] 75171: [NTOS]: Improve a bit CmpDeepCopyKeyInternal(): - Normally getting the SrcNode and DestNode must succeed (checked with assert); - Set the DestNode Flags member, in particular when...

Author: hbelusca
Date: Fri Jun 23 17:33:44 2017
New Revision: 75171

URL: http://svn.reactos.org/svn/reactos?rev=75171&view=rev
Log:
[NTOS]: Improve a bit CmpDeepCopyKeyInternal():
- Normally getting the SrcNode and DestNode must succeed (checked with assert);
- Set the DestNode Flags member, in particular when this is the new root node of the saved registry hive;
- Copy the key class cell (OK), and the key security cell (currently done in a hackish way; proper way: call the CmpAssignSecurity* function);
- Add more clean-up on failure;
- Warn in code about the fact that CmpDeepCopyKeyInternal is recursive, and will easily exhaust kernel stack. This function will need to be reworked later...
CORE-10793 CORE-10796

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	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/config/cmapi.c	[iso-8859-1] Fri Jun 23 17:33:44 2017
@@ -993,7 +993,7 @@
             goto Quickie;
         }

-        /* Ssanity checks */
+        /* Sanity checks */
         ASSERT(HvIsCellDirty(Hive, Parent->ValueList.List));
         ASSERT(HvIsCellDirty(Hive, ChildCell));

@@ -2376,7 +2376,6 @@

static
 NTSTATUS
-NTAPI
 CmpDeepCopyKeyInternal(IN PHHIVE SourceHive,
                        IN HCELL_INDEX SrcKeyCell,
                        IN PHHIVE DestinationHive,
@@ -2387,8 +2386,11 @@
     NTSTATUS Status;
     PCM_KEY_NODE SrcNode;
     PCM_KEY_NODE DestNode = NULL;
-    HCELL_INDEX NewKeyCell, SubKey, NewSubKey;
+    HCELL_INDEX NewKeyCell = HCELL_NIL;
+    HCELL_INDEX NewClassCell = HCELL_NIL, NewSecCell = HCELL_NIL;
+    HCELL_INDEX SubKey, NewSubKey;
     ULONG Index, SubKeyCount;
+
     PAGED_CODE();

DPRINT("CmpDeepCopyKeyInternal(0x%08X, 0x%08X, 0x%08X, 0x%08X, 0x%08X, 0x%08X)\n",
@@ -2401,6 +2403,7 @@

/* Get the source cell node */
     SrcNode = HvGetCell(SourceHive, SrcKeyCell);
+    ASSERT(SrcNode);

/* Sanity check */
     ASSERT(SrcNode->Signature == CM_KEY_NODE_SIGNATURE);
@@ -2419,12 +2422,55 @@

/* Get the destination cell node */
     DestNode = HvGetCell(DestinationHive, NewKeyCell);
-
-    /* Set the parent */
+    ASSERT(DestNode);
+
+    /* Set the parent and copy the flags */
     DestNode->Parent = Parent;
-
-    // TODO: These should also be copied!
-    DestNode->Security = DestNode->Class = HCELL_NIL;
+    DestNode->Flags  = (SrcNode->Flags & KEY_COMP_NAME); // Keep only the single permanent flag
+    if (Parent == HCELL_NIL)
+    {
+        /* This is the new root node */
+        DestNode->Flags |= KEY_HIVE_ENTRY | KEY_NO_DELETE;
+    }
+
+    /* Copy the class cell */
+    if (SrcNode->ClassLength > 0)
+    {
+        NewClassCell = CmpCopyCell(SourceHive,
+                                   SrcNode->Class,
+                                   DestinationHive,
+                                   StorageType);
+        if (NewClassCell == HCELL_NIL)
+        {
+            /* Not enough storage space */
+            Status = STATUS_INSUFFICIENT_RESOURCES;
+            goto Cleanup;
+        }
+
+        DestNode->Class = NewClassCell;
+        DestNode->ClassLength = SrcNode->ClassLength;
+    }
+    else
+    {
+        DestNode->Class = HCELL_NIL;
+        DestNode->ClassLength = 0;
+    }
+
+    /* Copy the security cell (FIXME: HACKish poor-man version) */
+    if (SrcNode->Security != HCELL_NIL)
+    {
+        NewSecCell = CmpCopyCell(SourceHive,
+                                 SrcNode->Security,
+                                 DestinationHive,
+                                 StorageType);
+        if (NewSecCell == HCELL_NIL)
+        {
+            /* Not enough storage space */
+            Status = STATUS_INSUFFICIENT_RESOURCES;
+            goto Cleanup;
+        }
+    }
+    DestNode->Security = NewSecCell;

/* Copy the value list */
     Status = CmpCopyKeyValueList(SourceHive,
@@ -2432,7 +2478,8 @@
                                  DestinationHive,
                                  &DestNode->ValueList,
                                  StorageType);
-    if (!NT_SUCCESS(Status)) goto Cleanup;
+    if (!NT_SUCCESS(Status))
+        goto Cleanup;

/* Clear the invalid subkey index */
     DestNode->SubKeyCounts[Stable] = DestNode->SubKeyCounts[Volatile] = 0;
@@ -2449,34 +2496,57 @@
         ASSERT(SubKey != HCELL_NIL);

/* Call the function recursively for the subkey */
+        //
+        // FIXME: Danger!! Kernel stack exhaustion!!
+        //
         Status = CmpDeepCopyKeyInternal(SourceHive,
                                         SubKey,
                                         DestinationHive,
                                         NewKeyCell,
                                         StorageType,
                                         &NewSubKey);
-        if (!NT_SUCCESS(Status)) goto Cleanup;
+        if (!NT_SUCCESS(Status))
+            goto Cleanup;

/* Add the copy of the subkey to the new key */
         if (!CmpAddSubKey(DestinationHive,
                           NewKeyCell,
                           NewSubKey))
         {
+            /* Cleanup allocated cell */
+            HvFreeCell(DestinationHive, NewSubKey);
+
             Status = STATUS_INSUFFICIENT_RESOURCES;
             goto Cleanup;
         }
     }

-    /* Set the cell index if requested and return success */
+    /* Set success */
+    Status = STATUS_SUCCESS;
+
+Cleanup:
+
+    /* Release the cells */
+    if (DestNode) HvReleaseCell(DestinationHive, NewKeyCell);
+    if (SrcNode) HvReleaseCell(SourceHive, SrcKeyCell);
+
+    /* Cleanup allocated cells in case of failure */
+    if (!NT_SUCCESS(Status))
+    {
+        if (NewSecCell != HCELL_NIL)
+            HvFreeCell(DestinationHive, NewSecCell);
+
+        if (NewClassCell != HCELL_NIL)
+            HvFreeCell(DestinationHive, NewClassCell);
+
+        if (NewKeyCell != HCELL_NIL)
+            HvFreeCell(DestinationHive, NewKeyCell);
+
+        NewKeyCell = HCELL_NIL;
+    }
+
+    /* Set the cell index if requested and return status */
     if (DestKeyCell) *DestKeyCell = NewKeyCell;
-    Status = STATUS_SUCCESS;
-
-Cleanup:
-
-    /* Release the cells */
-    if (SrcNode) HvReleaseCell(SourceHive, SrcKeyCell);
-    if (DestNode) HvReleaseCell(DestinationHive, NewKeyCell);
-
     return Status;
 }