about summary refs log tree commit diff
diff options
context:
space:
mode:
authorAdhemerval Zanella <adhemerval.zanella@linaro.org>2017-10-31 23:10:37 -0200
committerAdhemerval Zanella <adhemerval.zanella@linaro.org>2018-04-03 13:30:49 -0300
commitd2dc5467c67bc8625a4fc8f285b6a5443bf43df4 (patch)
tree27ddabfd779bfcbcc92f095288be5778ec4370ca
parent511ed56f2e792bea5e7145e1d5d5c93b9f47c41f (diff)
downloadglibc-d2dc5467c67bc8625a4fc8f285b6a5443bf43df4.tar.gz
glibc-d2dc5467c67bc8625a4fc8f285b6a5443bf43df4.tar.xz
glibc-d2dc5467c67bc8625a4fc8f285b6a5443bf43df4.zip
Filter out NPTL internal signals (BZ #22391)
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.
	* nptl/sigaction.c (__sigaction): Likewise.
	* 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/generic/internal-signals.h (__clear_internal_signals,
	__is_internal_signal, __libc_signal_block_all,
	__libc_signal_block_app, __libc_signal_restore_set): New functions.
	* sysdeps/nptl/sigfillset.c: Remove file.
	* sysdeps/unix/sysv/linux/internal-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.

Reported-by: Yury Norov <ynorov@caviumnetworks.com>
-rw-r--r--ChangeLog26
-rw-r--r--nptl/sigaction.c14
-rw-r--r--signal/sigaction.c2
-rw-r--r--signal/sigaddset.c5
-rw-r--r--signal/sigdelset.c5
-rw-r--r--signal/sigfillset.c10
-rw-r--r--signal/tst-sigset.c92
-rw-r--r--sysdeps/generic/internal-signals.h36
-rw-r--r--sysdeps/nptl/sigfillset.c20
-rw-r--r--sysdeps/posix/signal.c5
-rw-r--r--sysdeps/posix/sigset.c10
-rw-r--r--sysdeps/unix/sysv/linux/internal-signals.h4
-rw-r--r--sysdeps/unix/sysv/linux/sigtimedwait.c17
13 files changed, 150 insertions, 96 deletions
diff --git a/ChangeLog b/ChangeLog
index 28fa4a5e69..44a25a3baa 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,29 @@
+2018-04-03 Adhemerval Zanella  <adhemerval.zanella@linaro.org>
+
+	[BZ #22391]
+	* nptl/sigaction.c (__sigaction): Use __is_internal_signal to
+	check for internal nptl signals.
+	* nptl/sigaction.c (__sigaction): Likewise.
+	* 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/generic/internal-signals.h (__clear_internal_signals,
+	__is_internal_signal, __libc_signal_block_all,
+	__libc_signal_block_app, __libc_signal_restore_set): New functions.
+	* sysdeps/nptl/sigfillset.c: Remove file.
+	* sysdeps/unix/sysv/linux/internal-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.
+
 2018-04-03  Samuel Thibault  <samuel.thibault@ens-lyon.org>
 
 	* sysdeps/mach/hurd/if_index.c (__if_nametoindex): Always end
diff --git a/nptl/sigaction.c b/nptl/sigaction.c
index ddf6f5e2e4..79b6fdcde6 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 f761ca2612..c99001af42 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 d310890ec3..7238df4738 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 cd83dda57b..011978cf33 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 e586fd9295..83dd583b7b 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 d47adcc0d4..a2b764de5a 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 01e5b75b6b..17ed48c5bf 100644
--- a/sysdeps/generic/internal-signals.h
+++ b/sysdeps/generic/internal-signals.h
@@ -15,3 +15,39 @@
    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/>.  */
+
+#ifndef __INTERNAL_SIGNALS_H
+# define __INTERNAL_SIGNALS_H
+
+static inline bool
+__is_internal_signal (int sig)
+{
+  return false;
+}
+
+static inline void
+__clear_internal_signals (sigset_t *set)
+{
+}
+
+static inline int
+__libc_signal_block_all (sigset_t *set)
+{
+  return 0;
+}
+
+static inline int
+__libc_signal_block_app (sigset_t *set)
+{
+  return 0;
+}
+
+/* Restore current process signal mask.  */
+static inline int
+__libc_signal_restore_set (const sigset_t *set)
+{
+  return 0;
+}
+
+
+#endif /* __INTERNAL_SIGNALS_H  */
diff --git a/sysdeps/nptl/sigfillset.c b/sysdeps/nptl/sigfillset.c
deleted file mode 100644
index 94a7680aa3..0000000000
--- 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 a4a0875ae5..8a135c7b0e 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 b62aa3cc84..6ab4a48767 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 e007372f21..5ff4cf83d5 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 051a28575f..b4de8856dd 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