[RFC PATCH] ntdll: Swallow crashes once pthread_exit() has been called.
When pthread_exit() is called, it tries to rewind the stack, which includes calling cleanup routines registered with pthread_cleanup_push() or similar procedures. But we also unwind the stack in signal_exit_thread(). This means that if any cleanup routine was registered between the exit frame and where control was when the thread was interrupted, then pthread will try to call functions from frames that do not exist anymore and might have been overwritten, which easily results in segmentation faults or similar accidents. Since until PE separation is finished it is not easy to fix the real problem, this patch logs and swallows crashes occurring after pthread_exit() is called, so that at least damages are minimized and developers are not confused by irrelevant messages. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52213 --- dlls/ntdll/unix/signal_arm.c | 6 ++++++ dlls/ntdll/unix/signal_arm64.c | 6 ++++++ dlls/ntdll/unix/signal_i386.c | 6 ++++++ dlls/ntdll/unix/signal_x86_64.c | 6 ++++++ dlls/ntdll/unix/thread.c | 2 ++ dlls/ntdll/unix/unix_private.h | 1 + 6 files changed, 27 insertions(+) diff --git a/dlls/ntdll/unix/signal_arm.c b/dlls/ntdll/unix/signal_arm.c index ebc08984adf..b552bdb79f9 100644 --- a/dlls/ntdll/unix/signal_arm.c +++ b/dlls/ntdll/unix/signal_arm.c @@ -800,6 +800,12 @@ static void segv_handler( int signal, siginfo_t *siginfo, void *sigcontext ) EXCEPTION_RECORD rec = { 0 }; ucontext_t *context = sigcontext; + if (ntdll_get_thread_data()->exiting_pthread) + { + WARN("Swallowing crash after pthread is exiting\n"); + syscall(__NR_exit, 0); + } + switch (get_trap_code(signal, context)) { case TRAP_ARM_PRIVINFLT: /* Invalid opcode exception */ diff --git a/dlls/ntdll/unix/signal_arm64.c b/dlls/ntdll/unix/signal_arm64.c index f2612412366..10da9b8a366 100644 --- a/dlls/ntdll/unix/signal_arm64.c +++ b/dlls/ntdll/unix/signal_arm64.c @@ -854,6 +854,12 @@ static void segv_handler( int signal, siginfo_t *siginfo, void *sigcontext ) EXCEPTION_RECORD rec = { 0 }; ucontext_t *context = sigcontext; + if (ntdll_get_thread_data()->exiting_pthread) + { + WARN("Swallowing crash after pthread is exiting\n"); + syscall(__NR_exit, 0); + } + rec.NumberParameters = 2; rec.ExceptionInformation[0] = (get_fault_esr( context ) & 0x40) != 0; rec.ExceptionInformation[1] = (ULONG_PTR)siginfo->si_addr; diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index bf3abc1a587..1d4f8ec9df1 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -1774,6 +1774,12 @@ static void segv_handler( int signal, siginfo_t *siginfo, void *sigcontext ) ucontext_t *ucontext = sigcontext; void *stack = setup_exception_record( sigcontext, &rec, &xcontext ); + if (ntdll_get_thread_data()->exiting_pthread) + { + WARN("Swallowing crash after pthread is exiting\n"); + syscall(__NR_exit, 0); + } + switch (TRAP_sig(ucontext)) { case TRAP_x86_OFLOW: /* Overflow exception */ diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index d32e38875a4..6454d2e6b26 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -2551,6 +2551,12 @@ static void segv_handler( int signal, siginfo_t *siginfo, void *sigcontext ) struct xcontext context; ucontext_t *ucontext = sigcontext; + if (ntdll_get_thread_data()->exiting_pthread) + { + WARN("Swallowing crash after pthread is exiting\n"); + syscall(__NR_exit, 0); + } + rec.ExceptionAddress = (void *)RIP_sig(ucontext); save_context( &context, sigcontext ); diff --git a/dlls/ntdll/unix/thread.c b/dlls/ntdll/unix/thread.c index 00fe2b6fa1b..8a22b68605a 100644 --- a/dlls/ntdll/unix/thread.c +++ b/dlls/ntdll/unix/thread.c @@ -1055,6 +1055,7 @@ static void pthread_exit_wrapper( int status ) close( ntdll_get_thread_data()->wait_fd[1] ); close( ntdll_get_thread_data()->reply_fd ); close( ntdll_get_thread_data()->request_fd ); + ntdll_get_thread_data()->exiting_pthread = TRUE; pthread_exit( UIntToPtr(status) ); } @@ -1342,6 +1343,7 @@ NTSTATUS WINAPI NtCreateThreadEx( HANDLE *handle, ACCESS_MASK access, OBJECT_ATT thread_data->request_fd = request_pipe[1]; thread_data->start = start; thread_data->param = param; + thread_data->exiting_pthread = FALSE; pthread_attr_init( &pthread_attr ); pthread_attr_setstack( &pthread_attr, teb->DeallocationStack, diff --git a/dlls/ntdll/unix/unix_private.h b/dlls/ntdll/unix/unix_private.h index 0dcb09ad641..0d1117be2e3 100644 --- a/dlls/ntdll/unix/unix_private.h +++ b/dlls/ntdll/unix/unix_private.h @@ -61,6 +61,7 @@ struct ntdll_thread_data PRTL_THREAD_START_ROUTINE start; /* thread entry point */ void *param; /* thread entry point parameter */ void *jmp_buf; /* setjmp buffer for exception handling */ + BOOL exiting_pthread; }; C_ASSERT( sizeof(struct ntdll_thread_data) <= sizeof(((TEB *)0)->GdiTebBatch) ); -- 2.34.1
Hi, On 17/12/21 23:02, Chip Davis wrote:
This won't work on platforms other than Linux, and not just because only Linux has the __NR_* symbols for syscall numbers. Thanks for the feedback. The idea of that call is to immediately exit the thread without further cleanup, but signalling a clean exit. I don't think there is a way to do that portably. Which are the "supported" platforms I should care about for a non-portable patch to be accepted?
That said, the patch was a RFC mostly to ask whether the idea of the solution seems good or not. Giovanni.
Am 20.12.2021 um 09:38 schrieb Giovanni Mascellani <gmascellani(a)codeweavers.com>:
Hi,
On 17/12/21 23:02, Chip Davis wrote:
This won't work on platforms other than Linux, and not just because only Linux has the __NR_* symbols for syscall numbers. Thanks for the feedback. The idea of that call is to immediately exit the thread without further cleanup, but signalling a clean exit. I don't think there is a way to do that portably. Which are the "supported" platforms I should care about for a non-portable patch to be accepted?
You'd at least need #ifdefs to make sure other platforms compile, even if the kind of graceful exit you are trying to ensure doesn't work.
participants (3)
-
Chip Davis -
Giovanni Mascellani -
Stefan Dösinger