Hi Max, Thank you. applied and pushed,
best regards Waldemar Max Filippov wrote, > 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 <[email protected]> > --- > Changes v1 -> v2: > - initialize 'save' with NULL to avoid compiler warning. The code > cannot use uninitialized 'save' value, so no logic change is needed. > > libpthread/linuxthreads/signals.c | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/libpthread/linuxthreads/signals.c > b/libpthread/linuxthreads/signals.c > index 0c0f2b6b1d2a..0cde54a16d27 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; > } > -- > 2.30.2 > > _______________________________________________ > devel mailing list -- [email protected] > To unsubscribe send an email to [email protected] > _______________________________________________ devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
