From patchwork Tue Dec 31 19:31:28 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 37139 Received: (qmail 129274 invoked by alias); 31 Dec 2019 19:31:38 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 129266 invoked by uid 89); 31 Dec 2019 19:31:38 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-22.0 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=oss, Reserve, Safety X-HELO: mail-pf1-f194.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:subject:date:message-id; bh=ALvJSscCF3HsZCrWEnQViwYHBqjkActGJ/BAK0/f0Po=; b=S/qlMx+yXpTPn5epNqyjBccbs6TNvVRe4it32CnXV1SqIjsppRGxr6WK54SwFrcByG Zvndhy4hAyTUn/GPmbo5sfbdHq7cR/gJExTVfHcJygbyUawvTTyrfq0HqY0jvJikdVRm UUFnaNQ6uM3Rg33mp5XP/3v5gDk1nlHusRZpFFwTm5d4jcJVtOeCYxm9x7/xZzPxc86l qbBjrsxaGv1NTeJrQuQkvC8L7kRvVDMN/W3n/+EXNwgy6D2UeQmNzQaAHtzE3BMre2J/ ooyCIfeti8R3OzW01ejq8vRF6GRfYjQFNe4qD3H0z6wTg9ctzn9tDB1FSoxRAoBsFcrY AICA== Return-Path: From: Adhemerval Zanella To: libc-alpha@sourceware.org Subject: [PATCH 1/2] linux: Reserve a new signal for SIGTIMER Date: Tue, 31 Dec 2019 16:31:28 -0300 Message-Id: <20191231193129.2590-1-adhemerval.zanella@linaro.org> The fix for BZ#10815 requires decouple SIGTIMER and SIGCANCEL. The POSIX timer threads created to handle SIGEV_THREAD will have the SIGTIMER blocked but it should be able to cancel the created created (which in turn requires SIGCANCEL unblocked). This patch also a new internal symbol, __contain_internal_signal, which checks if the sigset_t contains any internal signal. It is used to refactor and move the logic to check and remove the internal signal to internal-signals.h. Checked on x86_64-linux-gnu and i686-linux-gnu. --- nptl/nptl-init.c | 9 --------- nptl/pthread_sigmask.c | 7 ++----- sysdeps/generic/internal-signals.h | 6 ++++++ sysdeps/nptl/allocrtsig.c | 8 ++------ sysdeps/unix/sysv/linux/internal-signals.h | 21 ++++++++++++++------- sysdeps/unix/sysv/linux/pthread_kill.c | 2 +- sysdeps/unix/sysv/linux/pthread_sigqueue.c | 2 +- sysdeps/unix/sysv/linux/sigprocmask.c | 7 ++----- sysdeps/unix/sysv/linux/timer_routines.c | 2 +- 9 files changed, 29 insertions(+), 35 deletions(-) diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c index acc0f3672b..0fa799aaac 100644 --- a/nptl/nptl-init.c +++ b/nptl/nptl-init.c @@ -141,15 +141,6 @@ __nptl_set_robust (struct pthread *self) static void sigcancel_handler (int sig, siginfo_t *si, void *ctx) { - /* Safety check. It would be possible to call this function for - other signals and send a signal from another process. This is not - correct and might even be a security problem. Try to catch as - many incorrect invocations as possible. */ - if (sig != SIGCANCEL - || si->si_pid != __getpid() - || si->si_code != SI_TKILL) - return; - struct pthread *self = THREAD_SELF; int oldval = THREAD_GETMEM (self, cancelhandling); diff --git a/nptl/pthread_sigmask.c b/nptl/pthread_sigmask.c index 4aa774d01d..31f6defcba 100644 --- a/nptl/pthread_sigmask.c +++ b/nptl/pthread_sigmask.c @@ -29,13 +29,10 @@ pthread_sigmask (int how, const sigset_t *newmask, sigset_t *oldmask) /* The only thing we have to make sure here is that SIGCANCEL and SIGSETXID is not blocked. */ - if (newmask != NULL - && (__builtin_expect (__sigismember (newmask, SIGCANCEL), 0) - || __builtin_expect (__sigismember (newmask, SIGSETXID), 0))) + if (newmask != NULL && __contain_internal_signal (newmask)) { local_newmask = *newmask; - __sigdelset (&local_newmask, SIGCANCEL); - __sigdelset (&local_newmask, SIGSETXID); + __clear_internal_signals (&local_newmask); newmask = &local_newmask; } diff --git a/sysdeps/generic/internal-signals.h b/sysdeps/generic/internal-signals.h index 41c24dc4b3..d78d919f62 100644 --- a/sysdeps/generic/internal-signals.h +++ b/sysdeps/generic/internal-signals.h @@ -29,6 +29,12 @@ __is_internal_signal (int sig) return false; } +static inline bool +__contain_internal_signal (const sigset_t *set) +{ + return false; +} + static inline void __clear_internal_signals (sigset_t *set) { diff --git a/sysdeps/nptl/allocrtsig.c b/sysdeps/nptl/allocrtsig.c index 3f62bf40e7..ffa2a48e65 100644 --- a/sysdeps/nptl/allocrtsig.c +++ b/sysdeps/nptl/allocrtsig.c @@ -19,13 +19,9 @@ #include #include -#if SIGTIMER != SIGCANCEL -# error "SIGTIMER and SIGCANCEL must be the same" -#endif - /* This tells the generic code (included below) how many signal numbers need to be reserved for libpthread's private uses - (SIGCANCEL and SIGSETXID). */ -#define RESERVED_SIGRT 2 + (SIGCANCEL, SIGSETXID, and SIGTIMER). */ +#define RESERVED_SIGRT 3 #include diff --git a/sysdeps/unix/sysv/linux/internal-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h index 2932e21fd0..06d1d38b7f 100644 --- a/sysdeps/unix/sysv/linux/internal-signals.h +++ b/sysdeps/unix/sysv/linux/internal-signals.h @@ -29,21 +29,27 @@ #define SIGCANCEL __SIGRTMIN -/* Signal needed for the kernel-supported POSIX timer implementation. - We can reuse the cancellation signal since we can distinguish - cancellation from timer expirations. */ -#define SIGTIMER SIGCANCEL - - /* Signal used to implement the setuid et.al. functions. */ #define SIGSETXID (__SIGRTMIN + 1) +/* Signal needed for the kernel-supported POSIX timer implementation. */ +#define SIGTIMER (__SIGRTMIN + 2) + + /* Return is sig is used internally. */ static inline bool __is_internal_signal (int sig) { - return (sig == SIGCANCEL) || (sig == SIGSETXID); + return (sig == SIGCANCEL) || (sig == SIGSETXID) || (sig == SIGTIMER); +} + +/* Return true is SIG is withing the signal set SET, or false otherwise. */ +static inline bool +__contain_internal_signal (const sigset_t *set) +{ + return __sigismember (set, SIGCANCEL) || __sigismember (set, SIGSETXID) + || __sigismember (set, SIGTIMER); } /* Remove internal glibc signal from the mask. */ @@ -52,6 +58,7 @@ __clear_internal_signals (sigset_t *set) { __sigdelset (set, SIGCANCEL); __sigdelset (set, SIGSETXID); + __sigdelset (set, SIGTIMER); } static const sigset_t sigall_set = { diff --git a/sysdeps/unix/sysv/linux/pthread_kill.c b/sysdeps/unix/sysv/linux/pthread_kill.c index 71305b90c6..6285ce5f75 100644 --- a/sysdeps/unix/sysv/linux/pthread_kill.c +++ b/sysdeps/unix/sysv/linux/pthread_kill.c @@ -44,7 +44,7 @@ __pthread_kill (pthread_t threadid, int signo) /* Disallow sending the signal we use for cancellation, timers, for the setxid implementation. */ - if (signo == SIGCANCEL || signo == SIGTIMER || signo == SIGSETXID) + if (__is_internal_signal (signo)) return EINVAL; /* We have a special syscall to do the work. */ diff --git a/sysdeps/unix/sysv/linux/pthread_sigqueue.c b/sysdeps/unix/sysv/linux/pthread_sigqueue.c index 62a7a24531..c4d8f4cd52 100644 --- a/sysdeps/unix/sysv/linux/pthread_sigqueue.c +++ b/sysdeps/unix/sysv/linux/pthread_sigqueue.c @@ -46,7 +46,7 @@ pthread_sigqueue (pthread_t threadid, int signo, const union sigval value) /* Disallow sending the signal we use for cancellation, timers, for the setxid implementation. */ - if (signo == SIGCANCEL || signo == SIGTIMER || signo == SIGSETXID) + if (__is_internal_signal (signo)) return EINVAL; pid_t pid = getpid (); diff --git a/sysdeps/unix/sysv/linux/sigprocmask.c b/sysdeps/unix/sysv/linux/sigprocmask.c index 73b0d0c19a..62820e577e 100644 --- a/sysdeps/unix/sysv/linux/sigprocmask.c +++ b/sysdeps/unix/sysv/linux/sigprocmask.c @@ -26,13 +26,10 @@ __sigprocmask (int how, const sigset_t *set, sigset_t *oset) /* The only thing we have to make sure here is that SIGCANCEL and SIGSETXID are not blocked. */ - if (set != NULL - && __glibc_unlikely (__sigismember (set, SIGCANCEL) - || __glibc_unlikely (__sigismember (set, SIGSETXID)))) + if (set != NULL && __contain_internal_signal (set)) { local_newmask = *set; - __sigdelset (&local_newmask, SIGCANCEL); - __sigdelset (&local_newmask, SIGSETXID); + __clear_internal_signals (&local_newmask); set = &local_newmask; } diff --git a/sysdeps/unix/sysv/linux/timer_routines.c b/sysdeps/unix/sysv/linux/timer_routines.c index d96d963177..84213dd4f7 100644 --- a/sysdeps/unix/sysv/linux/timer_routines.c +++ b/sysdeps/unix/sysv/linux/timer_routines.c @@ -167,7 +167,7 @@ __start_helper_thread (void) sigset_t ss; sigset_t oss; sigfillset (&ss); - __sigaddset (&ss, SIGCANCEL); + __sigaddset (&ss, SIGTIMER); INTERNAL_SYSCALL_DECL (err); INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_SETMASK, &ss, &oss, _NSIG / 8);