[PATCH 0/1] MR4705: ntdll: Use position independent syscall thunk for NtQueryInformationProcess on i386.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55967 Fixes a regression introduced by commit efd03f40e6e315d89cd1d09c48180aae82033f9f. The app reads syscall thunk for NtQueryInformationProcess() from ntdll file and just calls that from some memory location. That worked before the referenced commit because without performing relocations that was jumping to wine_syscall in the loaded ntdll.dll. Now when the actual ntdll load address is different that jumps to the same address which is now non sensual. The existing tests show that in general 32-bit syscall thunks need relocation. The same trick with some other Nt functions I tried doesn't work on Windows, but turns out NtQueryInformationProcess is special (the included test replicates that). The form of syscall thunk I used in the patch corresponds to what was in Wine-Staging before the upsream implementation of syscall thunks (that is, call to NtCurrentTeb()->WOW32Reserved). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4705
From: Paul Gofman <pgofman(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55967 Fixes a regression introduced by commit efd03f40e6e315d89cd1d09c48180aae82033f9f. --- dlls/ntdll/ntsyscalls.h | 2 +- dlls/ntdll/signal_arm.c | 2 ++ dlls/ntdll/signal_i386.c | 2 ++ dlls/ntdll/tests/virtual.c | 61 ++++++++++++++++++++++++++++++++++++-- dlls/ntdll/unix/loader.c | 4 +++ dlls/wow64/syscall.c | 4 +++ dlls/wow64/wow64_private.h | 2 ++ include/wine/asm.h | 6 ++++ 8 files changed, 80 insertions(+), 3 deletions(-) diff --git a/dlls/ntdll/ntsyscalls.h b/dlls/ntdll/ntsyscalls.h index 7ae20b31159..546a68cea28 100644 --- a/dlls/ntdll/ntsyscalls.h +++ b/dlls/ntdll/ntsyscalls.h @@ -132,7 +132,7 @@ SYSCALL_ENTRY( 0x0080, NtQueryInformationAtom, 20 ) \ SYSCALL_ENTRY( 0x0081, NtQueryInformationFile, 20 ) \ SYSCALL_ENTRY( 0x0082, NtQueryInformationJobObject, 20 ) \ - SYSCALL_ENTRY( 0x0083, NtQueryInformationProcess, 20 ) \ + SYSCALL_ENTRY2( 0x0083, NtQueryInformationProcess, 20 ) \ SYSCALL_ENTRY( 0x0084, NtQueryInformationThread, 20 ) \ SYSCALL_ENTRY( 0x0085, NtQueryInformationToken, 20 ) \ SYSCALL_ENTRY( 0x0086, NtQueryInstallUILanguage, 4 ) \ diff --git a/dlls/ntdll/signal_arm.c b/dlls/ntdll/signal_arm.c index 33233bb3a15..17081a95094 100644 --- a/dlls/ntdll/signal_arm.c +++ b/dlls/ntdll/signal_arm.c @@ -97,8 +97,10 @@ static inline BOOL is_valid_frame( ULONG_PTR frame ) * syscalls */ #define SYSCALL_ENTRY(id,name,args) __ASM_SYSCALL_FUNC( id, name, args ) +#define SYSCALL_ENTRY2 SYSCALL_ENTRY ALL_SYSCALLS32 DEFINE_SYSCALL_HELPER32() +#undef SYSCALL_ENTRY2 #undef SYSCALL_ENTRY diff --git a/dlls/ntdll/signal_i386.c b/dlls/ntdll/signal_i386.c index 4fff401c88d..68b8eb905af 100644 --- a/dlls/ntdll/signal_i386.c +++ b/dlls/ntdll/signal_i386.c @@ -77,8 +77,10 @@ extern DWORD EXC_CallHandler( EXCEPTION_RECORD *record, EXCEPTION_REGISTRATION_R * syscalls */ #define SYSCALL_ENTRY(id,name,args) __ASM_SYSCALL_FUNC( id, name, args ) +#define SYSCALL_ENTRY2(id,name,args) __ASM_SYSCALL_FUNC2( id, name, args ) ALL_SYSCALLS32 DEFINE_SYSCALL_HELPER32() +#undef SYSCALL_ENTRY2 #undef SYSCALL_ENTRY diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index a9dec1b5a8a..be150c080c8 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -2076,6 +2076,25 @@ static void perform_relocations( void *module, INT_PTR delta ) } } +#ifndef __x86_64__ +static ULONG_PTR get_va_file_offset( void *module, void *va_p ) +{ + IMAGE_NT_HEADERS *nt; + IMAGE_SECTION_HEADER *sec; + unsigned int i; + ULONG_PTR va = (ULONG_PTR)va_p - (ULONG_PTR)module; + + nt = RtlImageNtHeader( module ); + sec = IMAGE_FIRST_SECTION( nt ); + for (i = 0; i < nt->FileHeader.NumberOfSections; ++i) + { + if (va >= sec[i].VirtualAddress && va < sec[i].VirtualAddress + sec[i].SizeOfRawData) + return va - sec[i].VirtualAddress + sec[i].PointerToRawData; + } + ok( 0, "could not find file offset.\n" ); + return 0; +} +#endif static void test_syscalls(void) { @@ -2118,17 +2137,20 @@ static void test_syscalls(void) ok( mapping != NULL, "CreateFileMappingW failed err %lu\n", GetLastError() ); ptr = MapViewOfFile( mapping, FILE_MAP_READ, 0, 0, 0 ); ok( ptr != NULL, "MapViewOfFile failed err %lu\n", GetLastError() ); + CloseHandle( mapping ); - CloseHandle( file ); delta = (char *)ptr - (char *)module; if (memcmp( ptr, module, 0x1000 )) { skip( "modules are not identical (non-PE build?)\n" ); UnmapViewOfFile( ptr ); + CloseHandle( file ); return; } + perform_relocations( ptr, delta ); + pNtClose = (void *)GetProcAddress( module, "NtClose" ); if (pRtlFindExportedRoutineByName) @@ -2155,9 +2177,44 @@ static void test_syscalls(void) #ifdef __x86_64__ ok( 0, "syscall thunk relocated\n" ); #else - skip( "syscall thunk relocated\n" ); + NTSTATUS (WINAPI *pNtQueryInformationProcess)(HANDLE, PROCESSINFOCLASS, void *, ULONG, ULONG *); + PROCESS_BASIC_INFORMATION pbi; + void *exec_mem; + ULONG size; + BOOL ret; + + exec_mem = VirtualAlloc( NULL, 4096, MEM_RESERVE | MEM_COMMIT, PAGE_EXECUTE_READWRITE ); + ok( !!exec_mem, "got NULL.\n" ); + + if (0) + { + /* Crashes on Windows and Wine (as expected, as 32 bit syscall thunks need to be relocated). */ + pNtClose = (void *)GetProcAddress( module, "NtClose" ); + ret = SetFilePointer( file, get_va_file_offset( module, pNtClose ), NULL, FILE_BEGIN ); + ok( ret, "got %d, err %lu.\n", ret, GetLastError() ); + ret = ReadFile( file, exec_mem, 32, NULL, NULL ); + ok( ret, "got %d, err %lu.\n", ret, GetLastError() ); + pNtClose = exec_mem; + pNtClose( (HANDLE)0xdeadbeef ); + } + /* NtQueryInformationProcess is special. */ + pNtQueryInformationProcess = (void *)GetProcAddress( module, "NtQueryInformationProcess" ); + ret = SetFilePointer( file, get_va_file_offset( module, pNtQueryInformationProcess ), NULL, FILE_BEGIN ); + ok( ret, "got %d, err %lu.\n", ret, GetLastError() ); + ret = ReadFile( file, exec_mem, 32, NULL, NULL ); + ok( ret, "got %d, err %lu.\n", ret, GetLastError() ); + pNtQueryInformationProcess = exec_mem; + /* The thunk still works without relocation. */ + status = pNtQueryInformationProcess( GetCurrentProcess(), ProcessBasicInformation, &pbi, sizeof(pbi), &size ); + ok( !status, "got %#lx.\n", status ); + ok( size == sizeof(pbi), "got %lu.\n", size ); + ok( pbi.PebBaseAddress == NtCurrentTeb()->Peb, "got %p, %p.\n", pbi.PebBaseAddress, NtCurrentTeb()->Peb ); + + VirtualFree( exec_mem, 0, MEM_RELEASE ); + #endif } + CloseHandle( file ); UnmapViewOfFile( ptr ); } diff --git a/dlls/ntdll/unix/loader.c b/dlls/ntdll/unix/loader.c index 803d8079213..26cd8655179 100644 --- a/dlls/ntdll/unix/loader.c +++ b/dlls/ntdll/unix/loader.c @@ -119,22 +119,26 @@ SYSTEM_DLL_INIT_BLOCK *pLdrSystemDllInitBlock = NULL; static void * const syscalls[] = { #define SYSCALL_ENTRY(id,name,args) name, +#define SYSCALL_ENTRY2 SYSCALL_ENTRY #ifdef _WIN64 ALL_SYSCALLS64 #else ALL_SYSCALLS32 #endif +#undef SYSCALL_ENTRY2 #undef SYSCALL_ENTRY }; static BYTE syscall_args[ARRAY_SIZE(syscalls)] = { #define SYSCALL_ENTRY(id,name,args) args, +#define SYSCALL_ENTRY2 SYSCALL_ENTRY #ifdef _WIN64 ALL_SYSCALLS64 #else ALL_SYSCALLS32 #endif +#undef SYSCALL_ENTRY2 #undef SYSCALL_ENTRY }; diff --git a/dlls/wow64/syscall.c b/dlls/wow64/syscall.c index 2af7bf75de2..42e5d4479c4 100644 --- a/dlls/wow64/syscall.c +++ b/dlls/wow64/syscall.c @@ -45,14 +45,18 @@ typedef NTSTATUS (WINAPI *syscall_thunk)( UINT *args ); static const syscall_thunk syscall_thunks[] = { #define SYSCALL_ENTRY(id,name,args) wow64_ ## name, +#define SYSCALL_ENTRY2 SYSCALL_ENTRY ALL_SYSCALLS32 +#undef SYSCALL_ENTRY2 #undef SYSCALL_ENTRY }; static BYTE syscall_args[ARRAY_SIZE(syscall_thunks)] = { #define SYSCALL_ENTRY(id,name,args) args, +#define SYSCALL_ENTRY2 SYSCALL_ENTRY ALL_SYSCALLS32 +#undef SYSCALL_ENTRY2 #undef SYSCALL_ENTRY }; diff --git a/dlls/wow64/wow64_private.h b/dlls/wow64/wow64_private.h index 62eccd3474e..bb3454c0d95 100644 --- a/dlls/wow64/wow64_private.h +++ b/dlls/wow64/wow64_private.h @@ -25,7 +25,9 @@ #include "struct32.h" #define SYSCALL_ENTRY(id,name,_args) extern NTSTATUS WINAPI wow64_ ## name( UINT *args ); +#define SYSCALL_ENTRY2 SYSCALL_ENTRY ALL_SYSCALLS32 +#undef SYSCALL_ENTRY2 #undef SYSCALL_ENTRY extern void init_image_mapping( HMODULE module ); diff --git a/include/wine/asm.h b/include/wine/asm.h index 0fa2dfcd1b7..463dd07d9af 100644 --- a/include/wine/asm.h +++ b/include/wine/asm.h @@ -195,6 +195,7 @@ "movl $(" #id "),%eax\n\t" \ "call *%edx\n\t" \ "ret $" #args ) +# define __ASM_SYSCALL_FUNC2 __ASM_SYSCALL_FUNC # define DEFINE_SYSCALL_HELPER32() # else # define __ASM_SYSCALL_FUNC(id,name,args) \ @@ -203,6 +204,11 @@ "movl $" __ASM_NAME("__wine_syscall") ",%edx\n\t" \ "call *%edx\n\t" \ "ret $" #args ) +# define __ASM_SYSCALL_FUNC2(id,name,args) \ + __ASM_STDCALL_FUNC( name, args, \ + "movl $(" #id "),%eax\n\t" \ + ".byte 0x64\n\tcall *(0xc0)\n\t" \ + "ret $" #args ) # define DEFINE_SYSCALL_HELPER32() \ __ASM_GLOBAL_FUNC( __wine_syscall, "jmp *(" __ASM_NAME("__wine_syscall_dispatcher") ")" ) # endif -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4705
participants (2)
-
Paul Gofman -
Paul Gofman (＠gofman)