Hi Max,
there is a compiler warning with this patch added: /home/wbx/embedded-test/openadk/toolchain_qemu-arc_uclibc-ng_archs/usr/bin/arc-openadk-linux-uclibc-gcc -c libpthread/linuxthreads/signals.c -o libpthread/linuxthreads/signals.os -Wall -Wstrict-prototypes -Wstrip libpthread/linuxthreads/signals.c: In function 'sigaction': libpthread/linuxthreads/signals.c:168:29: warning: 'save' may be used uninitialized [-Wmaybe-uninitialized] 168 | sighandler[sig].old = save; | ~~~~~~~~~~~~~~~~~~~~^~~~~~ libpthread/linuxthreads/signals.c:137:9: note: 'save' was declared here 137 | void *save; | ^~~~
Can this be avoided?
Thanks for the patch.
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 jcmvbkbc@gmail.com
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;
#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-leave@uclibc-ng.org