[PATCH 0/3] MR4290: ntdll: A bit of KiUserExceptionDispatcher redesign.
An anticheat assumes some specific layout of KiUserExceptionDispatcher (in an 64 bit app). After checking two bytes (obviously instruction opcode) at (KiUserExceptionDispatcher + 1), it extracts memory address from this instruction and places its hook function address at that address, expecting that to be called on CPU exceptions (but not software raised exceptions). I tried to make something out of it so supporting this mechanics is not completely artificial and fits into our implementation. I don't know if that hook has really something to do with wow64 in Windows implementation (while I guess that's possible), but it seems to me in our implementation doing it this way is a bit more straightforward. Specifically, it seems to me a bit easier to follow when a single function (KiUserExceptionDispatcher) doesn't mix different unwind models (while having the same runtime function info). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4290
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/signal_x86_64.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/dlls/ntdll/signal_x86_64.c b/dlls/ntdll/signal_x86_64.c index ae658910868..5ce30461457 100644 --- a/dlls/ntdll/signal_x86_64.c +++ b/dlls/ntdll/signal_x86_64.c @@ -614,20 +614,25 @@ NTSTATUS WINAPI dispatch_wow_exception( EXCEPTION_RECORD *rec_ptr, CONTEXT *cont } +__ASM_GLOBAL_FUNC( dispatch_wow_exception_thunk, + "movw %cs,%ax\n\t" + "cmpw %ax,0x38(%rdx)\n\t" /* context->SegCs */ + "je 1f\n\t" + "movq %r14,%rsp\n\t" /* switch to 64-bit stack */ + "call " __ASM_NAME("dispatch_wow_exception") "\n\t" + "int3\n\t" + "1:\tret") + + /******************************************************************* * KiUserExceptionDispatcher (NTDLL.@) */ __ASM_GLOBAL_FUNC( KiUserExceptionDispatcher, - "mov 0x98(%rsp),%rcx\n\t" /* context->Rsp */ - "movw %cs,%ax\n\t" - "cmpw %ax,0x38(%rsp)\n\t" /* context->SegCs */ - "je 1f\n\t" "mov %rsp,%rdx\n\t" /* context */ "lea 0x4f0(%rsp),%rcx\n\t" /* rec */ - "movq %r14,%rsp\n\t" /* switch to 64-bit stack */ - "call " __ASM_NAME("dispatch_wow_exception") "\n\t" - "int3\n" - "1:\tmov 0xf8(%rsp),%rdx\n\t" /* context->Rip */ + "call " __ASM_NAME("dispatch_wow_exception_thunk") "\n\t" + "mov 0x98(%rsp),%rcx\n\t" /* context->Rsp */ + "mov 0xf8(%rsp),%rdx\n\t" /* context->Rip */ "mov %rdx,-0x8(%rcx)\n\t" "mov %rbp,-0x10(%rcx)\n\t" "mov %rdi,-0x18(%rcx)\n\t" -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4290
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/signal_x86_64.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/dlls/ntdll/signal_x86_64.c b/dlls/ntdll/signal_x86_64.c index 5ce30461457..ef9bfdd99c5 100644 --- a/dlls/ntdll/signal_x86_64.c +++ b/dlls/ntdll/signal_x86_64.c @@ -628,9 +628,6 @@ __ASM_GLOBAL_FUNC( dispatch_wow_exception_thunk, * KiUserExceptionDispatcher (NTDLL.@) */ __ASM_GLOBAL_FUNC( KiUserExceptionDispatcher, - "mov %rsp,%rdx\n\t" /* context */ - "lea 0x4f0(%rsp),%rcx\n\t" /* rec */ - "call " __ASM_NAME("dispatch_wow_exception_thunk") "\n\t" "mov 0x98(%rsp),%rcx\n\t" /* context->Rsp */ "mov 0xf8(%rsp),%rdx\n\t" /* context->Rip */ "mov %rdx,-0x8(%rcx)\n\t" @@ -638,8 +635,6 @@ __ASM_GLOBAL_FUNC( KiUserExceptionDispatcher, "mov %rdi,-0x18(%rcx)\n\t" "mov %rsi,-0x20(%rcx)\n\t" "lea -0x20(%rcx),%rbp\n\t" - "mov %rsp,%rdx\n\t" /* context */ - "lea 0x4f0(%rsp),%rcx\n\t" /* rec */ __ASM_SEH(".seh_pushreg %rbp\n\t") __ASM_SEH(".seh_pushreg %rdi\n\t") __ASM_SEH(".seh_pushreg %rsi\n\t") @@ -653,6 +648,12 @@ __ASM_GLOBAL_FUNC( KiUserExceptionDispatcher, __ASM_CFI(".cfi_rel_offset %rbp,0x10\n\t") __ASM_CFI(".cfi_rel_offset %rdi,0x8\n\t") __ASM_CFI(".cfi_rel_offset %rsi,0\n\t") + + "mov %rsp,%rdx\n\t" /* context */ + "lea 0x4f0(%rsp),%rcx\n\t" /* rec */ + "call " __ASM_NAME("dispatch_wow_exception_thunk") "\n\t" + "mov %rsp,%rdx\n\t" /* context */ + "lea 0x4f0(%rsp),%rcx\n\t" /* rec */ "call " __ASM_NAME("dispatch_exception") "\n\t" "int3") -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4290
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/signal_x86_64.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/dlls/ntdll/signal_x86_64.c b/dlls/ntdll/signal_x86_64.c index ef9bfdd99c5..d14b053a148 100644 --- a/dlls/ntdll/signal_x86_64.c +++ b/dlls/ntdll/signal_x86_64.c @@ -37,6 +37,8 @@ WINE_DECLARE_DEBUG_CHANNEL(seh); WINE_DECLARE_DEBUG_CHANNEL(relay); WINE_DECLARE_DEBUG_CHANNEL(threadname); +void (WINAPI * p_dispatch_wow_exception_thunk)( EXCEPTION_RECORD *, CONTEXT * ); + typedef struct _SCOPE_TABLE { ULONG Count; @@ -614,6 +616,7 @@ NTSTATUS WINAPI dispatch_wow_exception( EXCEPTION_RECORD *rec_ptr, CONTEXT *cont } +void WINAPI dispatch_wow_exception_thunk( EXCEPTION_RECORD *, CONTEXT * ); __ASM_GLOBAL_FUNC( dispatch_wow_exception_thunk, "movw %cs,%ax\n\t" "cmpw %ax,0x38(%rdx)\n\t" /* context->SegCs */ @@ -628,6 +631,10 @@ __ASM_GLOBAL_FUNC( dispatch_wow_exception_thunk, * KiUserExceptionDispatcher (NTDLL.@) */ __ASM_GLOBAL_FUNC( KiUserExceptionDispatcher, + "nop\n\t" + /* Some anticheats rely on this opcode at exact offset to hook KiUserExceptionDispatcher + * in 64 bit app. */ + "movq " __ASM_NAME("p_dispatch_wow_exception_thunk") "(%rip),%rax\n\t" "mov 0x98(%rsp),%rcx\n\t" /* context->Rsp */ "mov 0xf8(%rsp),%rdx\n\t" /* context->Rip */ "mov %rdx,-0x8(%rcx)\n\t" @@ -649,10 +656,12 @@ __ASM_GLOBAL_FUNC( KiUserExceptionDispatcher, __ASM_CFI(".cfi_rel_offset %rdi,0x8\n\t") __ASM_CFI(".cfi_rel_offset %rsi,0\n\t") + "test %rax,%rax\n\t" + "jz 1f\n\t" "mov %rsp,%rdx\n\t" /* context */ "lea 0x4f0(%rsp),%rcx\n\t" /* rec */ - "call " __ASM_NAME("dispatch_wow_exception_thunk") "\n\t" - "mov %rsp,%rdx\n\t" /* context */ + "call *%rax\n\t" + "1:\tmov %rsp,%rdx\n\t" /* context */ "lea 0x4f0(%rsp),%rcx\n\t" /* rec */ "call " __ASM_NAME("dispatch_exception") "\n\t" "int3") @@ -1712,6 +1721,10 @@ __ASM_GLOBAL_FUNC( signal_start_thread, */ void WINAPI LdrInitializeThunk( CONTEXT *context, ULONG_PTR unk2, ULONG_PTR unk3, ULONG_PTR unk4 ) { + if (NtCurrentTeb()->WowTebOffset) + InterlockedCompareExchangePointer( (void * volatile *)&p_dispatch_wow_exception_thunk, + dispatch_wow_exception_thunk, NULL ); + loader_init( context, (void **)&context->Rcx ); TRACE_(relay)( "\1Starting thread proc %p (arg=%p)\n", (void *)context->Rcx, (void *)context->Rdx ); signal_start_thread( context ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4290
Zebediah Figura (@zfigura) commented about dlls/ntdll/signal_x86_64.c:
*/ void WINAPI LdrInitializeThunk( CONTEXT *context, ULONG_PTR unk2, ULONG_PTR unk3, ULONG_PTR unk4 ) { + if (NtCurrentTeb()->WowTebOffset) + InterlockedCompareExchangePointer( (void * volatile *)&p_dispatch_wow_exception_thunk, + dispatch_wow_exception_thunk, NULL ); Can the global variable not just be initialized?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4290#note_50826
On Fri Nov 3 17:42:25 2023 +0000, Zebediah Figura wrote:
Can the global variable not just be initialized? It is not necessary to call `dispatch_wow_exception_thunk` on normal non-wow64 process, I guess it is better to only set that for wow64?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4290#note_50827
On Fri Nov 3 17:46:49 2023 +0000, Paul Gofman wrote:
It is not necessary to call `dispatch_wow_exception_thunk` on normal non-wow64 process, I guess it is better to only set that for wow64? Also I did a bit of local testing replicating the hooking which AC is doing, to test a bit of essential behaviour (I was mostly interested whether unwind works from this hook on pure 64 bit process, which it does, as well as testing that). Setting this variable to NULL doesn't break exception dispatcher, so we need to have a check in KiUserExceptionDispatcher anyway.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4290#note_50828
On Fri Nov 3 17:55:38 2023 +0000, Paul Gofman wrote:
Also I did a bit of local testing replicating the hooking which AC is doing, to test a bit of essential behaviour (I was mostly interested whether unwind works from this hook on pure 64 bit process, which it does, as well as testing that). Setting this variable to NULL doesn't break exception dispatcher, so we need to have a check in KiUserExceptionDispatcher anyway. UPDATE: Other essential bits are that the return value from the "hook" doesn't seem to matter and then normal exception handlers are called after the hook (unless the hook unwinds from it of course). Oh I'm sorry, I cannot at all read.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4290#note_50831
On Fri Nov 3 18:07:32 2023 +0000, Zebediah Figura wrote:
Oh I'm sorry, I cannot at all read. Why, it is indeed as well possible to always call it, but since I seem to have to keep NULL check in KiUserExceptionDispatcher anyway that doesn't seem too much of simplification, while more straightforward that it is not set on pure 64 where it is not needed.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4290#note_50834
I wonder if we are working on same Anti-Cheat :sweat_smile: I've been looking into getting League of Legends work and it's Anti-Cheat `packman` from Riot Game also patches `RtlAddVectoredExceptionHandler`/`RtlpAddVectoredHandler` Without this MR their exception handler causes recursion and dies with stack overflow causing other threads to deadlock. After applying this MR I'm not able to get that stack overflow anymore. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4290#note_54185
No, I wasn't looking at League of Legends. But maybe it is using similar way of hooking KiUserExceptionDispatcher. Without reverse engineering the AC it is possible to see if there is NtProtectVirtualMemory for around KiUserExceptionDispatcher address and whether the code of it actually changes during the game run. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4290#note_54186
It is a bit unfortunate to say "an anticheat" without specifying which... if the name can't be divulged, it'd be nice to at least make that clear. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4290#note_54187
On Wed Nov 29 01:27:17 2023 +0000, Paul Gofman wrote:
No, I wasn't looking at League of Legends. But maybe it is using similar way of hooking KiUserExceptionDispatcher. Without reverse engineering the AC it is possible to see if there is NtProtectVirtualMemory for around KiUserExceptionDispatcher address and whether the code of it actually changes during the game run. Oops, I actually wanted to say it does patch `KiUserExceptionDispatcher` but got myself confused :D It patches 18 methods in ntdll.dll
Patch for `KiUserExceptionDispatcher` looks like this (with this MR applied) ```diff - 59750: 90 nop - 59751: 48 8b 05 38 03 04 00 mov rax,QWORD PTR [rip+0x40338] # 0x99a90 + 59750: 40 e9 95 eb cf f8 rex jmp 0xf8d582eb + 59756: 04 00 add al,0x0 59758: 48 8b 8c 24 98 00 00 mov rcx,QWORD PTR [rsp+0x98] 5975f: 00 59760: 48 8b 94 24 f8 00 00 mov rdx,QWORD PTR [rsp+0xf8] 59767: 00 59768: 48 89 51 f8 mov QWORD PTR [rcx-0x8],rdx 5976c: 48 89 69 f0 mov QWORD PTR [rcx-0x10],rbp 59770: 48 89 79 e8 mov QWORD PTR [rcx-0x18],rdi 59774: 48 89 71 e0 mov QWORD PTR [rcx-0x20],rsi 59778: 48 8d 69 e0 lea rbp,[rcx-0x20] 5977c: 48 85 c0 test rax,rax 5977f: 74 0d je 0x5978e ``` Also spoke too soon, stack overflow is not fixed it just seems to have become way more rarer. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4290#note_54189
On Wed Nov 29 01:27:26 2023 +0000, Dāvis Mosāns (davispuh) wrote:
Oops, I actually wanted to say it does patch `KiUserExceptionDispatcher` but got myself confused :D It patches 18 methods in ntdll.dll Patch for `KiUserExceptionDispatcher` looks like this (with this MR applied) ```diff - 59750: 90 nop - 59751: 48 8b 05 38 03 04 00 mov rax,QWORD PTR [rip+0x40338] # 0x99a90 + 59750: 40 e9 95 eb cf f8 rex jmp 0xf8d582eb + 59756: 04 00 add al,0x0 59758: 48 8b 8c 24 98 00 00 mov rcx,QWORD PTR [rsp+0x98] 5975f: 00 59760: 48 8b 94 24 f8 00 00 mov rdx,QWORD PTR [rsp+0xf8] 59767: 00 59768: 48 89 51 f8 mov QWORD PTR [rcx-0x8],rdx 5976c: 48 89 69 f0 mov QWORD PTR [rcx-0x10],rbp 59770: 48 89 79 e8 mov QWORD PTR [rcx-0x18],rdi 59774: 48 89 71 e0 mov QWORD PTR [rcx-0x20],rsi 59778: 48 8d 69 e0 lea rbp,[rcx-0x20] 5977c: 48 85 c0 test rax,rax 5977f: 74 0d je 0x5978e ``` Also spoke too soon, stack overflow is not fixed it just seems to have become way more rarer. Now checked without this MR, it patches it like this:
```diff - 55aac: 48 8b 8c 24 98 00 00 mov rcx,QWORD PTR [rsp+0x98] - 55ab3: 00 + 55aac: 40 e9 39 ea c6 f6 rex jmp 0xf6cc44eb + 55ab2: 00 00 add BYTE PTR [rax],al 55ab4: 66 8c c8 mov ax,cs 55ab7: 66 39 44 24 38 cmp WORD PTR [rsp+0x38],ax 55abc: 74 14 je 0x55ad2 55abe: 48 89 e2 mov rdx,rsp 55ac1: 48 8d 8c 24 f0 04 00 lea rcx,[rsp+0x4f0] 55ac8: 00 55ac9: 4c 89 f4 mov rsp,r14 55acc: e8 bf 27 00 00 call 0x58290 55ad1: cc int3 ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4290#note_54336
This is superseded by 334f54c255d045f0f07b9e1cdf9b5aef8a30b88f, closing. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4290#note_54515
This merge request was closed by Paul Gofman. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4290
participants (4)
-
Dāvis Mosāns -
Paul Gofman
-
Paul Gofman (＠gofman)
-
Zebediah Figura (＠zfigura)