03/04: [SETUPLIB] Properly cleanup the temporary registry keys created for setting up the registry.

[Hermès Bélusca-Maïto]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=a635aa847518e844ff1e0…

commit a635aa847518e844ff1e099737c4879f80c430e4
Author:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
AuthorDate: Tue Jan 9 02:56:00 2018 +0100
Commit:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
CommitDate: Sun Oct 28 14:42:01 2018 +0100

    [SETUPLIB] Properly cleanup the temporary registry keys created for setting up the
registry.
    
    This includes also to remove the created symlinks.
    Symlinks deletion is special, as one has to open first the symlink
    itself (and not its target), then remove the "SymbolicLinkTarget" value
    before really deleting the key. Of course everything must be done under
    proper access rights.
    
    Additional changes:
    - Change prototype BOOLEAN CmpLinkKeyToHive(...) to NTSTATUS CreateSymLinkKey(...).
    - Silence few DPRINTs.
    - Document some FIXMEs that I need to inspect later on ReactOS.
    - HKEY --> HANDLE.
---
 base/setup/lib/registry.c      |  90 +++++++++++++++++++-----------
 base/setup/lib/utils/regutil.c | 122 ++++++++++++++++++++++++++++++++---------
 base/setup/lib/utils/regutil.h |  18 ++++--
 3 files changed, 166 insertions(+), 64 deletions(-)

diff --git a/base/setup/lib/registry.c b/base/setup/lib/registry.c
index 69f47c3fdb..810002d840 100644
--- a/base/setup/lib/registry.c
+++ b/base/setup/lib/registry.c
@@ -67,7 +67,7 @@
 #define Architecture L"ppc"
 #endif
 
-/* FUNCTIONS ****************************************************************/
+/* GLOBALS ******************************************************************/
 
 #define REGISTRY_SETUP_MACHINE 
L"\\Registry\\Machine\\SYSTEM\\USetup_Machine\\"
 #define REGISTRY_SETUP_USER     L"\\Registry\\Machine\\SYSTEM\\USetup_User\\"
@@ -90,6 +90,8 @@ ROOT_KEY RootKeys[] =
 #endif
 };
 
+/* FUNCTIONS ****************************************************************/
+
 #define IsPredefKey(HKey)       \
     (((ULONG_PTR)(HKey) & 0xF0000000) == 0x80000000)
 
