[PATCH v2 1/2] ntdll: Fix KiUserExceptionDispatcher ABI on x86.
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- v2: - Use __ASM_STDCALL_FUNC instead of coding linker name; - Use helper function to get KiUserDispactherAddress to properly handle PIC. dlls/ntdll/signal_i386.c | 9 ++- dlls/ntdll/tests/exception.c | 111 +++++++++++++++++++++++++++++++++- dlls/ntdll/unix/signal_i386.c | 11 ++-- 3 files changed, 125 insertions(+), 6 deletions(-) diff --git a/dlls/ntdll/signal_i386.c b/dlls/ntdll/signal_i386.c index 676329c5bd2..594c6f9a1d1 100644 --- a/dlls/ntdll/signal_i386.c +++ b/dlls/ntdll/signal_i386.c @@ -198,7 +198,7 @@ static NTSTATUS call_stack_handlers( EXCEPTION_RECORD *rec, CONTEXT *context ) /******************************************************************* * KiUserExceptionDispatcher (NTDLL.@) */ -NTSTATUS WINAPI KiUserExceptionDispatcher( EXCEPTION_RECORD *rec, CONTEXT *context ) +NTSTATUS WINAPI dispatch_exception( EXCEPTION_RECORD *rec, CONTEXT *context ) { NTSTATUS status; DWORD c; @@ -243,6 +243,13 @@ NTSTATUS WINAPI KiUserExceptionDispatcher( EXCEPTION_RECORD *rec, CONTEXT *conte return NtRaiseException( rec, context, FALSE ); } +__ASM_STDCALL_FUNC( KiUserExceptionDispatcher, 8, + /* hotpatch prologue. */ + "push %ebp\n\t" + "mov %esp,%ebp\n\t" + "pop %ebp\n\t" + "call " __ASM_NAME("dispatch_exception") "\n\t" + "int3") /*********************************************************************** * save_fpu diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c index 95d25375f77..a27dee21f00 100644 --- a/dlls/ntdll/tests/exception.c +++ b/dlls/ntdll/tests/exception.c @@ -1660,6 +1660,114 @@ static void test_thread_context(void) #undef COMPARE } +static BYTE saved_KiUserExceptionDispatcher_bytes[7]; +static void *pKiUserExceptionDispatcher; +static BOOL hook_called; +static void *hook_KiUserExceptionDispatcher_eip; +static void *dbg_except_continue_handler_eip; +static void *hook_exception_address; + +static DWORD dbg_except_continue_handler(EXCEPTION_RECORD *rec, EXCEPTION_REGISTRATION_RECORD *frame, + CONTEXT *context, EXCEPTION_REGISTRATION_RECORD **dispatcher) +{ + ok(hook_called, "Hook was not called.\n"); + got_exception = 1; + dbg_except_continue_handler_eip = (void *)context->Eip; + ++context->Eip; + return ExceptionContinueExecution; +} + +/* Use CDECL to leave arguments on stack. */ +void CDECL hook_KiUserExceptionDispatcher(EXCEPTION_RECORD *rec, CONTEXT *context) +{ + trace("rec %p, context %p.\n", rec, context); + trace("context->Eip %#x, context->Esp %#x, ContextFlags %#x.\n", + context->Eip, context->Esp, context->ContextFlags); + + hook_called = TRUE; + /* Broken on Win2008, probably rec offset in stack is different. */ + ok(rec->ExceptionCode == 0x80000003 || broken(!rec->ExceptionCode), + "Got unexpected ExceptionCode %#x.\n", rec->ExceptionCode); + + hook_KiUserExceptionDispatcher_eip = (void *)context->Eip; + hook_exception_address = rec->ExceptionAddress; + memcpy(pKiUserExceptionDispatcher, saved_KiUserExceptionDispatcher_bytes, + sizeof(saved_KiUserExceptionDispatcher_bytes)); +} + +static void test_kiuserexceptiondispatcher(void) +{ + HMODULE hntdll = GetModuleHandleA("ntdll.dll"); + static const BYTE except_code[] = + { + 0xcc, /* int3 */ + 0xc3, /* ret */ + }; + static BYTE hook_trampoline[] = + { + 0xff, 0x15, + /* offset: 2 bytes */ + 0x00, 0x00, 0x00, 0x00, /* callq *addr */ /* call hook implementation. */ + + 0xff, 0x25, + /* offset: 8 bytes */ + 0x00, 0x00, 0x00, 0x00, /* jmpq *addr */ /* jump to original function. */ + }; + void *phook_KiUserExceptionDispatcher = hook_KiUserExceptionDispatcher; + void *phook_trampoline = hook_trampoline; + DWORD old_protect1, old_protect2; + BYTE *ptr; + BOOL ret; + + pKiUserExceptionDispatcher = (void *)GetProcAddress(hntdll, "KiUserExceptionDispatcher"); + if (!pKiUserExceptionDispatcher) + { + win_skip("KiUserExceptionDispatcher is not available.\n"); + return; + } + + *(unsigned int *)(hook_trampoline + 2) = (ULONG_PTR)&phook_KiUserExceptionDispatcher; + *(unsigned int *)(hook_trampoline + 8) = (ULONG_PTR)&pKiUserExceptionDispatcher; + + ret = VirtualProtect(hook_trampoline, ARRAY_SIZE(hook_trampoline), PAGE_EXECUTE_READWRITE, &old_protect1); + ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); + + ret = VirtualProtect(pKiUserExceptionDispatcher, sizeof(saved_KiUserExceptionDispatcher_bytes), + PAGE_EXECUTE_READWRITE, &old_protect2); + ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); + + memcpy(saved_KiUserExceptionDispatcher_bytes, pKiUserExceptionDispatcher, + sizeof(saved_KiUserExceptionDispatcher_bytes)); + ptr = (BYTE *)pKiUserExceptionDispatcher; + /* mov hook_trampoline, %eax */ + *ptr++ = 0xa1; + *(void **)ptr = &phook_trampoline; + ptr += sizeof(void *); + /* jmp *eax */ + *ptr++ = 0xff; + *ptr++ = 0xe0; + + got_exception = 0; + run_exception_test(dbg_except_continue_handler, NULL, except_code, ARRAY_SIZE(except_code), + PAGE_EXECUTE_READ); + ok(got_exception, "Handler was not called.\n"); + ok(hook_called, "Hook was not called.\n"); + + ok(hook_exception_address == code_mem || broken(!hook_exception_address) /* Win2008 */, + "Got unexpected exception address %p, expected %p.\n", + hook_exception_address, code_mem); + todo_wine ok(hook_KiUserExceptionDispatcher_eip == code_mem, "Got unexpected exception address %p, expected %p.\n", + hook_KiUserExceptionDispatcher_eip, code_mem); + ok(dbg_except_continue_handler_eip == code_mem, "Got unexpected exception address %p, expected %p.\n", + dbg_except_continue_handler_eip, code_mem); + + ret = VirtualProtect(pKiUserExceptionDispatcher, sizeof(saved_KiUserExceptionDispatcher_bytes), + old_protect2, &old_protect2); + ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); + ret = VirtualProtect(hook_trampoline, ARRAY_SIZE(hook_trampoline), old_protect1, &old_protect1); + ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); +} + #elif defined(__x86_64__) #define is_wow64 0 @@ -3751,7 +3859,6 @@ START_TEST(exception) test_suspend_thread(); test_suspend_process(); test_unload_trace(); - test_kiuserexceptiondispatcher(); if (pRtlAddFunctionTable && pRtlDeleteFunctionTable && pRtlInstallFunctionTableCallback && pRtlLookupFunctionEntry) test_dynamic_unwind(); @@ -3766,5 +3873,7 @@ START_TEST(exception) #endif + test_kiuserexceptiondispatcher(); + VirtualFree(code_mem, 0, MEM_RELEASE); } diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index 730d0b1cb32..46fc1df43f6 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -428,7 +428,6 @@ static inline int set_thread_area( struct modify_ldt_s *ptr ) /* stack layout when calling an exception raise function */ struct stack_layout { - void *ret_addr; /* return address from raise_generic_exception */ EXCEPTION_RECORD *rec_ptr; /* first arg for raise_generic_exception */ CONTEXT *context_ptr; /* second arg for raise_generic_exception */ CONTEXT context; @@ -1583,16 +1582,20 @@ static void setup_raise_exception( ucontext_t *sigcontext, struct stack_layout * FS_sig(sigcontext) = get_fs(); GS_sig(sigcontext) = get_gs(); SS_sig(sigcontext) = get_ds(); - stack->ret_addr = (void *)0xdeadbabe; /* KiUserExceptionDispatcher must not return */ stack->rec_ptr = &stack->rec; /* arguments for KiUserExceptionDispatcher */ stack->context_ptr = &stack->context; } -void WINAPI call_user_exception_dispatcher( EXCEPTION_RECORD *rec, CONTEXT *context ) +void *get_KiUserExceptionDispatcher_address(void) { - pKiUserExceptionDispatcher( rec, context ); + return pKiUserExceptionDispatcher; } +__ASM_GLOBAL_FUNC( call_user_exception_dispatcher, + "add $4,%esp\n\t" + "call " __ASM_NAME("get_KiUserExceptionDispatcher_address") "\n\t" + "jmp *%eax") + /********************************************************************** * get_fpu_code * -- 2.26.2
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- v2: - no changes. dlls/ntdll/signal_x86_64.c | 11 ++++ dlls/ntdll/tests/exception.c | 114 ++++++++++++++++++++++++++++++++--- 2 files changed, 118 insertions(+), 7 deletions(-) diff --git a/dlls/ntdll/signal_x86_64.c b/dlls/ntdll/signal_x86_64.c index b575d638571..c5a3d766b4a 100644 --- a/dlls/ntdll/signal_x86_64.c +++ b/dlls/ntdll/signal_x86_64.c @@ -1331,7 +1331,18 @@ __ASM_GLOBAL_FUNC( RtlRaiseException, "movq %rax,0xf8(%rdx)\n\t" /* context->Rip */ "movq %rax,0x10(%rcx)\n\t" /* rec->ExceptionAddress */ "movl $1,%r8d\n\t" + ".byte 0x65\n\tmovq (0x30),%rax\n\t" /* Teb */ + "movq 0x60(%rax),%rax\n\t" /* Peb */ + "movb 0x02(%rax),%al\n\t" /* BeingDebugged */ + "testb %al,%al\n\t" + "jnz call_nt_raise_exception\n\t" + "call " __ASM_NAME("dispatch_exception") "\n\t" + "jmp done\n\t" + + "call_nt_raise_exception:\n\t" "call " __ASM_NAME("NtRaiseException") "\n\t" + + "done:" "movq %rax,%rcx\n\t" "call " __ASM_NAME("RtlRaiseStatus") /* does not return */ ); diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c index a27dee21f00..42161f60f7d 100644 --- a/dlls/ntdll/tests/exception.c +++ b/dlls/ntdll/tests/exception.c @@ -1677,6 +1677,19 @@ static DWORD dbg_except_continue_handler(EXCEPTION_RECORD *rec, EXCEPTION_REGIST return ExceptionContinueExecution; } +static LONG WINAPI dbg_except_continue_vectored_handler(struct _EXCEPTION_POINTERS *e) +{ + EXCEPTION_RECORD *rec = e->ExceptionRecord; + CONTEXT *context = e->ContextRecord; + + trace("dbg_except_continue_vectored_handler, code %#x, eip %#x.\n", rec->ExceptionCode, context->Eip); + + got_exception = 1; + ++context->Eip; + + return EXCEPTION_CONTINUE_EXECUTION; +} + /* Use CDECL to leave arguments on stack. */ void CDECL hook_KiUserExceptionDispatcher(EXCEPTION_RECORD *rec, CONTEXT *context) { @@ -1714,8 +1727,10 @@ static void test_kiuserexceptiondispatcher(void) 0x00, 0x00, 0x00, 0x00, /* jmpq *addr */ /* jump to original function. */ }; void *phook_KiUserExceptionDispatcher = hook_KiUserExceptionDispatcher; + BYTE patched_KiUserExceptionDispatcher_bytes[7]; void *phook_trampoline = hook_trampoline; DWORD old_protect1, old_protect2; + EXCEPTION_RECORD record; BYTE *ptr; BOOL ret; @@ -1738,7 +1753,8 @@ static void test_kiuserexceptiondispatcher(void) memcpy(saved_KiUserExceptionDispatcher_bytes, pKiUserExceptionDispatcher, sizeof(saved_KiUserExceptionDispatcher_bytes)); - ptr = (BYTE *)pKiUserExceptionDispatcher; + + ptr = patched_KiUserExceptionDispatcher_bytes; /* mov hook_trampoline, %eax */ *ptr++ = 0xa1; *(void **)ptr = &phook_trampoline; @@ -1747,9 +1763,12 @@ static void test_kiuserexceptiondispatcher(void) *ptr++ = 0xff; *ptr++ = 0xe0; + memcpy(pKiUserExceptionDispatcher, patched_KiUserExceptionDispatcher_bytes, + sizeof(patched_KiUserExceptionDispatcher_bytes)); got_exception = 0; run_exception_test(dbg_except_continue_handler, NULL, except_code, ARRAY_SIZE(except_code), PAGE_EXECUTE_READ); + ok(got_exception, "Handler was not called.\n"); ok(hook_called, "Hook was not called.\n"); @@ -1761,6 +1780,25 @@ static void test_kiuserexceptiondispatcher(void) ok(dbg_except_continue_handler_eip == code_mem, "Got unexpected exception address %p, expected %p.\n", dbg_except_continue_handler_eip, code_mem); + record.ExceptionCode = 0x80000003; + record.ExceptionFlags = 0; + record.ExceptionRecord = NULL; + record.ExceptionAddress = NULL; /* does not matter, copied return address */ + record.NumberParameters = 0; + + AddVectoredExceptionHandler(TRUE, dbg_except_continue_vectored_handler); + + memcpy(pKiUserExceptionDispatcher, patched_KiUserExceptionDispatcher_bytes, + sizeof(patched_KiUserExceptionDispatcher_bytes)); + got_exception = 0; + hook_called = FALSE; + + pRtlRaiseException(&record); + + ok(got_exception, "Handler was not called.\n"); + ok(hook_called, "Hook was not called.\n"); + + RemoveVectoredExceptionHandler(dbg_except_continue_vectored_handler); ret = VirtualProtect(pKiUserExceptionDispatcher, sizeof(saved_KiUserExceptionDispatcher_bytes), old_protect2, &old_protect2); ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); @@ -2777,17 +2815,37 @@ static void *hook_exception_address; static DWORD dbg_except_continue_handler(EXCEPTION_RECORD *rec, EXCEPTION_REGISTRATION_RECORD *frame, CONTEXT *context, EXCEPTION_REGISTRATION_RECORD **dispatcher) { - ok(hook_called, "Hook was not called.\n"); + trace("handler context->Rip %#lx, codemem %p.\n", context->Rip, code_mem); got_exception = 1; dbg_except_continue_handler_rip = (void *)context->Rip; ++context->Rip; + memcpy(pKiUserExceptionDispatcher, saved_KiUserExceptionDispatcher_bytes, + sizeof(saved_KiUserExceptionDispatcher_bytes)); + return ExceptionContinueExecution; } +static LONG WINAPI dbg_except_continue_vectored_handler(struct _EXCEPTION_POINTERS *e) +{ + EXCEPTION_RECORD *rec = e->ExceptionRecord; + CONTEXT *context = e->ContextRecord; + + trace("dbg_except_continue_vectored_handler, code %#x, Rip %#lx.\n", rec->ExceptionCode, context->Rip); + + got_exception = 1; + if (NtCurrentTeb()->Peb->BeingDebugged || !strcmp( winetest_platform, "wine" )) + { + todo_wine_if(!NtCurrentTeb()->Peb->BeingDebugged) + ok(NtCurrentTeb()->Peb->BeingDebugged, "context->Rip misplaced for dbg breakpoint exception.\n"); + ++context->Rip; + } + return EXCEPTION_CONTINUE_EXECUTION; +} + void WINAPI hook_KiUserExceptionDispatcher(EXCEPTION_RECORD *rec, CONTEXT *context) { trace("rec %p, context %p.\n", rec, context); - trace("context->Rip %#lx, context->Rsp %#lx, ContextFlags %#lx.\n", sizeof(*context), + trace("context->Rip %#lx, context->Rsp %#lx, ContextFlags %#lx.\n", context->Rip, context->Rsp, context->ContextFlags); hook_called = TRUE; @@ -2824,8 +2882,11 @@ static void test_kiuserexceptiondispatcher(void) /* offset: 27 bytes */ 0x00, 0x00, 0x00, 0x00, /* jmpq *addr */ /* jump to original function. */ }; + void *phook_KiUserExceptionDispatcher = hook_KiUserExceptionDispatcher; + BYTE patched_KiUserExceptionDispatcher_bytes[12]; DWORD old_protect1, old_protect2; + EXCEPTION_RECORD record; BYTE *ptr; BOOL ret; @@ -2847,11 +2908,13 @@ static void test_kiuserexceptiondispatcher(void) ret = VirtualProtect(hook_trampoline, ARRAY_SIZE(hook_trampoline), PAGE_EXECUTE_READWRITE, &old_protect1); ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); - ret = VirtualProtect(pKiUserExceptionDispatcher, 5, PAGE_EXECUTE_READWRITE, &old_protect2); + ret = VirtualProtect(pKiUserExceptionDispatcher, sizeof(saved_KiUserExceptionDispatcher_bytes), + PAGE_EXECUTE_READWRITE, &old_protect2); ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); - memcpy(saved_KiUserExceptionDispatcher_bytes, pKiUserExceptionDispatcher, sizeof(saved_KiUserExceptionDispatcher_bytes)); - ptr = (BYTE *)pKiUserExceptionDispatcher; + memcpy(saved_KiUserExceptionDispatcher_bytes, pKiUserExceptionDispatcher, + sizeof(saved_KiUserExceptionDispatcher_bytes)); + ptr = (BYTE *)patched_KiUserExceptionDispatcher_bytes; /* mov hook_trampoline, %rax */ *ptr++ = 0x48; *ptr++ = 0xb8; @@ -2861,6 +2924,8 @@ static void test_kiuserexceptiondispatcher(void) *ptr++ = 0xff; *ptr++ = 0xe0; + memcpy(pKiUserExceptionDispatcher, patched_KiUserExceptionDispatcher_bytes, + sizeof(patched_KiUserExceptionDispatcher_bytes)); got_exception = 0; run_exception_test(dbg_except_continue_handler, NULL, except_code, ARRAY_SIZE(except_code), PAGE_EXECUTE_READ); ok(got_exception, "Handler was not called.\n"); @@ -2874,7 +2939,42 @@ static void test_kiuserexceptiondispatcher(void) ok(dbg_except_continue_handler_rip == code_mem, "Got unexpected exception address %p, expected %p.\n", dbg_except_continue_handler_rip, code_mem); - ret = VirtualProtect(pKiUserExceptionDispatcher, 5, old_protect2, &old_protect2); + memset(&record, 0, sizeof(record)); + record.ExceptionCode = 0x80000003; + record.ExceptionFlags = 0; + record.ExceptionRecord = NULL; + record.ExceptionAddress = NULL; + record.NumberParameters = 0; + + AddVectoredExceptionHandler(TRUE, dbg_except_continue_vectored_handler); + + memcpy(pKiUserExceptionDispatcher, patched_KiUserExceptionDispatcher_bytes, + sizeof(patched_KiUserExceptionDispatcher_bytes)); + got_exception = 0; + hook_called = FALSE; + + pRtlRaiseException(&record); + + ok(got_exception, "Handler was not called.\n"); + ok(!hook_called, "Hook was called.\n"); + + memcpy(pKiUserExceptionDispatcher, patched_KiUserExceptionDispatcher_bytes, + sizeof(patched_KiUserExceptionDispatcher_bytes)); + got_exception = 0; + hook_called = FALSE; + NtCurrentTeb()->Peb->BeingDebugged = 1; + + pRtlRaiseException(&record); + + ok(got_exception, "Handler was not called.\n"); + ok(hook_called, "Hook was not called.\n"); + + NtCurrentTeb()->Peb->BeingDebugged = 0; + + RemoveVectoredExceptionHandler(dbg_except_continue_vectored_handler); + + ret = VirtualProtect(pKiUserExceptionDispatcher, sizeof(saved_KiUserExceptionDispatcher_bytes), + old_protect2, &old_protect2); ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); ret = VirtualProtect(hook_trampoline, ARRAY_SIZE(hook_trampoline), old_protect1, &old_protect1); ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); -- 2.26.2
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=74669 Your paranoid android. === debiant (64 bit WoW report) === ntdll: om.c:2118: Test failed: got 89
Paul Gofman <pgofman(a)codeweavers.com> writes:
-void WINAPI call_user_exception_dispatcher( EXCEPTION_RECORD *rec, CONTEXT *context ) +void *get_KiUserExceptionDispatcher_address(void) { - pKiUserExceptionDispatcher( rec, context ); + return pKiUserExceptionDispatcher; }
+__ASM_GLOBAL_FUNC( call_user_exception_dispatcher, + "add $4,%esp\n\t" + "call " __ASM_NAME("get_KiUserExceptionDispatcher_address") "\n\t" + "jmp *%eax")
That's not very nice. I'd suggest to pass the dispatcher address to call_user_exception_dispatcher(). -- Alexandre Julliard julliard(a)winehq.org
participants (3)
-
Alexandre Julliard -
Marvin -
Paul Gofman