[PATCH v2 0/2] MR2578: Fixing module paths in LdrPath (corner case).
The serie intent is to fix unexpected paths in module's list: This happens: - when running under old / (new) wow64 - when main module is located under the syswow64 directory - the 32 bit modules are stored in LdrData (and then exposed through a couple of ways) under syswow64 (they are normally stored under system32, letting the redirection come into play when needed) This triggers a couple of errors in winetest (as we're using c:\windows\syswow64\msinfo32.exe in many tests to trigger a wow64 process from a winetest program). This is the fix awaited in MR!2497. @julliard: I'm not 100% happy with the fix itself by reintroducting ref to the redirected DLLs in ntdll/PE but couldn't find a better idea. -- v2: ntdll: Store system DLL in LdrData under system32 for a wow64 process. kernel32: Harden some wow64 module tests. https://gitlab.winehq.org/wine/wine/-/merge_requests/2578
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernel32/tests/loader.c | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/dlls/kernel32/tests/loader.c b/dlls/kernel32/tests/loader.c index 19f112efcd8..c354ba8f24d 100644 --- a/dlls/kernel32/tests/loader.c +++ b/dlls/kernel32/tests/loader.c @@ -4047,10 +4047,15 @@ static void test_InMemoryOrderModuleList(void) ok(entry2 == mark2, "expected entry2 == mark2, got %p and %p\n", entry2, mark2); } -static void test_wow64_redirection_for_dll(const char *libname) +static void test_wow64_redirection_for_dll(const char *libname, BOOL will_fail) { HMODULE lib; char buf[256]; + static const char *system32dir = "c:\\windows\\system32\\"; + static const char *syswow64dir = "c:\\windows\\syswow64\\"; + const size_t sysdirlen = strlen(system32dir); + const size_t syswowlen = strlen(syswow64dir); + const char *modname; if (!GetModuleHandleA(libname)) { @@ -4066,6 +4071,13 @@ static void test_wow64_redirection_for_dll(const char *libname) ok(lib != NULL, "Loading %s from full path should succeed with WOW64 redirection disabled\n", libname); if (lib) FreeLibrary(lib); + modname = strrchr(libname, '\\'); + modname = modname ? modname + 1 : libname; + todo_wine_if(will_fail) + ok((!strncasecmp(buf, system32dir, sysdirlen) && !strcasecmp(buf + sysdirlen, modname)) || + /* Win7 report from syswow64 */ + broken(!strncasecmp(buf, syswow64dir, syswowlen) && !strcasecmp(buf + syswowlen, modname)), + "Unexpected loaded DLL name %s for %s\n", buf, libname); } } else @@ -4085,10 +4097,20 @@ static void test_wow64_redirection(void) * already be loaded in this process). */ ok(pWow64DisableWow64FsRedirection(&OldValue), "Disabling FS redirection failed\n"); - test_wow64_redirection_for_dll("wlanapi.dll"); - test_wow64_redirection_for_dll("dxgi.dll"); - test_wow64_redirection_for_dll("dwrite.dll"); + test_wow64_redirection_for_dll("wlanapi.dll", FALSE); + test_wow64_redirection_for_dll("dxgi.dll", FALSE); + test_wow64_redirection_for_dll("dwrite.dll", FALSE); + test_wow64_redirection_for_dll("c:\\windows\\syswow64\\wlanapi.dll", TRUE); + test_wow64_redirection_for_dll("c:\\windows\\syswow64\\dxgi.dll", TRUE); + test_wow64_redirection_for_dll("c:\\windows\\syswow64\\dwrite.dll", TRUE); ok(pWow64RevertWow64FsRedirection(OldValue), "Re-enabling FS redirection failed\n"); + /* and results don't depend whether redirection is enabled or not */ + test_wow64_redirection_for_dll("wlanapi.dll", FALSE); + test_wow64_redirection_for_dll("dxgi.dll", FALSE); + test_wow64_redirection_for_dll("dwrite.dll", FALSE); + test_wow64_redirection_for_dll("c:\\windows\\syswow64\\wlanapi.dll", TRUE); + test_wow64_redirection_for_dll("c:\\windows\\syswow64\\dxgi.dll", TRUE); + test_wow64_redirection_for_dll("c:\\windows\\syswow64\\dwrite.dll", TRUE); } static void test_dll_file( const char *name ) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2578
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/dbghelp/tests/dbghelp.c | 7 +------ dlls/kernel32/tests/loader.c | 27 +++++++++++++-------------- dlls/ntdll/loader.c | 17 +++++++++++++++++ dlls/ntdll/ntdll.spec | 1 + dlls/ntdll/unix/loader.c | 30 ++++++++++++++++++++++++++++++ dlls/psapi/tests/psapi_main.c | 2 -- dlls/wow64/file.c | 13 +++++++++++++ dlls/wow64/syscall.h | 3 ++- include/winternl.h | 2 ++ 9 files changed, 79 insertions(+), 23 deletions(-) diff --git a/dlls/dbghelp/tests/dbghelp.c b/dlls/dbghelp/tests/dbghelp.c index 276faf0101a..d6cd1c9da70 100644 --- a/dlls/dbghelp/tests/dbghelp.c +++ b/dlls/dbghelp/tests/dbghelp.c @@ -719,7 +719,7 @@ static void test_loaded_modules(void) break; case PCSKIND_64BIT: case PCSKIND_WOW64: - todo_wine + case PCSKIND_WINE_OLD_WOW64: ok(aggregation.count_systemdir > 2 && aggregation.count_wowdir == 1, "Wrong directory aggregation count %u %u\n", aggregation.count_systemdir, aggregation.count_wowdir); break; @@ -727,10 +727,6 @@ static void test_loaded_modules(void) ok(aggregation.count_systemdir > 2 && aggregation.count_wowdir == 0, "Wrong directory aggregation count %u %u\n", aggregation.count_systemdir, aggregation.count_wowdir); break; - case PCSKIND_WINE_OLD_WOW64: - ok(aggregation.count_systemdir == 1 && aggregation.count_wowdir > 2, "Wrong directory aggregation count %u %u\n", - aggregation.count_systemdir, aggregation.count_wowdir); - break; } } @@ -767,7 +763,6 @@ static void test_loaded_modules(void) aggregation.count_32bit, aggregation.count_64bit); ok(aggregation.count_exe == 1 && aggregation.count_ntdll == 1, "Wrong kind aggregation count %u %u\n", aggregation.count_exe, aggregation.count_ntdll); - todo_wine ok(aggregation.count_systemdir > 2 && aggregation.count_64bit == aggregation.count_systemdir && aggregation.count_wowdir == 1, "Wrong directory aggregation count %u %u\n", aggregation.count_systemdir, aggregation.count_wowdir); diff --git a/dlls/kernel32/tests/loader.c b/dlls/kernel32/tests/loader.c index c354ba8f24d..76d6951fff1 100644 --- a/dlls/kernel32/tests/loader.c +++ b/dlls/kernel32/tests/loader.c @@ -4047,7 +4047,7 @@ static void test_InMemoryOrderModuleList(void) ok(entry2 == mark2, "expected entry2 == mark2, got %p and %p\n", entry2, mark2); } -static void test_wow64_redirection_for_dll(const char *libname, BOOL will_fail) +static void test_wow64_redirection_for_dll(const char *libname) { HMODULE lib; char buf[256]; @@ -4073,7 +4073,6 @@ static void test_wow64_redirection_for_dll(const char *libname, BOOL will_fail) FreeLibrary(lib); modname = strrchr(libname, '\\'); modname = modname ? modname + 1 : libname; - todo_wine_if(will_fail) ok((!strncasecmp(buf, system32dir, sysdirlen) && !strcasecmp(buf + sysdirlen, modname)) || /* Win7 report from syswow64 */ broken(!strncasecmp(buf, syswow64dir, syswowlen) && !strcasecmp(buf + syswowlen, modname)), @@ -4097,20 +4096,20 @@ static void test_wow64_redirection(void) * already be loaded in this process). */ ok(pWow64DisableWow64FsRedirection(&OldValue), "Disabling FS redirection failed\n"); - test_wow64_redirection_for_dll("wlanapi.dll", FALSE); - test_wow64_redirection_for_dll("dxgi.dll", FALSE); - test_wow64_redirection_for_dll("dwrite.dll", FALSE); - test_wow64_redirection_for_dll("c:\\windows\\syswow64\\wlanapi.dll", TRUE); - test_wow64_redirection_for_dll("c:\\windows\\syswow64\\dxgi.dll", TRUE); - test_wow64_redirection_for_dll("c:\\windows\\syswow64\\dwrite.dll", TRUE); + test_wow64_redirection_for_dll("wlanapi.dll"); + test_wow64_redirection_for_dll("dxgi.dll"); + test_wow64_redirection_for_dll("dwrite.dll"); + test_wow64_redirection_for_dll("c:\\windows\\syswow64\\wlanapi.dll"); + test_wow64_redirection_for_dll("c:\\windows\\syswow64\\dxgi.dll"); + test_wow64_redirection_for_dll("c:\\windows\\syswow64\\dwrite.dll"); ok(pWow64RevertWow64FsRedirection(OldValue), "Re-enabling FS redirection failed\n"); /* and results don't depend whether redirection is enabled or not */ - test_wow64_redirection_for_dll("wlanapi.dll", FALSE); - test_wow64_redirection_for_dll("dxgi.dll", FALSE); - test_wow64_redirection_for_dll("dwrite.dll", FALSE); - test_wow64_redirection_for_dll("c:\\windows\\syswow64\\wlanapi.dll", TRUE); - test_wow64_redirection_for_dll("c:\\windows\\syswow64\\dxgi.dll", TRUE); - test_wow64_redirection_for_dll("c:\\windows\\syswow64\\dwrite.dll", TRUE); + test_wow64_redirection_for_dll("wlanapi.dll"); + test_wow64_redirection_for_dll("dxgi.dll"); + test_wow64_redirection_for_dll("dwrite.dll"); + test_wow64_redirection_for_dll("c:\\windows\\syswow64\\wlanapi.dll"); + test_wow64_redirection_for_dll("c:\\windows\\syswow64\\dxgi.dll"); + test_wow64_redirection_for_dll("c:\\windows\\syswow64\\dwrite.dll"); } static void test_dll_file( const char *name ) diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index f05cd1b4fd9..97a0f03290c 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -1426,6 +1426,7 @@ static WINE_MODREF *alloc_module( HMODULE hModule, const UNICODE_STRING *nt_name WINE_MODREF *wm; const WCHAR *p; const IMAGE_NT_HEADERS *nt = RtlImageNtHeader(hModule); + NTSTATUS status; if (!(wm = RtlAllocateHeap( GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*wm) ))) return NULL; @@ -1454,6 +1455,22 @@ static WINE_MODREF *alloc_module( HMODULE hModule, const UNICODE_STRING *nt_name memcpy( buffer, nt_name->Buffer + 4 /* \??\ prefix */, nt_name->Length - 4 * sizeof(WCHAR) ); buffer[nt_name->Length/sizeof(WCHAR) - 4] = 0; + if (NtCurrentTeb()->Peb->LdrData->InLoadOrderModuleList.Flink != &NtCurrentTeb()->Peb->LdrData->InLoadOrderModuleList) + { + status = wine_unredirect_file_name( buffer, buffer, nt_name->Length / sizeof(WCHAR) - 3 ); + if (status == STATUS_BUFFER_TOO_SMALL) + { + WCHAR *new = RtlAllocateHeap( GetProcessHeap(), 0, MAX_PATH * sizeof(WCHAR) ); + if (!new || wine_unredirect_file_name( buffer, new, MAX_PATH ) == STATUS_BUFFER_TOO_SMALL ) + { + RtlFreeHeap( GetProcessHeap(), 0, buffer ); + RtlFreeHeap( GetProcessHeap(), 0, wm ); + return NULL; + } + RtlFreeHeap( GetProcessHeap(), 0, buffer ); + buffer = new; + } + } if ((p = wcsrchr( buffer, '\\' ))) p++; else p = buffer; RtlInitUnicodeString( &wm->ldr.FullDllName, buffer ); diff --git a/dlls/ntdll/ntdll.spec b/dlls/ntdll/ntdll.spec index d5f6737e1aa..ddf6e53e056 100644 --- a/dlls/ntdll/ntdll.spec +++ b/dlls/ntdll/ntdll.spec @@ -1719,3 +1719,4 @@ # Filesystem @ stdcall -syscall wine_nt_to_unix_file_name(ptr ptr ptr long) @ stdcall -syscall wine_unix_to_nt_file_name(str ptr ptr) +@ stdcall -syscall wine_unredirect_file_name(ptr ptr long) diff --git a/dlls/ntdll/unix/loader.c b/dlls/ntdll/unix/loader.c index 16c044d502e..49c771c39d2 100644 --- a/dlls/ntdll/unix/loader.c +++ b/dlls/ntdll/unix/loader.c @@ -360,6 +360,7 @@ static void * const syscalls[] = NtYieldExecution, wine_nt_to_unix_file_name, wine_unix_to_nt_file_name, + wine_unredirect_file_name, }; static BYTE syscall_args[ARRAY_SIZE(syscalls)]; @@ -1666,6 +1667,35 @@ static const WCHAR *get_machine_wow64_dir( WORD machine ) } +NTSTATUS WINAPI wine_unredirect_file_name( const WCHAR *filename, WCHAR *output, ULONG size ) +{ + static WORD machines[] = {IMAGE_FILE_MACHINE_I386, IMAGE_FILE_MACHINE_ARMNT, IMAGE_FILE_MACHINE_AMD64, IMAGE_FILE_MACHINE_ARM64}; + int i; + + for (i = 0; i < ARRAY_SIZE(machines); i++) + { + const WCHAR *src = get_machine_wow64_dir( machines[i] ) + 4; + size_t srclen = wcslen( src ); + + if (!wcsnicmp( filename, src, srclen )) + { + const WCHAR *dst = get_machine_wow64_dir( IMAGE_FILE_MACHINE_TARGET_HOST ) + 4; + size_t dstlen = wcslen( dst ); + size_t filelen = wcslen( filename ); + + if (filelen + dstlen - srclen + 1 > size) + return STATUS_BUFFER_TOO_SMALL; + + /* filename and output could be the same buffer */ + memmove( output + dstlen, filename + srclen, (filelen - srclen + 1) * sizeof(WCHAR) ); + memcpy( output, dst, dstlen * sizeof(WCHAR) ); + return STATUS_SUCCESS; + } + } + return STATUS_NOT_FOUND; +} + + /*************************************************************************** * is_builtin_path * diff --git a/dlls/psapi/tests/psapi_main.c b/dlls/psapi/tests/psapi_main.c index 8d7b169703a..e1028d805ae 100644 --- a/dlls/psapi/tests/psapi_main.c +++ b/dlls/psapi/tests/psapi_main.c @@ -525,12 +525,10 @@ static void test_EnumProcessModulesEx(void) ret = GetSystemWow64DirectoryA(buffer, sizeof(buffer)); ok(ret, "GetSystemWow64DirectoryA failed: %lu\n", GetLastError()); count = snapshot_count_in_dir(snap, pi.hProcess, buffer); - todo_wine ok(count <= 1, "Wrong count %u in %s\n", count, buffer); /* msinfo32 can be from system wow64 */ ret = GetSystemDirectoryA(buffer, sizeof(buffer)); ok(ret, "GetSystemDirectoryA failed: %lu\n", GetLastError()); count = snapshot_count_in_dir(snap, pi.hProcess, buffer); - todo_wine ok(count > 2, "Wrong count %u in %s\n", count, buffer); /* in fact, this error is only returned when (list & 3 == 0), otherwise the corresponding diff --git a/dlls/wow64/file.c b/dlls/wow64/file.c index d32ef0b5455..adf60c5d2a4 100644 --- a/dlls/wow64/file.c +++ b/dlls/wow64/file.c @@ -955,3 +955,16 @@ NTSTATUS WINAPI wow64_wine_unix_to_nt_file_name( UINT *args ) return wine_unix_to_nt_file_name( name, buffer, size ); } + + +/********************************************************************** + * wow64___wine_unredirect_file_name + */ +NTSTATUS WINAPI wow64_wine_unredirect_file_name( UINT *args ) +{ + const WCHAR *name = get_ptr( &args ); + WCHAR *buffer = get_ptr( &args ); + ULONG size = get_ulong( &args ); + + return wine_unredirect_file_name( name, buffer, size ); +} diff --git a/dlls/wow64/syscall.h b/dlls/wow64/syscall.h index 59586d3fbfa..62648749909 100644 --- a/dlls/wow64/syscall.h +++ b/dlls/wow64/syscall.h @@ -259,6 +259,7 @@ SYSCALL_ENTRY( NtWriteVirtualMemory ) \ SYSCALL_ENTRY( NtYieldExecution ) \ SYSCALL_ENTRY( wine_nt_to_unix_file_name ) \ - SYSCALL_ENTRY( wine_unix_to_nt_file_name ) + SYSCALL_ENTRY( wine_unix_to_nt_file_name ) \ + SYSCALL_ENTRY( wine_unredirect_file_name ) #endif /* __WOW64_SYSCALL_H */ diff --git a/include/winternl.h b/include/winternl.h index ed0e51bae24..4e5fba01f47 100644 --- a/include/winternl.h +++ b/include/winternl.h @@ -4760,6 +4760,8 @@ NTSYSAPI NTSTATUS WINAPI wine_nt_to_unix_file_name( const OBJECT_ATTRIBUTES *att UINT disposition ); NTSYSAPI NTSTATUS WINAPI wine_unix_to_nt_file_name( const char *name, WCHAR *buffer, ULONG *size ); +NTSYSAPI NTSTATUS WINAPI wine_unredirect_file_name( const WCHAR *filename, WCHAR *output, ULONG size ); + /*********************************************************************** * Inline functions -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2578
V2 pushed: from tests (for a wow64 process only): - loading DLL from absolute path under syswow64 is stored with system32 in LdrData (test included) - loading DLL from absolute path under syswow64\foobar is stored with system32 in LdrData (test not included) - identical behavior whether redirection is enabled or not - main module (if process creation from syswow64\<exec>) is however stored in LdrData under syswow64 - when exec is from syswow64\<exec> (or subdirectories of syswow64) => the application search path is reported from the Ldr* functions from syswow64 (no unredirection involved) V1 -> V2: - changed trigger condition to be at module insertion time in LdrData, and perform a path based unredirection (that still looks strange, but that matches with above tests results) - move unredirection helper into ntdll.so -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2578#note_29610
there are tons of failures from Marvin build... quite a few in user32 that I can't reproduce locally something from with 32bit configuration? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2578#note_29618
On Thu Apr 6 12:11:43 2023 +0000, eric pouech wrote:
hmm... some more tests show that loading from a wow64 process c:\windows\syswow64\mydll.dll (with absolute path) ends up with a path reported from system32 in LdrData chain (tested with regular system DLL, but also by adding a dummy DLL in a created subdir of syswow64 => same rewrite to system32 in LdrData) (so this rules out predefined list of dll) (and a main image path from syswow64 actually shows up in the various Ldr* and Rtl* functions, which likely indicates that process image path isn't modified) which means that my initial patch is wrong in changing the default load path, but this looks like that DLLs paths *must* be "unredirected" when being inserted in LdrData (except main module) I'll resubmit with including some of the tests. (and moving "unredirection" logic to ntdll.so will require a new syscall) any feedback?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2578#note_31409
On Thu Apr 27 17:11:26 2023 +0000, eric pouech wrote:
any feedback? Why are you adding a Unix call for this? That doesn't seem necessary.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2578#note_31422
participants (3)
-
Alexandre Julliard (＠julliard) -
Eric Pouech
-
eric pouech (＠epo)