[PATCH v7 0/1] MR7064: Draft: 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 -- v7: ntdll: Check for invalid GS base in the 64-bit segv_handler on WoW64. 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 wine_syscall_dispatcher to immediately fault, as the syscall_frame is relative to it. Catch this early, and fix up up the thread's GS base register with one that matches NtCurrentTeb(). Alice: Madness Returns (2011) is one example of game that does this, among a few others currently known. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57444 --- dlls/ntdll/unix/signal_x86_64.c | 101 ++++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index caa85249896..fa2e1f8ba8e 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -1967,6 +1967,102 @@ static BOOL handle_syscall_trap( ucontext_t *sigcontext, siginfo_t *siginfo ) } +/*********************************************************************** + * 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 (syscall_flags & SYSCALL_HAVE_WRFSGSBASE) + __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)teb->ThreadLocalStoragePointer; +#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 (syscall_flags & SYSCALL_HAVE_WRFSGSBASE) + __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__) + __asm__ volatile ("movq %0,%%gs:%c1" :: "r" (teb->Tib.Self), + "n" (FIELD_OFFSET(TEB, Tib.Self))); + __asm__ volatile ("movq %0,%%gs:%c1" :: "r" (teb->ThreadLocalStoragePointer), + "n" (FIELD_OFFSET(TEB, ThreadLocalStoragePointer))); +#else +# error Please define setting %gs for your architecture +#endif + + return TRUE; +} + + /********************************************************************** * segv_handler * @@ -2025,6 +2121,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 (is_wow64() && 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) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7064
On Mon Dec 23 05:20:01 2024 +0000, William Horvath wrote:
changed this line in [version 5 of the diff](/wine/wine/-/merge_requests/7064/diffs?diff_id=150040&start_sha=f139c28c51980f6b1aef9f24a0d261e738d4f8a5#253a5ed0bf568582a6c96df257e6e01fe7220931_2138_2131) If I'm understanding what you meant here, you suggest we should only be performing the GS base fixup in the WoW64 layer's code instead of on the Unix side. Let me know if I'm still not clear about that.
Besides not being able to make this work (without a consequent seh `ACCESS_VIOLATION`), I can't find a reason as to why this is better/safer/more correct than fixing up at the earliest moment (Unix-side, syscall_dispatcher fault) instead of letting the issue propagate. It seems safe enough to fix up the GS base address based on what we get from e.g. `ARCH_GET_GS` and `NtCurrentTeb()`, along with the other checks. Also, while I haven't tried those patches on macOS, it fortunately seems like it's not quite necessary here in the scenarios I've looked at. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7064#note_91168
participants (2)
-
William Horvath -
William Horvath (＠whrvth)