[PATCH v4 0/2] MR8927: ntdll: Add offset when mapping image mappings.
Fixes backtraces in Visual Studio Remote Debugging when the remote program is running on wine. Not sure if this is entirely right yet, maybe the offset/size check should also be moved further up? Test file is attached ( [map_test.c](/uploads/4d5b426fd9ad931ef4027332f75b1bd9/map_test.c) ), however it's probably not ideal since it tests against the value that the compiler happens to put at offset 0x10000 (I tested with mingw64 GCC). In any case this value just needs to match windows, so for other compilers it can be obtained by running the test on windows. This could probably be improved with a linker script if wanted. -- v4: ntdll: Respect offset for image mappings. https://gitlab.winehq.org/wine/wine/-/merge_requests/8927
From: Charlotte Pabst <cpabst(a)codeweavers.com> --- dlls/ntdll/tests/virtual.c | 56 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index 02d963f0550..4fa262df608 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -1439,12 +1439,13 @@ static void test_NtMapViewOfSection(void) static const char data[] = "test data for NtMapViewOfSection"; char buffer[sizeof(data)]; HANDLE file, mapping, process; - void *ptr, *ptr2; + void *ptr, *ptr2, *mem, *mem2; BOOL ret; DWORD status, written; - SIZE_T size, result; + SIZE_T size, size2, result; LARGE_INTEGER offset; ULONG_PTR zero_bits; + SYSTEM_INFO si; if (!pIsWow64Process || !pIsWow64Process(NtCurrentProcess(), &is_wow64)) is_wow64 = FALSE; @@ -1698,6 +1699,57 @@ static void test_NtMapViewOfSection(void) CloseHandle(file); } + /* image offset */ + + GetSystemInfo(&si); + + file = CreateFileA("c:\\windows\\system32\\ntdll.dll", GENERIC_READ, 0, NULL, OPEN_EXISTING, 0, 0); + ok(file != INVALID_HANDLE_VALUE, "Failed to open ntdll.dll\n"); + + mapping = CreateFileMappingA(file, NULL, PAGE_READONLY|SEC_IMAGE, 0, 0, NULL); + ok(mapping != 0, "CreateFileMapping failed\n"); + + ptr = NULL; + size = 0; + offset.QuadPart = 0; + status = NtMapViewOfSection(mapping, process, &ptr, 0, 0, &offset, &size, 1, 0, PAGE_READONLY); + ok(status == STATUS_IMAGE_NOT_AT_BASE, "NtMapViewOfSection returned %08lx\n", status); + + ptr2 = NULL; + size2 = 0; + offset.QuadPart = si.dwAllocationGranularity; + status = NtMapViewOfSection(mapping, process, &ptr2, 0, 0, &offset, &size2, 1, 0, PAGE_READONLY); + ok(status == STATUS_IMAGE_NOT_AT_BASE, "NtMapViewOfSection returned %08lx\n", status); + + todo_wine + ok(size2 == size - si.dwAllocationGranularity, "got unexpected sizes %Ix, %Ix\n", size, size2); + size2 = size - si.dwAllocationGranularity; + + mem = malloc(size2); + ret = ReadProcessMemory(process, (char*)ptr + si.dwAllocationGranularity, mem, size2, &result); + ok(ret, "ReadProcessMemory failed\n"); + ok(size2 == result, "ReadProcessMemory didn't read all data (%Ix)\n", result); + + mem2 = malloc(size2); + ret = ReadProcessMemory(process, ptr2, mem2, size2, &result); + ok(ret, "ReadProcessMemory failed\n"); + ok(size2 == result, "ReadProcessMemory didn't read all data (%Ix)\n", result); + + todo_wine + ok(memcmp(mem, mem2, size2) == 0, "memory does not match\n"); + + free(mem); + free(mem2); + + status = NtUnmapViewOfSection(process, ptr); + ok(status == STATUS_SUCCESS, "NtUnmapViewOfSection returned %08lx\n", status); + + status = NtUnmapViewOfSection(process, ptr2); + ok(status == STATUS_SUCCESS, "NtUnmapViewOfSection returned %08lx\n", status); + + NtClose(mapping); + CloseHandle(file); + TerminateProcess(process, 0); CloseHandle(process); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8927
From: Charlotte Pabst <cpabst(a)codeweavers.com> --- dlls/ntdll/tests/virtual.c | 2 -- dlls/ntdll/unix/loader.c | 29 +++++++++++++++-------------- dlls/ntdll/unix/unix_private.h | 4 ++-- dlls/ntdll/unix/virtual.c | 29 ++++++++++++++++++++--------- server/mapping.c | 2 +- server/protocol.def | 1 + 6 files changed, 39 insertions(+), 28 deletions(-) diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index 4fa262df608..6730315d5fb 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -1721,7 +1721,6 @@ static void test_NtMapViewOfSection(void) status = NtMapViewOfSection(mapping, process, &ptr2, 0, 0, &offset, &size2, 1, 0, PAGE_READONLY); ok(status == STATUS_IMAGE_NOT_AT_BASE, "NtMapViewOfSection returned %08lx\n", status); - todo_wine ok(size2 == size - si.dwAllocationGranularity, "got unexpected sizes %Ix, %Ix\n", size, size2); size2 = size - si.dwAllocationGranularity; @@ -1735,7 +1734,6 @@ static void test_NtMapViewOfSection(void) ok(ret, "ReadProcessMemory failed\n"); ok(size2 == result, "ReadProcessMemory didn't read all data (%Ix)\n", result); - todo_wine ok(memcmp(mem, mem2, size2) == 0, "memory does not match\n"); free(mem); diff --git a/dlls/ntdll/unix/loader.c b/dlls/ntdll/unix/loader.c index 34e9c0d87e5..12b071eebd9 100644 --- a/dlls/ntdll/unix/loader.c +++ b/dlls/ntdll/unix/loader.c @@ -1218,7 +1218,7 @@ static NTSTATUS open_dll_file( const char *name, OBJECT_ATTRIBUTES *attr, HANDLE static NTSTATUS open_builtin_pe_file( const char *name, OBJECT_ATTRIBUTES *attr, void **module, SIZE_T *size, SECTION_IMAGE_INFORMATION *image_info, ULONG_PTR limit_low, ULONG_PTR limit_high, - WORD machine, BOOL prefer_native ) + WORD machine, BOOL prefer_native, off_t offset ) { NTSTATUS status; HANDLE mapping; @@ -1228,7 +1228,7 @@ static NTSTATUS open_builtin_pe_file( const char *name, OBJECT_ATTRIBUTES *attr, if (!status) { status = virtual_map_builtin_module( mapping, module, size, image_info, - limit_low, limit_high, machine, prefer_native ); + limit_low, limit_high, machine, prefer_native, offset ); NtClose( mapping ); } return status; @@ -1241,7 +1241,7 @@ static NTSTATUS open_builtin_pe_file( const char *name, OBJECT_ATTRIBUTES *attr, static NTSTATUS find_builtin_dll( UNICODE_STRING *nt_name, ANSI_STRING *exp_name, void **module, SIZE_T *size_ptr, SECTION_IMAGE_INFORMATION *image_info, ULONG_PTR limit_low, ULONG_PTR limit_high, USHORT search_machine, - USHORT load_machine, BOOL prefer_native ) + USHORT load_machine, BOOL prefer_native, off_t offset ) { unsigned int i, pos, len, namepos = 0, maxlen = 0; char *ptr = NULL, *file, *ext = NULL; @@ -1301,7 +1301,7 @@ static NTSTATUS find_builtin_dll( UNICODE_STRING *nt_name, ANSI_STRING *exp_name /* try as a dll */ ptr = prepend_build_dir_path( file + pos, ".dll", pe_dir, "/dlls", pe_build_dir ); status = open_builtin_pe_file( ptr, &attr, module, size_ptr, image_info, - limit_low, limit_high, load_machine, prefer_native ); + limit_low, limit_high, load_machine, prefer_native, offset ); ptr = prepend_build_dir_path( file + pos, ".dll", "", "/dlls", build_dir ); if (status != STATUS_DLL_NOT_FOUND) goto done; status = open_builtin_so_file( ptr, &attr, module, image_info, @@ -1311,7 +1311,7 @@ static NTSTATUS find_builtin_dll( UNICODE_STRING *nt_name, ANSI_STRING *exp_name /* now as a program */ ptr = prepend_build_dir_path( file + pos, ".exe", pe_dir, "/programs", pe_build_dir ); status = open_builtin_pe_file( ptr, &attr, module, size_ptr, image_info, - limit_low, limit_high, load_machine, prefer_native ); + limit_low, limit_high, load_machine, prefer_native, offset ); ptr = prepend_build_dir_path( file + pos, ".exe", "", "/programs", build_dir ); if (status != STATUS_DLL_NOT_FOUND) goto done; status = open_builtin_so_file( ptr, &attr, module, image_info, @@ -1325,7 +1325,7 @@ static NTSTATUS find_builtin_dll( UNICODE_STRING *nt_name, ANSI_STRING *exp_name ptr = prepend( ptr, pe_dir, strlen(pe_dir) ); ptr = prepend( ptr, dll_paths[i], strlen(dll_paths[i]) ); status = open_builtin_pe_file( ptr, &attr, module, size_ptr, image_info, limit_low, limit_high, - load_machine, prefer_native ); + load_machine, prefer_native, offset ); /* use so dir for unix lib */ ptr = file + pos; ptr = prepend( ptr, so_dir, strlen(so_dir) ); @@ -1336,7 +1336,7 @@ static NTSTATUS find_builtin_dll( UNICODE_STRING *nt_name, ANSI_STRING *exp_name if (status != STATUS_DLL_NOT_FOUND) goto done; ptr = prepend( file + pos, dll_paths[i], strlen(dll_paths[i]) ); status = open_builtin_pe_file( ptr, &attr, module, size_ptr, image_info, limit_low, limit_high, - load_machine, prefer_native ); + load_machine, prefer_native, offset ); if (status == STATUS_NOT_SUPPORTED) { found_image = TRUE; @@ -1370,7 +1370,8 @@ done: */ NTSTATUS load_builtin( const struct pe_image_info *image_info, UNICODE_STRING *nt_name, ANSI_STRING *exp_name, USHORT machine, SECTION_IMAGE_INFORMATION *info, - void **module, SIZE_T *size, ULONG_PTR limit_low, ULONG_PTR limit_high ) + void **module, SIZE_T *size, ULONG_PTR limit_low, ULONG_PTR limit_high, + off_t offset ) { NTSTATUS status; USHORT search_machine = image_info->machine; @@ -1400,10 +1401,10 @@ NTSTATUS load_builtin( const struct pe_image_info *image_info, UNICODE_STRING *n return STATUS_IMAGE_ALREADY_LOADED; case LO_BUILTIN: return find_builtin_dll( nt_name, exp_name, module, size, info, limit_low, limit_high, - search_machine, machine, FALSE ); + search_machine, machine, FALSE, offset ); default: status = find_builtin_dll( nt_name, exp_name, module, size, info, limit_low, limit_high, - search_machine, machine, (loadorder == LO_DEFAULT) ); + search_machine, machine, (loadorder == LO_DEFAULT), offset ); if (status == STATUS_DLL_NOT_FOUND || status == STATUS_NOT_SUPPORTED) return STATUS_IMAGE_ALREADY_LOADED; return status; @@ -1519,7 +1520,7 @@ NTSTATUS load_main_exe( UNICODE_STRING *nt_name, USHORT load_machine, void **mod /* if path is in system dir, we can load the builtin even if the file itself doesn't exist */ if (loadorder != LO_NATIVE && is_builtin_path( nt_name, &search_machine )) status = find_builtin_dll( nt_name, NULL, module, &size, &main_image_info, 0, 0, - search_machine, load_machine, FALSE ); + search_machine, load_machine, FALSE, 0 ); return status; } @@ -1539,7 +1540,7 @@ NTSTATUS load_start_exe( UNICODE_STRING *nt_name, void **module ) wcscpy( image, get_machine_wow64_dir( current_machine )); wcscat( image, startW ); init_unicode_string( nt_name, image ); - status = find_builtin_dll( nt_name, NULL, module, &size, &main_image_info, 0, 0, current_machine, 0, FALSE ); + status = find_builtin_dll( nt_name, NULL, module, &size, &main_image_info, 0, 0, current_machine, 0, FALSE, 0 ); if (!NT_SUCCESS(status)) { MESSAGE( "wine: failed to load start.exe: %x\n", status ); @@ -1754,7 +1755,7 @@ static void load_ntdll(void) else asprintf( &name, "%s%s/ntdll.dll", dll_dir, pe_dir ); if (is_arm64ec()) machine = main_image_info.Machine; - status = open_builtin_pe_file( name, &attr, &module, &size, &info, 0, 0, machine, FALSE ); + status = open_builtin_pe_file( name, &attr, &module, &size, &info, 0, 0, machine, FALSE, 0 ); if (status == STATUS_DLL_NOT_FOUND) { free( name ); @@ -1860,7 +1861,7 @@ static void load_wow64_ntdll( USHORT machine ) wcscpy( path, wow64_dir ); wcscat( path, ntdllW ); init_unicode_string( &nt_name, path ); - status = find_builtin_dll( &nt_name, NULL, &module, &size, &info, 0, 0, machine, 0, FALSE ); + status = find_builtin_dll( &nt_name, NULL, &module, &size, &info, 0, 0, machine, 0, FALSE, 0 ); if (status == STATUS_IMAGE_NOT_AT_BASE) status = virtual_relocate_module( module ); if (status) fatal_error( "failed to load %s error %x\n", debugstr_w(path), status ); load_ntdll_wow64_functions( module ); diff --git a/dlls/ntdll/unix/unix_private.h b/dlls/ntdll/unix/unix_private.h index 5d374017101..941c05f2bc2 100644 --- a/dlls/ntdll/unix/unix_private.h +++ b/dlls/ntdll/unix/unix_private.h @@ -214,7 +214,7 @@ extern char *get_alternate_wineloader( WORD machine ); extern NTSTATUS exec_wineloader( char **argv, int socketfd, const struct pe_image_info *pe_info ); extern NTSTATUS load_builtin( const struct pe_image_info *image_info, UNICODE_STRING *nt_name, ANSI_STRING *exp_name, USHORT machine, SECTION_IMAGE_INFORMATION *info, - void **module, SIZE_T *size, ULONG_PTR limit_low, ULONG_PTR limit_high ); + void **module, SIZE_T *size, ULONG_PTR limit_low, ULONG_PTR limit_high, off_t offset ); extern BOOL is_builtin_path( const UNICODE_STRING *path, WORD *machine ); extern NTSTATUS load_main_exe( UNICODE_STRING *nt_name, USHORT load_machine, void **module ); extern NTSTATUS load_start_exe( UNICODE_STRING *nt_name, void **module ); @@ -280,7 +280,7 @@ extern ULONG_PTR get_system_affinity_mask(void); extern void virtual_get_system_info( SYSTEM_BASIC_INFORMATION *info, BOOL wow64 ); extern NTSTATUS virtual_map_builtin_module( HANDLE mapping, void **module, SIZE_T *size, SECTION_IMAGE_INFORMATION *info, ULONG_PTR limit_low, - ULONG_PTR limit_high, WORD machine, BOOL prefer_native ); + ULONG_PTR limit_high, WORD machine, BOOL prefer_native, off_t offset ); extern NTSTATUS virtual_map_module( HANDLE mapping, void **module, SIZE_T *size, SECTION_IMAGE_INFORMATION *info, ULONG_PTR limit_low, ULONG_PTR limit_high, USHORT machine ); diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index c1efbce3526..0d6d5496f02 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -3404,7 +3404,7 @@ static NTSTATUS map_image_view( struct file_view **view_ret, struct pe_image_inf static NTSTATUS virtual_map_image( HANDLE mapping, void **addr_ptr, SIZE_T *size_ptr, HANDLE shared_file, ULONG_PTR limit_low, ULONG_PTR limit_high, ULONG alloc_type, USHORT machine, struct pe_image_info *image_info, - UNICODE_STRING *nt_name, BOOL is_builtin ) + UNICODE_STRING *nt_name, BOOL is_builtin, off_t offset) { int unix_fd = -1, needs_close; int shared_fd = -1, shared_needs_close = 0; @@ -3413,6 +3413,9 @@ static NTSTATUS virtual_map_image( HANDLE mapping, void **addr_ptr, SIZE_T *size unsigned int status; sigset_t sigset; + if (offset >= size) + return STATUS_INVALID_PARAMETER; + if ((status = server_get_unix_fd( mapping, 0, &unix_fd, &needs_close, NULL, NULL ))) return status; @@ -3443,6 +3446,12 @@ static NTSTATUS virtual_map_image( HANDLE mapping, void **addr_ptr, SIZE_T *size status = map_image_into_view( view, nt_name, unix_fd, image_info, machine, shared_fd, needs_close ); if (status == STATUS_SUCCESS) { + if (offset) + { + free_pages( view, view->base, offset ); + size -= offset; + } + image_info->base = wine_server_client_ptr( view->base ); SERVER_START_REQ( map_image_view ) { @@ -3451,13 +3460,14 @@ static NTSTATUS virtual_map_image( HANDLE mapping, void **addr_ptr, SIZE_T *size req->size = size; req->entry = image_info->entry_point; req->machine = image_info->machine; + req->offset = offset; status = wine_server_call( req ); } SERVER_END_REQ; } if (NT_SUCCESS(status)) { - if (is_builtin) add_builtin_module( view->base, NULL ); + if (is_builtin && !offset) add_builtin_module( view->base, NULL ); *addr_ptr = view->base; *size_ptr = size; VIRTUAL_DEBUG_DUMP_VIEW( view ); @@ -3523,6 +3533,8 @@ static unsigned int virtual_map_section( HANDLE handle, PVOID *addr_ptr, ULONG_P &image_info, &nt_name, &exp_name ); if (res) return res; + offset.QuadPart = offset_ptr ? offset_ptr->QuadPart : 0; + if (image_info) { SECTION_IMAGE_INFORMATION info; @@ -3535,10 +3547,10 @@ static unsigned int virtual_map_section( HANDLE handle, PVOID *addr_ptr, ULONG_P } /* check if we can replace that mapping with the builtin */ res = load_builtin( image_info, &nt_name, &exp_name, machine, &info, - addr_ptr, size_ptr, limit_low, limit_high ); + addr_ptr, size_ptr, limit_low, limit_high, offset.QuadPart ); if (res == STATUS_IMAGE_ALREADY_LOADED) res = virtual_map_image( handle, addr_ptr, size_ptr, shared_file, limit_low, limit_high, - alloc_type, machine, image_info, &nt_name, FALSE ); + alloc_type, machine, image_info, &nt_name, FALSE, offset.QuadPart ); if (shared_file) NtClose( shared_file ); free( image_info ); if (NtCurrentTeb64()) NtCurrentTeb64()->Tib.ArbitraryUserPointer = prev; @@ -3546,7 +3558,6 @@ static unsigned int virtual_map_section( HANDLE handle, PVOID *addr_ptr, ULONG_P } base = *addr_ptr; - offset.QuadPart = offset_ptr ? offset_ptr->QuadPart : 0; if (offset.QuadPart >= full_size) return STATUS_INVALID_PARAMETER; if (*size_ptr) { @@ -3782,7 +3793,7 @@ void virtual_get_system_info( SYSTEM_BASIC_INFORMATION *info, BOOL wow64 ) */ NTSTATUS virtual_map_builtin_module( HANDLE mapping, void **module, SIZE_T *size, SECTION_IMAGE_INFORMATION *info, ULONG_PTR limit_low, - ULONG_PTR limit_high, WORD machine, BOOL prefer_native ) + ULONG_PTR limit_high, WORD machine, BOOL prefer_native, off_t offset ) { mem_size_t full_size; unsigned int sec_flags; @@ -3814,7 +3825,7 @@ NTSTATUS virtual_map_builtin_module( HANDLE mapping, void **module, SIZE_T *size else { status = virtual_map_image( mapping, module, size, shared_file, limit_low, limit_high, 0, - machine, image_info, &nt_name, TRUE ); + machine, image_info, &nt_name, TRUE, offset ); virtual_fill_image_information( image_info, info ); } @@ -3849,11 +3860,11 @@ NTSTATUS virtual_map_module( HANDLE mapping, void **module, SIZE_T *size, SECTIO /* check if we can replace that mapping with the builtin */ status = load_builtin( image_info, &nt_name, &exp_name, machine, info, - module, size, limit_low, limit_high ); + module, size, limit_low, limit_high, 0 ); if (status == STATUS_IMAGE_ALREADY_LOADED) { status = virtual_map_image( mapping, module, size, shared_file, limit_low, limit_high, 0, - machine, image_info, &nt_name, FALSE ); + machine, image_info, &nt_name, FALSE, 0 ); virtual_fill_image_information( image_info, info ); } if (shared_file) NtClose( shared_file ); diff --git a/server/mapping.c b/server/mapping.c index d56fe2b188a..905d528b04f 100644 --- a/server/mapping.c +++ b/server/mapping.c @@ -1677,7 +1677,7 @@ DECL_HANDLER(map_image_view) native_machine : req->machine; } - if (view->base != (mapping->image.map_addr ? mapping->image.map_addr : mapping->image.base)) + if (view->base != (mapping->image.map_addr ? mapping->image.map_addr : mapping->image.base) + req->offset) set_error( STATUS_IMAGE_NOT_AT_BASE ); if (req->machine != current->process->machine) { diff --git a/server/protocol.def b/server/protocol.def index 93e574b61a3..74557d2374d 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -1854,6 +1854,7 @@ enum server_fd_type mem_size_t size; /* view size */ unsigned int entry; /* entry point in mapped view */ unsigned short machine; /* machine in the mapped view */ + mem_size_t offset; /* offset into the image */ @END -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8927
@julliard this is ready for re-review. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8927#note_120012
participants (2)
-
Charlotte Pabst -
Charlotte Pabst (＠CharlottePabst)