diff mbox series

[v4,2/4] Filter out NPTL internal signals (BZ #22391)

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

Commit Message

Adhemerval Zanella Netto Feb. 12, 2018, 12:42 p.m. UTC
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

Comments

Adhemerval Zanella Netto Feb. 19, 2018, 5:50 p.m. UTC | #1
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

>
Adhemerval Zanella Netto April 3, 2018, 2:24 p.m. UTC | #2
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

>>
Zack Weinberg April 3, 2018, 3:12 p.m. UTC | #3
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

>>>
Adhemerval Zanella Netto April 3, 2018, 4:13 p.m. UTC | #4
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 mbox series

Patch

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