[PATCH v7 0/2] MR8466: ntdll: Also restore rbp before calling user mode callback.
If wine dlls are built with frame pointers enabled, the frame pointer will be used during unwinding. If we don't restore frame pointer before calling the user mode callback, then later when the unwinder encounters the user mode callback frame, it will set the frame pointer to something unexpected (depends on what it was during `call_user_mode_callback`). Then for the subsequent frame it adjusts the stack pointer based on the frame pointer, thus derailing the unwinding process. -- v7: ntdll: Also restore rbp before calling user mode callback. user32/tests: Test unwinding through a user callback. https://gitlab.winehq.org/wine/wine/-/merge_requests/8466
From: Yuxuan Shui <yshui(a)codeweavers.com> Check the state of non-volatile registers before a user callback is entered. --- dlls/user32/tests/win.c | 121 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c index f899008f394..d462ce3bcab 100644 --- a/dlls/user32/tests/win.c +++ b/dlls/user32/tests/win.c @@ -32,6 +32,8 @@ #include "winuser.h" #include "winreg.h" +#include "wine/asm.h" +#include "wine/exception.h" #include "wine/test.h" #ifndef WM_SYSTIMER @@ -43,6 +45,7 @@ void dump_region(HRGN hrgn); +BOOL (WINAPI *pDestroyWindow)(HWND); static BOOL (WINAPI *pGetWindowInfo)(HWND,WINDOWINFO*); static UINT (WINAPI *pGetWindowModuleFileNameA)(HWND,LPSTR,UINT); static BOOL (WINAPI *pGetLayeredWindowAttributes)(HWND,COLORREF*,BYTE*,DWORD*); @@ -13750,12 +13753,127 @@ static void test_startupinfo_showwindow( char **argv ) } } +#if defined(_WIN64) && defined(__WINE_PE_BUILD) +void *unwind_target = NULL; +void *target_frame; + +static LRESULT unwinding_wnd_proc(HWND w, UINT msg, WPARAM p2, LPARAM p3) +{ + CONTEXT context; + int frames; + UNWIND_HISTORY_TABLE table; + RUNTIME_FUNCTION *func; + ULONG_PTR frame, base; + void *data; + BOOL found = FALSE; + + RtlCaptureContext(&context); + + switch (msg) + { + case WM_NCDESTROY: + for (frames = 0; frames < 16; frames++) + { + func = RtlLookupFunctionEntry(context.Rip, &base, &table); + if (RtlVirtualUnwind(UNW_FLAG_NHANDLER, base, context.Rip, func, &context, &data, &frame, NULL)) + break; + if (!context.Rip) break; + if (!frame) break; + if (context.Rip == (DWORD64)unwind_target || frame == (DWORD64)target_frame) + { + found = TRUE; + + /* check that non-volatile registers are zeroed before entering user callback. */ + ok(!context.Rbx, "unexpected register value, %%rbx = %#I64x\n", context.Rbx); + ok(!context.R12, "unexpected register value, %%r12 = %#I64x\n", context.R12); + ok(!context.R13, "unexpected register value, %%r13 = %#I64x\n", context.R13); + ok(!context.R14, "unexpected register value, %%r14 = %#I64x\n", context.R14); + ok(!context.R15, "unexpected register value, %%r15 = %#I64x\n", context.R15); + ok(context.Rbp == 0xdeadbeaf, "unexpected register value, %%rbp = %#I64x\n", context.Rbp); + break; + } + } + ok(found, "couldn't find target frame in parent frames\n"); + break; + default: break; + } + return DefWindowProcA(w, msg, p2, p3); +} + +/* setup register context so later we can check what changed and what hasn't. */ +extern void destroy_window_trampoline(HWND w); +__ASM_GLOBAL_FUNC(destroy_window_trampoline, + "push %rbp\n\t" + __ASM_SEH(".seh_pushreg %rbp\n\t") + "push %r15\n\t" + __ASM_SEH(".seh_pushreg %r15\n\t") + "pushq %r14\n\t" + __ASM_SEH(".seh_pushreg %r14\n\t") + "pushq %r13\n\t" + __ASM_SEH(".seh_pushreg %r13\n\t") + "pushq %r12\n\t" + __ASM_SEH(".seh_pushreg %r12\n\t") + "pushq %rbx\n\t" + __ASM_SEH(".seh_pushreg %rbx\n\t") + "subq $40, %rsp\n\t" /* allocate some random amount */ + __ASM_SEH(".seh_stackalloc 40\n\t") + __ASM_SEH(".seh_endprologue\n\t") + "mov $0xdeadbeaf,%rbp\n\t" + "mov $0xdeadbeaf,%rbx\n\t" + "mov $0xdeadbeaf,%r12\n\t" + "mov $0xdeadbeaf,%r13\n\t" + "mov $0xdeadbeaf,%r14\n\t" + "mov %rsp,target_frame(%rip)\n\t" + "lea 1f(%rip),%rdx\n\t" + "mov %rdx,unwind_target(%rip)\n\t" + "call *pDestroyWindow(%rip)\n\t" + "1:nop\n\t" /* some padding before epilogue */ + "addq $40, %rsp\n\t" + "pop %rbx\n\t" + "pop %r12\n\t" + "pop %r13\n\t" + "pop %r14\n\t" + "pop %r15\n\t" + "pop %rbp\n\t" + "ret"); + +static void test_user_callback_registers(void) +{ + WNDCLASSA cls = {0}; + HWND hwnd; + ATOM atom; + // Fill in the window class structure with parameters + // that describe the main window. + + cls.style = CS_HREDRAW | CS_VREDRAW; + cls.lpfnWndProc = unwinding_wnd_proc; // points to window procedure + cls.hInstance = GetModuleHandleA(0); + cls.lpszClassName = "test_user_callback_registers_class"; // name of window class + + // Register the window class. + + atom = RegisterClassA(&cls); + ok(!!atom, "RegisterClassA failed, error %#lx.\n", GetLastError()); + + // Create the main window. + + hwnd = CreateWindowExA(WS_EX_TOPMOST, cls.lpszClassName, "", WS_POPUP | WS_VISIBLE, 100, 100, + 100, 100, NULL, NULL, 0, NULL); + ok(!!hwnd, "CreateWindowA failed, error %#lx.\n", GetLastError()); + + destroy_window_trampoline(hwnd); + UnregisterClassA(cls.lpszClassName, GetModuleHandleA(0)); +} + +#endif + START_TEST(win) { char **argv; int argc = winetest_get_mainargs( &argv ); HMODULE user32 = GetModuleHandleA( "user32.dll" ); HMODULE gdi32 = GetModuleHandleA("gdi32.dll"); + pDestroyWindow = (void *)GetProcAddress( user32, "DestroyWindow" ); pGetWindowInfo = (void *)GetProcAddress( user32, "GetWindowInfo" ); pGetWindowModuleFileNameA = (void *)GetProcAddress( user32, "GetWindowModuleFileNameA" ); pGetLayeredWindowAttributes = (void *)GetProcAddress( user32, "GetLayeredWindowAttributes" ); @@ -13943,6 +14061,9 @@ START_TEST(win) test_ReleaseCapture(); test_SetProcessLaunchForegroundPolicy(); test_startupinfo_showwindow(argv); +#if defined(_WIN64) && defined(__WINE_PE_BUILD) + test_user_callback_registers(); +#endif /* add the tests above this line */ if (hhook) UnhookWindowsHookEx(hhook); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8466
From: Yuxuan Shui <yshui(a)codeweavers.com> If wine dlls are built with frame pointers enabled, the frame pointer will be used during unwinding. If we don't restore frame pointer before calling the user mode callback, then later when the unwinder encounters the user mode callback frame, it will set the frame pointer to something unexpected (depends on what it was during `call_user_mode_callback`). Then for the subsequent frame it adjusts the stack pointer based on the frame pointer, thus derailing the unwinding process. --- dlls/ntdll/unix/signal_x86_64.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index 6ec662b9850..4694b949071 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -1642,8 +1642,8 @@ __ASM_GLOBAL_FUNC( call_user_mode_callback, "andq $~63,%rsp\n\t" "leaq 0x10(%rbp),%rax\n\t" "movq %rax,0xa8(%rsp)\n\t" /* frame->syscall_cfa */ - "movq 0x378(%r13),%r10\n\t" /* thread_data->syscall_frame */ - "movq %r10,0xa0(%rsp)\n\t" /* frame->prev_frame */ + "movq 0x378(%r13),%r14\n\t" /* thread_data->syscall_frame */ + "movq %r14,0xa0(%rsp)\n\t" /* frame->prev_frame */ "movq %rsp,0x378(%r13)\n\t" /* thread_data->syscall_frame */ "testl $1,0x380(%r13)\n\t" /* thread_data->syscall_trace */ "jz 1f\n\t" @@ -1658,6 +1658,7 @@ __ASM_GLOBAL_FUNC( call_user_mode_callback, "movq %r15,%rcx\n" /* func */ /* switch to user stack */ "1:\tmovq %rdi,%rsp\n\t" /* user_rsp */ + "movq 0x98(%r14),%rbp\n\t" /* user_frame->Rbp */ #ifdef __linux__ "movw 0x338(%r13),%ax\n" /* amd64_thread_data()->fs */ "testw %ax,%ax\n\t" @@ -1677,7 +1678,12 @@ __ASM_GLOBAL_FUNC( call_user_mode_callback, "test %r10,%r10\n\t" "jz 1f\n\t" "xchgq %rcx,%r10\n\t" - "1\t:jmpq *%rcx" ) /* func */ + "1:\txor %rbx,%rbx\n\t" /* zero non-volatile registers. */ + "xor %r12,%r12\n\t" + "xor %r13,%r13\n\t" + "xor %r14,%r14\n\t" + "xor %r15,%r15\n\t" + "jmpq *%rcx" ) /* func */ /*********************************************************************** -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8466
participants (2)
-
Yuxuan Shui -
Yuxuan Shui (＠yshui)