| Message ID | 1518439345-6013-2-git-send-email-adhemerval.zanella@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v4,1/4] Rename nptl-signals.h to internal-signals.h | expand |
Ping. On 12/02/2018 10:42, Adhemerval Zanella wrote: > This patch filters out the internal NPTL signals (SIGCANCEL/SIGTIMER and > SIGSETXID) from signal functions. GLIBC on Linux requires both signals to > proper implement pthread cancellation, posix timers, and set*id posix > thread synchronization. > > And not filtering out the internal signal is troublesome: > > - A conformant program on a architecture that does not filter out the > signals might inadvertently disable pthread asynchronous cancellation, > set*id synchronization or posix timers. > > - It might also to security issues if SIGSETXID is masked and set*id > functions are called (some threads might have effective user or group > id different from the rest). > > The changes are basically: > > - Change __is_internal_signal to bool and used on all signal function > that has a signal number as input. Also for signal function which accepts > signals sets (sigset_t) it assumes that canonical function were used to > add/remove signals which lead to some input simplification. > > - Fix tst-sigset.c to avoid check for SIGCANCEL/SIGTIMER and SIGSETXID. > It is rewritten to check each signal indidually and to check realtime > signals using canonical macros. > > - Add generic __clear_internal_signals and __is_internal_signal > version since both symbols are used on generic implementations. > > - Remove superflous sysdeps/nptl/sigfillset.c. > > - Remove superflous SIGTIMER handling on Linux __is_internal_signal > since it is the same of SIGCANCEL. > > - Remove dangling define and obvious comment on nptl/sigaction.c. > > Checked on x86_64-linux-gnu. > > [BZ #22391] > * nptl/sigaction.c (__sigaction): Use __is_internal_signal to > check for internal nptl signals. > * signal/sigaddset.c (sigaddset): Likewise. > * signal/sigdelset.c (sigdelset): Likewise. > * sysdeps/posix/signal.c (__bsd_signal): Likewise. > * sysdeps/posix/sigset.c (sigset): Call and check sigaddset return > value. > * signal/sigfillset.c (sigfillset): User __clear_internal_signals > to filter out internal nptl signals. > * signal/tst-sigset.c (do_test): Check ech signal indidually and > also check realtime signals using standard macros. > * sysdeps/nptl/nptl-signals.h (__clear_internal_signals, > __is_internal_signal): New functions. > * sysdeps/nptl/sigfillset.c: Remove file. > * sysdeps/unix/sysv/linux/nptl-signals.h (__is_internal_signal): > Change return to bool. > (__clear_internal_signals): Remove SIGTIMER clean since it is > equal to SIGCANEL on Linux. > * sysdeps/unix/sysv/linux/sigtimedwait.c (__sigtimedwait): Assume > signal set was constructed using standard functions. > * sysdeps/unix/sysv/linux/sigwait.c (do_sigtwait): Likewise. > > Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > Reported-by: Yury Norov <ynorov@caviumnetworks.com> > --- > ChangeLog | 23 ++++++++ > nptl/sigaction.c | 14 +---- > signal/sigaction.c | 2 +- > signal/sigaddset.c | 5 +- > signal/sigdelset.c | 5 +- > signal/sigfillset.c | 10 +--- > signal/tst-sigset.c | 92 ++++++++++++++++++++++-------- > sysdeps/generic/internal-signals.h | 11 ++++ > sysdeps/nptl/sigfillset.c | 20 ------- > sysdeps/posix/signal.c | 5 +- > sysdeps/posix/sigset.c | 10 +--- > sysdeps/unix/sysv/linux/internal-signals.h | 4 +- > sysdeps/unix/sysv/linux/sigtimedwait.c | 17 +----- > 13 files changed, 122 insertions(+), 96 deletions(-) > delete mode 100644 sysdeps/nptl/sigfillset.c > > diff --git a/nptl/sigaction.c b/nptl/sigaction.c > index ddf6f5e..79b6fdc 100644 > --- a/nptl/sigaction.c > +++ b/nptl/sigaction.c > @@ -16,22 +16,12 @@ > License along with the GNU C Library; if not, see > <http://www.gnu.org/licenses/>. */ > > - > -/* This is no complete implementation. The file is meant to be > - included in the real implementation to provide the wrapper around > - __libc_sigaction. */ > - > -#include <nptl/pthreadP.h> > - > -/* We use the libc implementation but we tell it to not allow > - SIGCANCEL or SIGTIMER to be handled. */ > -#define LIBC_SIGACTION 1 > - > +#include <internal-signals.h> > > int > __sigaction (int sig, const struct sigaction *act, struct sigaction *oact) > { > - if (__glibc_unlikely (sig == SIGCANCEL || sig == SIGSETXID)) > + if (sig <= 0 || sig >= NSIG || __is_internal_signal (sig)) > { > __set_errno (EINVAL); > return -1; > diff --git a/signal/sigaction.c b/signal/sigaction.c > index f761ca2..c99001a 100644 > --- a/signal/sigaction.c > +++ b/signal/sigaction.c > @@ -24,7 +24,7 @@ > int > __sigaction (int sig, const struct sigaction *act, struct sigaction *oact) > { > - if (sig <= 0 || sig >= NSIG) > + if (sig <= 0 || sig >= NSIG || __is_internal_signal (sig)) > { > __set_errno (EINVAL); > return -1; > diff --git a/signal/sigaddset.c b/signal/sigaddset.c > index d310890..7238df4 100644 > --- a/signal/sigaddset.c > +++ b/signal/sigaddset.c > @@ -17,13 +17,14 @@ > > #include <errno.h> > #include <signal.h> > -#include <sigsetops.h> > +#include <internal-signals.h> > > /* Add SIGNO to SET. */ > int > sigaddset (sigset_t *set, int signo) > { > - if (set == NULL || signo <= 0 || signo >= NSIG) > + if (set == NULL || signo <= 0 || signo >= NSIG > + || __is_internal_signal (signo)) > { > __set_errno (EINVAL); > return -1; > diff --git a/signal/sigdelset.c b/signal/sigdelset.c > index cd83dda..011978c 100644 > --- a/signal/sigdelset.c > +++ b/signal/sigdelset.c > @@ -17,13 +17,14 @@ > > #include <errno.h> > #include <signal.h> > -#include <sigsetops.h> > +#include <internal-signals.h> > > /* Add SIGNO to SET. */ > int > sigdelset (sigset_t *set, int signo) > { > - if (set == NULL || signo <= 0 || signo >= NSIG) > + if (set == NULL || signo <= 0 || signo >= NSIG > + || __is_internal_signal (signo)) > { > __set_errno (EINVAL); > return -1; > diff --git a/signal/sigfillset.c b/signal/sigfillset.c > index e586fd9..83dd583 100644 > --- a/signal/sigfillset.c > +++ b/signal/sigfillset.c > @@ -18,6 +18,7 @@ > #include <errno.h> > #include <signal.h> > #include <string.h> > +#include <internal-signals.h> > > /* Set all signals in SET. */ > int > @@ -31,14 +32,7 @@ sigfillset (sigset_t *set) > > memset (set, 0xff, sizeof (sigset_t)); > > - /* If the implementation uses a cancellation signal don't set the bit. */ > -#ifdef SIGCANCEL > - __sigdelset (set, SIGCANCEL); > -#endif > - /* Likewise for the signal to implement setxid. */ > -#ifdef SIGSETXID > - __sigdelset (set, SIGSETXID); > -#endif > + __clear_internal_signals (set); > > return 0; > } > diff --git a/signal/tst-sigset.c b/signal/tst-sigset.c > index d47adcc..a2b764d 100644 > --- a/signal/tst-sigset.c > +++ b/signal/tst-sigset.c > @@ -1,43 +1,85 @@ > /* Test sig*set functions. */ > > #include <signal.h> > -#include <stdio.h> > > -#define TEST_FUNCTION do_test () > +#include <support/check.h> > + > static int > do_test (void) > { > - int result = 0; > - int sig = -1; > + sigset_t set; > + TEST_VERIFY (sigemptyset (&set) == 0); > > -#define TRY(call) \ > - if (call) \ > - { \ > - printf ("%s (sig = %d): %m\n", #call, sig); \ > - result = 1; \ > - } \ > - else > +#define VERIFY(set, sig) \ > + TEST_VERIFY (sigismember (&set, sig) == 0); \ > + TEST_VERIFY (sigaddset (&set, sig) == 0); \ > + TEST_VERIFY (sigismember (&set, sig) != 0); \ > + TEST_VERIFY (sigdelset (&set, sig) == 0); \ > + TEST_VERIFY (sigismember (&set, sig) == 0) > > + /* ISO C99 signals. */ > + VERIFY (set, SIGINT); > + VERIFY (set, SIGILL); > + VERIFY (set, SIGABRT); > + VERIFY (set, SIGFPE); > + VERIFY (set, SIGSEGV); > + VERIFY (set, SIGTERM); > > - sigset_t set; > - TRY (sigemptyset (&set) != 0); > + /* Historical signals specified by POSIX. */ > + VERIFY (set, SIGHUP); > + VERIFY (set, SIGQUIT); > + VERIFY (set, SIGTRAP); > + VERIFY (set, SIGKILL); > + VERIFY (set, SIGBUS); > + VERIFY (set, SIGSYS); > + VERIFY (set, SIGPIPE); > + VERIFY (set, SIGALRM); > + > + /* New(er) POSIX signals (1003.1-2008, 1003.1-2013). */ > + VERIFY (set, SIGURG); > + VERIFY (set, SIGSTOP); > + VERIFY (set, SIGTSTP); > + VERIFY (set, SIGCONT); > + VERIFY (set, SIGCHLD); > + VERIFY (set, SIGTTIN); > + VERIFY (set, SIGTTOU); > + VERIFY (set, SIGPOLL); > + VERIFY (set, SIGXCPU); > + VERIFY (set, SIGXFSZ); > + VERIFY (set, SIGVTALRM); > + VERIFY (set, SIGPROF); > + VERIFY (set, SIGUSR1); > + VERIFY (set, SIGUSR2); > + > + /* Nonstandard signals found in all modern POSIX systems > + (including both BSD and Linux). */ > + VERIFY (set, SIGWINCH); > > -#ifdef SIGRTMAX > - int max_sig = SIGRTMAX; > -#else > - int max_sig = NSIG - 1; > + /* Arch-specific signals. */ > +#ifdef SIGEMT > + VERIFY (set, SIGEMT); > +#endif > +#ifdef SIGLOST > + VERIFY (set, SIGLOST); > +#endif > +#ifdef SIGINFO > + VERIFY (set, SIGINFO); > +#endif > +#ifdef SIGSTKFLT > + VERIFY (set, SIGSTKFLT); > +#endif > +#ifdef SIGPWR > + VERIFY (set, SIGPWR); > #endif > > - for (sig = 1; sig <= max_sig; ++sig) > + /* Read-time signals (POSIX.1b real-time extensions). If they are > + supported SIGRTMAX value is greater than SIGRTMIN. */ > + for (int rtsig = SIGRTMIN; rtsig <= SIGRTMAX; rtsig++) > { > - TRY (sigismember (&set, sig) != 0); > - TRY (sigaddset (&set, sig) != 0); > - TRY (sigismember (&set, sig) == 0); > - TRY (sigdelset (&set, sig) != 0); > - TRY (sigismember (&set, sig) != 0); > + VERIFY (set, rtsig); > } > > - return result; > + return 0; > } > > -#include "../test-skeleton.c" > +#include <support/test-driver.c> > diff --git a/sysdeps/generic/internal-signals.h b/sysdeps/generic/internal-signals.h > index 01e5b75..ab0b22e 100644 > --- a/sysdeps/generic/internal-signals.h > +++ b/sysdeps/generic/internal-signals.h > @@ -15,3 +15,14 @@ > You should have received a copy of the GNU Lesser General Public > License along with the GNU C Library; if not, see > <http://www.gnu.org/licenses/>. */ > + > +static inline void > +__clear_internal_signals (sigset_t *set) > +{ > +} > + > +static inline bool > +__is_internal_signal (int sig) > +{ > + return false; > +} > diff --git a/sysdeps/nptl/sigfillset.c b/sysdeps/nptl/sigfillset.c > deleted file mode 100644 > index 94a7680..0000000 > --- a/sysdeps/nptl/sigfillset.c > +++ /dev/null > @@ -1,20 +0,0 @@ > -/* Copyright (C) 2003-2018 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 > - <http://www.gnu.org/licenses/>. */ > - > -#include <nptl/pthreadP.h> > - > -#include <signal/sigfillset.c> > diff --git a/sysdeps/posix/signal.c b/sysdeps/posix/signal.c > index a4a0875..8a135c7 100644 > --- a/sysdeps/posix/signal.c > +++ b/sysdeps/posix/signal.c > @@ -18,8 +18,8 @@ > > #include <errno.h> > #include <signal.h> > -#include <string.h> /* For the real memset prototype. */ > #include <sigsetops.h> > +#include <internal-signals.h> > > sigset_t _sigintr attribute_hidden; /* Set by siginterrupt. */ > > @@ -31,7 +31,8 @@ __bsd_signal (int sig, __sighandler_t handler) > struct sigaction act, oact; > > /* Check signal extents to protect __sigismember. */ > - if (handler == SIG_ERR || sig < 1 || sig >= NSIG) > + if (handler == SIG_ERR || sig < 1 || sig >= NSIG > + || __is_internal_signal (sig)) > { > __set_errno (EINVAL); > return SIG_ERR; > diff --git a/sysdeps/posix/sigset.c b/sysdeps/posix/sigset.c > index b62aa3c..6ab4a48 100644 > --- a/sysdeps/posix/sigset.c > +++ b/sysdeps/posix/sigset.c > @@ -31,15 +31,9 @@ sigset (int sig, __sighandler_t disp) > sigset_t set; > sigset_t oset; > > - /* Check signal extents to protect __sigismember. */ > - if (disp == SIG_ERR || sig < 1 || sig >= NSIG) > - { > - __set_errno (EINVAL); > - return SIG_ERR; > - } > - > __sigemptyset (&set); > - __sigaddset (&set, sig); > + if (sigaddset (&set, sig) < 0) > + return SIG_ERR; > > if (disp == SIG_HOLD) > { > diff --git a/sysdeps/unix/sysv/linux/internal-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h > index e007372..5ff4cf8 100644 > --- a/sysdeps/unix/sysv/linux/internal-signals.h > +++ b/sysdeps/unix/sysv/linux/internal-signals.h > @@ -21,6 +21,8 @@ > > #include <signal.h> > #include <sigsetops.h> > +#include <stdbool.h> > +#include <sysdep.h> > > /* The signal used for asynchronous cancelation. */ > #define SIGCANCEL __SIGRTMIN > @@ -37,7 +39,7 @@ > > > /* Return is sig is used internally. */ > -static inline int > +static inline bool > __is_internal_signal (int sig) > { > return (sig == SIGCANCEL) || (sig == SIGSETXID); > diff --git a/sysdeps/unix/sysv/linux/sigtimedwait.c b/sysdeps/unix/sysv/linux/sigtimedwait.c > index 051a285..b4de885 100644 > --- a/sysdeps/unix/sysv/linux/sigtimedwait.c > +++ b/sysdeps/unix/sysv/linux/sigtimedwait.c > @@ -24,21 +24,8 @@ int > __sigtimedwait (const sigset_t *set, siginfo_t *info, > const struct timespec *timeout) > { > - sigset_t tmpset; > - if (set != NULL > - && (__builtin_expect (__sigismember (set, SIGCANCEL), 0) > - || __builtin_expect (__sigismember (set, SIGSETXID), 0))) > - { > - /* Create a temporary mask without the bit for SIGCANCEL set. */ > - // We are not copying more than we have to. > - memcpy (&tmpset, set, _NSIG / 8); > - __sigdelset (&tmpset, SIGCANCEL); > - __sigdelset (&tmpset, SIGSETXID); > - set = &tmpset; > - } > - > - /* XXX The size argument hopefully will have to be changed to the > - real size of the user-level sigset_t. */ > + /* XXX The size argument hopefully will have to be changed to the > + real size of the user-level sigset_t. */ > int result = SYSCALL_CANCEL (rt_sigtimedwait, set, info, timeout, _NSIG / 8); > > /* The kernel generates a SI_TKILL code in si_code in case tkill is >
If no one opposes, I will commit this shortly. On 19/02/2018 14:50, Adhemerval Zanella wrote: > Ping. > > On 12/02/2018 10:42, Adhemerval Zanella wrote: >> This patch filters out the internal NPTL signals (SIGCANCEL/SIGTIMER and >> SIGSETXID) from signal functions. GLIBC on Linux requires both signals to >> proper implement pthread cancellation, posix timers, and set*id posix >> thread synchronization. >> >> And not filtering out the internal signal is troublesome: >> >> - A conformant program on a architecture that does not filter out the >> signals might inadvertently disable pthread asynchronous cancellation, >> set*id synchronization or posix timers. >> >> - It might also to security issues if SIGSETXID is masked and set*id >> functions are called (some threads might have effective user or group >> id different from the rest). >> >> The changes are basically: >> >> - Change __is_internal_signal to bool and used on all signal function >> that has a signal number as input. Also for signal function which accepts >> signals sets (sigset_t) it assumes that canonical function were used to >> add/remove signals which lead to some input simplification. >> >> - Fix tst-sigset.c to avoid check for SIGCANCEL/SIGTIMER and SIGSETXID. >> It is rewritten to check each signal indidually and to check realtime >> signals using canonical macros. >> >> - Add generic __clear_internal_signals and __is_internal_signal >> version since both symbols are used on generic implementations. >> >> - Remove superflous sysdeps/nptl/sigfillset.c. >> >> - Remove superflous SIGTIMER handling on Linux __is_internal_signal >> since it is the same of SIGCANCEL. >> >> - Remove dangling define and obvious comment on nptl/sigaction.c. >> >> Checked on x86_64-linux-gnu. >> >> [BZ #22391] >> * nptl/sigaction.c (__sigaction): Use __is_internal_signal to >> check for internal nptl signals. >> * signal/sigaddset.c (sigaddset): Likewise. >> * signal/sigdelset.c (sigdelset): Likewise. >> * sysdeps/posix/signal.c (__bsd_signal): Likewise. >> * sysdeps/posix/sigset.c (sigset): Call and check sigaddset return >> value. >> * signal/sigfillset.c (sigfillset): User __clear_internal_signals >> to filter out internal nptl signals. >> * signal/tst-sigset.c (do_test): Check ech signal indidually and >> also check realtime signals using standard macros. >> * sysdeps/nptl/nptl-signals.h (__clear_internal_signals, >> __is_internal_signal): New functions. >> * sysdeps/nptl/sigfillset.c: Remove file. >> * sysdeps/unix/sysv/linux/nptl-signals.h (__is_internal_signal): >> Change return to bool. >> (__clear_internal_signals): Remove SIGTIMER clean since it is >> equal to SIGCANEL on Linux. >> * sysdeps/unix/sysv/linux/sigtimedwait.c (__sigtimedwait): Assume >> signal set was constructed using standard functions. >> * sysdeps/unix/sysv/linux/sigwait.c (do_sigtwait): Likewise. >> >> Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> >> Reported-by: Yury Norov <ynorov@caviumnetworks.com> >> --- >> ChangeLog | 23 ++++++++ >> nptl/sigaction.c | 14 +---- >> signal/sigaction.c | 2 +- >> signal/sigaddset.c | 5 +- >> signal/sigdelset.c | 5 +- >> signal/sigfillset.c | 10 +--- >> signal/tst-sigset.c | 92 ++++++++++++++++++++++-------- >> sysdeps/generic/internal-signals.h | 11 ++++ >> sysdeps/nptl/sigfillset.c | 20 ------- >> sysdeps/posix/signal.c | 5 +- >> sysdeps/posix/sigset.c | 10 +--- >> sysdeps/unix/sysv/linux/internal-signals.h | 4 +- >> sysdeps/unix/sysv/linux/sigtimedwait.c | 17 +----- >> 13 files changed, 122 insertions(+), 96 deletions(-) >> delete mode 100644 sysdeps/nptl/sigfillset.c >> >> diff --git a/nptl/sigaction.c b/nptl/sigaction.c >> index ddf6f5e..79b6fdc 100644 >> --- a/nptl/sigaction.c >> +++ b/nptl/sigaction.c >> @@ -16,22 +16,12 @@ >> License along with the GNU C Library; if not, see >> <http://www.gnu.org/licenses/>. */ >> >> - >> -/* This is no complete implementation. The file is meant to be >> - included in the real implementation to provide the wrapper around >> - __libc_sigaction. */ >> - >> -#include <nptl/pthreadP.h> >> - >> -/* We use the libc implementation but we tell it to not allow >> - SIGCANCEL or SIGTIMER to be handled. */ >> -#define LIBC_SIGACTION 1 >> - >> +#include <internal-signals.h> >> >> int >> __sigaction (int sig, const struct sigaction *act, struct sigaction *oact) >> { >> - if (__glibc_unlikely (sig == SIGCANCEL || sig == SIGSETXID)) >> + if (sig <= 0 || sig >= NSIG || __is_internal_signal (sig)) >> { >> __set_errno (EINVAL); >> return -1; >> diff --git a/signal/sigaction.c b/signal/sigaction.c >> index f761ca2..c99001a 100644 >> --- a/signal/sigaction.c >> +++ b/signal/sigaction.c >> @@ -24,7 +24,7 @@ >> int >> __sigaction (int sig, const struct sigaction *act, struct sigaction *oact) >> { >> - if (sig <= 0 || sig >= NSIG) >> + if (sig <= 0 || sig >= NSIG || __is_internal_signal (sig)) >> { >> __set_errno (EINVAL); >> return -1; >> diff --git a/signal/sigaddset.c b/signal/sigaddset.c >> index d310890..7238df4 100644 >> --- a/signal/sigaddset.c >> +++ b/signal/sigaddset.c >> @@ -17,13 +17,14 @@ >> >> #include <errno.h> >> #include <signal.h> >> -#include <sigsetops.h> >> +#include <internal-signals.h> >> >> /* Add SIGNO to SET. */ >> int >> sigaddset (sigset_t *set, int signo) >> { >> - if (set == NULL || signo <= 0 || signo >= NSIG) >> + if (set == NULL || signo <= 0 || signo >= NSIG >> + || __is_internal_signal (signo)) >> { >> __set_errno (EINVAL); >> return -1; >> diff --git a/signal/sigdelset.c b/signal/sigdelset.c >> index cd83dda..011978c 100644 >> --- a/signal/sigdelset.c >> +++ b/signal/sigdelset.c >> @@ -17,13 +17,14 @@ >> >> #include <errno.h> >> #include <signal.h> >> -#include <sigsetops.h> >> +#include <internal-signals.h> >> >> /* Add SIGNO to SET. */ >> int >> sigdelset (sigset_t *set, int signo) >> { >> - if (set == NULL || signo <= 0 || signo >= NSIG) >> + if (set == NULL || signo <= 0 || signo >= NSIG >> + || __is_internal_signal (signo)) >> { >> __set_errno (EINVAL); >> return -1; >> diff --git a/signal/sigfillset.c b/signal/sigfillset.c >> index e586fd9..83dd583 100644 >> --- a/signal/sigfillset.c >> +++ b/signal/sigfillset.c >> @@ -18,6 +18,7 @@ >> #include <errno.h> >> #include <signal.h> >> #include <string.h> >> +#include <internal-signals.h> >> >> /* Set all signals in SET. */ >> int >> @@ -31,14 +32,7 @@ sigfillset (sigset_t *set) >> >> memset (set, 0xff, sizeof (sigset_t)); >> >> - /* If the implementation uses a cancellation signal don't set the bit. */ >> -#ifdef SIGCANCEL >> - __sigdelset (set, SIGCANCEL); >> -#endif >> - /* Likewise for the signal to implement setxid. */ >> -#ifdef SIGSETXID >> - __sigdelset (set, SIGSETXID); >> -#endif >> + __clear_internal_signals (set); >> >> return 0; >> } >> diff --git a/signal/tst-sigset.c b/signal/tst-sigset.c >> index d47adcc..a2b764d 100644 >> --- a/signal/tst-sigset.c >> +++ b/signal/tst-sigset.c >> @@ -1,43 +1,85 @@ >> /* Test sig*set functions. */ >> >> #include <signal.h> >> -#include <stdio.h> >> >> -#define TEST_FUNCTION do_test () >> +#include <support/check.h> >> + >> static int >> do_test (void) >> { >> - int result = 0; >> - int sig = -1; >> + sigset_t set; >> + TEST_VERIFY (sigemptyset (&set) == 0); >> >> -#define TRY(call) \ >> - if (call) \ >> - { \ >> - printf ("%s (sig = %d): %m\n", #call, sig); \ >> - result = 1; \ >> - } \ >> - else >> +#define VERIFY(set, sig) \ >> + TEST_VERIFY (sigismember (&set, sig) == 0); \ >> + TEST_VERIFY (sigaddset (&set, sig) == 0); \ >> + TEST_VERIFY (sigismember (&set, sig) != 0); \ >> + TEST_VERIFY (sigdelset (&set, sig) == 0); \ >> + TEST_VERIFY (sigismember (&set, sig) == 0) >> >> + /* ISO C99 signals. */ >> + VERIFY (set, SIGINT); >> + VERIFY (set, SIGILL); >> + VERIFY (set, SIGABRT); >> + VERIFY (set, SIGFPE); >> + VERIFY (set, SIGSEGV); >> + VERIFY (set, SIGTERM); >> >> - sigset_t set; >> - TRY (sigemptyset (&set) != 0); >> + /* Historical signals specified by POSIX. */ >> + VERIFY (set, SIGHUP); >> + VERIFY (set, SIGQUIT); >> + VERIFY (set, SIGTRAP); >> + VERIFY (set, SIGKILL); >> + VERIFY (set, SIGBUS); >> + VERIFY (set, SIGSYS); >> + VERIFY (set, SIGPIPE); >> + VERIFY (set, SIGALRM); >> + >> + /* New(er) POSIX signals (1003.1-2008, 1003.1-2013). */ >> + VERIFY (set, SIGURG); >> + VERIFY (set, SIGSTOP); >> + VERIFY (set, SIGTSTP); >> + VERIFY (set, SIGCONT); >> + VERIFY (set, SIGCHLD); >> + VERIFY (set, SIGTTIN); >> + VERIFY (set, SIGTTOU); >> + VERIFY (set, SIGPOLL); >> + VERIFY (set, SIGXCPU); >> + VERIFY (set, SIGXFSZ); >> + VERIFY (set, SIGVTALRM); >> + VERIFY (set, SIGPROF); >> + VERIFY (set, SIGUSR1); >> + VERIFY (set, SIGUSR2); >> + >> + /* Nonstandard signals found in all modern POSIX systems >> + (including both BSD and Linux). */ >> + VERIFY (set, SIGWINCH); >> >> -#ifdef SIGRTMAX >> - int max_sig = SIGRTMAX; >> -#else >> - int max_sig = NSIG - 1; >> + /* Arch-specific signals. */ >> +#ifdef SIGEMT >> + VERIFY (set, SIGEMT); >> +#endif >> +#ifdef SIGLOST >> + VERIFY (set, SIGLOST); >> +#endif >> +#ifdef SIGINFO >> + VERIFY (set, SIGINFO); >> +#endif >> +#ifdef SIGSTKFLT >> + VERIFY (set, SIGSTKFLT); >> +#endif >> +#ifdef SIGPWR >> + VERIFY (set, SIGPWR); >> #endif >> >> - for (sig = 1; sig <= max_sig; ++sig) >> + /* Read-time signals (POSIX.1b real-time extensions). If they are >> + supported SIGRTMAX value is greater than SIGRTMIN. */ >> + for (int rtsig = SIGRTMIN; rtsig <= SIGRTMAX; rtsig++) >> { >> - TRY (sigismember (&set, sig) != 0); >> - TRY (sigaddset (&set, sig) != 0); >> - TRY (sigismember (&set, sig) == 0); >> - TRY (sigdelset (&set, sig) != 0); >> - TRY (sigismember (&set, sig) != 0); >> + VERIFY (set, rtsig); >> } >> >> - return result; >> + return 0; >> } >> >> -#include "../test-skeleton.c" >> +#include <support/test-driver.c> >> diff --git a/sysdeps/generic/internal-signals.h b/sysdeps/generic/internal-signals.h >> index 01e5b75..ab0b22e 100644 >> --- a/sysdeps/generic/internal-signals.h >> +++ b/sysdeps/generic/internal-signals.h >> @@ -15,3 +15,14 @@ >> You should have received a copy of the GNU Lesser General Public >> License along with the GNU C Library; if not, see >> <http://www.gnu.org/licenses/>. */ >> + >> +static inline void >> +__clear_internal_signals (sigset_t *set) >> +{ >> +} >> + >> +static inline bool >> +__is_internal_signal (int sig) >> +{ >> + return false; >> +} >> diff --git a/sysdeps/nptl/sigfillset.c b/sysdeps/nptl/sigfillset.c >> deleted file mode 100644 >> index 94a7680..0000000 >> --- a/sysdeps/nptl/sigfillset.c >> +++ /dev/null >> @@ -1,20 +0,0 @@ >> -/* Copyright (C) 2003-2018 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 >> - <http://www.gnu.org/licenses/>. */ >> - >> -#include <nptl/pthreadP.h> >> - >> -#include <signal/sigfillset.c> >> diff --git a/sysdeps/posix/signal.c b/sysdeps/posix/signal.c >> index a4a0875..8a135c7 100644 >> --- a/sysdeps/posix/signal.c >> +++ b/sysdeps/posix/signal.c >> @@ -18,8 +18,8 @@ >> >> #include <errno.h> >> #include <signal.h> >> -#include <string.h> /* For the real memset prototype. */ >> #include <sigsetops.h> >> +#include <internal-signals.h> >> >> sigset_t _sigintr attribute_hidden; /* Set by siginterrupt. */ >> >> @@ -31,7 +31,8 @@ __bsd_signal (int sig, __sighandler_t handler) >> struct sigaction act, oact; >> >> /* Check signal extents to protect __sigismember. */ >> - if (handler == SIG_ERR || sig < 1 || sig >= NSIG) >> + if (handler == SIG_ERR || sig < 1 || sig >= NSIG >> + || __is_internal_signal (sig)) >> { >> __set_errno (EINVAL); >> return SIG_ERR; >> diff --git a/sysdeps/posix/sigset.c b/sysdeps/posix/sigset.c >> index b62aa3c..6ab4a48 100644 >> --- a/sysdeps/posix/sigset.c >> +++ b/sysdeps/posix/sigset.c >> @@ -31,15 +31,9 @@ sigset (int sig, __sighandler_t disp) >> sigset_t set; >> sigset_t oset; >> >> - /* Check signal extents to protect __sigismember. */ >> - if (disp == SIG_ERR || sig < 1 || sig >= NSIG) >> - { >> - __set_errno (EINVAL); >> - return SIG_ERR; >> - } >> - >> __sigemptyset (&set); >> - __sigaddset (&set, sig); >> + if (sigaddset (&set, sig) < 0) >> + return SIG_ERR; >> >> if (disp == SIG_HOLD) >> { >> diff --git a/sysdeps/unix/sysv/linux/internal-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h >> index e007372..5ff4cf8 100644 >> --- a/sysdeps/unix/sysv/linux/internal-signals.h >> +++ b/sysdeps/unix/sysv/linux/internal-signals.h >> @@ -21,6 +21,8 @@ >> >> #include <signal.h> >> #include <sigsetops.h> >> +#include <stdbool.h> >> +#include <sysdep.h> >> >> /* The signal used for asynchronous cancelation. */ >> #define SIGCANCEL __SIGRTMIN >> @@ -37,7 +39,7 @@ >> >> >> /* Return is sig is used internally. */ >> -static inline int >> +static inline bool >> __is_internal_signal (int sig) >> { >> return (sig == SIGCANCEL) || (sig == SIGSETXID); >> diff --git a/sysdeps/unix/sysv/linux/sigtimedwait.c b/sysdeps/unix/sysv/linux/sigtimedwait.c >> index 051a285..b4de885 100644 >> --- a/sysdeps/unix/sysv/linux/sigtimedwait.c >> +++ b/sysdeps/unix/sysv/linux/sigtimedwait.c >> @@ -24,21 +24,8 @@ int >> __sigtimedwait (const sigset_t *set, siginfo_t *info, >> const struct timespec *timeout) >> { >> - sigset_t tmpset; >> - if (set != NULL >> - && (__builtin_expect (__sigismember (set, SIGCANCEL), 0) >> - || __builtin_expect (__sigismember (set, SIGSETXID), 0))) >> - { >> - /* Create a temporary mask without the bit for SIGCANCEL set. */ >> - // We are not copying more than we have to. >> - memcpy (&tmpset, set, _NSIG / 8); >> - __sigdelset (&tmpset, SIGCANCEL); >> - __sigdelset (&tmpset, SIGSETXID); >> - set = &tmpset; >> - } >> - >> - /* XXX The size argument hopefully will have to be changed to the >> - real size of the user-level sigset_t. */ >> + /* XXX The size argument hopefully will have to be changed to the >> + real size of the user-level sigset_t. */ >> int result = SYSCALL_CANCEL (rt_sigtimedwait, set, info, timeout, _NSIG / 8); >> >> /* The kernel generates a SI_TKILL code in si_code in case tkill is >>
LGTM except I think sysdeps/generic/internal-signals.h should define all of the functions that sysdeps/u/s/l/internal-signals.h does. On Tue, Apr 3, 2018 at 10:24 AM, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > If no one opposes, I will commit this shortly. > > On 19/02/2018 14:50, Adhemerval Zanella wrote: >> Ping. >> >> On 12/02/2018 10:42, Adhemerval Zanella wrote: >>> This patch filters out the internal NPTL signals (SIGCANCEL/SIGTIMER and >>> SIGSETXID) from signal functions. GLIBC on Linux requires both signals to >>> proper implement pthread cancellation, posix timers, and set*id posix >>> thread synchronization. >>> >>> And not filtering out the internal signal is troublesome: >>> >>> - A conformant program on a architecture that does not filter out the >>> signals might inadvertently disable pthread asynchronous cancellation, >>> set*id synchronization or posix timers. >>> >>> - It might also to security issues if SIGSETXID is masked and set*id >>> functions are called (some threads might have effective user or group >>> id different from the rest). >>> >>> The changes are basically: >>> >>> - Change __is_internal_signal to bool and used on all signal function >>> that has a signal number as input. Also for signal function which accepts >>> signals sets (sigset_t) it assumes that canonical function were used to >>> add/remove signals which lead to some input simplification. >>> >>> - Fix tst-sigset.c to avoid check for SIGCANCEL/SIGTIMER and SIGSETXID. >>> It is rewritten to check each signal indidually and to check realtime >>> signals using canonical macros. >>> >>> - Add generic __clear_internal_signals and __is_internal_signal >>> version since both symbols are used on generic implementations. >>> >>> - Remove superflous sysdeps/nptl/sigfillset.c. >>> >>> - Remove superflous SIGTIMER handling on Linux __is_internal_signal >>> since it is the same of SIGCANCEL. >>> >>> - Remove dangling define and obvious comment on nptl/sigaction.c. >>> >>> Checked on x86_64-linux-gnu. >>> >>> [BZ #22391] >>> * nptl/sigaction.c (__sigaction): Use __is_internal_signal to >>> check for internal nptl signals. >>> * signal/sigaddset.c (sigaddset): Likewise. >>> * signal/sigdelset.c (sigdelset): Likewise. >>> * sysdeps/posix/signal.c (__bsd_signal): Likewise. >>> * sysdeps/posix/sigset.c (sigset): Call and check sigaddset return >>> value. >>> * signal/sigfillset.c (sigfillset): User __clear_internal_signals >>> to filter out internal nptl signals. >>> * signal/tst-sigset.c (do_test): Check ech signal indidually and >>> also check realtime signals using standard macros. >>> * sysdeps/nptl/nptl-signals.h (__clear_internal_signals, >>> __is_internal_signal): New functions. >>> * sysdeps/nptl/sigfillset.c: Remove file. >>> * sysdeps/unix/sysv/linux/nptl-signals.h (__is_internal_signal): >>> Change return to bool. >>> (__clear_internal_signals): Remove SIGTIMER clean since it is >>> equal to SIGCANEL on Linux. >>> * sysdeps/unix/sysv/linux/sigtimedwait.c (__sigtimedwait): Assume >>> signal set was constructed using standard functions. >>> * sysdeps/unix/sysv/linux/sigwait.c (do_sigtwait): Likewise. >>> >>> Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> >>> Reported-by: Yury Norov <ynorov@caviumnetworks.com> >>> --- >>> ChangeLog | 23 ++++++++ >>> nptl/sigaction.c | 14 +---- >>> signal/sigaction.c | 2 +- >>> signal/sigaddset.c | 5 +- >>> signal/sigdelset.c | 5 +- >>> signal/sigfillset.c | 10 +--- >>> signal/tst-sigset.c | 92 ++++++++++++++++++++++-------- >>> sysdeps/generic/internal-signals.h | 11 ++++ >>> sysdeps/nptl/sigfillset.c | 20 ------- >>> sysdeps/posix/signal.c | 5 +- >>> sysdeps/posix/sigset.c | 10 +--- >>> sysdeps/unix/sysv/linux/internal-signals.h | 4 +- >>> sysdeps/unix/sysv/linux/sigtimedwait.c | 17 +----- >>> 13 files changed, 122 insertions(+), 96 deletions(-) >>> delete mode 100644 sysdeps/nptl/sigfillset.c >>> >>> diff --git a/nptl/sigaction.c b/nptl/sigaction.c >>> index ddf6f5e..79b6fdc 100644 >>> --- a/nptl/sigaction.c >>> +++ b/nptl/sigaction.c >>> @@ -16,22 +16,12 @@ >>> License along with the GNU C Library; if not, see >>> <http://www.gnu.org/licenses/>. */ >>> >>> - >>> -/* This is no complete implementation. The file is meant to be >>> - included in the real implementation to provide the wrapper around >>> - __libc_sigaction. */ >>> - >>> -#include <nptl/pthreadP.h> >>> - >>> -/* We use the libc implementation but we tell it to not allow >>> - SIGCANCEL or SIGTIMER to be handled. */ >>> -#define LIBC_SIGACTION 1 >>> - >>> +#include <internal-signals.h> >>> >>> int >>> __sigaction (int sig, const struct sigaction *act, struct sigaction *oact) >>> { >>> - if (__glibc_unlikely (sig == SIGCANCEL || sig == SIGSETXID)) >>> + if (sig <= 0 || sig >= NSIG || __is_internal_signal (sig)) >>> { >>> __set_errno (EINVAL); >>> return -1; >>> diff --git a/signal/sigaction.c b/signal/sigaction.c >>> index f761ca2..c99001a 100644 >>> --- a/signal/sigaction.c >>> +++ b/signal/sigaction.c >>> @@ -24,7 +24,7 @@ >>> int >>> __sigaction (int sig, const struct sigaction *act, struct sigaction *oact) >>> { >>> - if (sig <= 0 || sig >= NSIG) >>> + if (sig <= 0 || sig >= NSIG || __is_internal_signal (sig)) >>> { >>> __set_errno (EINVAL); >>> return -1; >>> diff --git a/signal/sigaddset.c b/signal/sigaddset.c >>> index d310890..7238df4 100644 >>> --- a/signal/sigaddset.c >>> +++ b/signal/sigaddset.c >>> @@ -17,13 +17,14 @@ >>> >>> #include <errno.h> >>> #include <signal.h> >>> -#include <sigsetops.h> >>> +#include <internal-signals.h> >>> >>> /* Add SIGNO to SET. */ >>> int >>> sigaddset (sigset_t *set, int signo) >>> { >>> - if (set == NULL || signo <= 0 || signo >= NSIG) >>> + if (set == NULL || signo <= 0 || signo >= NSIG >>> + || __is_internal_signal (signo)) >>> { >>> __set_errno (EINVAL); >>> return -1; >>> diff --git a/signal/sigdelset.c b/signal/sigdelset.c >>> index cd83dda..011978c 100644 >>> --- a/signal/sigdelset.c >>> +++ b/signal/sigdelset.c >>> @@ -17,13 +17,14 @@ >>> >>> #include <errno.h> >>> #include <signal.h> >>> -#include <sigsetops.h> >>> +#include <internal-signals.h> >>> >>> /* Add SIGNO to SET. */ >>> int >>> sigdelset (sigset_t *set, int signo) >>> { >>> - if (set == NULL || signo <= 0 || signo >= NSIG) >>> + if (set == NULL || signo <= 0 || signo >= NSIG >>> + || __is_internal_signal (signo)) >>> { >>> __set_errno (EINVAL); >>> return -1; >>> diff --git a/signal/sigfillset.c b/signal/sigfillset.c >>> index e586fd9..83dd583 100644 >>> --- a/signal/sigfillset.c >>> +++ b/signal/sigfillset.c >>> @@ -18,6 +18,7 @@ >>> #include <errno.h> >>> #include <signal.h> >>> #include <string.h> >>> +#include <internal-signals.h> >>> >>> /* Set all signals in SET. */ >>> int >>> @@ -31,14 +32,7 @@ sigfillset (sigset_t *set) >>> >>> memset (set, 0xff, sizeof (sigset_t)); >>> >>> - /* If the implementation uses a cancellation signal don't set the bit. */ >>> -#ifdef SIGCANCEL >>> - __sigdelset (set, SIGCANCEL); >>> -#endif >>> - /* Likewise for the signal to implement setxid. */ >>> -#ifdef SIGSETXID >>> - __sigdelset (set, SIGSETXID); >>> -#endif >>> + __clear_internal_signals (set); >>> >>> return 0; >>> } >>> diff --git a/signal/tst-sigset.c b/signal/tst-sigset.c >>> index d47adcc..a2b764d 100644 >>> --- a/signal/tst-sigset.c >>> +++ b/signal/tst-sigset.c >>> @@ -1,43 +1,85 @@ >>> /* Test sig*set functions. */ >>> >>> #include <signal.h> >>> -#include <stdio.h> >>> >>> -#define TEST_FUNCTION do_test () >>> +#include <support/check.h> >>> + >>> static int >>> do_test (void) >>> { >>> - int result = 0; >>> - int sig = -1; >>> + sigset_t set; >>> + TEST_VERIFY (sigemptyset (&set) == 0); >>> >>> -#define TRY(call) \ >>> - if (call) \ >>> - { \ >>> - printf ("%s (sig = %d): %m\n", #call, sig); \ >>> - result = 1; \ >>> - } \ >>> - else >>> +#define VERIFY(set, sig) \ >>> + TEST_VERIFY (sigismember (&set, sig) == 0); \ >>> + TEST_VERIFY (sigaddset (&set, sig) == 0); \ >>> + TEST_VERIFY (sigismember (&set, sig) != 0); \ >>> + TEST_VERIFY (sigdelset (&set, sig) == 0); \ >>> + TEST_VERIFY (sigismember (&set, sig) == 0) >>> >>> + /* ISO C99 signals. */ >>> + VERIFY (set, SIGINT); >>> + VERIFY (set, SIGILL); >>> + VERIFY (set, SIGABRT); >>> + VERIFY (set, SIGFPE); >>> + VERIFY (set, SIGSEGV); >>> + VERIFY (set, SIGTERM); >>> >>> - sigset_t set; >>> - TRY (sigemptyset (&set) != 0); >>> + /* Historical signals specified by POSIX. */ >>> + VERIFY (set, SIGHUP); >>> + VERIFY (set, SIGQUIT); >>> + VERIFY (set, SIGTRAP); >>> + VERIFY (set, SIGKILL); >>> + VERIFY (set, SIGBUS); >>> + VERIFY (set, SIGSYS); >>> + VERIFY (set, SIGPIPE); >>> + VERIFY (set, SIGALRM); >>> + >>> + /* New(er) POSIX signals (1003.1-2008, 1003.1-2013). */ >>> + VERIFY (set, SIGURG); >>> + VERIFY (set, SIGSTOP); >>> + VERIFY (set, SIGTSTP); >>> + VERIFY (set, SIGCONT); >>> + VERIFY (set, SIGCHLD); >>> + VERIFY (set, SIGTTIN); >>> + VERIFY (set, SIGTTOU); >>> + VERIFY (set, SIGPOLL); >>> + VERIFY (set, SIGXCPU); >>> + VERIFY (set, SIGXFSZ); >>> + VERIFY (set, SIGVTALRM); >>> + VERIFY (set, SIGPROF); >>> + VERIFY (set, SIGUSR1); >>> + VERIFY (set, SIGUSR2); >>> + >>> + /* Nonstandard signals found in all modern POSIX systems >>> + (including both BSD and Linux). */ >>> + VERIFY (set, SIGWINCH); >>> >>> -#ifdef SIGRTMAX >>> - int max_sig = SIGRTMAX; >>> -#else >>> - int max_sig = NSIG - 1; >>> + /* Arch-specific signals. */ >>> +#ifdef SIGEMT >>> + VERIFY (set, SIGEMT); >>> +#endif >>> +#ifdef SIGLOST >>> + VERIFY (set, SIGLOST); >>> +#endif >>> +#ifdef SIGINFO >>> + VERIFY (set, SIGINFO); >>> +#endif >>> +#ifdef SIGSTKFLT >>> + VERIFY (set, SIGSTKFLT); >>> +#endif >>> +#ifdef SIGPWR >>> + VERIFY (set, SIGPWR); >>> #endif >>> >>> - for (sig = 1; sig <= max_sig; ++sig) >>> + /* Read-time signals (POSIX.1b real-time extensions). If they are >>> + supported SIGRTMAX value is greater than SIGRTMIN. */ >>> + for (int rtsig = SIGRTMIN; rtsig <= SIGRTMAX; rtsig++) >>> { >>> - TRY (sigismember (&set, sig) != 0); >>> - TRY (sigaddset (&set, sig) != 0); >>> - TRY (sigismember (&set, sig) == 0); >>> - TRY (sigdelset (&set, sig) != 0); >>> - TRY (sigismember (&set, sig) != 0); >>> + VERIFY (set, rtsig); >>> } >>> >>> - return result; >>> + return 0; >>> } >>> >>> -#include "../test-skeleton.c" >>> +#include <support/test-driver.c> >>> diff --git a/sysdeps/generic/internal-signals.h b/sysdeps/generic/internal-signals.h >>> index 01e5b75..ab0b22e 100644 >>> --- a/sysdeps/generic/internal-signals.h >>> +++ b/sysdeps/generic/internal-signals.h >>> @@ -15,3 +15,14 @@ >>> You should have received a copy of the GNU Lesser General Public >>> License along with the GNU C Library; if not, see >>> <http://www.gnu.org/licenses/>. */ >>> + >>> +static inline void >>> +__clear_internal_signals (sigset_t *set) >>> +{ >>> +} >>> + >>> +static inline bool >>> +__is_internal_signal (int sig) >>> +{ >>> + return false; >>> +} >>> diff --git a/sysdeps/nptl/sigfillset.c b/sysdeps/nptl/sigfillset.c >>> deleted file mode 100644 >>> index 94a7680..0000000 >>> --- a/sysdeps/nptl/sigfillset.c >>> +++ /dev/null >>> @@ -1,20 +0,0 @@ >>> -/* Copyright (C) 2003-2018 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 >>> - <http://www.gnu.org/licenses/>. */ >>> - >>> -#include <nptl/pthreadP.h> >>> - >>> -#include <signal/sigfillset.c> >>> diff --git a/sysdeps/posix/signal.c b/sysdeps/posix/signal.c >>> index a4a0875..8a135c7 100644 >>> --- a/sysdeps/posix/signal.c >>> +++ b/sysdeps/posix/signal.c >>> @@ -18,8 +18,8 @@ >>> >>> #include <errno.h> >>> #include <signal.h> >>> -#include <string.h> /* For the real memset prototype. */ >>> #include <sigsetops.h> >>> +#include <internal-signals.h> >>> >>> sigset_t _sigintr attribute_hidden; /* Set by siginterrupt. */ >>> >>> @@ -31,7 +31,8 @@ __bsd_signal (int sig, __sighandler_t handler) >>> struct sigaction act, oact; >>> >>> /* Check signal extents to protect __sigismember. */ >>> - if (handler == SIG_ERR || sig < 1 || sig >= NSIG) >>> + if (handler == SIG_ERR || sig < 1 || sig >= NSIG >>> + || __is_internal_signal (sig)) >>> { >>> __set_errno (EINVAL); >>> return SIG_ERR; >>> diff --git a/sysdeps/posix/sigset.c b/sysdeps/posix/sigset.c >>> index b62aa3c..6ab4a48 100644 >>> --- a/sysdeps/posix/sigset.c >>> +++ b/sysdeps/posix/sigset.c >>> @@ -31,15 +31,9 @@ sigset (int sig, __sighandler_t disp) >>> sigset_t set; >>> sigset_t oset; >>> >>> - /* Check signal extents to protect __sigismember. */ >>> - if (disp == SIG_ERR || sig < 1 || sig >= NSIG) >>> - { >>> - __set_errno (EINVAL); >>> - return SIG_ERR; >>> - } >>> - >>> __sigemptyset (&set); >>> - __sigaddset (&set, sig); >>> + if (sigaddset (&set, sig) < 0) >>> + return SIG_ERR; >>> >>> if (disp == SIG_HOLD) >>> { >>> diff --git a/sysdeps/unix/sysv/linux/internal-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h >>> index e007372..5ff4cf8 100644 >>> --- a/sysdeps/unix/sysv/linux/internal-signals.h >>> +++ b/sysdeps/unix/sysv/linux/internal-signals.h >>> @@ -21,6 +21,8 @@ >>> >>> #include <signal.h> >>> #include <sigsetops.h> >>> +#include <stdbool.h> >>> +#include <sysdep.h> >>> >>> /* The signal used for asynchronous cancelation. */ >>> #define SIGCANCEL __SIGRTMIN >>> @@ -37,7 +39,7 @@ >>> >>> >>> /* Return is sig is used internally. */ >>> -static inline int >>> +static inline bool >>> __is_internal_signal (int sig) >>> { >>> return (sig == SIGCANCEL) || (sig == SIGSETXID); >>> diff --git a/sysdeps/unix/sysv/linux/sigtimedwait.c b/sysdeps/unix/sysv/linux/sigtimedwait.c >>> index 051a285..b4de885 100644 >>> --- a/sysdeps/unix/sysv/linux/sigtimedwait.c >>> +++ b/sysdeps/unix/sysv/linux/sigtimedwait.c >>> @@ -24,21 +24,8 @@ int >>> __sigtimedwait (const sigset_t *set, siginfo_t *info, >>> const struct timespec *timeout) >>> { >>> - sigset_t tmpset; >>> - if (set != NULL >>> - && (__builtin_expect (__sigismember (set, SIGCANCEL), 0) >>> - || __builtin_expect (__sigismember (set, SIGSETXID), 0))) >>> - { >>> - /* Create a temporary mask without the bit for SIGCANCEL set. */ >>> - // We are not copying more than we have to. >>> - memcpy (&tmpset, set, _NSIG / 8); >>> - __sigdelset (&tmpset, SIGCANCEL); >>> - __sigdelset (&tmpset, SIGSETXID); >>> - set = &tmpset; >>> - } >>> - >>> - /* XXX The size argument hopefully will have to be changed to the >>> - real size of the user-level sigset_t. */ >>> + /* XXX The size argument hopefully will have to be changed to the >>> + real size of the user-level sigset_t. */ >>> int result = SYSCALL_CANCEL (rt_sigtimedwait, set, info, timeout, _NSIG / 8); >>> >>> /* The kernel generates a SI_TKILL code in si_code in case tkill is >>>
Right, I will fix it. On 03/04/2018 12:12, Zack Weinberg wrote: > LGTM except I think sysdeps/generic/internal-signals.h should define > all of the functions that sysdeps/u/s/l/internal-signals.h does. > > On Tue, Apr 3, 2018 at 10:24 AM, Adhemerval Zanella > <adhemerval.zanella@linaro.org> wrote: >> If no one opposes, I will commit this shortly. >> >> On 19/02/2018 14:50, Adhemerval Zanella wrote: >>> Ping. >>> >>> On 12/02/2018 10:42, Adhemerval Zanella wrote: >>>> This patch filters out the internal NPTL signals (SIGCANCEL/SIGTIMER and >>>> SIGSETXID) from signal functions. GLIBC on Linux requires both signals to >>>> proper implement pthread cancellation, posix timers, and set*id posix >>>> thread synchronization. >>>> >>>> And not filtering out the internal signal is troublesome: >>>> >>>> - A conformant program on a architecture that does not filter out the >>>> signals might inadvertently disable pthread asynchronous cancellation, >>>> set*id synchronization or posix timers. >>>> >>>> - It might also to security issues if SIGSETXID is masked and set*id >>>> functions are called (some threads might have effective user or group >>>> id different from the rest). >>>> >>>> The changes are basically: >>>> >>>> - Change __is_internal_signal to bool and used on all signal function >>>> that has a signal number as input. Also for signal function which accepts >>>> signals sets (sigset_t) it assumes that canonical function were used to >>>> add/remove signals which lead to some input simplification. >>>> >>>> - Fix tst-sigset.c to avoid check for SIGCANCEL/SIGTIMER and SIGSETXID. >>>> It is rewritten to check each signal indidually and to check realtime >>>> signals using canonical macros. >>>> >>>> - Add generic __clear_internal_signals and __is_internal_signal >>>> version since both symbols are used on generic implementations. >>>> >>>> - Remove superflous sysdeps/nptl/sigfillset.c. >>>> >>>> - Remove superflous SIGTIMER handling on Linux __is_internal_signal >>>> since it is the same of SIGCANCEL. >>>> >>>> - Remove dangling define and obvious comment on nptl/sigaction.c. >>>> >>>> Checked on x86_64-linux-gnu. >>>> >>>> [BZ #22391] >>>> * nptl/sigaction.c (__sigaction): Use __is_internal_signal to >>>> check for internal nptl signals. >>>> * signal/sigaddset.c (sigaddset): Likewise. >>>> * signal/sigdelset.c (sigdelset): Likewise. >>>> * sysdeps/posix/signal.c (__bsd_signal): Likewise. >>>> * sysdeps/posix/sigset.c (sigset): Call and check sigaddset return >>>> value. >>>> * signal/sigfillset.c (sigfillset): User __clear_internal_signals >>>> to filter out internal nptl signals. >>>> * signal/tst-sigset.c (do_test): Check ech signal indidually and >>>> also check realtime signals using standard macros. >>>> * sysdeps/nptl/nptl-signals.h (__clear_internal_signals, >>>> __is_internal_signal): New functions. >>>> * sysdeps/nptl/sigfillset.c: Remove file. >>>> * sysdeps/unix/sysv/linux/nptl-signals.h (__is_internal_signal): >>>> Change return to bool. >>>> (__clear_internal_signals): Remove SIGTIMER clean since it is >>>> equal to SIGCANEL on Linux. >>>> * sysdeps/unix/sysv/linux/sigtimedwait.c (__sigtimedwait): Assume >>>> signal set was constructed using standard functions. >>>> * sysdeps/unix/sysv/linux/sigwait.c (do_sigtwait): Likewise. >>>> >>>> Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> >>>> Reported-by: Yury Norov <ynorov@caviumnetworks.com> >>>> --- >>>> ChangeLog | 23 ++++++++ >>>> nptl/sigaction.c | 14 +---- >>>> signal/sigaction.c | 2 +- >>>> signal/sigaddset.c | 5 +- >>>> signal/sigdelset.c | 5 +- >>>> signal/sigfillset.c | 10 +--- >>>> signal/tst-sigset.c | 92 ++++++++++++++++++++++-------- >>>> sysdeps/generic/internal-signals.h | 11 ++++ >>>> sysdeps/nptl/sigfillset.c | 20 ------- >>>> sysdeps/posix/signal.c | 5 +- >>>> sysdeps/posix/sigset.c | 10 +--- >>>> sysdeps/unix/sysv/linux/internal-signals.h | 4 +- >>>> sysdeps/unix/sysv/linux/sigtimedwait.c | 17 +----- >>>> 13 files changed, 122 insertions(+), 96 deletions(-) >>>> delete mode 100644 sysdeps/nptl/sigfillset.c >>>> >>>> diff --git a/nptl/sigaction.c b/nptl/sigaction.c >>>> index ddf6f5e..79b6fdc 100644 >>>> --- a/nptl/sigaction.c >>>> +++ b/nptl/sigaction.c >>>> @@ -16,22 +16,12 @@ >>>> License along with the GNU C Library; if not, see >>>> <http://www.gnu.org/licenses/>. */ >>>> >>>> - >>>> -/* This is no complete implementation. The file is meant to be >>>> - included in the real implementation to provide the wrapper around >>>> - __libc_sigaction. */ >>>> - >>>> -#include <nptl/pthreadP.h> >>>> - >>>> -/* We use the libc implementation but we tell it to not allow >>>> - SIGCANCEL or SIGTIMER to be handled. */ >>>> -#define LIBC_SIGACTION 1 >>>> - >>>> +#include <internal-signals.h> >>>> >>>> int >>>> __sigaction (int sig, const struct sigaction *act, struct sigaction *oact) >>>> { >>>> - if (__glibc_unlikely (sig == SIGCANCEL || sig == SIGSETXID)) >>>> + if (sig <= 0 || sig >= NSIG || __is_internal_signal (sig)) >>>> { >>>> __set_errno (EINVAL); >>>> return -1; >>>> diff --git a/signal/sigaction.c b/signal/sigaction.c >>>> index f761ca2..c99001a 100644 >>>> --- a/signal/sigaction.c >>>> +++ b/signal/sigaction.c >>>> @@ -24,7 +24,7 @@ >>>> int >>>> __sigaction (int sig, const struct sigaction *act, struct sigaction *oact) >>>> { >>>> - if (sig <= 0 || sig >= NSIG) >>>> + if (sig <= 0 || sig >= NSIG || __is_internal_signal (sig)) >>>> { >>>> __set_errno (EINVAL); >>>> return -1; >>>> diff --git a/signal/sigaddset.c b/signal/sigaddset.c >>>> index d310890..7238df4 100644 >>>> --- a/signal/sigaddset.c >>>> +++ b/signal/sigaddset.c >>>> @@ -17,13 +17,14 @@ >>>> >>>> #include <errno.h> >>>> #include <signal.h> >>>> -#include <sigsetops.h> >>>> +#include <internal-signals.h> >>>> >>>> /* Add SIGNO to SET. */ >>>> int >>>> sigaddset (sigset_t *set, int signo) >>>> { >>>> - if (set == NULL || signo <= 0 || signo >= NSIG) >>>> + if (set == NULL || signo <= 0 || signo >= NSIG >>>> + || __is_internal_signal (signo)) >>>> { >>>> __set_errno (EINVAL); >>>> return -1; >>>> diff --git a/signal/sigdelset.c b/signal/sigdelset.c >>>> index cd83dda..011978c 100644 >>>> --- a/signal/sigdelset.c >>>> +++ b/signal/sigdelset.c >>>> @@ -17,13 +17,14 @@ >>>> >>>> #include <errno.h> >>>> #include <signal.h> >>>> -#include <sigsetops.h> >>>> +#include <internal-signals.h> >>>> >>>> /* Add SIGNO to SET. */ >>>> int >>>> sigdelset (sigset_t *set, int signo) >>>> { >>>> - if (set == NULL || signo <= 0 || signo >= NSIG) >>>> + if (set == NULL || signo <= 0 || signo >= NSIG >>>> + || __is_internal_signal (signo)) >>>> { >>>> __set_errno (EINVAL); >>>> return -1; >>>> diff --git a/signal/sigfillset.c b/signal/sigfillset.c >>>> index e586fd9..83dd583 100644 >>>> --- a/signal/sigfillset.c >>>> +++ b/signal/sigfillset.c >>>> @@ -18,6 +18,7 @@ >>>> #include <errno.h> >>>> #include <signal.h> >>>> #include <string.h> >>>> +#include <internal-signals.h> >>>> >>>> /* Set all signals in SET. */ >>>> int >>>> @@ -31,14 +32,7 @@ sigfillset (sigset_t *set) >>>> >>>> memset (set, 0xff, sizeof (sigset_t)); >>>> >>>> - /* If the implementation uses a cancellation signal don't set the bit. */ >>>> -#ifdef SIGCANCEL >>>> - __sigdelset (set, SIGCANCEL); >>>> -#endif >>>> - /* Likewise for the signal to implement setxid. */ >>>> -#ifdef SIGSETXID >>>> - __sigdelset (set, SIGSETXID); >>>> -#endif >>>> + __clear_internal_signals (set); >>>> >>>> return 0; >>>> } >>>> diff --git a/signal/tst-sigset.c b/signal/tst-sigset.c >>>> index d47adcc..a2b764d 100644 >>>> --- a/signal/tst-sigset.c >>>> +++ b/signal/tst-sigset.c >>>> @@ -1,43 +1,85 @@ >>>> /* Test sig*set functions. */ >>>> >>>> #include <signal.h> >>>> -#include <stdio.h> >>>> >>>> -#define TEST_FUNCTION do_test () >>>> +#include <support/check.h> >>>> + >>>> static int >>>> do_test (void) >>>> { >>>> - int result = 0; >>>> - int sig = -1; >>>> + sigset_t set; >>>> + TEST_VERIFY (sigemptyset (&set) == 0); >>>> >>>> -#define TRY(call) \ >>>> - if (call) \ >>>> - { \ >>>> - printf ("%s (sig = %d): %m\n", #call, sig); \ >>>> - result = 1; \ >>>> - } \ >>>> - else >>>> +#define VERIFY(set, sig) \ >>>> + TEST_VERIFY (sigismember (&set, sig) == 0); \ >>>> + TEST_VERIFY (sigaddset (&set, sig) == 0); \ >>>> + TEST_VERIFY (sigismember (&set, sig) != 0); \ >>>> + TEST_VERIFY (sigdelset (&set, sig) == 0); \ >>>> + TEST_VERIFY (sigismember (&set, sig) == 0) >>>> >>>> + /* ISO C99 signals. */ >>>> + VERIFY (set, SIGINT); >>>> + VERIFY (set, SIGILL); >>>> + VERIFY (set, SIGABRT); >>>> + VERIFY (set, SIGFPE); >>>> + VERIFY (set, SIGSEGV); >>>> + VERIFY (set, SIGTERM); >>>> >>>> - sigset_t set; >>>> - TRY (sigemptyset (&set) != 0); >>>> + /* Historical signals specified by POSIX. */ >>>> + VERIFY (set, SIGHUP); >>>> + VERIFY (set, SIGQUIT); >>>> + VERIFY (set, SIGTRAP); >>>> + VERIFY (set, SIGKILL); >>>> + VERIFY (set, SIGBUS); >>>> + VERIFY (set, SIGSYS); >>>> + VERIFY (set, SIGPIPE); >>>> + VERIFY (set, SIGALRM); >>>> + >>>> + /* New(er) POSIX signals (1003.1-2008, 1003.1-2013). */ >>>> + VERIFY (set, SIGURG); >>>> + VERIFY (set, SIGSTOP); >>>> + VERIFY (set, SIGTSTP); >>>> + VERIFY (set, SIGCONT); >>>> + VERIFY (set, SIGCHLD); >>>> + VERIFY (set, SIGTTIN); >>>> + VERIFY (set, SIGTTOU); >>>> + VERIFY (set, SIGPOLL); >>>> + VERIFY (set, SIGXCPU); >>>> + VERIFY (set, SIGXFSZ); >>>> + VERIFY (set, SIGVTALRM); >>>> + VERIFY (set, SIGPROF); >>>> + VERIFY (set, SIGUSR1); >>>> + VERIFY (set, SIGUSR2); >>>> + >>>> + /* Nonstandard signals found in all modern POSIX systems >>>> + (including both BSD and Linux). */ >>>> + VERIFY (set, SIGWINCH); >>>> >>>> -#ifdef SIGRTMAX >>>> - int max_sig = SIGRTMAX; >>>> -#else >>>> - int max_sig = NSIG - 1; >>>> + /* Arch-specific signals. */ >>>> +#ifdef SIGEMT >>>> + VERIFY (set, SIGEMT); >>>> +#endif >>>> +#ifdef SIGLOST >>>> + VERIFY (set, SIGLOST); >>>> +#endif >>>> +#ifdef SIGINFO >>>> + VERIFY (set, SIGINFO); >>>> +#endif >>>> +#ifdef SIGSTKFLT >>>> + VERIFY (set, SIGSTKFLT); >>>> +#endif >>>> +#ifdef SIGPWR >>>> + VERIFY (set, SIGPWR); >>>> #endif >>>> >>>> - for (sig = 1; sig <= max_sig; ++sig) >>>> + /* Read-time signals (POSIX.1b real-time extensions). If they are >>>> + supported SIGRTMAX value is greater than SIGRTMIN. */ >>>> + for (int rtsig = SIGRTMIN; rtsig <= SIGRTMAX; rtsig++) >>>> { >>>> - TRY (sigismember (&set, sig) != 0); >>>> - TRY (sigaddset (&set, sig) != 0); >>>> - TRY (sigismember (&set, sig) == 0); >>>> - TRY (sigdelset (&set, sig) != 0); >>>> - TRY (sigismember (&set, sig) != 0); >>>> + VERIFY (set, rtsig); >>>> } >>>> >>>> - return result; >>>> + return 0; >>>> } >>>> >>>> -#include "../test-skeleton.c" >>>> +#include <support/test-driver.c> >>>> diff --git a/sysdeps/generic/internal-signals.h b/sysdeps/generic/internal-signals.h >>>> index 01e5b75..ab0b22e 100644 >>>> --- a/sysdeps/generic/internal-signals.h >>>> +++ b/sysdeps/generic/internal-signals.h >>>> @@ -15,3 +15,14 @@ >>>> You should have received a copy of the GNU Lesser General Public >>>> License along with the GNU C Library; if not, see >>>> <http://www.gnu.org/licenses/>. */ >>>> + >>>> +static inline void >>>> +__clear_internal_signals (sigset_t *set) >>>> +{ >>>> +} >>>> + >>>> +static inline bool >>>> +__is_internal_signal (int sig) >>>> +{ >>>> + return false; >>>> +} >>>> diff --git a/sysdeps/nptl/sigfillset.c b/sysdeps/nptl/sigfillset.c >>>> deleted file mode 100644 >>>> index 94a7680..0000000 >>>> --- a/sysdeps/nptl/sigfillset.c >>>> +++ /dev/null >>>> @@ -1,20 +0,0 @@ >>>> -/* Copyright (C) 2003-2018 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 >>>> - <http://www.gnu.org/licenses/>. */ >>>> - >>>> -#include <nptl/pthreadP.h> >>>> - >>>> -#include <signal/sigfillset.c> >>>> diff --git a/sysdeps/posix/signal.c b/sysdeps/posix/signal.c >>>> index a4a0875..8a135c7 100644 >>>> --- a/sysdeps/posix/signal.c >>>> +++ b/sysdeps/posix/signal.c >>>> @@ -18,8 +18,8 @@ >>>> >>>> #include <errno.h> >>>> #include <signal.h> >>>> -#include <string.h> /* For the real memset prototype. */ >>>> #include <sigsetops.h> >>>> +#include <internal-signals.h> >>>> >>>> sigset_t _sigintr attribute_hidden; /* Set by siginterrupt. */ >>>> >>>> @@ -31,7 +31,8 @@ __bsd_signal (int sig, __sighandler_t handler) >>>> struct sigaction act, oact; >>>> >>>> /* Check signal extents to protect __sigismember. */ >>>> - if (handler == SIG_ERR || sig < 1 || sig >= NSIG) >>>> + if (handler == SIG_ERR || sig < 1 || sig >= NSIG >>>> + || __is_internal_signal (sig)) >>>> { >>>> __set_errno (EINVAL); >>>> return SIG_ERR; >>>> diff --git a/sysdeps/posix/sigset.c b/sysdeps/posix/sigset.c >>>> index b62aa3c..6ab4a48 100644 >>>> --- a/sysdeps/posix/sigset.c >>>> +++ b/sysdeps/posix/sigset.c >>>> @@ -31,15 +31,9 @@ sigset (int sig, __sighandler_t disp) >>>> sigset_t set; >>>> sigset_t oset; >>>> >>>> - /* Check signal extents to protect __sigismember. */ >>>> - if (disp == SIG_ERR || sig < 1 || sig >= NSIG) >>>> - { >>>> - __set_errno (EINVAL); >>>> - return SIG_ERR; >>>> - } >>>> - >>>> __sigemptyset (&set); >>>> - __sigaddset (&set, sig); >>>> + if (sigaddset (&set, sig) < 0) >>>> + return SIG_ERR; >>>> >>>> if (disp == SIG_HOLD) >>>> { >>>> diff --git a/sysdeps/unix/sysv/linux/internal-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h >>>> index e007372..5ff4cf8 100644 >>>> --- a/sysdeps/unix/sysv/linux/internal-signals.h >>>> +++ b/sysdeps/unix/sysv/linux/internal-signals.h >>>> @@ -21,6 +21,8 @@ >>>> >>>> #include <signal.h> >>>> #include <sigsetops.h> >>>> +#include <stdbool.h> >>>> +#include <sysdep.h> >>>> >>>> /* The signal used for asynchronous cancelation. */ >>>> #define SIGCANCEL __SIGRTMIN >>>> @@ -37,7 +39,7 @@ >>>> >>>> >>>> /* Return is sig is used internally. */ >>>> -static inline int >>>> +static inline bool >>>> __is_internal_signal (int sig) >>>> { >>>> return (sig == SIGCANCEL) || (sig == SIGSETXID); >>>> diff --git a/sysdeps/unix/sysv/linux/sigtimedwait.c b/sysdeps/unix/sysv/linux/sigtimedwait.c >>>> index 051a285..b4de885 100644 >>>> --- a/sysdeps/unix/sysv/linux/sigtimedwait.c >>>> +++ b/sysdeps/unix/sysv/linux/sigtimedwait.c >>>> @@ -24,21 +24,8 @@ int >>>> __sigtimedwait (const sigset_t *set, siginfo_t *info, >>>> const struct timespec *timeout) >>>> { >>>> - sigset_t tmpset; >>>> - if (set != NULL >>>> - && (__builtin_expect (__sigismember (set, SIGCANCEL), 0) >>>> - || __builtin_expect (__sigismember (set, SIGSETXID), 0))) >>>> - { >>>> - /* Create a temporary mask without the bit for SIGCANCEL set. */ >>>> - // We are not copying more than we have to. >>>> - memcpy (&tmpset, set, _NSIG / 8); >>>> - __sigdelset (&tmpset, SIGCANCEL); >>>> - __sigdelset (&tmpset, SIGSETXID); >>>> - set = &tmpset; >>>> - } >>>> - >>>> - /* XXX The size argument hopefully will have to be changed to the >>>> - real size of the user-level sigset_t. */ >>>> + /* XXX The size argument hopefully will have to be changed to the >>>> + real size of the user-level sigset_t. */ >>>> int result = SYSCALL_CANCEL (rt_sigtimedwait, set, info, timeout, _NSIG / 8); >>>> >>>> /* The kernel generates a SI_TKILL code in si_code in case tkill is >>>>
diff --git a/nptl/sigaction.c b/nptl/sigaction.c index ddf6f5e..79b6fdc 100644 --- a/nptl/sigaction.c +++ b/nptl/sigaction.c @@ -16,22 +16,12 @@ License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ - -/* This is no complete implementation. The file is meant to be - included in the real implementation to provide the wrapper around - __libc_sigaction. */ - -#include <nptl/pthreadP.h> - -/* We use the libc implementation but we tell it to not allow - SIGCANCEL or SIGTIMER to be handled. */ -#define LIBC_SIGACTION 1 - +#include <internal-signals.h> int __sigaction (int sig, const struct sigaction *act, struct sigaction *oact) { - if (__glibc_unlikely (sig == SIGCANCEL || sig == SIGSETXID)) + if (sig <= 0 || sig >= NSIG || __is_internal_signal (sig)) { __set_errno (EINVAL); return -1; diff --git a/signal/sigaction.c b/signal/sigaction.c index f761ca2..c99001a 100644 --- a/signal/sigaction.c +++ b/signal/sigaction.c @@ -24,7 +24,7 @@ int __sigaction (int sig, const struct sigaction *act, struct sigaction *oact) { - if (sig <= 0 || sig >= NSIG) + if (sig <= 0 || sig >= NSIG || __is_internal_signal (sig)) { __set_errno (EINVAL); return -1; diff --git a/signal/sigaddset.c b/signal/sigaddset.c index d310890..7238df4 100644 --- a/signal/sigaddset.c +++ b/signal/sigaddset.c @@ -17,13 +17,14 @@ #include <errno.h> #include <signal.h> -#include <sigsetops.h> +#include <internal-signals.h> /* Add SIGNO to SET. */ int sigaddset (sigset_t *set, int signo) { - if (set == NULL || signo <= 0 || signo >= NSIG) + if (set == NULL || signo <= 0 || signo >= NSIG + || __is_internal_signal (signo)) { __set_errno (EINVAL); return -1; diff --git a/signal/sigdelset.c b/signal/sigdelset.c index cd83dda..011978c 100644 --- a/signal/sigdelset.c +++ b/signal/sigdelset.c @@ -17,13 +17,14 @@ #include <errno.h> #include <signal.h> -#include <sigsetops.h> +#include <internal-signals.h> /* Add SIGNO to SET. */ int sigdelset (sigset_t *set, int signo) { - if (set == NULL || signo <= 0 || signo >= NSIG) + if (set == NULL || signo <= 0 || signo >= NSIG + || __is_internal_signal (signo)) { __set_errno (EINVAL); return -1; diff --git a/signal/sigfillset.c b/signal/sigfillset.c index e586fd9..83dd583 100644 --- a/signal/sigfillset.c +++ b/signal/sigfillset.c @@ -18,6 +18,7 @@ #include <errno.h> #include <signal.h> #include <string.h> +#include <internal-signals.h> /* Set all signals in SET. */ int @@ -31,14 +32,7 @@ sigfillset (sigset_t *set) memset (set, 0xff, sizeof (sigset_t)); - /* If the implementation uses a cancellation signal don't set the bit. */ -#ifdef SIGCANCEL - __sigdelset (set, SIGCANCEL); -#endif - /* Likewise for the signal to implement setxid. */ -#ifdef SIGSETXID - __sigdelset (set, SIGSETXID); -#endif + __clear_internal_signals (set); return 0; } diff --git a/signal/tst-sigset.c b/signal/tst-sigset.c index d47adcc..a2b764d 100644 --- a/signal/tst-sigset.c +++ b/signal/tst-sigset.c @@ -1,43 +1,85 @@ /* Test sig*set functions. */ #include <signal.h> -#include <stdio.h> -#define TEST_FUNCTION do_test () +#include <support/check.h> + static int do_test (void) { - int result = 0; - int sig = -1; + sigset_t set; + TEST_VERIFY (sigemptyset (&set) == 0); -#define TRY(call) \ - if (call) \ - { \ - printf ("%s (sig = %d): %m\n", #call, sig); \ - result = 1; \ - } \ - else +#define VERIFY(set, sig) \ + TEST_VERIFY (sigismember (&set, sig) == 0); \ + TEST_VERIFY (sigaddset (&set, sig) == 0); \ + TEST_VERIFY (sigismember (&set, sig) != 0); \ + TEST_VERIFY (sigdelset (&set, sig) == 0); \ + TEST_VERIFY (sigismember (&set, sig) == 0) + /* ISO C99 signals. */ + VERIFY (set, SIGINT); + VERIFY (set, SIGILL); + VERIFY (set, SIGABRT); + VERIFY (set, SIGFPE); + VERIFY (set, SIGSEGV); + VERIFY (set, SIGTERM); - sigset_t set; - TRY (sigemptyset (&set) != 0); + /* Historical signals specified by POSIX. */ + VERIFY (set, SIGHUP); + VERIFY (set, SIGQUIT); + VERIFY (set, SIGTRAP); + VERIFY (set, SIGKILL); + VERIFY (set, SIGBUS); + VERIFY (set, SIGSYS); + VERIFY (set, SIGPIPE); + VERIFY (set, SIGALRM); + + /* New(er) POSIX signals (1003.1-2008, 1003.1-2013). */ + VERIFY (set, SIGURG); + VERIFY (set, SIGSTOP); + VERIFY (set, SIGTSTP); + VERIFY (set, SIGCONT); + VERIFY (set, SIGCHLD); + VERIFY (set, SIGTTIN); + VERIFY (set, SIGTTOU); + VERIFY (set, SIGPOLL); + VERIFY (set, SIGXCPU); + VERIFY (set, SIGXFSZ); + VERIFY (set, SIGVTALRM); + VERIFY (set, SIGPROF); + VERIFY (set, SIGUSR1); + VERIFY (set, SIGUSR2); + + /* Nonstandard signals found in all modern POSIX systems + (including both BSD and Linux). */ + VERIFY (set, SIGWINCH); -#ifdef SIGRTMAX - int max_sig = SIGRTMAX; -#else - int max_sig = NSIG - 1; + /* Arch-specific signals. */ +#ifdef SIGEMT + VERIFY (set, SIGEMT); +#endif +#ifdef SIGLOST + VERIFY (set, SIGLOST); +#endif +#ifdef SIGINFO + VERIFY (set, SIGINFO); +#endif +#ifdef SIGSTKFLT + VERIFY (set, SIGSTKFLT); +#endif +#ifdef SIGPWR + VERIFY (set, SIGPWR); #endif - for (sig = 1; sig <= max_sig; ++sig) + /* Read-time signals (POSIX.1b real-time extensions). If they are + supported SIGRTMAX value is greater than SIGRTMIN. */ + for (int rtsig = SIGRTMIN; rtsig <= SIGRTMAX; rtsig++) { - TRY (sigismember (&set, sig) != 0); - TRY (sigaddset (&set, sig) != 0); - TRY (sigismember (&set, sig) == 0); - TRY (sigdelset (&set, sig) != 0); - TRY (sigismember (&set, sig) != 0); + VERIFY (set, rtsig); } - return result; + return 0; } -#include "../test-skeleton.c" +#include <support/test-driver.c> diff --git a/sysdeps/generic/internal-signals.h b/sysdeps/generic/internal-signals.h index 01e5b75..ab0b22e 100644 --- a/sysdeps/generic/internal-signals.h +++ b/sysdeps/generic/internal-signals.h @@ -15,3 +15,14 @@ You should have received a copy of the GNU Lesser General Public License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ + +static inline void +__clear_internal_signals (sigset_t *set) +{ +} + +static inline bool +__is_internal_signal (int sig) +{ + return false; +} diff --git a/sysdeps/nptl/sigfillset.c b/sysdeps/nptl/sigfillset.c deleted file mode 100644 index 94a7680..0000000 --- a/sysdeps/nptl/sigfillset.c +++ /dev/null @@ -1,20 +0,0 @@ -/* Copyright (C) 2003-2018 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 - <http://www.gnu.org/licenses/>. */ - -#include <nptl/pthreadP.h> - -#include <signal/sigfillset.c> diff --git a/sysdeps/posix/signal.c b/sysdeps/posix/signal.c index a4a0875..8a135c7 100644 --- a/sysdeps/posix/signal.c +++ b/sysdeps/posix/signal.c @@ -18,8 +18,8 @@ #include <errno.h> #include <signal.h> -#include <string.h> /* For the real memset prototype. */ #include <sigsetops.h> +#include <internal-signals.h> sigset_t _sigintr attribute_hidden; /* Set by siginterrupt. */ @@ -31,7 +31,8 @@ __bsd_signal (int sig, __sighandler_t handler) struct sigaction act, oact; /* Check signal extents to protect __sigismember. */ - if (handler == SIG_ERR || sig < 1 || sig >= NSIG) + if (handler == SIG_ERR || sig < 1 || sig >= NSIG + || __is_internal_signal (sig)) { __set_errno (EINVAL); return SIG_ERR; diff --git a/sysdeps/posix/sigset.c b/sysdeps/posix/sigset.c index b62aa3c..6ab4a48 100644 --- a/sysdeps/posix/sigset.c +++ b/sysdeps/posix/sigset.c @@ -31,15 +31,9 @@ sigset (int sig, __sighandler_t disp) sigset_t set; sigset_t oset; - /* Check signal extents to protect __sigismember. */ - if (disp == SIG_ERR || sig < 1 || sig >= NSIG) - { - __set_errno (EINVAL); - return SIG_ERR; - } - __sigemptyset (&set); - __sigaddset (&set, sig); + if (sigaddset (&set, sig) < 0) + return SIG_ERR; if (disp == SIG_HOLD) { diff --git a/sysdeps/unix/sysv/linux/internal-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h index e007372..5ff4cf8 100644 --- a/sysdeps/unix/sysv/linux/internal-signals.h +++ b/sysdeps/unix/sysv/linux/internal-signals.h @@ -21,6 +21,8 @@ #include <signal.h> #include <sigsetops.h> +#include <stdbool.h> +#include <sysdep.h> /* The signal used for asynchronous cancelation. */ #define SIGCANCEL __SIGRTMIN @@ -37,7 +39,7 @@ /* Return is sig is used internally. */ -static inline int +static inline bool __is_internal_signal (int sig) { return (sig == SIGCANCEL) || (sig == SIGSETXID); diff --git a/sysdeps/unix/sysv/linux/sigtimedwait.c b/sysdeps/unix/sysv/linux/sigtimedwait.c index 051a285..b4de885 100644 --- a/sysdeps/unix/sysv/linux/sigtimedwait.c +++ b/sysdeps/unix/sysv/linux/sigtimedwait.c @@ -24,21 +24,8 @@ int __sigtimedwait (const sigset_t *set, siginfo_t *info, const struct timespec *timeout) { - sigset_t tmpset; - if (set != NULL - && (__builtin_expect (__sigismember (set, SIGCANCEL), 0) - || __builtin_expect (__sigismember (set, SIGSETXID), 0))) - { - /* Create a temporary mask without the bit for SIGCANCEL set. */ - // We are not copying more than we have to. - memcpy (&tmpset, set, _NSIG / 8); - __sigdelset (&tmpset, SIGCANCEL); - __sigdelset (&tmpset, SIGSETXID); - set = &tmpset; - } - - /* XXX The size argument hopefully will have to be changed to the - real size of the user-level sigset_t. */ + /* XXX The size argument hopefully will have to be changed to the + real size of the user-level sigset_t. */ int result = SYSCALL_CANCEL (rt_sigtimedwait, set, info, timeout, _NSIG / 8); /* The kernel generates a SI_TKILL code in si_code in case tkill is
This patch filters out the internal NPTL signals (SIGCANCEL/SIGTIMER and SIGSETXID) from signal functions. GLIBC on Linux requires both signals to proper implement pthread cancellation, posix timers, and set*id posix thread synchronization. And not filtering out the internal signal is troublesome: - A conformant program on a architecture that does not filter out the signals might inadvertently disable pthread asynchronous cancellation, set*id synchronization or posix timers. - It might also to security issues if SIGSETXID is masked and set*id functions are called (some threads might have effective user or group id different from the rest). The changes are basically: - Change __is_internal_signal to bool and used on all signal function that has a signal number as input. Also for signal function which accepts signals sets (sigset_t) it assumes that canonical function were used to add/remove signals which lead to some input simplification. - Fix tst-sigset.c to avoid check for SIGCANCEL/SIGTIMER and SIGSETXID. It is rewritten to check each signal indidually and to check realtime signals using canonical macros. - Add generic __clear_internal_signals and __is_internal_signal version since both symbols are used on generic implementations. - Remove superflous sysdeps/nptl/sigfillset.c. - Remove superflous SIGTIMER handling on Linux __is_internal_signal since it is the same of SIGCANCEL. - Remove dangling define and obvious comment on nptl/sigaction.c. Checked on x86_64-linux-gnu. [BZ #22391] * nptl/sigaction.c (__sigaction): Use __is_internal_signal to check for internal nptl signals. * signal/sigaddset.c (sigaddset): Likewise. * signal/sigdelset.c (sigdelset): Likewise. * sysdeps/posix/signal.c (__bsd_signal): Likewise. * sysdeps/posix/sigset.c (sigset): Call and check sigaddset return value. * signal/sigfillset.c (sigfillset): User __clear_internal_signals to filter out internal nptl signals. * signal/tst-sigset.c (do_test): Check ech signal indidually and also check realtime signals using standard macros. * sysdeps/nptl/nptl-signals.h (__clear_internal_signals, __is_internal_signal): New functions. * sysdeps/nptl/sigfillset.c: Remove file. * sysdeps/unix/sysv/linux/nptl-signals.h (__is_internal_signal): Change return to bool. (__clear_internal_signals): Remove SIGTIMER clean since it is equal to SIGCANEL on Linux. * sysdeps/unix/sysv/linux/sigtimedwait.c (__sigtimedwait): Assume signal set was constructed using standard functions. * sysdeps/unix/sysv/linux/sigwait.c (do_sigtwait): Likewise. Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> Reported-by: Yury Norov <ynorov@caviumnetworks.com> --- ChangeLog | 23 ++++++++ nptl/sigaction.c | 14 +---- signal/sigaction.c | 2 +- signal/sigaddset.c | 5 +- signal/sigdelset.c | 5 +- signal/sigfillset.c | 10 +--- signal/tst-sigset.c | 92 ++++++++++++++++++++++-------- sysdeps/generic/internal-signals.h | 11 ++++ sysdeps/nptl/sigfillset.c | 20 ------- sysdeps/posix/signal.c | 5 +- sysdeps/posix/sigset.c | 10 +--- sysdeps/unix/sysv/linux/internal-signals.h | 4 +- sysdeps/unix/sysv/linux/sigtimedwait.c | 17 +----- 13 files changed, 122 insertions(+), 96 deletions(-) delete mode 100644 sysdeps/nptl/sigfillset.c -- 2.7.4