Handling stack overflow

[Dāvis Mosāns]

Hi,

I've been looking to get League of Legends work in Wine.
The issue is that their Anti-Cheat hooks bunch of functions in
ntdll.dll directly by prepending `jmp their_code` before the real
function and it seems it messes something up.

Anyway, topic of this email is about handling of stack overflow.
When their exception handler gets messed up it is invoked recursively
until stack overflow.
And because it happened while it was in critical section it causes
other threads to deadlock.

My question is how such case should be handled?
When thread is killed should lock be released so other threads can
still proceed?
Or maybe just whole process should be killed if any thread has stack overflow?

To me it looks like in Windows when exception handler goes too deep
then Just in Time debugger is invoked or if that's not enabled then
process is killed with EXCEPTION_ACCESS_VIOLATION (maybe because it
tried to access beyond end of stack?)

Log looks like:

[...]
0120:trace:seh:call_handler calling handler 00006FFFFFC97030
(rec=00007F3214C54360, frame=00007F3214D4E710
context=00007F3214C538D0, dispatch=00007F3214C537A0)
0120:trace:seh:call_handler handler at 00006FFFFFC97030 returned 2
0120:trace:seh:call_stack_handlers nested exception
0120:trace:seh:call_handler calling handler 00000001413B347C
(rec=00007F3214C54360, frame=00007F3214D4F570
context=00007F3214C538D0, dispatch=00007F3214C537A0)
0120:err:virtual:virtual_setup_exception stack overflow 2576 bytes
addr 0x6ffff9e66327 stack 0x7f3214c505f0
(0x7f3214c50000-0x7f3214c51000-0x7f3214d50000)
0124:err:sync:RtlpWaitForCriticalSection section 00006FFFFFCAE4C0
"../wine/dlls/ntdll/loader.c: loader_section" wait timed out in thread
0124, blocked by 0120, retrying (60 sec)
011c:err:sync:RtlpWaitForCriticalSection section 00006FFFFFCAE4C0
"../wine/dlls/ntdll/loader.c: loader_section" wait timed out in thread
011c, blocked by 0120, retrying (60 sec)
[...]
// stuck like this

And several threads stuck like this

Thread 8 (Thread 472 "011c"):
#0  0x00006fffffc966d4 in NtWaitForAlertByThreadId () from
/opt/wine-lol-staging/lib/wine/x86_64-windows/ntdll.dll
#1  0x00006fffffc9db68 in RtlWaitOnAddress
(addr=addr@entry=0x6fffffcae4d8 <loader_section+24>,
cmp=cmp@entry=0x6fffffcbda90 <zero>, size=size@entry=4,
timeout=timeout@entry=0x7fc37b38f700) at ../wine/dlls/ntdll/sync.c:912
#2  0x00006fffffc9dd06 in wait_semaphore (timeout=<optimized out>,
crit=0x6fffffcae4c0 <loader_section>) at ../wine/dlls/ntdll/sync.c:196
#3  RtlpWaitForCriticalSection (crit=crit@entry=0x6fffffcae4c0
<loader_section>) at ../wine/dlls/ntdll/sync.c:314
#4  0x00006fffffc9df71 in RtlEnterCriticalSection
(crit=crit@entry=0x6fffffcae4c0 <loader_section>) at
../wine/dlls/ntdll/sync.c:383
#5  0x00006fffffc7667b in loader_init
(context=context@entry=0x7fc37b38fb00,
entry=entry@entry=0x7fc37b38fb80) at ../wine/dlls/ntdll/loader.c:4405
#6  0x00006fffffc9a444 in LdrInitializeThunk (context=0x7fc37b38fb00,
unk2=<optimized out>, unk3=<optimized out>, unk4=<optimized out>) at
../wine/dlls/ntdll/signal_x86_64.c:1716
#7  0x00006ffff88f7954 in ?? () from /mnt/Riot Games/League of
Legends/wine/dosdevices/s:/Riot Games/League of Legends/Game/stub.dll

Best regards,
Dāvis

[Stefan Dösinger]

