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: 182686 Delivered-To: patch@linaro.org Received: by 2002:a92:815a:0:0:0:0:0 with SMTP id e87csp7927723ild; Tue, 31 Dec 2019 11:31:48 -0800 (PST) X-Google-Smtp-Source: APXvYqwTmRdjTlq2CDgEZc4i2f6nGSYncsjaHc4GQV9vu2EUsdnzRt61g50Mv0Tmy9N2XZylIQce X-Received: by 2002:a17:906:958e:: with SMTP id r14mr76939290ejx.228.1577820708676; Tue, 31 Dec 2019 11:31:48 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1577820708; cv=none; d=google.com; s=arc-20160816; b=Y9ZSzdcQCN28Btu3dSvUgn7gPRbBxZlZYIqByBqpgQWFW8zCMf8J3b2B7CilmgfCY1 xEWlP3b0d8pgHTbQlCMn2b+E+g5Sfrb0b77/oSdUARqIpbTV95C3XLtYuByrxAr2tiPW OYNjzijB8mJ6/67uizMDf8ei+6K3EWJxQlYED5chg3v2K8UE5OW36jlZrZLdPC4d83fS uXFmK3BIpXaHhao30uCAChiOep6+fkHpN4uOAOzKpS54yfiFgCg7ugCp2DkvK/y30NNB y0fXSqrLri65XzJ1ZZRHI0oeEXGohSI+waWB9OT4z1A/fatvTzNmdXyTOLbSoPW6/Q9a OcYg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=message-id:date:subject:to:from:dkim-signature:delivered-to:sender :list-help:list-post:list-archive:list-subscribe:list-unsubscribe :list-id:precedence:mailing-list:dkim-signature:domainkey-signature; bh=ALvJSscCF3HsZCrWEnQViwYHBqjkActGJ/BAK0/f0Po=; b=K5C5b0Aim2HnmbLQwe6UOLC++m2kmPyFjSvxTp5qxfuYhlE0wluz/iFv2HeuTc4KtJ SzUSQwmDWtIYll7BBWQ8ybzI+MAC34h0UTEBliCXkoNR7djl31+tc1WeFlJhYDqYObHk PxBA94QGToC/HGFHwBJep8EeWSHlm3ajwZEi2Iq1ol5EnLPXxcIECoZ6J17CERuFN1sw 7/eCVAVBnONhyLa/qUX/ZC6+mZFSH7xdAuoLsiS7HKBEG9rMDvoFP7Aichtcfc+xf666 7sa6daWIGxrlUZVQ7YP19oi/6s56WNh4975W0r/nMyvV/iEcSXMd8Lf8P53AdDjKqQ+6 e1bg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b=CvzGjjL7; dkim=pass header.i=@linaro.org header.s=google header.b="S/qlMx+y"; spf=pass (google.com: domain of libc-alpha-return-108390-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom="libc-alpha-return-108390-patch=linaro.org@sourceware.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id ay20si31361652edb.62.2019.12.31.11.31.47 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 31 Dec 2019 11:31:48 -0800 (PST) Received-SPF: pass (google.com: domain of libc-alpha-return-108390-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b=CvzGjjL7; dkim=pass header.i=@linaro.org header.s=google header.b="S/qlMx+y"; spf=pass (google.com: domain of libc-alpha-return-108390-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom="libc-alpha-return-108390-patch=linaro.org@sourceware.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id; q=dns; s= default; b=fRoaZiPugprucudMa6n3HDCpwdFzRB2tRBcFtoMB6kKREl2hxSeDi 4OpRgW0FN3dG/tXifZwOFcWIRM+SFuT7LyLj+xXQHpkLVMrK4T94fb3Rom2qE8Ty 3YlTeg7AdygQYfVSVmV7Rs+8zAennJd7Vesm/hl0K+67nnllLMesNs= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id; s=default; bh=N6/pib1WOXYTu8s3iRPI9UFwuZo=; b=CvzGjjL7aFynytbgYFHinvzkOq73 /w5F3P+lOckvCrD6giD+z7R/qBWY7RDUaMe8VNWUTjvXc7+6fL/FlehN2M8hFYlr JKvo5MDy9VbOvPLImOmaRrfFm+mDkk3RNoiDQAXbateRmh58Fs4bdA4qRe10sQAk kjgTJhOJuP+Vijo= 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(-) -- 2.17.1 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); From patchwork Tue Dec 31 19:31:29 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: 182687 Delivered-To: patch@linaro.org Received: by 2002:a92:815a:0:0:0:0:0 with SMTP id e87csp7927882ild; Tue, 31 Dec 2019 11:31:58 -0800 (PST) X-Google-Smtp-Source: APXvYqzAA61F/+V4fRXn1DSmhOHkeYZP/lDttZ557PR95PLHSpJOLgeMdUuveHozAbWx/v/koFK/ X-Received: by 2002:adf:fd84:: with SMTP id d4mr73284142wrr.211.1577820717887; Tue, 31 Dec 2019 11:31:57 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1577820717; cv=none; d=google.com; s=arc-20160816; b=iZeJRV9aDKbKEdpf0bHZLZde0BEN8pNpbBPInTfRR4Y9p9wdc7g4vy2ui2rmYKyPxv D4rmDoQFQF4njgR/cwHgKMWbynL3yB3XTWQVvfOcJWebfAiSa63XbU3xUJBzxnRXHnij gxMslK19Sva1HX5BkcGXm/7/FyG5rdlmAzRLG8nmvp6HP3Kam2voH0XRGnYxpFEQR4IY V32njBaupS7ErufCAlNPlJn85hLeVXrZxWN914vhF2jKm2AESeVlA5g/xac9qFtYds9N wd2pBu5wMSKNhnIG8kENaTuEirQO78g0bCzoVv97DSq1EYfJXlTQd96zxhzXJycRxBKI L+uQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=references:in-reply-to:message-id:date:subject:to:from :dkim-signature:delivered-to:sender:list-help:list-post:list-archive :list-subscribe:list-unsubscribe:list-id:precedence:mailing-list :dkim-signature:domainkey-signature; bh=vV9gg4i+w2bhiXuLcCxHGqalMFZwaJvW6hqy67zMHLE=; b=sZQUIL0NGfjm9vrjlANp9KRkXZAjHnTAXdqS95ha2YPperSsoV4RyfG/N5DtIFIrno 1cYsukQRqBVbqBkw0qxQ3rO5d9Z0Q4ZO7HrF3i9ammiWAOANkeKwTXQJPkiF4+sA5lZX zpxxynGT4TysiRN2k4C3HXMEdwK4dunOreHYRHA4+2jdWLBfvmMSzM3F2MYg8CbUjGVu ++D1lC/4eCcVG70lF/0sC13bVGNYbcYu5J5gKLpXGBq9B06ykRIH0mdCkOMvs5LprhBO cgS52tK4lnHgkeNkNVu/p9lqps1FDrHhiyd7e4DN6sMQsbvfKDYgsyE9qSccCeOKu+fk aEjw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b=gEcpPX+L; dkim=pass header.i=@linaro.org header.s=google header.b=Bfntbe6C; spf=pass (google.com: domain of libc-alpha-return-108391-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom="libc-alpha-return-108391-patch=linaro.org@sourceware.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id by1si30871788edb.198.2019.12.31.11.31.56 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 31 Dec 2019 11:31:57 -0800 (PST) Received-SPF: pass (google.com: domain of libc-alpha-return-108391-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b=gEcpPX+L; dkim=pass header.i=@linaro.org header.s=google header.b=Bfntbe6C; spf=pass (google.com: domain of libc-alpha-return-108391-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom="libc-alpha-return-108391-patch=linaro.org@sourceware.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id:in-reply-to :references; q=dns; s=default; b=PF1Q4jc2Wrav3JDCqwCsW/qTvgKJ5gT m/oTclb74yBr9VKnOggIQCf4mtXdWqGS5X5WrGrkNHgOe0UrOMn7h4taV3l0mayt 3eJ7zpn4xomumHankTG7vyJMEmOm4n30mg3mwD46jfVus71OWKKkNEITkDnJ8mlR OryPaqwIGn+k= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id:in-reply-to :references; s=default; bh=EP2+V+nfKouH9KLDMHFsx0nGBxI=; b=gEcpP X+LRanmdkIpJHEVYnY2oy1P+sHUT9iUh9wNTZlU2UJcs6yrvszR5J7uRmH0dNHRq XZOKpyTzTwjYyzq5SDteDyPi3I2dcvJv9Kdcrb+8OfAIlft4wBo8pmmje2zSOwhQ onIlZYWtJOSR03wGTxEoFjpnBJCi9xrSGVVDmI= Received: (qmail 129675 invoked by alias); 31 Dec 2019 19:31:41 -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 129613 invoked by uid 89); 31 Dec 2019 19:31:40 -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, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=1000000, canceled, opens, xxx X-HELO: mail-pg1-f177.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:subject:date:message-id:in-reply-to:references; bh=vV9gg4i+w2bhiXuLcCxHGqalMFZwaJvW6hqy67zMHLE=; b=Bfntbe6CvGey2Ypwauk35zRnC/kPDqUvzzeBsKkZDIjFYGu4CwUxWYUviItATH8QD6 MNml+kwW7lkoDh9mDqokD1CkZlK6SOm0U/TMXwHwEGYcUTBO9tvxxE1DGdan/yrYoPbO zkzbkflcYMeb0olBpdN5QzfUEJ/EG4CMJBbj9tyGOGjR9sPlNGPepaaJQCBtuStGSsGx Dg6sujxr7iMmIWIB8wHMJjQeBPGLqmCydwyCcWZy2mJ131n63tj+VyWyHVUcEVddF+z1 aOr4Jawo3QZt+RL6F0qchuFgDVoTIh0wCuhnymu41CB6HCFP68rmoH1RvfoVdBhH5Y40 WK3Q== Return-Path: From: Adhemerval Zanella To: libc-alpha@sourceware.org Subject: [PATCH v2 2/2] Block all signals on timer_create thread (BZ#10815) Date: Tue, 31 Dec 2019 16:31:29 -0300 Message-Id: <20191231193129.2590-2-adhemerval.zanella@linaro.org> In-Reply-To: <20191231193129.2590-1-adhemerval.zanella@linaro.org> References: <20191231193129.2590-1-adhemerval.zanella@linaro.org> Changes from previous version: - Use xpthread on tst-timer-sigmask. - It does not block SIGCANCEL and it adds tst-cancel28.c to check if the created thread is cancellable. -- The behavior of the signal mask on threads created by timer_create for SIGEV_THREAD timers are implementation-defined and glibc explicit unblocks all signals before calling the user-defined function. This behavior, although not incorrect standard-wise, opens a race if a program using a blocked rt-signal plus sigwaitinfo (and without an installed signal handler for the rt-signal) receives a signal while executing the used-defined function for SIGEV_THREAD. A better alternative discussed in bug report is to rather block all signals (besides the internal ones not available to application usage). This patch fixes this issue by only unblocking SIGSETXID (used on set*uid function) and SIGCANCEL (used for thread cancellation). Checked on x86_64-linux-gnu and i686-linux-gnu. --- nptl/Makefile | 5 +- nptl/tst-cancel28.c | 80 ++++++++++++++++++ rt/Makefile | 7 +- rt/tst-timer-sigmask.c | 85 +++++++++++++++++++ sysdeps/unix/sysv/linux/internal-signals.h | 20 +++++ sysdeps/unix/sysv/linux/timer_routines.c | 96 ++++++++-------------- 6 files changed, 227 insertions(+), 66 deletions(-) create mode 100644 nptl/tst-cancel28.c create mode 100644 rt/tst-timer-sigmask.c -- 2.17.1 diff --git a/nptl/Makefile b/nptl/Makefile index 4a57213ddb..ca4d14d541 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -286,7 +286,7 @@ tests = tst-attr1 tst-attr2 tst-attr3 tst-default-attr \ tst-cancel11 tst-cancel12 tst-cancel13 tst-cancel14 tst-cancel15 \ tst-cancel16 tst-cancel17 tst-cancel18 tst-cancel19 tst-cancel20 \ tst-cancel21 tst-cancel22 tst-cancel23 tst-cancel24 \ - tst-cancel26 tst-cancel27 \ + tst-cancel26 tst-cancel27 tst-cancel28 \ tst-cancel-self tst-cancel-self-cancelstate \ tst-cancel-self-canceltype tst-cancel-self-testcancel \ tst-cleanup0 tst-cleanup1 tst-cleanup2 tst-cleanup3 tst-cleanup4 \ @@ -600,6 +600,9 @@ $(objpfx)tst-tls6.out: tst-tls6.sh $(objpfx)tst-tls5 \ $(BASH) $< $(common-objpfx) '$(test-via-rtld-prefix)' \ '$(test-wrapper-env)' '$(run-program-env)' > $@; \ $(evaluate-test) +$(objpfx)tst-cancel28: $(common-objpfx)rt/librt.so +else +$(objpfx)tst-cancel28: $(common-objpfx)rt/librt.a endif $(objpfx)tst-join7: $(libdl) $(shared-thread-library) diff --git a/nptl/tst-cancel28.c b/nptl/tst-cancel28.c new file mode 100644 index 0000000000..ad70aeb548 --- /dev/null +++ b/nptl/tst-cancel28.c @@ -0,0 +1,80 @@ +/* Check if the thread created by POSIX timer using SIGEV_THREAD is + cancellable. + Copyright (C) 2019 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 + . */ + +#include +#include +#include +#include +#include + +#include +#include +#include + +static pthread_barrier_t barrier; +static pthread_t timer_thread; + +static void +cl (void *arg) +{ + xpthread_barrier_wait (&barrier); +} + +static void +thread_handler (union sigval sv) +{ + timer_thread = pthread_self (); + + xpthread_barrier_wait (&barrier); + + pthread_cleanup_push (cl, NULL); + while (1) + clock_nanosleep (CLOCK_REALTIME, 0, &(struct timespec) { 1, 0 }, NULL); + pthread_cleanup_pop (0); +} + +static int +do_test (void) +{ + struct sigevent sev = { 0 }; + sev.sigev_notify = SIGEV_THREAD; + sev.sigev_notify_function = &thread_handler; + + timer_t timerid; + TEST_COMPARE (timer_create (CLOCK_REALTIME, &sev, &timerid), 0); + + xpthread_barrier_init (&barrier, NULL, 2); + + struct itimerspec trigger = { 0 }; + trigger.it_value.tv_nsec = 1000000; + TEST_COMPARE (timer_settime (timerid, 0, &trigger, NULL), 0); + + xpthread_barrier_wait (&barrier); + + xpthread_cancel (timer_thread); + + xpthread_barrier_init (&barrier, NULL, 2); + xpthread_barrier_wait (&barrier); + + return 0; +} + +/* A stall in cancellation is a regression. */ +#define TIMEOUT 5 +#include diff --git a/rt/Makefile b/rt/Makefile index 6c8365e0c0..a7a82879c7 100644 --- a/rt/Makefile +++ b/rt/Makefile @@ -47,6 +47,7 @@ tests := tst-shm tst-timer tst-timer2 \ tst-timer3 tst-timer4 tst-timer5 \ tst-cpuclock2 tst-cputimer1 tst-cputimer2 tst-cputimer3 \ tst-shm-cancel +tests-internal := tst-timer-sigmask extra-libs := librt extra-libs-others := $(extra-libs) @@ -63,9 +64,11 @@ LDFLAGS-rt.so = -Wl,--enable-new-dtags,-z,nodelete $(objpfx)librt.so: $(shared-thread-library) ifeq (yes,$(build-shared)) -$(addprefix $(objpfx),$(tests)): $(objpfx)librt.so $(shared-thread-library) +$(addprefix $(objpfx),$(tests) $(tests-internal)): \ + $(objpfx)librt.so $(shared-thread-library) else -$(addprefix $(objpfx),$(tests)): $(objpfx)librt.a $(static-thread-library) +$(addprefix $(objpfx),$(tests)) $(tests-internal): \ + $(objpfx)librt.a $(static-thread-library) endif tst-mqueue7-ARGS = -- $(host-test-program-cmd) diff --git a/rt/tst-timer-sigmask.c b/rt/tst-timer-sigmask.c new file mode 100644 index 0000000000..063004120a --- /dev/null +++ b/rt/tst-timer-sigmask.c @@ -0,0 +1,85 @@ +/* Check resulting signal mask from POSIX timer using SIGEV_THREAD. + Copyright (C) 2019 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 + . */ + +#include +#include +#include +#include + +#include +#include +#include + +#include + +static pthread_barrier_t barrier; +static bool thread_handler_failure = false; + +static void +thread_handler (union sigval sv) +{ + sigset_t ss; + sigprocmask (SIG_BLOCK, NULL, &ss); + if (test_verbose > 0) + printf ("%s: blocked signal mask = { ", __func__); + for (int sig = 1; sig < NSIG; sig++) + { +#ifdef __linux__ + /* POSIX timers threads created to handle SIGEV_THREAD block all + signals except SIGKILL, SIGSTOP and glibc internals one except + SIGTIMER. */ + if (!sigismember (&ss, sig) + && (sig != SIGSETXID && sig != SIGCANCEL + && sig != SIGKILL && sig != SIGSTOP)) + { + thread_handler_failure = true; + } +#endif + if (test_verbose && sigismember (&ss, sig)) + printf ("%d, ", sig); + } + if (test_verbose > 0) + printf ("}\n"); + + xpthread_barrier_wait (&barrier); +} + +static int +do_test (void) +{ + struct sigevent sev = { 0 }; + sev.sigev_notify = SIGEV_THREAD; + sev.sigev_notify_function = &thread_handler; + + timer_t timerid; + TEST_COMPARE (timer_create (CLOCK_REALTIME, &sev, &timerid), 0); + + xpthread_barrier_init (&barrier, NULL, 2); + + struct itimerspec trigger = { 0 }; + trigger.it_value.tv_nsec = 1000000; + TEST_COMPARE (timer_settime (timerid, 0, &trigger, NULL), 0); + + xpthread_barrier_wait (&barrier); + + TEST_COMPARE (thread_handler_failure, false); + + return 0; +} + +#include diff --git a/sysdeps/unix/sysv/linux/internal-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h index 06d1d38b7f..df406ea522 100644 --- a/sysdeps/unix/sysv/linux/internal-signals.h +++ b/sysdeps/unix/sysv/linux/internal-signals.h @@ -65,6 +65,17 @@ static const sigset_t sigall_set = { .__val = {[0 ... _SIGSET_NWORDS-1 ] = -1 } }; +static const sigset_t sigtimer_set = { +#if ULONG_MAX == 0xffffffff + .__val = { [0] = 0, + [1] = __sigmask (SIGTIMER), + [2 ... _SIGSET_NWORDS-1] = 0 } +#else + .__val = { [0] = __sigmask (SIGTIMER), + [1 ... _SIGSET_NWORDS-1] = 0 } +#endif +}; + /* Block all signals, including internal glibc ones. */ static inline void __libc_signal_block_all (sigset_t *set) @@ -85,6 +96,15 @@ __libc_signal_block_app (sigset_t *set) _NSIG / 8); } +/* Block only the SIGTIMER set. */ +static inline void +__libc_signal_block_sigtimer (sigset_t *set) +{ + INTERNAL_SYSCALL_DECL (err); + INTERNAL_SYSCALL_CALL (rt_sigprocmask, err, SIG_BLOCK, &sigtimer_set, set, + _NSIG / 8); +} + /* Restore current process signal mask. */ static inline void __libc_signal_restore_set (const sigset_t *set) diff --git a/sysdeps/unix/sysv/linux/timer_routines.c b/sysdeps/unix/sysv/linux/timer_routines.c index 84213dd4f7..832cc9190a 100644 --- a/sysdeps/unix/sysv/linux/timer_routines.c +++ b/sysdeps/unix/sysv/linux/timer_routines.c @@ -42,14 +42,6 @@ struct thread_start_data static void * timer_sigev_thread (void *arg) { - /* The parent thread has all signals blocked. This is a bit - surprising for user code, although valid. We unblock all - signals. */ - sigset_t ss; - sigemptyset (&ss); - INTERNAL_SYSCALL_DECL (err); - INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_SETMASK, &ss, NULL, _NSIG / 8); - struct thread_start_data *td = (struct thread_start_data *) arg; void (*thrfunc) (sigval_t) = td->thrfunc; @@ -69,65 +61,49 @@ timer_sigev_thread (void *arg) static void * timer_helper_thread (void *arg) { - /* Wait for the SIGTIMER signal, allowing the setXid signal, and - none else. */ - sigset_t ss; - sigemptyset (&ss); - __sigaddset (&ss, SIGTIMER); - /* Endless loop of waiting for signals. The loop is only ended when the thread is canceled. */ while (1) { siginfo_t si; - /* sigwaitinfo cannot be used here, since it deletes - SIGCANCEL == SIGTIMER from the set. */ - - /* 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, &ss, &si, NULL, _NSIG / 8); - - if (result > 0) + while (sigwaitinfo (&sigtimer_set, &si) < 0); + if (si.si_code == SI_TIMER) { - if (si.si_code == SI_TIMER) - { - struct timer *tk = (struct timer *) si.si_ptr; + struct timer *tk = (struct timer *) si.si_ptr; + + /* Check the timer is still used and will not go away + while we are reading the values here. */ + pthread_mutex_lock (&__active_timer_sigev_thread_lock); - /* Check the timer is still used and will not go away - while we are reading the values here. */ - pthread_mutex_lock (&__active_timer_sigev_thread_lock); + struct timer *runp = __active_timer_sigev_thread; + while (runp != NULL) + if (runp == tk) + break; + else + runp = runp->next; - struct timer *runp = __active_timer_sigev_thread; - while (runp != NULL) - if (runp == tk) - break; - else - runp = runp->next; + if (runp != NULL) + { + struct thread_start_data *td = malloc (sizeof (*td)); - if (runp != NULL) + /* There is not much we can do if the allocation fails. */ + if (td != NULL) { - struct thread_start_data *td = malloc (sizeof (*td)); - - /* There is not much we can do if the allocation fails. */ - if (td != NULL) - { - /* This is the signal we are waiting for. */ - td->thrfunc = tk->thrfunc; - td->sival = tk->sival; - - pthread_t th; - (void) pthread_create (&th, &tk->attr, - timer_sigev_thread, td); - } - } + /* This is the signal we are waiting for. */ + td->thrfunc = tk->thrfunc; + td->sival = tk->sival; - pthread_mutex_unlock (&__active_timer_sigev_thread_lock); + pthread_t th; + pthread_create (&th, &tk->attr, timer_sigev_thread, td); + } } - else if (si.si_code == SI_TKILL) - /* The thread is canceled. */ - pthread_exit (NULL); + + pthread_mutex_unlock (&__active_timer_sigev_thread_lock); } + else if (si.si_code == SI_TKILL) + /* The thread is canceled. */ + pthread_exit (NULL); } } @@ -161,15 +137,10 @@ __start_helper_thread (void) /* Block all signals in the helper thread but SIGSETXID. To do this thoroughly we temporarily have to block all signals here. The - helper can lose wakeups if SIGCANCEL is not blocked throughout, - but sigfillset omits it SIGSETXID. So, we add SIGCANCEL back - explicitly here. */ + helper can lose wakeups if SIGTIMER is not blocked throughout. */ sigset_t ss; - sigset_t oss; - sigfillset (&ss); - __sigaddset (&ss, SIGTIMER); - INTERNAL_SYSCALL_DECL (err); - INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_SETMASK, &ss, &oss, _NSIG / 8); + __libc_signal_block_app (&ss); + __libc_signal_block_sigtimer (NULL); /* Create the helper thread for this timer. */ pthread_t th; @@ -179,8 +150,7 @@ __start_helper_thread (void) __helper_tid = ((struct pthread *) th)->tid; /* Restore the signal mask. */ - INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_SETMASK, &oss, NULL, - _NSIG / 8); + __libc_signal_restore_set (&ss); /* No need for the attribute anymore. */ (void) pthread_attr_destroy (&attr);