@@ -789,7 +791,9 @@ RegInitializeRegistry(
                          &ObjectAttributes,
                          0,
                          NULL,
-                         REG_OPTION_NON_VOLATILE, // REG_OPTION_VOLATILE, // FIXME!
+                    // FIXME: Using REG_OPTION_VOLATILE works OK on Windows,
+                    // but I need to check whether it works OK on ReactOS too.
+                         REG_OPTION_NON_VOLATILE, // REG_OPTION_VOLATILE,
                          &Disposition);
     if (!NT_SUCCESS(Status))
     {
@@ -811,7 +815,9 @@ RegInitializeRegistry(
                          &ObjectAttributes,
                          0,
                          NULL,
-                         REG_OPTION_NON_VOLATILE, // REG_OPTION_VOLATILE, // FIXME!
+                    // FIXME: Using REG_OPTION_VOLATILE works OK on Windows,
+                    // but I need to check whether it works OK on ReactOS too.
+                         REG_OPTION_NON_VOLATILE, // REG_OPTION_VOLATILE,
                          &Disposition);
     if (!NT_SUCCESS(Status))
     {
@@ -838,15 +844,18 @@ RegInitializeRegistry(
                                      /* SystemSecurity, sizeof(SystemSecurity) */);
             if (!NT_SUCCESS(Status))
             {
-                DPRINT1("ConnectRegistry(%S) failed, Status 0x%08lx\n",
RegistryHives[i].HiveName, Status);
+                DPRINT1("ConnectRegistry(%S) failed, Status 0x%08lx\n",
+                        RegistryHives[i].HiveName, Status);
             }
 
             /* Create the registry symlink to this key */
-            if
(!CmpLinkKeyToHive(RootKeys[GetPredefKeyIndex(RegistryHives[i].PredefKeyHandle)].Handle,
-                                  RegistryHives[i].RegSymLink,
-                                  RegistryHives[i].HiveRegistryPath))
+            Status =
CreateSymLinkKey(RootKeys[GetPredefKeyIndex(RegistryHives[i].PredefKeyHandle)].Handle,
+                                      RegistryHives[i].RegSymLink,
+                                      RegistryHives[i].HiveRegistryPath);
+            if (!NT_SUCCESS(Status))
             {
-                DPRINT1("CmpLinkKeyToHive(%S) failed!\n",
RegistryHives[i].HiveName);
+                DPRINT1("CreateSymLinkKey(%S) failed, Status 0x%08lx\n",
+                        RegistryHives[i].RegSymLink, Status);
             }
         }
         else
@@ -865,7 +874,9 @@ RegInitializeRegistry(
                                  &ObjectAttributes,
                                  0,
                                  NULL,
-                                 REG_OPTION_NON_VOLATILE, // REG_OPTION_VOLATILE, //
FIXME!
+                            // FIXME: Using REG_OPTION_VOLATILE works OK on Windows,
+                            // but I need to check whether it works OK on ReactOS too.
+                                 REG_OPTION_NON_VOLATILE, // REG_OPTION_VOLATILE,
                                  &Disposition);
             if (!NT_SUCCESS(Status))
             {
@@ -935,9 +946,9 @@ RegInitializeRegistry(
     }
     else
     {
-        DPRINT1("NtCreateKey() succeeded to %s the %wZ key (Status %lx)\n",
-                Disposition == REG_CREATED_NEW_KEY ? "create" : /*
REG_OPENED_EXISTING_KEY */ "open",
-                &KeyName, Status);
+        DPRINT("NtCreateKey() succeeded to %s the %wZ key (Status %lx)\n",
+               Disposition == REG_CREATED_NEW_KEY ? "create" : /*
REG_OPENED_EXISTING_KEY */ "open",
+               &KeyName, Status);
     }
     RootKeys[GetPredefKeyIndex(HKEY_CLASSES_ROOT)].Handle = KeyHandle;
 
@@ -967,18 +978,19 @@ RegInitializeRegistry(
     }
     else
     {
-        DPRINT1("NtCreateKey() succeeded to %s the ControlSet001 key (Status
%lx)\n",
-                Disposition == REG_CREATED_NEW_KEY ? "create" : /*
REG_OPENED_EXISTING_KEY */ "open",
-                Status);
+        DPRINT("NtCreateKey() succeeded to %s the ControlSet001 key (Status
%lx)\n",
+               Disposition == REG_CREATED_NEW_KEY ? "create" : /*
REG_OPENED_EXISTING_KEY */ "open",
+               Status);
     }
     NtClose(KeyHandle);
 
     /* Create the 'HKLM\SYSTEM\CurrentControlSet' symlink */
-    if (!CmpLinkKeyToHive(RootKeys[GetPredefKeyIndex(HKEY_LOCAL_MACHINE)].Handle,
-                          L"SYSTEM\\CurrentControlSet",
-                          REGISTRY_SETUP_MACHINE L"SYSTEM\\ControlSet001"))
+    Status = CreateSymLinkKey(RootKeys[GetPredefKeyIndex(HKEY_LOCAL_MACHINE)].Handle,
+                              L"SYSTEM\\CurrentControlSet",
+                              REGISTRY_SETUP_MACHINE
L"SYSTEM\\ControlSet001");
+    if (!NT_SUCCESS(Status))
     {
-        DPRINT1("CmpLinkKeyToHive(CurrentControlSet) failed!\n");
+        DPRINT1("CreateSymLinkKey(CurrentControlSet) failed, Status 0x%08lx\n",
Status);
     }
 
 