Am Montag, 27. November 2023, 06:27:51 EAT schrieb Dāvis Mosāns:
> Hi,
>
> I've been looking to get League of Legends work in Wine.
> The issue is that their Anti-Cheat hooks bunch of functions in
> ntdll.dll directly by prepending `jmp their_code` before the real
> function and it seems it messes something up.
>
> Anyway, topic of this email is about handling of stack overflow.
> When their exception handler gets messed up it is invoked recursively
> until stack overflow.
> And because it happened while it was in critical section it causes
> other threads to deadlock.

Since this is anti-cheat it is tricky, and things might not be what they seem.

When you say "it messes something up", are you sure the hook is working 
correctly to begin with? If not, that needs to be fixed rather than handling 
the fallout exception.

Do you know if the recursive invocation happens on Windows too? You mention 
that the process is killed, so it doesn't sound like it is supposed to happen.

Anti-cheat / Anti-debugger / DRM systems usually don't tell you nicely if they 
think something is wrong. When they think you are trying to mess with them 
they usually pretend to proceed for a while, do something else and then at a 
much later point deliberately crash the process in a weird way. So your 
exception handler recursion might be the Anti-cheat's underhanded attempt to 
kill the process.

[Erich E. Hoover]

On Thu, Nov 30, 2023 at 2:46 AM Stefan Dösinger
 wrote:
> ...
> Anti-cheat / Anti-debugger / DRM systems usually don't tell you nicely if they
> think something is wrong. When they think you are trying to mess with them
> they usually pretend to proceed for a while, do something else and then at a
> much later point deliberately crash the process in a weird way. So your
> exception handler recursion might be the Anti-cheat's underhanded attempt to
> kill the process.

I would just like to second that, when I was getting Silverlight
working I ran down a number of rabbit holes trying to "fix" things
that the DRM was doing wrong on purpose.  It's my suspicion that they
tested it in some different VMs and found various "failing to fail"
ways to detect the presence of the virtualizer.

Best,
Erich

[Dāvis Mosāns]

> [...]
>
> Since this is anti-cheat it is tricky, and things might not be what they seem.
>
> When you say "it messes something up", are you sure the hook is working
> correctly to begin with? If not, that needs to be fixed rather than handling
> the fallout exception.

I don't know if it's working correctly, but that's a separate issue,
I'm looking into
both things.
> Do you know if the recursive invocation happens on Windows too? You mention
> that the process is killed, so it doesn't sound like it is supposed to happen.

Yeah it's definitely not supposed to happen but Wine still could
handle it better
(hence this email).
Here is small example that will deadlock Wine in exactly same way
https://gist.github.com/davispuh/6c0ae7e5a10500a5b7a759b817e66214
In Windows such program will be killed and you'll get dialog with
button to attach
debugger.
> Anti-cheat / Anti-debugger / DRM systems usually don't tell you nicely if they
> think something is wrong. When they think you are trying to mess with them
> they usually pretend to proceed for a while, do something else and then at a
> much later point deliberately crash the process in a weird way. So your
> exception handler recursion might be the Anti-cheat's underhanded attempt to
> kill the process.

True, but in this case it wasn't intended way, fixing LoadLibraryEx
non-NULL hFile
prevented stack overflow and deadlock
https://gitlab.winehq.org/wine/wine/-/merge_requests/4587
^ But I don't know if that was anti-debug/reversing trick or just
working around some
bigger issue.
Game still doesn't work but now behaviour is way better because it
doesn't deadlock
and 100% of time presents game's internal crash dialog (it's used for
all purposes,
including if debugger is detected).

[Dāvis Mosāns]

pirmd., 2023. g. 4. dec., plkst. 03:24 — lietotājs Dāvis Mosāns
() rakstīja:
> [...]
> Here is small example that will deadlock Wine in exactly same way
> https://gist.github.com/davispuh/6c0ae7e5a10500a5b7a759b817e66214
> In Windows such program will be killed and you'll get dialog with
> button to attach
> debugger.

So any thoughts how this could be fixed?
It seems that in virtual_setup_exception() we need better
abort_thread() that would be able to kill process that has some
deadlocked threads.

void abort_thread( int status )
{
   pthread_sigmask( SIG_BLOCK, &server_block_set, NULL );
   if (InterlockedDecrement( &nb_threads ) <= 0) abort_process( status );
   pthread_exit_wrapper( status );
}