[PATCH 0/2] MR8962: dbghelp: Fix pointer truncation issues when parsing 64-bit Mach-O binaries in a Wow64 process
When running `SymInitializeW(GetCurrentProcess(), NULL, TRUE)` in a 32-bit EXE on 64-bit macOS, there are a number of syscall faults resulting from 64-bit pointers being truncated to 32 bits and then passed to `ReadProcessMemory()`. Fix these, mostly by widening variables to 64 bits. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8962
From: Brendan Shanks <bshanks(a)codeweavers.com> --- dlls/dbghelp/dbghelp_private.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dlls/dbghelp/dbghelp_private.h b/dlls/dbghelp/dbghelp_private.h index 1249d02b666..695d234616c 100644 --- a/dlls/dbghelp/dbghelp_private.h +++ b/dlls/dbghelp/dbghelp_private.h @@ -599,6 +599,9 @@ struct process static inline BOOL read_process_memory(const struct process *process, UINT64 addr, void *buf, size_t size) { + if (addr != (UINT_PTR)addr) + return FALSE; + return ReadProcessMemory(process->handle, (void*)(UINT_PTR)addr, buf, size, NULL); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8962
From: Brendan Shanks <bshanks(a)codeweavers.com> --- dlls/dbghelp/dbghelp_private.h | 4 ++-- dlls/dbghelp/image_private.h | 4 ++-- dlls/dbghelp/macho_module.c | 31 +++++++++++++++---------------- 3 files changed, 19 insertions(+), 20 deletions(-) diff --git a/dlls/dbghelp/dbghelp_private.h b/dlls/dbghelp/dbghelp_private.h index 695d234616c..bc5693b7e08 100644 --- a/dlls/dbghelp/dbghelp_private.h +++ b/dlls/dbghelp/dbghelp_private.h @@ -584,7 +584,7 @@ struct process DWORD64 reg_user; struct module* lmodules; - ULONG_PTR dbg_hdr_addr; + DWORD64 dbg_hdr_addr; IMAGEHLP_STACK_FRAME ctx_frame; DWORD64 localscope_pc; @@ -789,7 +789,7 @@ struct elf_thunk_area; extern int elf_is_in_thunk_area(ULONG_PTR addr, const struct elf_thunk_area* thunks); /* macho_module.c */ -extern BOOL macho_read_wine_loader_dbg_info(struct process* pcs, ULONG_PTR addr); +extern BOOL macho_read_wine_loader_dbg_info(struct process* pcs, ULONG64 addr); /* minidump.c */ void minidump_add_memory_block(struct dump_context* dc, ULONG64 base, ULONG size, ULONG rva); diff --git a/dlls/dbghelp/image_private.h b/dlls/dbghelp/image_private.h index d5daac9dd47..3dfacf4d9de 100644 --- a/dlls/dbghelp/image_private.h +++ b/dlls/dbghelp/image_private.h @@ -124,8 +124,8 @@ struct image_file_map } elf; struct macho_file_map { - size_t segs_size; - size_t segs_start; + UINT64 segs_size; + UINT64 segs_start; HANDLE handle; struct image_file_map* dsym; /* the debug symbols file associated with this one */ size_t header_size; /* size of real header in file */ diff --git a/dlls/dbghelp/macho_module.c b/dlls/dbghelp/macho_module.c index 152dd16284f..3f95678166e 100644 --- a/dlls/dbghelp/macho_module.c +++ b/dlls/dbghelp/macho_module.c @@ -177,7 +177,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(dbghelp_macho); struct macho_module_info { struct image_file_map file_map; - ULONG_PTR load_addr; + UINT64 load_addr; unsigned short in_use : 1, is_loader : 1; }; @@ -639,8 +639,8 @@ static int macho_load_section_info(struct image_file_map* ifm, const struct mach struct section_info* info = user; BOOL ignore; int i; - ULONG_PTR tmp, page_mask = sysinfo.dwPageSize - 1; - UINT64 vmaddr, vmsize; + ULONG_PTR page_mask = sysinfo.dwPageSize - 1; + UINT64 tmp, vmaddr, vmsize; char segname[16]; size_t nsects; const void *sections; @@ -664,10 +664,10 @@ static int macho_load_section_info(struct image_file_map* ifm, const struct mach sections = (const void *)(sc + 1); } - TRACE("(%p/%p, %p, %p) before: 0x%08Ix - 0x%08Ix\n", fmap, fmap->handle, lc, user, - (ULONG_PTR)fmap->segs_start, (ULONG_PTR)fmap->segs_size); - TRACE("Segment command vm: 0x%08Ix - 0x%08Ix\n", (ULONG_PTR)vmaddr, - (ULONG_PTR)(vmaddr + vmsize)); + TRACE("(%p/%p, %p, %p) before: %#08I64x - %#08I64x\n", fmap, fmap->handle, lc, user, + fmap->segs_start, fmap->segs_size); + TRACE("Segment command vm: %#08I64x - %#08I64x\n", vmaddr, + (UINT64)(vmaddr + vmsize)); /* Images in the dyld shared cache have their segments mapped non-contiguously. We don't know how to properly locate any of the segments other than __TEXT, @@ -690,7 +690,7 @@ static int macho_load_section_info(struct image_file_map* ifm, const struct mach tmp = (vmaddr + vmsize + page_mask) & ~page_mask; if (fmap->segs_size < tmp) fmap->segs_size = tmp; - TRACE("after: 0x%08Ix - 0x%08Ix\n", (ULONG_PTR)fmap->segs_start, (ULONG_PTR)fmap->segs_size); + TRACE("after: %#08I64x - %#08I64x\n", fmap->segs_start, fmap->segs_size); } for (i = 0; i < nsects; i++) @@ -875,8 +875,7 @@ static BOOL macho_map_file(struct process *pcs, const WCHAR *filenameW, } fmap->segs_size -= fmap->segs_start; - TRACE("segs_start: 0x%08Ix, segs_size: 0x%08Ix\n", (ULONG_PTR)fmap->segs_start, - (ULONG_PTR)fmap->segs_size); + TRACE("segs_start: %#08I64x, segs_size: %#08I64x\n", fmap->segs_start, fmap->segs_size); if (macho_enum_load_commands(ifm, MACHO_LC_UUID, find_uuid, NULL) < 0) goto done; @@ -1349,7 +1348,7 @@ found: * The image header has to be loaded from the process's memory * because the relevant flag is only set in memory, not in the file. */ -static BOOL image_uses_split_segs(struct process* process, ULONG_PTR load_addr) +static BOOL image_uses_split_segs(struct process* process, UINT64 load_addr) { BOOL split_segs = FALSE; @@ -1376,7 +1375,7 @@ static BOOL image_uses_split_segs(struct process* process, ULONG_PTR load_addr) * For a module identified by its load address, return the machine field * of the (loaded) Macho header. */ -static USHORT image_get_machine(struct process *process, ULONG_PTR load_addr) +static USHORT image_get_machine(struct process *process, UINT64 load_addr) { if (load_addr) { @@ -1502,13 +1501,13 @@ static const struct module_format_vtable macho_module_format_vtable = * TRUE on success */ static BOOL macho_load_file(struct process* pcs, const WCHAR* filename, - ULONG_PTR load_addr, struct macho_info* macho_info, BOOL with_image) + UINT64 load_addr, struct macho_info* macho_info, BOOL with_image) { BOOL ret = TRUE; BOOL split_segs; struct image_file_map fmap; - TRACE("(%p/%p, %s, 0x%08Ix, %p/0x%08x, %u)\n", pcs, pcs->handle, debugstr_w(filename), + TRACE("(%p/%p, %s, %#08I64x, %p/%#08x, %u)\n", pcs, pcs->handle, debugstr_w(filename), load_addr, macho_info, macho_info->flags, with_image); split_segs = image_uses_split_segs(pcs, load_addr); @@ -1969,7 +1968,7 @@ static const struct loader_ops macho_loader_ops = * * Try to find a decent wine executable which could have loaded the debuggee */ -BOOL macho_read_wine_loader_dbg_info(struct process* pcs, ULONG_PTR addr) +BOOL macho_read_wine_loader_dbg_info(struct process* pcs, ULONG64 addr) { struct macho_info macho_info; @@ -1980,6 +1979,6 @@ BOOL macho_read_wine_loader_dbg_info(struct process* pcs, ULONG_PTR addr) macho_info.module->format_info[DFI_MACHO]->u.macho_info->is_loader = 1; module_set_module(macho_info.module, S_WineLoaderW); pcs->loader = &macho_loader_ops; - TRACE("Found macho debug header %#Ix\n", pcs->dbg_hdr_addr); + TRACE("Found macho debug header %#I64x\n", pcs->dbg_hdr_addr); return TRUE; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8962
does this simpler approach works? (assuming wine loader is above 4G) [dh.patch](/uploads/5375ed08db3804a4026726c4735f8d9c/dh.patch) I think it's better to disable fully system modules searching & loading in that case (32bit dbghelp managing a live new wow process) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8962#note_115737
participants (3)
-
Brendan Shanks -
Brendan Shanks (＠bshanks)
-
eric pouech (＠epo)