[PATCH] Revert "ntdll: Skip dlls of the wrong machine type when searching through the load path."
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 <bshanks(a)codeweavers.com> --- 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; -- 2.17.1
Brendan Shanks <bshanks(a)codeweavers.com> 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 <bshanks(a)codeweavers.com> --- 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. -- Alexandre Julliard julliard(a)winehq.org
On Oct 23, 2019, at 12:57 PM, Alexandre Julliard <julliard(a)winehq.org> wrote:
Brendan Shanks <bshanks(a)codeweavers.com> 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 <bshanks(a)codeweavers.com> --- 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
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
participants (3)
-
Alexandre Julliard -
Brendan Shanks -
Marvin