[PATCH v5 0/1] MR2126: ntdll: Print an error if we receive a signal not on the signal stack.
Without this patch the process will silently die. I managed to trigger this while trying to trace Child of Light with Renderdoc, the latter of which crashed in its own "TargetControlServerThread". This, confusingly, manifested in the game restarting itself without the Ubisoft overlay; apparently the game or one of its launchers was capable of recognizing when the process had died and restarting it, but would not try to inject the overlay a second time. I have not investigated the cause of the crash; it is not unlikely that it resulted from the overlay injection (despite the fact that that should only directly affect PE code.) -- v5: ntdll: Print an error if we receive a signal not on the signal stack. https://gitlab.winehq.org/wine/wine/-/merge_requests/2126
From: Zebediah Figura <zfigura(a)codeweavers.com> Without this patch the process will silently die. I managed to trigger this while trying to trace Child of Light with Renderdoc, the latter of which crashed in its own "TargetControlServerThread". This, confusingly, manifested in the game restarting itself without the Ubisoft overlay; apparently the game or one of its launchers was capable of recognizing when the process had died and restarting it, but would not try to inject the overlay a second time. I have not investigated the cause of the crash; it is not unlikely that it resulted from the overlay injection (despite the fact that that should only directly affect PE code.) --- dlls/ntdll/unix/signal_arm.c | 19 +++++++++++++++++++ dlls/ntdll/unix/signal_arm64.c | 25 ++++++++++++++++++++++++- dlls/ntdll/unix/signal_i386.c | 5 ++++- dlls/ntdll/unix/signal_x86_64.c | 2 ++ dlls/ntdll/unix/thread.c | 28 ++++++++++++++++++++++++++++ dlls/ntdll/unix/unix_private.h | 1 + 6 files changed, 78 insertions(+), 2 deletions(-) diff --git a/dlls/ntdll/unix/signal_arm.c b/dlls/ntdll/unix/signal_arm.c index 681c2e3c0c1..f82d8e4d182 100644 --- a/dlls/ntdll/unix/signal_arm.c +++ b/dlls/ntdll/unix/signal_arm.c @@ -750,6 +750,15 @@ NTSTATUS unwind_builtin_dll( void *args ) } +/*********************************************************************** + * init_handler + */ +static inline void init_handler( const ucontext_t *sigcontext ) +{ + check_signal_stack(); +} + + /*********************************************************************** * get_trap_code * @@ -833,6 +842,8 @@ static void save_context( CONTEXT *context, const ucontext_t *sigcontext ) C(0); C(1); C(2); C(3); C(4); C(5); C(6); C(7); C(8); C(9); C(10); #undef C + init_handler( sigcontext ); + context->ContextFlags = CONTEXT_FULL; context->Sp = SP_sig(sigcontext); /* Stack pointer */ context->Lr = LR_sig(sigcontext); /* Link register */ @@ -1302,6 +1313,8 @@ static void segv_handler( int signal, siginfo_t *siginfo, void *sigcontext ) EXCEPTION_RECORD rec = { 0 }; ucontext_t *context = sigcontext; + init_handler( sigcontext ); + switch (get_trap_code(signal, context)) { case TRAP_ARM_PRIVINFLT: /* Invalid opcode exception */ @@ -1445,6 +1458,8 @@ static void int_handler( int signal, siginfo_t *siginfo, void *sigcontext ) { HANDLE handle; + init_handler( sigcontext ); + if (!p__wine_ctrl_routine) return; if (!NtCreateThreadEx( &handle, THREAD_ALL_ACCESS, NULL, NtCurrentProcess(), p__wine_ctrl_routine, 0 /* CTRL_C_EVENT */, 0, 0, 0, 0, NULL )) @@ -1472,6 +1487,8 @@ static void abrt_handler( int signal, siginfo_t *siginfo, void *sigcontext ) */ static void quit_handler( int signal, siginfo_t *siginfo, void *sigcontext ) { + init_handler( sigcontext ); + abort_thread(0); } @@ -1485,6 +1502,8 @@ static void usr1_handler( int signal, siginfo_t *siginfo, void *sigcontext ) { CONTEXT context; + init_handler( sigcontext ); + if (is_inside_syscall( sigcontext )) { context.ContextFlags = CONTEXT_FULL; diff --git a/dlls/ntdll/unix/signal_arm64.c b/dlls/ntdll/unix/signal_arm64.c index 3855afc34f4..16fb912a448 100644 --- a/dlls/ntdll/unix/signal_arm64.c +++ b/dlls/ntdll/unix/signal_arm64.c @@ -479,6 +479,15 @@ NTSTATUS unwind_builtin_dll( void *args ) } +/*********************************************************************** + * init_handler + */ +static inline void init_handler( const ucontext_t *sigcontext ) +{ + check_signal_stack(); +} + + /*********************************************************************** * save_fpu * @@ -534,6 +543,8 @@ static void save_context( CONTEXT *context, const ucontext_t *sigcontext ) { DWORD i; + init_handler( sigcontext ); + context->ContextFlags = CONTEXT_FULL; context->u.s.Fp = FP_sig(sigcontext); /* Frame pointer */ context->u.s.Lr = LR_sig(sigcontext); /* Link register */ @@ -1265,6 +1276,8 @@ static void segv_handler( int signal, siginfo_t *siginfo, void *sigcontext ) EXCEPTION_RECORD rec = { 0 }; ucontext_t *context = sigcontext; + init_handler( sigcontext ); + rec.NumberParameters = 2; rec.ExceptionInformation[0] = (get_fault_esr( context ) & 0x40) != 0; rec.ExceptionInformation[1] = (ULONG_PTR)siginfo->si_addr; @@ -1408,6 +1421,8 @@ static void int_handler( int signal, siginfo_t *siginfo, void *sigcontext ) { HANDLE handle; + init_handler( sigcontext ); + if (!p__wine_ctrl_routine) return; if (!NtCreateThreadEx( &handle, THREAD_ALL_ACCESS, NULL, NtCurrentProcess(), p__wine_ctrl_routine, 0 /* CTRL_C_EVENT */, 0, 0, 0, 0, NULL )) @@ -1435,6 +1450,8 @@ static void abrt_handler( int signal, siginfo_t *siginfo, void *sigcontext ) */ static void quit_handler( int signal, siginfo_t *siginfo, void *sigcontext ) { + init_handler( sigcontext ); + abort_thread(0); } @@ -1448,6 +1465,8 @@ static void usr1_handler( int signal, siginfo_t *siginfo, void *sigcontext ) { CONTEXT context; + init_handler( sigcontext ); + if (is_inside_syscall( sigcontext )) { context.ContextFlags = CONTEXT_FULL; @@ -1471,10 +1490,14 @@ static void usr1_handler( int signal, siginfo_t *siginfo, void *sigcontext ) */ static void usr2_handler( int signal, siginfo_t *siginfo, void *sigcontext ) { - struct syscall_frame *frame = arm64_thread_data()->syscall_frame; + struct syscall_frame *frame; ucontext_t *context = sigcontext; DWORD i; + init_handler( sigcontext ); + + frame = arm64_thread_data()->syscall_frame; + if (!is_inside_syscall( sigcontext )) return; FP_sig(context) = frame->fp; diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index a567aa31b2a..a57734dcdf4 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -670,7 +670,10 @@ __ASM_GLOBAL_FUNC( clear_alignment_flag, */ static inline void *init_handler( const ucontext_t *sigcontext ) { - TEB *teb = get_current_teb(); + TEB *teb; + + check_signal_stack(); + teb = get_current_teb(); clear_alignment_flag(); diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index 07ffb269d19..2aee4b0a37e 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -814,6 +814,8 @@ static inline void set_sigcontext( const CONTEXT *context, ucontext_t *sigcontex */ static inline ucontext_t *init_handler( void *sigcontext ) { + check_signal_stack(); + #ifdef __linux__ if (fs32_sel) { diff --git a/dlls/ntdll/unix/thread.c b/dlls/ntdll/unix/thread.c index d56962e1721..3a5386ad245 100644 --- a/dlls/ntdll/unix/thread.c +++ b/dlls/ntdll/unix/thread.c @@ -1530,6 +1530,34 @@ NTSTATUS WINAPI NtRaiseException( EXCEPTION_RECORD *rec, CONTEXT *context, BOOL } +/*********************************************************************** + * check_signal_stack + */ +void check_signal_stack(void) +{ + stack_t ss; + + sigaltstack( NULL, &ss ); + if (!(ss.ss_flags & SS_ONSTACK)) + { + static const char msg[] = "wine: received signal not on the signal stack; aborting\n"; + struct sigaction sa; + + /* if this happened we probably got a signal from a non-wine thread. + * regardless of the reason, we can't get access to the TEB (or there + * isn't one), so print a message and bail */ + write( 2, msg, sizeof(msg) - 1 ); + + /* exit with SIGABRT so that we dump core */ + sa.sa_flags = SA_RESETHAND | SA_NODEFER; + sigemptyset( &sa.sa_mask ); + sa.sa_handler = NULL; + sigaction( SIGABRT, &sa, NULL ); + abort(); + } +} + + /********************************************************************** * NtCurrentTeb (NTDLL.@) */ diff --git a/dlls/ntdll/unix/unix_private.h b/dlls/ntdll/unix/unix_private.h index d7d1d0d2ac0..5d89551c622 100644 --- a/dlls/ntdll/unix/unix_private.h +++ b/dlls/ntdll/unix/unix_private.h @@ -192,6 +192,7 @@ extern NTSTATUS get_thread_context( HANDLE handle, void *context, BOOL *self, US extern unsigned int alloc_object_attributes( const OBJECT_ATTRIBUTES *attr, struct object_attributes **ret, data_size_t *ret_len ) DECLSPEC_HIDDEN; extern NTSTATUS system_time_precise( void *args ) DECLSPEC_HIDDEN; +extern void check_signal_stack(void) DECLSPEC_HIDDEN; extern void *anon_mmap_fixed( void *start, size_t size, int prot, int flags ) DECLSPEC_HIDDEN; extern void *anon_mmap_alloc( size_t size, int prot ) DECLSPEC_HIDDEN; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2126
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 tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=130486 Your paranoid android. === debian11 (32 bit report) === Report validation errors: ntdll:exception has no test summary line (early exit of the main process?) === debian11 (32 bit zh:CN report) === Report validation errors: ntdll:exception has no test summary line (early exit of the main process?)
So the test failure is, well, very annoying. I had assumed that, although we were indeed calling into glibc to get at sigaltstack(), that glibc would not depend on having TLS at that point. That was probably a foolish assumption on my part. glibc does indeed depend on valid %gs for 32-bit x86 sigaltstack()†, and Linux does not restore it to a sane value. [Neither does Sun, as init_handler() shows.] It's still possible to manually call sigaltstack on linux with int $0x80, and retrieve the correct %gs value that way. But the point has been made, any other target could have the same problem. I really hate to just drop this patch, because it was very annoying to debug the crash that motivated it, but I don't see a good way to achieve that warning without separate assembly for every OS and architecture. † Why? Unlike x86, it has the option of int $0x80 or sysenter, and it uses TLS to determine which. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2126#note_27715
This merge request was closed by Zebediah Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2126
participants (3)
-
Marvin -
Zebediah Figura -
Zebediah Figura (＠zfigura)