| Message ID | 87sfvtcjua.fsf@oldenburg.str.redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | nptl: Do not set signal mask on second setjmp return [BZ #28607] | expand |
On 18/11/2021 17:03, Florian Weimer via Libc-alpha wrote: > __libc_signal_restore_set was in the wrong place: It also ran > when setjmp returned the second time (after pthread_exit or > pthread_cancel). This is observable with blocked pending > signals during thread exit. > > Tested on i686-linux-gnu and x86_64-linux-gnu., LGTM, thanks. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > --- > nptl/pthread_create.c | 4 +-- > sysdeps/pthread/Makefile | 1 + > sysdeps/pthread/tst-pthread-exit-signal.c | 45 +++++++++++++++++++++++++++++++ > 3 files changed, 48 insertions(+), 2 deletions(-) > > diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c > index d6ea43a754..bad9eeb52f 100644 > --- a/nptl/pthread_create.c > +++ b/nptl/pthread_create.c > @@ -406,8 +406,6 @@ start_thread (void *arg) > unwind_buf.priv.data.prev = NULL; > unwind_buf.priv.data.cleanup = NULL; > > - __libc_signal_restore_set (&pd->sigmask); > - > /* Allow setxid from now onwards. */ > if (__glibc_unlikely (atomic_exchange_acq (&pd->setxid_futex, 0) == -2)) > futex_wake (&pd->setxid_futex, 1, FUTEX_PRIVATE); > @@ -417,6 +415,8 @@ start_thread (void *arg) > /* Store the new cleanup handler info. */ > THREAD_SETMEM (pd, cleanup_jmp_buf, &unwind_buf); > > + __libc_signal_restore_set (&pd->sigmask); > + > LIBC_PROBE (pthread_start, 3, (pthread_t) pd, pd->start_routine, pd->arg); > > /* Run the code the user provided. */ Ok. > diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile > index df8943f486..c657101696 100644 > --- a/sysdeps/pthread/Makefile > +++ b/sysdeps/pthread/Makefile > @@ -118,6 +118,7 @@ tests += tst-cnd-basic tst-mtx-trylock tst-cnd-broadcast \ > tst-unload \ > tst-unwind-thread \ > tst-pt-vfork1 tst-pt-vfork2 tst-vfork1x tst-vfork2x \ > + tst-pthread-exit-signal \ > tst-pthread-setuid-loop \ > tst-pthread_cancel-exited \ > tst-pthread_cancel-select-loop \ Ok. > diff --git a/sysdeps/pthread/tst-pthread-exit-signal.c b/sysdeps/pthread/tst-pthread-exit-signal.c > new file mode 100644 > index 0000000000..b4526fe663 > --- /dev/null > +++ b/sysdeps/pthread/tst-pthread-exit-signal.c > @@ -0,0 +1,45 @@ > +/* Test that pending signals are not delivered on thread exit (bug 28607). > + Copyright (C) 2021 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <https://www.gnu.org/licenses/>. */ > + > +/* Due to bug 28607, pthread_kill (or pthread_cancel) restored the > + signal mask during during thread exit, triggering the delivery of a > + blocked pending signal (SIGUSR1 in this test). */ > + > +#include <support/xthread.h> > +#include <support/xsignal.h> > + > +static void * > +threadfunc (void *closure) > +{ > + sigset_t sigmask; > + sigfillset (&sigmask); > + xpthread_sigmask (SIG_SETMASK, &sigmask, NULL); > + xpthread_kill (pthread_self (), SIGUSR1); > + pthread_exit (NULL); > + return NULL; > +} > + > +static int > +do_test (void) > +{ > + pthread_t thr = xpthread_create (NULL, threadfunc, NULL); > + xpthread_join (thr); > + return 0; > +} > + > +#include <support/test-driver.c> > Ok.
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index d6ea43a754..bad9eeb52f 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -406,8 +406,6 @@ start_thread (void *arg) unwind_buf.priv.data.prev = NULL; unwind_buf.priv.data.cleanup = NULL; - __libc_signal_restore_set (&pd->sigmask); - /* Allow setxid from now onwards. */ if (__glibc_unlikely (atomic_exchange_acq (&pd->setxid_futex, 0) == -2)) futex_wake (&pd->setxid_futex, 1, FUTEX_PRIVATE); @@ -417,6 +415,8 @@ start_thread (void *arg) /* Store the new cleanup handler info. */ THREAD_SETMEM (pd, cleanup_jmp_buf, &unwind_buf); + __libc_signal_restore_set (&pd->sigmask); + LIBC_PROBE (pthread_start, 3, (pthread_t) pd, pd->start_routine, pd->arg); /* Run the code the user provided. */ diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile index df8943f486..c657101696 100644 --- a/sysdeps/pthread/Makefile +++ b/sysdeps/pthread/Makefile @@ -118,6 +118,7 @@ tests += tst-cnd-basic tst-mtx-trylock tst-cnd-broadcast \ tst-unload \ tst-unwind-thread \ tst-pt-vfork1 tst-pt-vfork2 tst-vfork1x tst-vfork2x \ + tst-pthread-exit-signal \ tst-pthread-setuid-loop \ tst-pthread_cancel-exited \ tst-pthread_cancel-select-loop \ diff --git a/sysdeps/pthread/tst-pthread-exit-signal.c b/sysdeps/pthread/tst-pthread-exit-signal.c new file mode 100644 index 0000000000..b4526fe663 --- /dev/null +++ b/sysdeps/pthread/tst-pthread-exit-signal.c @@ -0,0 +1,45 @@ +/* Test that pending signals are not delivered on thread exit (bug 28607). + Copyright (C) 2021 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +/* Due to bug 28607, pthread_kill (or pthread_cancel) restored the + signal mask during during thread exit, triggering the delivery of a + blocked pending signal (SIGUSR1 in this test). */ + +#include <support/xthread.h> +#include <support/xsignal.h> + +static void * +threadfunc (void *closure) +{ + sigset_t sigmask; + sigfillset (&sigmask); + xpthread_sigmask (SIG_SETMASK, &sigmask, NULL); + xpthread_kill (pthread_self (), SIGUSR1); + pthread_exit (NULL); + return NULL; +} + +static int +do_test (void) +{ + pthread_t thr = xpthread_create (NULL, threadfunc, NULL); + xpthread_join (thr); + return 0; +} + +#include <support/test-driver.c>