[PATCH v7 0/8] MR1445: kernelbase: Some refactoring of the open_key() and create_key() functions.

[Sven Baars (＠sbaars)]

This is in preparation of implementing all logic from the server in kernelbase, which is needed because Nt*Key ignores KEY_WOW64_32KEY. Unfortunately this requires a lot of server calls.

The full branch can be found here

https://gitlab.winehq.org/sbaars/wine/-/tree/shared-classes-new

The old approach that doesn't duplicate code to kernelbase can be found here

https://gitlab.winehq.org/sbaars/wine/-/tree/shared-classes-old

I ran make_requests by the way, not sure if we should still leave that out with the gitlab workflow.

--
  v7: kernelbase: Factor creating a subkey out of create_key().
      kernelbase: Use open_subkey() in create_key().
      kernelbase: Always try to open the Wow6432Node in create_key()
      kernelbase: Add a fast path to create_key().
      kernelbase: Move create_key() below open_key().
      kernelbase: Factor opening a subkey out of open_key().
      kernelbase: Always try to open the Wow6432Node in open_key().
      kernelbase: Restructure the open_key() loop.

https://gitlab.winehq.org/wine/wine/-/merge_requests/1445

[PATCH v7 1/8] kernelbase: Restructure the open_key() loop.

[Sven Baars]

From: Sven Baars

---
 dlls/kernelbase/registry.c | 50 +++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/dlls/kernelbase/registry.c b/dlls/kernelbase/registry.c
index 91462d80e06..9ebf281ef71 100644
--- a/dlls/kernelbase/registry.c
+++ b/dlls/kernelbase/registry.c
@@ -100,7 +100,7 @@ static inline BOOL is_version_nt(void)

static BOOL is_wow6432node( const UNICODE_STRING *name )
 {
-    return (name->Length == 11 * sizeof(WCHAR) && !wcsnicmp( name->Buffer, L"Wow6432Node", 11 ));
+    return (name->Length >= 11 * sizeof(WCHAR) && !wcsnicmp( name->Buffer, L"Wow6432Node", 11 ));
 }

