[PATCH] Revert "ntdll: Skip dlls of the wrong machine type when searching through the load path."

[Brendan Shanks]

This reverts commit 9839bb7691a1b1c57a4ca501d03825420c1609d7.
Windows 7/2008R2 and up are able to load system DLLs with WoW64
redirection disabled, and badly-written installers crash without this.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47912
Signed-off-by: Brendan Shanks 
---
 dlls/kernel32/tests/loader.c |  4 ++--
 dlls/ntdll/loader.c          | 31 +++++++++++--------------------
 2 files changed, 13 insertions(+), 22 deletions(-)

diff --git a/dlls/kernel32/tests/loader.c b/dlls/kernel32/tests/loader.c
index e5b8c3b28e..f165b3ee05 100644
--- a/dlls/kernel32/tests/loader.c
+++ b/dlls/kernel32/tests/loader.c
@@ -688,7 +688,7 @@ static NTSTATUS map_image_section( const IMAGE_NT_HEADERS *nt_header, const IMAG
         /* some dlls with invalid entry point will crash, but this means we loaded the test dll */
         ok( !expect_fallback, "%u: got test dll but expected fallback\n", line );
     }
-    else
+    else todo_wine_if( !expect_status )
     {
         ok( ldr_status == expect_status ||
             broken(il_only && !expect_status && ldr_status == STATUS_INVALID_IMAGE_FORMAT),
@@ -3822,7 +3822,7 @@ static void test_wow64_redirection_for_dll(const char *libname)
     if (!GetModuleHandleA(libname))
     {
         lib = LoadLibraryExA(libname, NULL, 0);
-        todo_wine ok (broken(lib == NULL) /* Vista/2008 */ ||
+        ok (broken(lib == NULL) /* Vista/2008 */ ||
             lib != NULL, "Loading %s should succeed with WOW64 redirection disabled\n", libname);
         if (lib)
         {
diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c
index 2bae3c7bd5..9e7aa7778d 100644
--- a/dlls/ntdll/loader.c
+++ b/dlls/ntdll/loader.c
@@ -2326,13 +2326,6 @@ static NTSTATUS open_dll_file( UNICODE_STRING *nt_name, WINE_MODREF **pwm,
         if (status == STATUS_IMAGE_NOT_AT_BASE) status = STATUS_SUCCESS;
         NtClose( mapping );
     }
-    if (!status && !is_valid_binary( *module, image_info ))
-    {
-        TRACE( "%s is for arch %x, continuing search\n", debugstr_us(nt_name), image_info->machine );
-        NtUnmapViewOfSection( NtCurrentProcess(), *module );
-        *module = NULL;
-        status = STATUS_IMAGE_MACHINE_TYPE_MISMATCH;
-    }
     return status;
 }

@@ -2351,6 +2344,12 @@ static NTSTATUS load_native_dll( LPCWSTR load_path, const UNICODE_STRING *nt_nam

TRACE("Trying %s dll %s\n", dll_type, debugstr_us(nt_name) );

+    if (!is_valid_binary( *module, image_info ))
+    {
+        NtUnmapViewOfSection( NtCurrentProcess(), module );
+        return STATUS_INVALID_IMAGE_FORMAT;
+    }
+
     /* perform base relocation, if necessary */

if ((status = perform_relocations( *module, nt, image_info->map_size ))) return status;
@@ -2824,7 +2823,6 @@ static NTSTATUS search_dll_file( LPCWSTR paths, LPCWSTR search, UNICODE_STRING *
                                  struct stat *st )
 {
     WCHAR *name;
-    BOOL found_image = FALSE;
     NTSTATUS status = STATUS_DLL_NOT_FOUND;
     ULONG len = strlenW( paths );

@@ -2849,20 +2847,15 @@ static NTSTATUS search_dll_file( LPCWSTR paths, LPCWSTR search, UNICODE_STRING *
         if ((status = RtlDosPathNameToNtPathName_U_WithStatus( name, nt_name, NULL, NULL ))) goto done;

status = open_dll_file( nt_name, pwm, module, image_info, st );
-        if (status == STATUS_IMAGE_MACHINE_TYPE_MISMATCH) found_image = TRUE;
-        else if (status != STATUS_DLL_NOT_FOUND) goto done;
+        if (status != STATUS_DLL_NOT_FOUND) goto done;
         RtlFreeUnicodeString( nt_name );
         paths = ptr;
     }

-    if (!found_image)
-    {
-        /* not found, return file in the system dir to be loaded as builtin */
-        strcpyW( name, system_dir );
-        strcatW( name, search );
-        if (!RtlDosPathNameToNtPathName_U( name, nt_name, NULL, NULL )) status = STATUS_NO_MEMORY;
-    }
-    else status = STATUS_IMAGE_MACHINE_TYPE_MISMATCH;
+    /* not found, return file in the system dir to be loaded as builtin */
+    strcpyW( name, system_dir );
+    strcatW( name, search );
+    if (!RtlDosPathNameToNtPathName_U( name, nt_name, NULL, NULL )) status = STATUS_NO_MEMORY;

done:
     RtlFreeHeap( GetProcessHeap(), 0, name );
@@ -2926,8 +2919,6 @@ static NTSTATUS find_dll_file( const WCHAR *load_path, const WCHAR *libname,
     else if (!(status = RtlDosPathNameToNtPathName_U_WithStatus( libname, nt_name, NULL, NULL )))
         status = open_dll_file( nt_name, pwm, module, image_info, st );

-    if (status == STATUS_IMAGE_MACHINE_TYPE_MISMATCH) status = STATUS_INVALID_IMAGE_FORMAT;
-
 done:
     RtlFreeHeap( GetProcessHeap(), 0, dllname );
     return status;

[Alexandre Julliard]

Brendan Shanks  writes:
> This reverts commit 9839bb7691a1b1c57a4ca501d03825420c1609d7.
> Windows 7/2008R2 and up are able to load system DLLs with WoW64
> redirection disabled, and badly-written installers crash without this.
>
> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47912
> Signed-off-by: Brendan Shanks
>
> dlls/kernel32/tests/loader.c |  4 ++--
>  dlls/ntdll/loader.c          | 31 +++++++++++--------------------
>  2 files changed, 13 insertions(+), 22 deletions(-)

That's going to reintroduce the problems that were fixed by that commit,
it's not a step forward. If the fix was incomplete it should be
improved, not reverted.

[Brendan Shanks]

> On Oct 23, 2019, at 12:57 PM, Alexandre Julliard  wrote:
>
> Brendan Shanks  writes:
> > This reverts commit 9839bb7691a1b1c57a4ca501d03825420c1609d7.
> > Windows 7/2008R2 and up are able to load system DLLs with WoW64
> > redirection disabled, and badly-written installers crash without this.
> >
> > Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47912
> > Signed-off-by: Brendan Shanks
> >
> > dlls/kernel32/tests/loader.c |  4 ++--
> > dlls/ntdll/loader.c          | 31 +++++++++++--------------------
> > 2 files changed, 13 insertions(+), 22 deletions(-)
>
> That's going to reintroduce the problems that were fixed by that commit,
> it's not a step forward. If the fix was incomplete it should be
> improved, not reverted.

Yes you’re right, I worked on the problem more and I think found the right solution (re-enabling filesystem redirection when loading libraries). I’ll send that patch in today.

Brendan

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=58312

Your paranoid android.

=== w7pro64 (32 bit report) ===

kernel32:
loader: Timeout

=== w7pro64 (task log) ===

Task errors:
An error occurred while waiting for the test to complete: network read timed out (wait2/connect:AgentVersion.h:0/9)
The test VM has crashed, rebooted or lost connectivity (or the TestAgent server died)
The previous 2 run(s) terminated abnormally

=== w1064v1809_ar (32 bit report) ===

kernel32:
loader.c:3181: Test failed: child process failed to terminate

=== w1064v1809_he (32 bit report) ===

kernel32:
loader.c:3209: Test failed: child process failed to terminate

=== w1064v1809_ja (32 bit report) ===

kernel32:
loader.c:3181: Test failed: child process failed to terminate

=== w1064v1809_zh_CN (32 bit report) ===

kernel32:
loader.c:3181: Test failed: child process failed to terminate

=== w1064v1809 (64 bit report) ===

kernel32:
loader.c:689: Test failed: 1389: got test dll but expected fallback
loader.c:689: Test failed: 1395: got test dll but expected fallback
loader.c:689: Test failed: 1401: got test dll but expected fallback
loader.c:689: Test failed: 1408: got test dll but expected fallback
loader.c:689: Test failed: 1435: got test dll but expected fallback