Hello
here are my last patches
Mit freundlichem Gruß,
With best regards,
Ramin Seyed-Moussavi
Lead Software Developer
Phone: +49 30 3499834 0
Fax: +49 30 3499834 28
[cid:image001.png@01D9C916.916A8140]
Gustav-Meyer-Allee 25
Haus 12.2
D - 13355 Berlin
Amtsgericht Berlin, Charlottenburg HRB 79187
USt.-IdNr: DE814030948
Geschäftsführer: M.Sc. Markus Nigg
YACOUB im Internet: www.yacoub.de<http://www.yacoub.de/>
The information contained in this email is intended only for its addressee and may contain confidential and/or legally proteced information. If the reader of this email is not the intended recipient, you are hereby notified that saving, distribution or use of the content of this email in any way is prohibited. If you have received this email in error, please notify the sender and delete the email. We do not accept any responsibility for any damages caused by email transmitted.
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(a)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
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(a)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