@@ -1026,15 +1038,37 @@ RegCleanupRegistry(
     }
 
     /*
-     * Note that we don't need to explicitly remove the symlinks we have created
-     * since they are created volatile, inside registry keys that will be however
-     * removed explictly in the following.
+     * To keep the running system clean we need first to remove the symlinks
+     * we have created and then unmounting the hives. Finally we delete the
+     * master registry keys.
      */
 
     for (i = 0; i < ARRAYSIZE(RegistryHives); ++i)
     {
-        if (RegistryHives[i].State != Create && RegistryHives[i].State !=
Repair)
+        if (RegistryHives[i].State == Create || RegistryHives[i].State == Repair)
+        {
+            /* Delete the registry symlink to this key */
+            Status =
DeleteSymLinkKey(RootKeys[GetPredefKeyIndex(RegistryHives[i].PredefKeyHandle)].Handle,
+                                      RegistryHives[i].RegSymLink);
+            if (!NT_SUCCESS(Status))
+            {
+                DPRINT1("DeleteSymLinkKey(%S) failed, Status 0x%08lx\n",
+                        RegistryHives[i].RegSymLink, Status);
+            }
+
+            /* Unmount the hive */
+            Status = DisconnectRegistry(NULL,
+                                        RegistryHives[i].HiveRegistryPath,
+                                        1 /* REG_FORCE_UNLOAD */);
+            DPRINT1("Unmounting '%S' %s\n",
RegistryHives[i].HiveRegistryPath, NT_SUCCESS(Status) ? "succeeded" :
"failed");
+
+            /* Switch the hive state to 'Update' */
+            RegistryHives[i].State = Update;
+        }
+        else
         {
+            /* Delete the *DUMMY* volatile hives created for the update procedure */
+
             RtlInitUnicodeString(&KeyName, RegistryHives[i].RegSymLink);
             InitializeObjectAttributes(&ObjectAttributes,
                                        &KeyName,
@@ -1054,16 +1088,6 @@ RegCleanupRegistry(
             NtDeleteKey(KeyHandle);
             NtClose(KeyHandle);
         }
-        else
-        {
-            Status = DisconnectRegistry(NULL,
-                                        RegistryHives[i].HiveRegistryPath,
-                                        1 /* REG_FORCE_UNLOAD */);
-            DPRINT1("Unmounting '%S' %s\n",
RegistryHives[i].HiveRegistryPath, NT_SUCCESS(Status) ? "succeeded" :
"failed");
-
-            /* Switch the hive state to 'Update' */
-            RegistryHives[i].State = Update;
-        }
     }
 
     /*
diff --git a/base/setup/lib/utils/regutil.c b/base/setup/lib/utils/regutil.c
index f70d9545e2..60e0e9fd95 100644
--- a/base/setup/lib/utils/regutil.c
+++ b/base/setup/lib/utils/regutil.c
@@ -16,6 +16,11 @@
 #define NDEBUG
 #include <debug.h>
 
+/* GLOBALS ******************************************************************/
+
+static UNICODE_STRING SymbolicLinkValueName =
+    RTL_CONSTANT_STRING(L"SymbolicLinkValue");
+
 /* FUNCTIONS ****************************************************************/
 
 /*
@@ -80,7 +85,7 @@ CreateNestedKey(PHANDLE KeyHandle,
                              &LocalObjectAttributes,
                              0,
                              NULL,
-                             REG_OPTION_NON_VOLATILE,
+                             REG_OPTION_NON_VOLATILE, // FIXME ?
                              &Disposition);
         DPRINT("NtCreateKey(%wZ) called (Status %lx)\n", &LocalKeyName,
Status);
         if (!NT_SUCCESS(Status) && Status != STATUS_OBJECT_NAME_NOT_FOUND)
@@ -244,19 +249,17 @@ CreateRegistryFile(
     return Status;
 }
 
-BOOLEAN
-CmpLinkKeyToHive(
-    IN HANDLE RootLinkKeyHandle OPTIONAL,
+/* Adapted from ntoskrnl/config/cmsysini.c:CmpLinkKeyToHive() */
+NTSTATUS
+CreateSymLinkKey(
+    IN HANDLE RootKey OPTIONAL,
     IN PCWSTR LinkKeyName,
     IN PCWSTR TargetKeyName)
 {
-    static UNICODE_STRING CmSymbolicLinkValueName =
-        RTL_CONSTANT_STRING(L"SymbolicLinkValue");
-
     NTSTATUS Status;
     OBJECT_ATTRIBUTES ObjectAttributes;
     UNICODE_STRING KeyName;
-    HANDLE TargetKeyHandle;
+    HANDLE LinkKeyHandle;
     ULONG Disposition;
 
     /* Initialize the object attributes */
@@ -264,11 +267,11 @@ CmpLinkKeyToHive(
     InitializeObjectAttributes(&ObjectAttributes,
                                &KeyName,
                                OBJ_CASE_INSENSITIVE,
-                               RootLinkKeyHandle,
+                               RootKey,
                                NULL);
 
     /* Create the link key */
-    Status = NtCreateKey(&TargetKeyHandle,
+    Status = NtCreateKey(&LinkKeyHandle,
                          KEY_SET_VALUE | KEY_CREATE_LINK,
                          &ObjectAttributes,
                          0,
@@ -277,39 +280,108 @@ CmpLinkKeyToHive(
                          &Disposition);
     if (!NT_SUCCESS(Status))
     {
-        DPRINT1("CmpLinkKeyToHive: couldn't create %S, Status =
0x%08lx\n",
+        DPRINT1("CreateSymLinkKey: couldn't create '%S', Status =
0x%08lx\n",
                 LinkKeyName, Status);
-        return FALSE;
+        return Status;
     }
 
     /* Check if the new key was actually created */
     if (Disposition != REG_CREATED_NEW_KEY)
     {
-        DPRINT1("CmpLinkKeyToHive: %S already exists!\n", LinkKeyName);
-        NtClose(TargetKeyHandle);
-        return FALSE;
+        DPRINT1("CreateSymLinkKey: %S already exists!\n", LinkKeyName);
+        NtClose(LinkKeyHandle);
+        return STATUS_OBJECT_NAME_EXISTS; // STATUS_OBJECT_NAME_COLLISION;
     }
 
     /* Set the target key name as link target */
     RtlInitUnicodeString(&KeyName, TargetKeyName);
-    Status = NtSetValueKey(TargetKeyHandle,
-                           &CmSymbolicLinkValueName,
+    Status = NtSetValueKey(LinkKeyHandle,
+                           &SymbolicLinkValueName,
                            0,
                            REG_LINK,
                            KeyName.Buffer,
                            KeyName.Length);
 
     /* Close the link key handle */
-    NtClose(TargetKeyHandle);
+    NtClose(LinkKeyHandle);
 
     if (!NT_SUCCESS(Status))
     {
-        DPRINT1("CmpLinkKeyToHive: couldn't create symbolic link for %S, Status
= 0x%08lx\n",
-                TargetKeyName, Status);
-        return FALSE;
+        DPRINT1("CreateSymLinkKey: couldn't create symbolic link '%S'
for '%S', Status = 0x%08lx\n",
+                LinkKeyName, TargetKeyName, Status);
     }
 
-    return TRUE;
+    return Status;
+}
+
+NTSTATUS
+DeleteSymLinkKey(
+    IN HANDLE RootKey OPTIONAL,
+    IN PCWSTR LinkKeyName)
+{
+    NTSTATUS Status;
+    OBJECT_ATTRIBUTES ObjectAttributes;
+    UNICODE_STRING KeyName;
+    HANDLE LinkKeyHandle;
+    // ULONG Disposition;
+
+    /* Initialize the object attributes */
+    RtlInitUnicodeString(&KeyName, LinkKeyName);
+    InitializeObjectAttributes(&ObjectAttributes,
+                               &KeyName,
+            /* Open the symlink key itself if it exists, and not its target */
+                               OBJ_CASE_INSENSITIVE | OBJ_OPENIF | OBJ_OPENLINK,
+                               RootKey,
+                               NULL);
+
+    /*
+     * Note: We could use here NtOpenKey() but it does not allow to pass
+     * opening options. NtOpenKeyEx() could do it but is Windows 7+.
+     * So we use the good old NtCreateKey() that can open the key.
+     */
+#if 0
+    Status = NtCreateKey(&LinkKeyHandle,
+                         DELETE | KEY_SET_VALUE | KEY_CREATE_LINK,
+                         &ObjectAttributes,
+                         0,
+                         NULL,
+                         /*REG_OPTION_VOLATILE |*/ REG_OPTION_OPEN_LINK,
+                         &Disposition);
+#else
+    Status = NtOpenKey(&LinkKeyHandle,
+                       DELETE | KEY_SET_VALUE | KEY_CREATE_LINK,
+                       &ObjectAttributes);
+#endif
+    if (!NT_SUCCESS(Status))
+    {
+        DPRINT1("NtOpenKey(%wZ) failed (Status %lx)\n", &KeyName, Status);
+        return Status;
+    }
+
+    /*
+     * Delete the special "SymbolicLinkValue" value.
+     * This is technically not needed since we are going to remove
+     * the key anyways, but it is good practice to do it.
+     */
+    Status = NtDeleteValueKey(LinkKeyHandle, &SymbolicLinkValueName);
+    if (!NT_SUCCESS(Status))
+    {
+        DPRINT1("NtDeleteValueKey(%wZ) failed (Status %lx)\n", &KeyName,
Status);
+        NtClose(LinkKeyHandle);
+        return Status;
+    }
+
+    /* Finally delete the key itself and close the link key handle */
+    Status = NtDeleteKey(LinkKeyHandle);
+    NtClose(LinkKeyHandle);
+
+    if (!NT_SUCCESS(Status))
+    {
+        DPRINT1("DeleteSymLinkKey: couldn't delete symbolic link '%S',
Status = 0x%08lx\n",
+                LinkKeyName, Status);
+    }
+
+    return Status;
 }
 
 /*
@@ -317,7 +389,7 @@ CmpLinkKeyToHive(
  */
 NTSTATUS
 ConnectRegistry(
-    IN HKEY RootKey OPTIONAL,
+    IN HANDLE RootKey OPTIONAL,
     IN PCWSTR RegMountPoint,
     // IN HANDLE RootDirectory OPTIONAL,
     IN PUNICODE_STRING NtSystemRoot,
@@ -358,7 +430,7 @@ ConnectRegistry(
  */
 NTSTATUS
 DisconnectRegistry(
-    IN HKEY RootKey OPTIONAL,
+    IN HANDLE RootKey OPTIONAL,
     IN PCWSTR RegMountPoint,
     IN ULONG Flags)
 {
@@ -381,7 +453,7 @@ DisconnectRegistry(
  */
 NTSTATUS
 VerifyRegistryHive(
-    // IN HKEY RootKey OPTIONAL,
+    // IN HANDLE RootKey OPTIONAL,
     // // IN HANDLE RootDirectory OPTIONAL,
     IN PUNICODE_STRING NtSystemRoot,
     IN PCWSTR RegistryKey /* ,
diff --git a/base/setup/lib/utils/regutil.h b/base/setup/lib/utils/regutil.h
index 401638717e..c7c74b507a 100644
--- a/base/setup/lib/utils/regutil.h
+++ b/base/setup/lib/utils/regutil.h
@@ -35,18 +35,24 @@ CreateRegistryFile(
 */
     );
 
-BOOLEAN
-CmpLinkKeyToHive(
-    IN HANDLE RootLinkKeyHandle OPTIONAL,
+/* Adapted from ntoskrnl/config/cmsysini.c:CmpLinkKeyToHive() */
+NTSTATUS
+CreateSymLinkKey(
+    IN HANDLE RootKey OPTIONAL,
     IN PCWSTR LinkKeyName,
     IN PCWSTR TargetKeyName);
 
+NTSTATUS
+DeleteSymLinkKey(
+    IN HANDLE RootKey OPTIONAL,
+    IN PCWSTR LinkKeyName);
+
 /*
  * Should be called under SE_RESTORE_PRIVILEGE privilege
  */
 NTSTATUS
 ConnectRegistry(
-    IN HKEY RootKey OPTIONAL,
+    IN HANDLE RootKey OPTIONAL,
     IN PCWSTR RegMountPoint,
     // IN HANDLE RootDirectory OPTIONAL,
     IN PUNICODE_STRING NtSystemRoot,
@@ -62,7 +68,7 @@ ConnectRegistry(
  */
 NTSTATUS
 DisconnectRegistry(
-    IN HKEY RootKey OPTIONAL,
+    IN HANDLE RootKey OPTIONAL,
     IN PCWSTR RegMountPoint,
     IN ULONG Flags);
 
@@ -71,7 +77,7 @@ DisconnectRegistry(
  */
 NTSTATUS
 VerifyRegistryHive(
-    // IN HKEY RootKey OPTIONAL,
+    // IN HANDLE RootKey OPTIONAL,
     // // IN HANDLE RootDirectory OPTIONAL,
     IN PUNICODE_STRING NtSystemRoot,
     IN PCWSTR RegistryKey /* ,