[PATCH] ntdll: Only call wine exception handlers on the current stack.

[Paul Gofman]

On 2/1/22 14:54, Bernhard Übelacker wrote:
> Am 31.01.22 um 16:24 schrieb Rémi Bernon:
>> MK11 creates an alternate stack and sometimes throws an exception which
>> gets incorrectly handled by a Wine exception handler, causing the game
>> to crash.
>>
>> Signed-off-by: Rémi Bernon <rbernon at codeweavers.com>
>> ---
>>   dlls/ntdll/signal_x86_64.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/dlls/ntdll/signal_x86_64.c b/dlls/ntdll/signal_x86_64.c
>> index 7e77329363c..36985832e4a 100644
>> --- a/dlls/ntdll/signal_x86_64.c
>> +++ b/dlls/ntdll/signal_x86_64.c
>> @@ -463,7 +463,9 @@ static NTSTATUS call_stack_handlers( 
>> EXCEPTION_RECORD *rec, CONTEXT *orig_contex
>>               }
>>           }
>>           /* hack: call wine handlers registered in the tib list */
>> -        else while ((ULONG64)teb_frame < context.Rsp)
>> +        else while ((ULONG64)teb_frame < context.Rsp &&
>> +                    (ULONG64)teb_frame >= 
>> (ULONG64)NtCurrentTeb()->Tib.StackLimit &&
>> +                    (ULONG64)teb_frame <= 
>> (ULONG64)NtCurrentTeb()->Tib.StackBase)
>>           {
>>               TRACE_(seh)( "found wine frame %p rsp %p handler %p\n",
>>                            teb_frame, (void *)context.Rsp, 
>> teb_frame->Handler );
>
> I am not sure but this seems kind of similar to what I think I found in
> this bug: https://bugs.winehq.org/show_bug.cgi?id=52159
>
Yes, that looks quite similar. All these fix attempts workaround the 
problem where it is first visibly hit (calling wrong handler) but ignore 
what may happen to these handlers after. What if the app, e. g., 
switches stack back? And RtlUnwindEx()?

Actually the majority of Wine __TRY blocks should have rather low 
chances to hit app's stack switch in between the handler is installed 
and removed (unless the app is doing something really fun with 
hotpatching). Those IsBad..Ptr(), a bunch of cases when the parameter 
access is wrapped into __TRY, OutputDebugString() are not much likely to 
hit such an issue. There are relatively few __TRY blocks which embrace 
application functions: calling thread func, calling DLL entry points, 
exception handlers calls in signal_x86_64.c (and maybe I missed some). 
Since you probably hit the issue with Proton that is not the nested 
exception handlers as those in Proton already have manual asm wrappers 
to use straightforward x64 .seh handling and don't install Teb handlers. 
If maybe there are just one or two of other "global" handlers which are 
problematic maybe it worth solving the issue with some sort of manual 
.seh specifically for those ones?