[PATCH v9 0/3] MR7064: ntdll: Check for invalid GS base in the 64-bit segv_handler.
Adapted from [check_invalid_gs](https://gitlab.winehq.org/wine/wine/-/blob/d7a7cae2e2d1ad34af6dbefcb06fbef2f...) in signal_i386.c. This prevents crashing when the %gs register is manipulated, such as in 32-bit code on "new-style" WoW64. "Exertus: Darkness Approaches" is a 32-bit game that triggers this crash (on WoW64), which can be downloaded from the Bugzilla page's attachments. For another example, [test.c](https://bugs.winehq.org/attachment.cgi?id=77444) can reproduce it, courtesy of Fabian Maurer from the Bugzilla comments. This will crash whether it's compiled as 32-bit or 64-bit, while it works fine on Windows. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57444 -- v9: ntdll/tests: Re-enable a previously crashing test. ntdll: Check for invalid gs_base in the 64-bit segv_handler. ntdll/tests: Add an exception test for accessing a modified %gs on x86_64. https://gitlab.winehq.org/wine/wine/-/merge_requests/7064
From: William Horvath <william(a)horvath.blog> --- dlls/ntdll/tests/exception.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c index 025923b7d8e..41f25cc6c8b 100644 --- a/dlls/ntdll/tests/exception.c +++ b/dlls/ntdll/tests/exception.c @@ -2922,6 +2922,14 @@ static const struct exception { { 0xb8, 0x01, 0x00, 0x00, 0x00, /* mov $0x01, %eax */ 0xcd, 0x2d, 0xfa, 0xc3 }, /* int $0x2d; cli; ret */ 8, 0, STATUS_SUCCESS, 0 }, +#if 0 /* Disabled for the same reason as the #if 0 blocks above (gs_base zeroed) */ + { { 0x66, 0x0f, 0xa8, /* push %gs */ + 0x66, 0x0f, 0xa9, /* pop %gs */ + 0x65, 0x48, 0x8b, 0x04, 0x25, /* movq %gs:0x30,%rax (NtCurrentTeb) */ + 0x30, 0x00, 0x00, 0x00, + 0xc3 }, /* ret */ + 8, 0, STATUS_SUCCESS, 0 }, +#endif }; static int got_exception; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7064
From: William Horvath <william(a)horvath.blog> Adapted from check_invalid_gs in signal_i386.c. PE-side code can manipulate %gs and cause the next call to NtCurrentTeb to segfault, as the gs_base may be cleared with writes to %gs on x86_64 [1]. This would cause a recursive exception loop, as any PE-side code in the exception handling chain after the segv_handler would run into the same problem. So, catch this early, and manually repair the thread's gs_base with the pthread TEB from the Unix side. The 32-bit game "Alice: Madness Returns" is one example of this problem occurring in the real world, when running under WoW64. However, this is currently handled in Windows under both WoW64 and native 64-bit, so we should handle both architectures as well. [1]: https://bugs.winehq.org/show_bug.cgi?id=51152 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57444 --- dlls/ntdll/tests/exception.c | 2 - dlls/ntdll/unix/signal_x86_64.c | 135 +++++++++++++++++++++++++++----- 2 files changed, 117 insertions(+), 20 deletions(-) diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c index 41f25cc6c8b..281cbb08865 100644 --- a/dlls/ntdll/tests/exception.c +++ b/dlls/ntdll/tests/exception.c @@ -2922,14 +2922,12 @@ static const struct exception { { 0xb8, 0x01, 0x00, 0x00, 0x00, /* mov $0x01, %eax */ 0xcd, 0x2d, 0xfa, 0xc3 }, /* int $0x2d; cli; ret */ 8, 0, STATUS_SUCCESS, 0 }, -#if 0 /* Disabled for the same reason as the #if 0 blocks above (gs_base zeroed) */ { { 0x66, 0x0f, 0xa8, /* push %gs */ 0x66, 0x0f, 0xa9, /* pop %gs */ 0x65, 0x48, 0x8b, 0x04, 0x25, /* movq %gs:0x30,%rax (NtCurrentTeb) */ 0x30, 0x00, 0x00, 0x00, 0xc3 }, /* ret */ 8, 0, STATUS_SUCCESS, 0 }, -#endif }; static int got_exception; diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index 058e3976ef1..a3132087702 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -2007,6 +2007,118 @@ static BOOL handle_syscall_trap( ucontext_t *sigcontext, siginfo_t *siginfo ) } +#ifdef __APPLE__ +/********************************************************************** + * mac_thread_gsbase + */ +static void *mac_thread_gsbase(void) +{ + struct thread_identifier_info tiinfo; + unsigned int info_count = THREAD_IDENTIFIER_INFO_COUNT; + + mach_port_t self = mach_thread_self(); + kern_return_t kr = thread_info(self, THREAD_IDENTIFIER_INFO, (thread_info_t) &tiinfo, &info_count); + mach_port_deallocate(mach_task_self(), self); + + if (kr == KERN_SUCCESS) return (void*)tiinfo.thread_handle; + return NULL; +} +#endif + + +/*********************************************************************** + * check_invalid_gsbase + * + * Check for fault caused by invalid %gs value (some copy protection schemes mess with it). + */ +static inline BOOL check_invalid_gsbase( ucontext_t *sigcontext, CONTEXT *context ) +{ + unsigned int prefix_count = 0; + const BYTE *instr = (const BYTE *)context->Rip; + TEB *teb = NtCurrentTeb(); + ULONG_PTR cur_gsbase = 0; + +#ifdef __linux__ + if (user_shared_data->ProcessorFeatures[PF_RDWRFSGSBASE_AVAILABLE]) + __asm__ volatile ("rdgsbase %0" : "=r" (cur_gsbase)); + else + arch_prctl( ARCH_GET_GS, &cur_gsbase ); +#elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) + amd64_get_gsbase( &cur_gsbase ); +#elif defined(__NetBSD__) + sysarch( X86_64_GET_GSBASE, &cur_gsbase ); +#elif defined(__APPLE__) + cur_gsbase = (ULONG_PTR)mac_thread_gsbase(); +#else +# error Please define getting %gs for your architecture +#endif + + if (cur_gsbase == (ULONG_PTR)teb) return FALSE; + + for (;;) + { + switch(*instr) + { + /* instruction prefixes */ + case 0x2e: /* %cs: */ + case 0x36: /* %ss: */ + case 0x3e: /* %ds: */ + case 0x26: /* %es: */ + case 0x40: /* rex */ + case 0x41: /* rex */ + case 0x42: /* rex */ + case 0x43: /* rex */ + case 0x44: /* rex */ + case 0x45: /* rex */ + case 0x46: /* rex */ + case 0x47: /* rex */ + case 0x48: /* rex */ + case 0x49: /* rex */ + case 0x4a: /* rex */ + case 0x4b: /* rex */ + case 0x4c: /* rex */ + case 0x4d: /* rex */ + case 0x4e: /* rex */ + case 0x4f: /* rex */ + case 0x64: /* %fs: */ + case 0x66: /* opcode size */ + case 0x67: /* addr size */ + case 0xf0: /* lock */ + case 0xf2: /* repne */ + case 0xf3: /* repe */ + if (++prefix_count >= 15) return FALSE; + instr++; + continue; + case 0x65: /* %gs: */ + GS_sig(sigcontext) = ds64_sel; + break; + default: + return FALSE; + } + break; /* %gs: */ + } + + TRACE( "gsbase %016lx teb %p at instr %p, fixing up\n", cur_gsbase, teb, instr ); + +#ifdef __linux__ + if (user_shared_data->ProcessorFeatures[PF_RDWRFSGSBASE_AVAILABLE]) + __asm__ volatile ("wrgsbase %0" :: "r" (teb)); + else + arch_prctl( ARCH_SET_GS, teb ); +#elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) + amd64_set_gsbase( teb ); +#elif defined(__NetBSD__) + sysarch( X86_64_SET_GSBASE, &teb ); +#elif defined(__APPLE__) + _thread_set_tsd_base((uint64_t)teb); +#else +# error Please define setting %gs for your architecture +#endif + + return TRUE; +} + + /********************************************************************** * segv_handler * @@ -2065,6 +2177,11 @@ static void segv_handler( int signal, siginfo_t *siginfo, void *sigcontext ) /* send EXCEPTION_EXECUTE_FAULT only if data execution prevention is enabled */ if (!(flags & MEM_EXECUTE_OPTION_DISABLE)) rec.ExceptionInformation[0] = EXCEPTION_READ_FAULT; } + if (CS_sig(ucontext) == cs64_sel && check_invalid_gsbase( ucontext, &context.c )) + { + leave_handler( ucontext ); + return; + } break; case TRAP_x86_ALIGNFLT: /* Alignment check exception */ if (EFL_sig(ucontext) & 0x00040000) @@ -2473,24 +2590,6 @@ void signal_free_thread( TEB *teb ) } } -#ifdef __APPLE__ -/********************************************************************** - * mac_thread_gsbase - */ -static void *mac_thread_gsbase(void) -{ - struct thread_identifier_info tiinfo; - unsigned int info_count = THREAD_IDENTIFIER_INFO_COUNT; - - mach_port_t self = mach_thread_self(); - kern_return_t kr = thread_info(self, THREAD_IDENTIFIER_INFO, (thread_info_t) &tiinfo, &info_count); - mach_port_deallocate(mach_task_self(), self); - - if (kr == KERN_SUCCESS) return (void*)tiinfo.thread_handle; - return NULL; -} -#endif - /********************************************************************** * signal_init_process -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7064
From: William Horvath <william(a)horvath.blog> See https://bugs.winehq.org/show_bug.cgi?id=51152 for the bug that led to commit 4e4847dd71a3c682356559a51705ccec93b2490e. We can re-enable the %gs case now, as that no longer causes a crash. --- dlls/ntdll/tests/exception.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c index 281cbb08865..0969897c12b 100644 --- a/dlls/ntdll/tests/exception.c +++ b/dlls/ntdll/tests/exception.c @@ -2884,24 +2884,21 @@ static const struct exception /* It is observed that fs/gs base is reset on some CPUs when setting the segment value even to 0 (regardless of CPU spec - saying otherwise) and it is not currently - handled in Wine. + saying otherwise) and the fs base case + is not currently handled in Wine. Disable this part to avoid crashing the test. */ 0x8e, 0xe0, /* mov %eax,%fs */ - 0x8e, 0xe8, /* mov %eax,%gs */ #else 0x90, 0x90, /* nop */ - 0x90, 0x90, /* nop */ #endif + 0x8e, 0xe8, /* mov %eax,%gs */ 0xfa, /* cli */ 0x58, /* pop %rax */ -#if 0 0x8e, 0xe8, /* mov %eax,%gs */ 0x58, /* pop %rax */ +#if 0 0x8e, 0xe0, /* mov %eax,%fs */ #else - 0x58, /* pop %rax */ - 0x90, 0x90, /* nop */ 0x90, 0x90, /* nop */ #endif 0x58, /* pop %rax */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7064
This fixes Shiny Entertainment's "MDK". So I think this should be merged, seems to be pretty much ready. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7064#note_109749
Looking forward to this fix getting merged :smile: [multiple games](https://bugs.winehq.org/show_bug.cgi?id=57444) are currently affected and not all users are capable of compiling their own build of Wine. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7064#note_110793
participants (4)
-
Bob Bob (＠bob-drinks-wine) -
Manuel Alfayate (＠vanfanel)
-
William Horvath
-
William Horvath (＠whrvt)