/* open the Wow6432Node subkey of the specified key */
@@ -221,7 +221,7 @@ static NTSTATUS open_key( HKEY *retkey, DWORD options, ACCESS_MASK access, OBJEC
     BOOL force_wow32 = is_win64 && (access & KEY_WOW64_32KEY);
     HANDLE subkey, root = attr->RootDirectory;
     WCHAR *buffer = attr->ObjectName->Buffer;
-    DWORD pos = 0, i = 0, len = attr->ObjectName->Length / sizeof(WCHAR);
+    DWORD i, len = attr->ObjectName->Length / sizeof(WCHAR);
     UNICODE_STRING str;

*retkey = NULL;
@@ -238,24 +238,17 @@ static NTSTATUS open_key( HKEY *retkey, DWORD options, ACCESS_MASK access, OBJEC
         return status;
     }

-    if (len && buffer[0] == '\') return STATUS_OBJECT_PATH_INVALID;
-    while (i < len && buffer[i] != '\') i++;
     attr->ObjectName = &str;

-    for (;;)
+    while (len)
     {
-        str.Buffer = buffer + pos;
-        str.Length = (i - pos) * sizeof(WCHAR);
-        if (force_wow32 && pos)
-        {
-            if (is_wow6432node( &str )) force_wow32 = FALSE;
-            else if ((subkey = open_wow6432node( attr->RootDirectory )))
-            {
-                if (attr->RootDirectory != root) NtClose( attr->RootDirectory );
-                attr->RootDirectory = subkey;
-                force_wow32 = FALSE;
-            }
-        }
+        i = 0;
+        if (buffer[0] == '\') return STATUS_OBJECT_PATH_INVALID;
+        while (i < len && buffer[i] != '\') i++;
+
+        str.Buffer = buffer;
+        str.Length = i * sizeof(WCHAR);
+
         if (i == len)
         {
             if (options & REG_OPTION_OPEN_LINK) attr->Attributes |= OBJ_OPENLINK;
@@ -269,15 +262,22 @@ static NTSTATUS open_key( HKEY *retkey, DWORD options, ACCESS_MASK access, OBJEC
         if (attr->RootDirectory != root) NtClose( attr->RootDirectory );
         if (status) return status;
         attr->RootDirectory = subkey;
-        if (i == len) break;
         while (i < len && buffer[i] == '\') i++;
-        pos = i;
-        while (i < len && buffer[i] != '\') i++;
-    }
-    if (force_wow32 && (subkey = open_wow6432node( attr->RootDirectory )))
-    {
-        if (attr->RootDirectory != root) NtClose( attr->RootDirectory );
-        attr->RootDirectory = subkey;
+        buffer += i;
+        len -= i;
+
+        if (force_wow32)
+        {
+            str.Buffer = buffer;
+            str.Length = len * sizeof(WCHAR);
+            if (is_wow6432node( &str )) force_wow32 = FALSE;
+            else if ((subkey = open_wow6432node( attr->RootDirectory )))
+            {
+                NtClose( attr->RootDirectory );
+                attr->RootDirectory = subkey;
+                force_wow32 = FALSE;
+            }
+        }
     }
     if (status == STATUS_PREDEFINED_HANDLE)
     {

[PATCH v7 2/8] kernelbase: Always try to open the Wow6432Node in open_key().

[Sven Baars]

From: Sven Baars

---
 dlls/kernelbase/registry.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/dlls/kernelbase/registry.c b/dlls/kernelbase/registry.c
index 9ebf281ef71..668bceac57a 100644
--- a/dlls/kernelbase/registry.c
+++ b/dlls/kernelbase/registry.c
@@ -218,7 +218,6 @@ static NTSTATUS create_key( HKEY *retkey, ACCESS_MASK access, OBJECT_ATTRIBUTES
 static NTSTATUS open_key( HKEY *retkey, DWORD options, ACCESS_MASK access, OBJECT_ATTRIBUTES *attr )
 {
     NTSTATUS status;
-    BOOL force_wow32 = is_win64 && (access & KEY_WOW64_32KEY);
     HANDLE subkey, root = attr->RootDirectory;
     WCHAR *buffer = attr->ObjectName->Buffer;
     DWORD i, len = attr->ObjectName->Length / sizeof(WCHAR);
@@ -226,7 +225,7 @@ static NTSTATUS open_key( HKEY *retkey, DWORD options, ACCESS_MASK access, OBJEC

*retkey = NULL;

-    if (!force_wow32)
+    if (!(is_win64 && (access & KEY_WOW64_32KEY)))
     {
         if (options & REG_OPTION_OPEN_LINK) attr->Attributes |= OBJ_OPENLINK;
         status = NtOpenKeyEx( (HANDLE *)retkey, access, attr, options );
@@ -266,16 +265,14 @@ static NTSTATUS open_key( HKEY *retkey, DWORD options, ACCESS_MASK access, OBJEC
         buffer += i;
         len -= i;

-        if (force_wow32)
+        str.Buffer = buffer;
+        str.Length = len * sizeof(WCHAR);
+        if (!is_wow6432node( &str ))
         {
-            str.Buffer = buffer;
-            str.Length = len * sizeof(WCHAR);
-            if (is_wow6432node( &str )) force_wow32 = FALSE;
-            else if ((subkey = open_wow6432node( attr->RootDirectory )))
+            if ((subkey = open_wow6432node( attr->RootDirectory )))
             {
                 NtClose( attr->RootDirectory );
                 attr->RootDirectory = subkey;
-                force_wow32 = FALSE;
             }
         }
     }

[PATCH v7 3/8] kernelbase: Factor opening a subkey out of open_key().

[Sven Baars]

From: Sven Baars

---
 dlls/kernelbase/registry.c | 117 ++++++++++++++++++++++---------------
 1 file changed, 71 insertions(+), 46 deletions(-)

diff --git a/dlls/kernelbase/registry.c b/dlls/kernelbase/registry.c
index 668bceac57a..2e3726096ad 100644
--- a/dlls/kernelbase/registry.c
+++ b/dlls/kernelbase/registry.c
@@ -214,14 +214,71 @@ static NTSTATUS create_key( HKEY *retkey, ACCESS_MASK access, OBJECT_ATTRIBUTES
     return status;
 }

+static NTSTATUS open_subkey( HKEY *subkey, HKEY root, UNICODE_STRING *name, DWORD options, ACCESS_MASK access )
+{
+    DWORD i = 0, len = name->Length / sizeof(WCHAR);
+    WCHAR *buffer = name->Buffer;
+    OBJECT_ATTRIBUTES attr;
+    UNICODE_STRING str;
+    NTSTATUS status = 0;
+
+    attr.Length = sizeof(attr);
+    attr.RootDirectory = (HANDLE)root;
+    attr.ObjectName = &str;
+    attr.Attributes = 0;
+    attr.SecurityDescriptor = NULL;
+    attr.SecurityQualityOfService = NULL;
+
+    if (buffer[0] == '\') return STATUS_OBJECT_PATH_INVALID;
+    while (i < len && buffer[i] != '\') i++;
+
+    str.Buffer = name->Buffer;
+    str.Length = i * sizeof(WCHAR);
+
+    if (i == len)
+    {
+        if (options & REG_OPTION_OPEN_LINK) attr.Attributes |= OBJ_OPENLINK;
+    }
+    else
+    {
+        if (!(options & REG_OPTION_OPEN_LINK)) attr.Attributes &= ~OBJ_OPENLINK;
+        options &= ~REG_OPTION_OPEN_LINK;
+    }
+
+    status = NtOpenKeyEx( (HANDLE *)subkey, access, &attr, options );
+    if (status == STATUS_PREDEFINED_HANDLE)
+    {
+        *subkey = get_perflib_key( *subkey );
+        status = STATUS_SUCCESS;
+    }
+
+    if (!status)
+    {
+        while (i < len && buffer[i] == '\') i++;
+
+        name->Buffer += i;
+        name->Length -= i * sizeof(WCHAR);
+
+        if (!is_wow6432node( name ))
+        {
+            HKEY wow6432node = open_wow6432node( *subkey );
+            if (wow6432node)
+            {
+                NtClose( *subkey );
+                *subkey = wow6432node;
+            }
+        }
+    }
+
+    return status;
+}
+
 /* wrapper for NtOpenKeyEx to handle Wow6432 nodes */
 static NTSTATUS open_key( HKEY *retkey, DWORD options, ACCESS_MASK access, OBJECT_ATTRIBUTES *attr )
 {
-    NTSTATUS status;
-    HANDLE subkey, root = attr->RootDirectory;
-    WCHAR *buffer = attr->ObjectName->Buffer;
-    DWORD i, len = attr->ObjectName->Length / sizeof(WCHAR);
-    UNICODE_STRING str;
+    HKEY root = attr->RootDirectory, subkey, subkey_root = root;
+    UNICODE_STRING name;
+    NTSTATUS status = 0;

*retkey = NULL;

@@ -237,51 +294,19 @@ static NTSTATUS open_key( HKEY *retkey, DWORD options, ACCESS_MASK access, OBJEC
         return status;
     }

-    attr->ObjectName = &str;
+    name.Buffer = attr->ObjectName->Buffer;
+    name.Length = attr->ObjectName->Length;

-    while (len)
+    while (!status && name.Length)
     {
-        i = 0;
-        if (buffer[0] == '\') return STATUS_OBJECT_PATH_INVALID;
-        while (i < len && buffer[i] != '\') i++;
-
-        str.Buffer = buffer;
-        str.Length = i * sizeof(WCHAR);
+        status = open_subkey( &subkey, subkey_root, &name, options, access );
+        if (subkey_root && subkey_root != root) NtClose( subkey_root );
+        subkey_root = subkey;
+    }

-        if (i == len)
-        {
-            if (options & REG_OPTION_OPEN_LINK) attr->Attributes |= OBJ_OPENLINK;
-            status = NtOpenKeyEx( &subkey, access, attr, options );
-        }
-        else
-        {
-            if (!(options & REG_OPTION_OPEN_LINK)) attr->Attributes &= ~OBJ_OPENLINK;
-            status = NtOpenKeyEx( &subkey, access, attr, options & ~REG_OPTION_OPEN_LINK );
-        }
-        if (attr->RootDirectory != root) NtClose( attr->RootDirectory );
-        if (status) return status;
-        attr->RootDirectory = subkey;
-        while (i < len && buffer[i] == '\') i++;
-        buffer += i;
-        len -= i;
+    if (!status)
+        *retkey = subkey_root;

-        str.Buffer = buffer;
-        str.Length = len * sizeof(WCHAR);
-        if (!is_wow6432node( &str ))
-        {
-            if ((subkey = open_wow6432node( attr->RootDirectory )))
-            {
-                NtClose( attr->RootDirectory );
-                attr->RootDirectory = subkey;
-            }
-        }
-    }
-    if (status == STATUS_PREDEFINED_HANDLE)
-    {
-        attr->RootDirectory = get_perflib_key( attr->RootDirectory );
-        status = STATUS_SUCCESS;
-    }
-    *retkey = attr->RootDirectory;
     return status;
 }

[PATCH v7 4/8] kernelbase: Move create_key() below open_key().

[Sven Baars]

From: Sven Baars

---
 dlls/kernelbase/registry.c | 148 ++++++++++++++++++-------------------
 1 file changed, 74 insertions(+), 74 deletions(-)

diff --git a/dlls/kernelbase/registry.c b/dlls/kernelbase/registry.c
index 2e3726096ad..f0c70508380 100644
--- a/dlls/kernelbase/registry.c
+++ b/dlls/kernelbase/registry.c
@@ -140,80 +140,6 @@ static HKEY get_perflib_key( HANDLE key )
     return key;
 }

-/* wrapper for NtCreateKey that creates the key recursively if necessary */
-static NTSTATUS create_key( HKEY *retkey, ACCESS_MASK access, OBJECT_ATTRIBUTES *attr,
-                            const UNICODE_STRING *class, ULONG options, PULONG dispos )
-{
-    BOOL force_wow32 = is_win64 && (access & KEY_WOW64_32KEY);
-    NTSTATUS status = STATUS_OBJECT_NAME_NOT_FOUND;
-    HANDLE subkey, root = attr->RootDirectory;
-
-    if (!force_wow32) status = NtCreateKey( &subkey, access, attr, 0, class, options, dispos );
-
-    if (status == STATUS_OBJECT_NAME_NOT_FOUND)
-    {
-        WCHAR *buffer = attr->ObjectName->Buffer;
-        DWORD attrs, pos = 0, i = 0, len = attr->ObjectName->Length / sizeof(WCHAR);
-        UNICODE_STRING str;
-
-        /* don't try to create registry root */
-        if (!attr->RootDirectory && len > 10 && !wcsnicmp( buffer, L"\Registry\", 10 )) i += 10;
-
-        while (i < len && buffer[i] != '\') i++;
-        if (i == len && !force_wow32) return status;
-
-        attrs = attr->Attributes;
-        attr->ObjectName = &str;
-
-        for (;;)
-        {
-            str.Buffer = buffer + pos;
-            str.Length = (i - pos) * sizeof(WCHAR);
-            if (force_wow32 && pos)
-            {
-                if (is_wow6432node( &str )) force_wow32 = FALSE;
-                else if ((subkey = open_wow6432node( attr->RootDirectory )))
-                {
-                    if (attr->RootDirectory != root) NtClose( attr->RootDirectory );
-                    attr->RootDirectory = subkey;
-                    force_wow32 = FALSE;
-                }
-            }
-            if (i == len)
-            {
-                attr->Attributes = attrs;
-                status = NtCreateKey( &subkey, access, attr, 0, class, options, dispos );
-            }
-            else
-            {
-                attr->Attributes = attrs & ~OBJ_OPENLINK;
-                status = NtCreateKey( &subkey, access, attr, 0, class,
-                                      options & ~REG_OPTION_CREATE_LINK, dispos );
-            }
-            if (attr->RootDirectory != root) NtClose( attr->RootDirectory );
-            if (!NT_SUCCESS(status)) return status;
-            if (i == len) break;
-            attr->RootDirectory = subkey;
-            while (i < len && buffer[i] == '\') i++;
-            pos = i;
-            while (i < len && buffer[i] != '\') i++;
-        }
-    }
-    attr->RootDirectory = subkey;
-    if (force_wow32 && (subkey = open_wow6432node( attr->RootDirectory )))
-    {
-        if (attr->RootDirectory != root) NtClose( attr->RootDirectory );
-        attr->RootDirectory = subkey;
-    }
-    if (status == STATUS_PREDEFINED_HANDLE)
-    {
-        attr->RootDirectory = get_perflib_key( attr->RootDirectory );
-        status = STATUS_SUCCESS;
-    }
-    *retkey = attr->RootDirectory;
-    return status;
-}
-
 static NTSTATUS open_subkey( HKEY *subkey, HKEY root, UNICODE_STRING *name, DWORD options, ACCESS_MASK access )
 {
     DWORD i = 0, len = name->Length / sizeof(WCHAR);
@@ -310,6 +236,80 @@ static NTSTATUS open_key( HKEY *retkey, DWORD options, ACCESS_MASK access, OBJEC
     return status;
 }

+/* wrapper for NtCreateKey that creates the key recursively if necessary */
+static NTSTATUS create_key( HKEY *retkey, ACCESS_MASK access, OBJECT_ATTRIBUTES *attr,
+                            const UNICODE_STRING *class, ULONG options, PULONG dispos )
+{
+    BOOL force_wow32 = is_win64 && (access & KEY_WOW64_32KEY);
+    NTSTATUS status = STATUS_OBJECT_NAME_NOT_FOUND;
+    HANDLE subkey, root = attr->RootDirectory;
+
+    if (!force_wow32) status = NtCreateKey( &subkey, access, attr, 0, class, options, dispos );
+
+    if (status == STATUS_OBJECT_NAME_NOT_FOUND)
+    {
+        WCHAR *buffer = attr->ObjectName->Buffer;
+        DWORD attrs, pos = 0, i = 0, len = attr->ObjectName->Length / sizeof(WCHAR);
+        UNICODE_STRING str;
+
+        /* don't try to create registry root */
+        if (!attr->RootDirectory && len > 10 && !wcsnicmp( buffer, L"\Registry\", 10 )) i += 10;
+
+        while (i < len && buffer[i] != '\') i++;
+        if (i == len && !force_wow32) return status;
+
+        attrs = attr->Attributes;
+        attr->ObjectName = &str;
+
+        for (;;)
+        {
+            str.Buffer = buffer + pos;
+            str.Length = (i - pos) * sizeof(WCHAR);
+            if (force_wow32 && pos)
+            {
+                if (is_wow6432node( &str )) force_wow32 = FALSE;
+                else if ((subkey = open_wow6432node( attr->RootDirectory )))
+                {
+                    if (attr->RootDirectory != root) NtClose( attr->RootDirectory );
+                    attr->RootDirectory = subkey;
+                    force_wow32 = FALSE;
+                }
+            }
+            if (i == len)
+            {
+                attr->Attributes = attrs;
+                status = NtCreateKey( &subkey, access, attr, 0, class, options, dispos );
+            }
+            else
+            {
+                attr->Attributes = attrs & ~OBJ_OPENLINK;
+                status = NtCreateKey( &subkey, access, attr, 0, class,
+                                      options & ~REG_OPTION_CREATE_LINK, dispos );
+            }
+            if (attr->RootDirectory != root) NtClose( attr->RootDirectory );
+            if (!NT_SUCCESS(status)) return status;
+            if (i == len) break;
+            attr->RootDirectory = subkey;
+            while (i < len && buffer[i] == '\') i++;
+            pos = i;
+            while (i < len && buffer[i] != '\') i++;
+        }
+    }
+    attr->RootDirectory = subkey;
+    if (force_wow32 && (subkey = open_wow6432node( attr->RootDirectory )))
+    {
+        if (attr->RootDirectory != root) NtClose( attr->RootDirectory );
+        attr->RootDirectory = subkey;
+    }
+    if (status == STATUS_PREDEFINED_HANDLE)
+    {
+        attr->RootDirectory = get_perflib_key( attr->RootDirectory );
+        status = STATUS_SUCCESS;
+    }
+    *retkey = attr->RootDirectory;
+    return status;
+}
+
 /* create one of the HKEY_* special root keys */
 static HKEY create_special_root_hkey( HKEY hkey, DWORD access )
 {

[PATCH v7 5/8] kernelbase: Add a fast path to create_key().

[Sven Baars]

From: Sven Baars

---
 dlls/kernelbase/registry.c | 69 +++++++++++++++++++++-----------------
 1 file changed, 39 insertions(+), 30 deletions(-)

diff --git a/dlls/kernelbase/registry.c b/dlls/kernelbase/registry.c
index f0c70508380..2cded4ede6b 100644
--- a/dlls/kernelbase/registry.c
+++ b/dlls/kernelbase/registry.c
@@ -244,37 +244,40 @@ static NTSTATUS create_key( HKEY *retkey, ACCESS_MASK access, OBJECT_ATTRIBUTES
     NTSTATUS status = STATUS_OBJECT_NAME_NOT_FOUND;
     HANDLE subkey, root = attr->RootDirectory;

-    if (!force_wow32) status = NtCreateKey( &subkey, access, attr, 0, class, options, dispos );
+    if (!force_wow32)
+    {
+        if (options & REG_OPTION_OPEN_LINK) attr->Attributes |= OBJ_OPENLINK;
+        status = NtCreateKey( (HANDLE *)retkey, access, attr, 0, class, options, dispos );
+        if (status == STATUS_PREDEFINED_HANDLE)
+        {
+            *retkey = get_perflib_key( *retkey );
+            status = STATUS_SUCCESS;
+        }
+
+        if (!status || status != STATUS_OBJECT_NAME_NOT_FOUND)
+            return status;
+    }

if (status == STATUS_OBJECT_NAME_NOT_FOUND)
     {
         WCHAR *buffer = attr->ObjectName->Buffer;
-        DWORD attrs, pos = 0, i = 0, len = attr->ObjectName->Length / sizeof(WCHAR);
+        DWORD attrs, i = 0, len = attr->ObjectName->Length / sizeof(WCHAR);
         UNICODE_STRING str;

-        /* don't try to create registry root */
-        if (!attr->RootDirectory && len > 10 && !wcsnicmp( buffer, L"\Registry\", 10 )) i += 10;
-
-        while (i < len && buffer[i] != '\') i++;
-        if (i == len && !force_wow32) return status;
-
         attrs = attr->Attributes;
         attr->ObjectName = &str;

-        for (;;)
+        while (len)
         {
-            str.Buffer = buffer + pos;
-            str.Length = (i - pos) * sizeof(WCHAR);
-            if (force_wow32 && pos)
-            {
-                if (is_wow6432node( &str )) force_wow32 = FALSE;
-                else if ((subkey = open_wow6432node( attr->RootDirectory )))
-                {
-                    if (attr->RootDirectory != root) NtClose( attr->RootDirectory );
-                    attr->RootDirectory = subkey;
-                    force_wow32 = FALSE;
-                }
-            }
+            i = 0;
+
+            /* don't try to create registry root */
+            if (!attr->RootDirectory && len > 10 && !wcsnicmp( buffer, L"\Registry\", 10 )) i += 10;
+            while (i < len && buffer[i] != '\') i++;
+
+            str.Buffer = buffer;
+            str.Length = i * sizeof(WCHAR);
+
             if (i == len)
             {
                 attr->Attributes = attrs;
@@ -288,19 +291,25 @@ static NTSTATUS create_key( HKEY *retkey, ACCESS_MASK access, OBJECT_ATTRIBUTES
             }
             if (attr->RootDirectory != root) NtClose( attr->RootDirectory );
             if (!NT_SUCCESS(status)) return status;
-            if (i == len) break;
             attr->RootDirectory = subkey;
             while (i < len && buffer[i] == '\') i++;
-            pos = i;
-            while (i < len && buffer[i] != '\') i++;
+            buffer += i;
+            len -= i;
+
+            if (force_wow32)
+            {
+                str.Buffer = buffer;
+                str.Length = len * sizeof(WCHAR);
+                if (is_wow6432node( &str )) force_wow32 = FALSE;
+                else if ((subkey = open_wow6432node( attr->RootDirectory )))
+                {
+                    NtClose( attr->RootDirectory );
+                    attr->RootDirectory = subkey;
+                    force_wow32 = FALSE;
+                }
+            }
         }
     }
-    attr->RootDirectory = subkey;
-    if (force_wow32 && (subkey = open_wow6432node( attr->RootDirectory )))
-    {
-        if (attr->RootDirectory != root) NtClose( attr->RootDirectory );
-        attr->RootDirectory = subkey;
-    }
     if (status == STATUS_PREDEFINED_HANDLE)
     {
         attr->RootDirectory = get_perflib_key( attr->RootDirectory );

[PATCH v7 6/8] kernelbase: Always try to open the Wow6432Node in create_key()

[Sven Baars]

From: Sven Baars

---
 dlls/kernelbase/registry.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/dlls/kernelbase/registry.c b/dlls/kernelbase/registry.c
index 2cded4ede6b..3c2352a3a16 100644
--- a/dlls/kernelbase/registry.c
+++ b/dlls/kernelbase/registry.c
@@ -240,11 +240,10 @@ static NTSTATUS open_key( HKEY *retkey, DWORD options, ACCESS_MASK access, OBJEC
 static NTSTATUS create_key( HKEY *retkey, ACCESS_MASK access, OBJECT_ATTRIBUTES *attr,
                             const UNICODE_STRING *class, ULONG options, PULONG dispos )
 {
-    BOOL force_wow32 = is_win64 && (access & KEY_WOW64_32KEY);
     NTSTATUS status = STATUS_OBJECT_NAME_NOT_FOUND;
     HANDLE subkey, root = attr->RootDirectory;

-    if (!force_wow32)
+    if (!(is_win64 && (access & KEY_WOW64_32KEY)))
     {
         if (options & REG_OPTION_OPEN_LINK) attr->Attributes |= OBJ_OPENLINK;
         status = NtCreateKey( (HANDLE *)retkey, access, attr, 0, class, options, dispos );
@@ -296,16 +295,14 @@ static NTSTATUS create_key( HKEY *retkey, ACCESS_MASK access, OBJECT_ATTRIBUTES
             buffer += i;
             len -= i;

-            if (force_wow32)
+            str.Buffer = buffer;
+            str.Length = len * sizeof(WCHAR);
+            if (!is_wow6432node( &str ))
             {
-                str.Buffer = buffer;
-                str.Length = len * sizeof(WCHAR);
-                if (is_wow6432node( &str )) force_wow32 = FALSE;
-                else if ((subkey = open_wow6432node( attr->RootDirectory )))
+                if ((subkey = open_wow6432node( attr->RootDirectory )))
                 {
                     NtClose( attr->RootDirectory );
                     attr->RootDirectory = subkey;
-                    force_wow32 = FALSE;
                 }
             }
         }

[PATCH v7 7/8] kernelbase: Use open_subkey() in create_key().

[Sven Baars]

From: Sven Baars

---
 dlls/kernelbase/registry.c | 56 +++++++++++++++++++++++---------------
 1 file changed, 34 insertions(+), 22 deletions(-)

diff --git a/dlls/kernelbase/registry.c b/dlls/kernelbase/registry.c
index 3c2352a3a16..757ced4f00a 100644
--- a/dlls/kernelbase/registry.c
+++ b/dlls/kernelbase/registry.c
@@ -142,6 +142,8 @@ static HKEY get_perflib_key( HANDLE key )

static NTSTATUS open_subkey( HKEY *subkey, HKEY root, UNICODE_STRING *name, DWORD options, ACCESS_MASK access )
 {
+    BOOL is_wow64_key = (access & KEY_WOW64_32KEY) || (is_wow64 && !(access & KEY_WOW64_64KEY));
+    ACCESS_MASK access_64 = (access & ~KEY_WOW64_32KEY) | KEY_WOW64_64KEY;
     DWORD i = 0, len = name->Length / sizeof(WCHAR);
     WCHAR *buffer = name->Buffer;
     OBJECT_ATTRIBUTES attr;
@@ -155,7 +157,8 @@ static NTSTATUS open_subkey( HKEY *subkey, HKEY root, UNICODE_STRING *name, DWOR
     attr.SecurityDescriptor = NULL;
     attr.SecurityQualityOfService = NULL;

-    if (buffer[0] == '\') return STATUS_OBJECT_PATH_INVALID;
+    if (!root && len > 10 && !wcsnicmp( buffer, L"\Registry\", 10 )) i += 10;
+    if (i < len && buffer[i] == '\') return STATUS_OBJECT_PATH_INVALID;
     while (i < len && buffer[i] != '\') i++;

str.Buffer = name->Buffer;
@@ -171,7 +174,7 @@ static NTSTATUS open_subkey( HKEY *subkey, HKEY root, UNICODE_STRING *name, DWOR
         options &= ~REG_OPTION_OPEN_LINK;
     }

-    status = NtOpenKeyEx( (HANDLE *)subkey, access, &attr, options );
+    status = NtOpenKeyEx( (HANDLE *)subkey, access_64, &attr, options );
     if (status == STATUS_PREDEFINED_HANDLE)
     {
         *subkey = get_perflib_key( *subkey );
@@ -185,7 +188,7 @@ static NTSTATUS open_subkey( HKEY *subkey, HKEY root, UNICODE_STRING *name, DWOR
         name->Buffer += i;
         name->Length -= i * sizeof(WCHAR);

-        if (!is_wow6432node( name ))
+        if (is_wow64_key && !is_wow6432node( name ))
         {
             HKEY wow6432node = open_wow6432node( *subkey );
             if (wow6432node)
@@ -241,7 +244,8 @@ static NTSTATUS create_key( HKEY *retkey, ACCESS_MASK access, OBJECT_ATTRIBUTES
                             const UNICODE_STRING *class, ULONG options, PULONG dispos )
 {
     NTSTATUS status = STATUS_OBJECT_NAME_NOT_FOUND;
-    HANDLE subkey, root = attr->RootDirectory;
+    HKEY subkey, root = attr->RootDirectory, subkey_root = root;
+    UNICODE_STRING name;

if (!(is_win64 && (access & KEY_WOW64_32KEY)))
     {
@@ -257,10 +261,31 @@ static NTSTATUS create_key( HKEY *retkey, ACCESS_MASK access, OBJECT_ATTRIBUTES
             return status;
     }

+    name.Buffer = attr->ObjectName->Buffer;
+    name.Length = attr->ObjectName->Length;
+
+    while (name.Length)
+    {
+        status = open_subkey( &subkey, subkey_root, &name, options & REG_OPTION_OPEN_LINK, access );
+        if (status) break;
+        if (subkey_root && subkey_root != root) NtClose( subkey_root );
+        subkey_root = subkey;
+    }
+    attr->RootDirectory = subkey_root;
+
+    if (!status && (options & REG_OPTION_CREATE_LINK))
+    {
+        NtClose( subkey_root );
+        status = STATUS_OBJECT_NAME_COLLISION;
+    }
+
+    if (!status)
+        if (dispos) *dispos = REG_OPENED_EXISTING_KEY;
+
     if (status == STATUS_OBJECT_NAME_NOT_FOUND)
     {
-        WCHAR *buffer = attr->ObjectName->Buffer;
-        DWORD attrs, i = 0, len = attr->ObjectName->Length / sizeof(WCHAR);
+        WCHAR *buffer = name.Buffer;
+        DWORD attrs, i = 0, len = name.Length / sizeof(WCHAR);
         UNICODE_STRING str;

attrs = attr->Attributes;
@@ -269,9 +294,7 @@ static NTSTATUS create_key( HKEY *retkey, ACCESS_MASK access, OBJECT_ATTRIBUTES
         while (len)
         {
             i = 0;
-
-            /* don't try to create registry root */
-            if (!attr->RootDirectory && len > 10 && !wcsnicmp( buffer, L"\Registry\", 10 )) i += 10;
+            if (buffer[0] == '\') return STATUS_OBJECT_PATH_INVALID;
             while (i < len && buffer[i] != '\') i++;

str.Buffer = buffer;
@@ -280,12 +303,12 @@ static NTSTATUS create_key( HKEY *retkey, ACCESS_MASK access, OBJECT_ATTRIBUTES
             if (i == len)
             {
                 attr->Attributes = attrs;
-                status = NtCreateKey( &subkey, access, attr, 0, class, options, dispos );
+                status = NtCreateKey( (HANDLE *)&subkey, access, attr, 0, class, options, dispos );
             }
             else
             {
                 attr->Attributes = attrs & ~OBJ_OPENLINK;
-                status = NtCreateKey( &subkey, access, attr, 0, class,
+                status = NtCreateKey( (HANDLE *)&subkey, access, attr, 0, class,
                                       options & ~REG_OPTION_CREATE_LINK, dispos );
             }
             if (attr->RootDirectory != root) NtClose( attr->RootDirectory );
@@ -294,17 +317,6 @@ static NTSTATUS create_key( HKEY *retkey, ACCESS_MASK access, OBJECT_ATTRIBUTES
             while (i < len && buffer[i] == '\') i++;
             buffer += i;
             len -= i;
-
-            str.Buffer = buffer;
-            str.Length = len * sizeof(WCHAR);
-            if (!is_wow6432node( &str ))
-            {
-                if ((subkey = open_wow6432node( attr->RootDirectory )))
-                {
-                    NtClose( attr->RootDirectory );
-                    attr->RootDirectory = subkey;
-                }
-            }
         }
     }
     if (status == STATUS_PREDEFINED_HANDLE)

[PATCH v7 8/8] kernelbase: Factor creating a subkey out of create_key().

[Sven Baars]

From: Sven Baars

---
 dlls/kernelbase/registry.c | 101 ++++++++++++++++++++++---------------
 1 file changed, 61 insertions(+), 40 deletions(-)

diff --git a/dlls/kernelbase/registry.c b/dlls/kernelbase/registry.c
index 757ced4f00a..64d6e4a0ae9 100644
--- a/dlls/kernelbase/registry.c
+++ b/dlls/kernelbase/registry.c
@@ -239,12 +239,61 @@ static NTSTATUS open_key( HKEY *retkey, DWORD options, ACCESS_MASK access, OBJEC
     return status;
 }

+static NTSTATUS create_subkey( HKEY *subkey, HKEY root, UNICODE_STRING *name, DWORD options, ACCESS_MASK access,
+                               const UNICODE_STRING *class, PULONG dispos )
+{
+    DWORD i = 0, len = name->Length / sizeof(WCHAR);
+    WCHAR *buffer = name->Buffer;
+    OBJECT_ATTRIBUTES attr;
+    UNICODE_STRING str;
+    NTSTATUS status;
+
+    attr.Length = sizeof(attr);
+    attr.RootDirectory = (HANDLE)root;
+    attr.ObjectName = &str;
+    attr.Attributes = 0;
+    attr.SecurityDescriptor = NULL;
+    attr.SecurityQualityOfService = NULL;
+
+    if (i < len && buffer[i] == '\') return STATUS_OBJECT_PATH_INVALID;
+    while (i < len && buffer[i] != '\') i++;
+
+    str.Buffer = name->Buffer;
+    str.Length = i * sizeof(WCHAR);
+
+    if (i == len)
+    {
+        if (options & REG_OPTION_OPEN_LINK) attr.Attributes |= OBJ_OPENLINK;
+    }
+    else
+    {
+        options &= ~REG_OPTION_CREATE_LINK;
+    }
+
+    status = NtCreateKey( (HANDLE *)subkey, access, &attr, 0, class, options, dispos );
+    if (status == STATUS_PREDEFINED_HANDLE)
+    {
+        *subkey = get_perflib_key( *subkey );
+        status = STATUS_SUCCESS;
+    }
+
+    if (!status)
+    {
+        while (i < len && buffer[i] == '\') i++;
+
+        name->Buffer += i;
+        name->Length -= i * sizeof(WCHAR);
+    }
+
+    return status;
+}
+
 /* wrapper for NtCreateKey that creates the key recursively if necessary */
 static NTSTATUS create_key( HKEY *retkey, ACCESS_MASK access, OBJECT_ATTRIBUTES *attr,
                             const UNICODE_STRING *class, ULONG options, PULONG dispos )
 {
-    NTSTATUS status = STATUS_OBJECT_NAME_NOT_FOUND;
     HKEY subkey, root = attr->RootDirectory, subkey_root = root;
+    NTSTATUS status = STATUS_OBJECT_NAME_NOT_FOUND;
     UNICODE_STRING name;

if (!(is_win64 && (access & KEY_WOW64_32KEY)))
@@ -261,6 +310,8 @@ static NTSTATUS create_key( HKEY *retkey, ACCESS_MASK access, OBJECT_ATTRIBUTES
             return status;
     }

+    *retkey = NULL;
+
     name.Buffer = attr->ObjectName->Buffer;
     name.Length = attr->ObjectName->Length;

@@ -271,7 +322,6 @@ static NTSTATUS create_key( HKEY *retkey, ACCESS_MASK access, OBJECT_ATTRIBUTES
         if (subkey_root && subkey_root != root) NtClose( subkey_root );
         subkey_root = subkey;
     }
-    attr->RootDirectory = subkey_root;

if (!status && (options & REG_OPTION_CREATE_LINK))
     {
@@ -284,47 +334,18 @@ static NTSTATUS create_key( HKEY *retkey, ACCESS_MASK access, OBJECT_ATTRIBUTES

if (status == STATUS_OBJECT_NAME_NOT_FOUND)
     {
-        WCHAR *buffer = name.Buffer;
-        DWORD attrs, i = 0, len = name.Length / sizeof(WCHAR);
-        UNICODE_STRING str;
-
-        attrs = attr->Attributes;
-        attr->ObjectName = &str;
-
-        while (len)
+        status = STATUS_SUCCESS;
+        while (!status && name.Length)
         {
-            i = 0;
-            if (buffer[0] == '\') return STATUS_OBJECT_PATH_INVALID;
-            while (i < len && buffer[i] != '\') i++;
-
-            str.Buffer = buffer;
-            str.Length = i * sizeof(WCHAR);
-
-            if (i == len)
-            {
-                attr->Attributes = attrs;
-                status = NtCreateKey( (HANDLE *)&subkey, access, attr, 0, class, options, dispos );
-            }
-            else
-            {
-                attr->Attributes = attrs & ~OBJ_OPENLINK;
-                status = NtCreateKey( (HANDLE *)&subkey, access, attr, 0, class,
-                                      options & ~REG_OPTION_CREATE_LINK, dispos );
-            }
-            if (attr->RootDirectory != root) NtClose( attr->RootDirectory );
-            if (!NT_SUCCESS(status)) return status;
-            attr->RootDirectory = subkey;
-            while (i < len && buffer[i] == '\') i++;
-            buffer += i;
-            len -= i;
+            status = create_subkey( &subkey, subkey_root, &name, options, access, class, dispos );
+            if (subkey_root && subkey_root != root) NtClose( subkey_root );
+            subkey_root = subkey;
         }
     }
-    if (status == STATUS_PREDEFINED_HANDLE)
-    {
-        attr->RootDirectory = get_perflib_key( attr->RootDirectory );
-        status = STATUS_SUCCESS;
-    }
-    *retkey = attr->RootDirectory;
+
+    if (!status)
+        *retkey = subkey_root;
+
     return status;
 }

[Sven Baars (＠sbaars)]

I split the first two patches into 8 patches to make them more readable, as per Zeb's comment. The only functional change here is that we don't disable looking for the wow6432node after one has been entered. This is because a scenario like this may happen:

```
\Registry\Machine
\Registry\Machine\Software
\Registry\Machine\Software\Wow6432Node
\Registry\Machine\Software\Wow6432Node\Classes <-- This is a link
\Registry\Machine\Software\Classes\Wow6432Node
\Registry\Machine\Software\Classes <-- This will happen in the server, because it's shared
\Registry\Machine\Software\Classes\Wow6432Node
\Registry\Machine\Software\Classes\Wow6432Node\Interface
```

So here we need to go into the Wow6432Node twice.