From eb532bc1b7a635932fe5cef472d0442620a173a2 Mon Sep 17 00:00:00 2001 From: Max Filippov Date: Wed, 2 Aug 2023 10:49:47 -0700 Subject: linuxthreads/signal: improve sigaction behavior Setting signal handler in the kernel and then updating sighandler[sig] results in a crash if a signal which handler is being changed from SIG_DFL to a non-default was pending. Improve the race a little and update the sighandler[sig] before the sigaction syscall. It doesn't eliminate the race entirely, but fixes this particular failing case. E.g. this fixes the 100% reproducible segfault in the busybox hush shell built with FEATURE_EDITING_WINCH on ssh client's terminal window resize, but in that case there's one more even bigger issue: busybox calls sigaction with both old and new signal pointers pointing to the same structure instance, as a result act->sa_handler after the sigaction syscall is not what the user requested, but the previous handler. Signed-off-by: Max Filippov --- libpthread/linuxthreads/signals.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) (limited to 'libpthread') diff --git a/libpthread/linuxthreads/signals.c b/libpthread/linuxthreads/signals.c index 0c0f2b6b1..91b88e2c7 100644 --- a/libpthread/linuxthreads/signals.c +++ b/libpthread/linuxthreads/signals.c @@ -134,6 +134,7 @@ int sigaction(int sig, const struct sigaction * act, { struct sigaction newact; struct sigaction *newactp; + void *save = NULL; #ifdef DEBUG_PT printf(__FUNCTION__": pthreads wrapper!\n"); @@ -142,6 +143,8 @@ printf(__FUNCTION__": pthreads wrapper!\n"); sig == __pthread_sig_cancel || (sig == __pthread_sig_debug && __pthread_sig_debug > 0)) return EINVAL; + if (sig > 0 && sig < NSIG) + save = sighandler[sig].old; if (act) { newact = *act; @@ -154,22 +157,24 @@ printf(__FUNCTION__": pthreads wrapper!\n"); newact.sa_handler = (__sighandler_t) pthread_sighandler; } newactp = &newact; + if (sig > 0 && sig < NSIG) + sighandler[sig].old = (arch_sighandler_t) act->sa_handler; } else newactp = NULL; if (__libc_sigaction(sig, newactp, oact) == -1) - return -1; + { + if (act && sig > 0 && sig < NSIG) + sighandler[sig].old = save; + return -1; + } #ifdef DEBUG_PT printf(__FUNCTION__": sighandler installed, sigaction successful\n"); #endif if (sig > 0 && sig < NSIG) { if (oact != NULL) - oact->sa_handler = (__sighandler_t) sighandler[sig].old; - if (act) - /* For the assignment is does not matter whether it's a normal - or real-time signal. */ - sighandler[sig].old = (arch_sighandler_t) act->sa_handler; + oact->sa_handler = save; } return 0; } -- cgit v1.2.3