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 <jcmvb...@gmail.com>
---
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 -- devel@uclibc-ng.org
To unsubscribe send an email to devel-le...@uclibc-ng.org

Reply via email to