[PATCH 0/3] MR1152: ntdll: Fixes for Overwatch 2.
Overwatch 2 doesn't like the win32u `-syscall` conversion that happened a while back, because it hooks KiUserCallbackDispatcher. This MR fixes it up to match what Overwatch 2 expects. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1152
From: Torge Matthies <tmatthies(a)codeweavers.com> Overwatch hooks KiUserCallbackDispatcher and expects the stack pointer to be aligned to a multiple of 16 bytes, instead of the usual 8-byte misalignment, otherwise it will crash on a misaligned movaps. Fix this by aligning the stack pointer when calling the dispatcher and again inside the dispatcher. Signed-off-by: Torge Matthies <openglfreak(a)googlemail.com> --- dlls/ntdll/signal_x86_64.c | 28 ++++++++++++++++++++++------ dlls/ntdll/unix/signal_x86_64.c | 2 +- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/dlls/ntdll/signal_x86_64.c b/dlls/ntdll/signal_x86_64.c index b1ab4933b93..5af82a6a09f 100644 --- a/dlls/ntdll/signal_x86_64.c +++ b/dlls/ntdll/signal_x86_64.c @@ -654,12 +654,7 @@ __ASM_GLOBAL_FUNC( KiUserApcDispatcher, "int3") -/******************************************************************* - * KiUserCallbackDispatcher (NTDLL.@) - * - * FIXME: not binary compatible - */ -void WINAPI KiUserCallbackDispatcher( ULONG id, void *args, ULONG len ) +void WINAPI user_callback_dispatcher( ULONG id, void *args, ULONG len ) { NTSTATUS status; @@ -678,6 +673,27 @@ void WINAPI KiUserCallbackDispatcher( ULONG id, void *args, ULONG len ) RtlRaiseStatus( status ); } +/******************************************************************* + * KiUserCallbackDispatcher (NTDLL.@) + * + * FIXME: not binary compatible + */ +#ifdef __x86_64__ +__ASM_GLOBAL_FUNC( KiUserCallbackDispatcher, + "movq %rsp,%rbp\n\t" + __ASM_SEH(".seh_setframe %rbp, 0\n\t") + __ASM_CFI(".cfi_def_cfa rbp, 8\n\t") + "andq $0xFFFFFFFFFFFFFFF0, %rsp\n\t" + __ASM_SEH(".seh_endprologue\n\t") + "call " __ASM_NAME("user_callback_dispatcher") "\n\t" + "int3") +#else +void WINAPI DECLSPEC_HOTPATCH KiUserCallbackDispatcher( ULONG id, void *args, ULONG len ) +{ + return user_callback_dispatcher( id, args, len ); +} +#endif + static ULONG64 get_int_reg( CONTEXT *context, int reg ) { diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index 5787f1dc6f9..0526db9d762 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -2399,7 +2399,7 @@ NTSTATUS WINAPI KeUserModeCallback( ULONG id, const void *args, ULONG len, void if (!__wine_setjmpex( &callback_frame.jmpbuf, NULL )) { struct syscall_frame *frame = amd64_thread_data()->syscall_frame; - void *args_data = (void *)((frame->rsp - len) & ~15); + void *args_data = (void *)(((frame->rsp - len) & ~15) - 8); memcpy( args_data, args, len ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1152
From: Torge Matthies <tmatthies(a)codeweavers.com> Overwatch 2 hooks KiUserCallbackDispatcher by overwriting the first five bytes with a jump, and returning to just after the jump. Make sure there is a five-byte instruction for it to replace. Signed-off-by: Torge Matthies <openglfreak(a)googlemail.com> --- dlls/ntdll/signal_x86_64.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/ntdll/signal_x86_64.c b/dlls/ntdll/signal_x86_64.c index 5af82a6a09f..7757b7b61bd 100644 --- a/dlls/ntdll/signal_x86_64.c +++ b/dlls/ntdll/signal_x86_64.c @@ -680,6 +680,7 @@ void WINAPI user_callback_dispatcher( ULONG id, void *args, ULONG len ) */ #ifdef __x86_64__ __ASM_GLOBAL_FUNC( KiUserCallbackDispatcher, + ".byte 0x0f, 0x1f, 0x44, 0x00, 0x00\n\t" /* Overwatch 2 replaces the first 5 bytes with a jump */ "movq %rsp,%rbp\n\t" __ASM_SEH(".seh_setframe %rbp, 0\n\t") __ASM_CFI(".cfi_def_cfa rbp, 8\n\t") -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1152
From: Torge Matthies <tmatthies(a)codeweavers.com> In addition to in registers. Overwatch 2 hooks KiUserCallbackDispatcher and expects to be able to use all the caller-saved registers, but also expects the callback id to be in ecx. Signed-off-by: Torge Matthies <openglfreak(a)googlemail.com> --- dlls/ntdll/signal_x86_64.c | 3 +++ dlls/ntdll/unix/signal_x86_64.c | 10 +++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/dlls/ntdll/signal_x86_64.c b/dlls/ntdll/signal_x86_64.c index 7757b7b61bd..2bebd9cd934 100644 --- a/dlls/ntdll/signal_x86_64.c +++ b/dlls/ntdll/signal_x86_64.c @@ -686,6 +686,9 @@ __ASM_GLOBAL_FUNC( KiUserCallbackDispatcher, __ASM_CFI(".cfi_def_cfa rbp, 8\n\t") "andq $0xFFFFFFFFFFFFFFF0, %rsp\n\t" __ASM_SEH(".seh_endprologue\n\t") + "movq 0x28(%rbp), %rdx\n\t" + "movl 0x30(%rbp), %ecx\n\t" + "movl 0x34(%rbp), %r8d\n\t" "call " __ASM_NAME("user_callback_dispatcher") "\n\t" "int3") #else diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index 0526db9d762..019d1ecefa2 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -2400,8 +2400,16 @@ NTSTATUS WINAPI KeUserModeCallback( ULONG id, const void *args, ULONG len, void { struct syscall_frame *frame = amd64_thread_data()->syscall_frame; void *args_data = (void *)(((frame->rsp - len) & ~15) - 8); + struct { + void *args; + ULONG id; + ULONG len; + } *params = (void *)((ULONG_PTR)args_data - 0x10); memcpy( args_data, args, len ); + params->args = args_data; + params->id = id; + params->len = len; callback_frame.frame.rcx = id; callback_frame.frame.rdx = (ULONG_PTR)args; @@ -2410,7 +2418,7 @@ NTSTATUS WINAPI KeUserModeCallback( ULONG id, const void *args, ULONG len, void callback_frame.frame.fs = amd64_thread_data()->fs; callback_frame.frame.gs = ds64_sel; callback_frame.frame.ss = ds64_sel; - callback_frame.frame.rsp = (ULONG_PTR)args_data - 0x28; + callback_frame.frame.rsp = (ULONG_PTR)params - 0x28; callback_frame.frame.rip = (ULONG_PTR)pKiUserCallbackDispatcher; callback_frame.frame.eflags = 0x200; callback_frame.frame.restore_flags = CONTEXT_CONTROL | CONTEXT_INTEGER; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1152
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=125362 Your paranoid android. === debian11 (build log) === Task: Could not create the win32 wineprefix: Failed to disable the crash dialogs: Task: WineTest did not produce the win32 report
I wonder if we could unwind through KiUserCallbackDispatcher, if we put the previous rip on the stack. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1152#note_11915
participants (3)
-
Marvin -
Torge Matthies -
Torge Matthies (＠tmatthies)