From patchwork Mon Feb 12 12:42:23 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 128017 Delivered-To: patch@linaro.org Received: by 10.46.124.24 with SMTP id x24csp3171424ljc; Mon, 12 Feb 2018 04:42:54 -0800 (PST) X-Google-Smtp-Source: AH8x2248SXDdAe+hnVlKGG67/FEJt7Y8KCB46UmOVVVDSj4ez1ev6YMbM2Js889vQzUHWMZhCiao X-Received: by 2002:a17:902:8ec4:: with SMTP id x4-v6mr10294684plo.271.1518439374863; Mon, 12 Feb 2018 04:42:54 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1518439374; cv=none; d=google.com; s=arc-20160816; b=XNzUdMoaWhmnEX1LgEBJadELz+U9/skurhTkgteEGM4KBvZt6hK9yoQvPkhd3fCpSW 2kUEfLHFxTkaSQkqWPzHKPz6NPMj37ogqCVTwgnDmEcCllAKVJbE+2DPmCkk6y8E+/qn RuViBpg0yr34ycKTCzSFY5XScDSjv+b965XCclNKQpjSj4nyj4331DNid/bu55X0x1WG kzNjvVIWijArXrJkXXVtILa5g/8gHx7B4DGO3vZXTxN6dWGuRFmcH5rUfSrZ2hPH1uKO yzxsXUljxIGs0ZR7b+BA+uin0IdgpgW3GlXCB22jEHMxwiodADPIIqexyA1dILWaNOuB zuFQ== 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:delivered-to :sender:list-help:list-post:list-archive:list-subscribe :list-unsubscribe:list-id:precedence:mailing-list:dkim-signature :domainkey-signature:arc-authentication-results; bh=E19VUyJfEkXLOWNtIkWH9c3D4sZBDS6RGh8OqJl9W2A=; b=EnZ+y80WowAugdr2vGHH7usTB7DRdYfDYmGGsz+V4D3OnMsyl384QzI1bQX6pPuQVH xmRhDgIusVaV3VvUrC91TcoPQov8OrlhfTDgJ+pGrP17bzBuH9pFbiS4aRykLnJIgyWx 2S5QKhcTvxF5s3tO5Rhdr6jo4ap4kV/ccjlgad+v3uYX3nBsQva5RBlnoW3TGV2esswQ +vJua+VJTE/dnPwki3rih8gyrp37K72XWDZessGtJaJupFqWdvqV6KawCyxMZLuHbQXY o+UK6+Omf1SV4IVSo+E6DMPTmM8E6I8+bfjt+GGFmw/gGRXiySG3gzXCbv0/oqY3B+WY RZUw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b=WJT13Lzz; spf=pass (google.com: domain of libc-alpha-return-90258-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=libc-alpha-return-90258-patch=linaro.org@sourceware.org; dmarc=fail (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 c17-v6si1198190plo.118.2018.02.12.04.42.54 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 12 Feb 2018 04:42:54 -0800 (PST) Received-SPF: pass (google.com: domain of libc-alpha-return-90258-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=WJT13Lzz; spf=pass (google.com: domain of libc-alpha-return-90258-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=libc-alpha-return-90258-patch=linaro.org@sourceware.org; dmarc=fail (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=fqrB90gO95g6rJLZlgMtDXM+RxOJvjX VSqwUZwf+4+eSbrRr3D7im2xn7d7UJ1qH7NHn7qhyorCJuDYUg153OnKOi3To9c6 dwR/s/5vkQa5ADN1JcYNsM3E8Qx+YQysWUWmpnnPvI6WLjcmM5Ug9O90HfKmvsBk Wi+9sSFQAb50= 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=BrlCZDkers+J4R/iQN7OmVm1eH0=; b=WJT13 LzzmzPlLqPVpGmkesxJ6Y3YsgVfy5CG3jVu0AZp3drEtNkIwkIxfvloMOIStP34K Fv1IDbmyg/Kqjewt+CGBDeG7msqkV90RTa8ZdbXxXXkr5YecWxxjTgrgLBpBEDQs Nec2Do4DpJb5Zn3PBrumafQmwJoFV7vB66BEBk= Received: (qmail 51747 invoked by alias); 12 Feb 2018 12:42: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 51617 invoked by uid 89); 12 Feb 2018 12:42:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=2421, TRY X-HELO: mail-qt0-f178.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=E19VUyJfEkXLOWNtIkWH9c3D4sZBDS6RGh8OqJl9W2A=; b=ARTMMkboPx7KhYF5fJ5GM5zV+5Pnfe5gLXjFvFt8SDQiebe/ggzBOiNX6/KxyCxczX SZCrONlCfF3XbAF2ITZAWiZJWtFoKYBAwIqTTpTE9k9Lzy3QiU79eJJAYCtzddpCPo5H nIZjVX5DRdt1uSACWpsgekwDrDZU8dp3UpkXEMIzvZ1kFXk2T4b968Sgkx7qkpsWiYVQ tswiskrX8xkcZskTLIz9dy+A0PFSU6EaH61h/dEekCa2mbuabxrb5YIBTiZjNl87sd20 oN5XqdY23DRH2VA6nvb/zA7d4/QvgwtTf9YraPrw2QWR+emSsjEiXCNW1J7d3tB3x1IE Cl8A== X-Gm-Message-State: APf1xPBnc6KkyGg2okaT4LZ0IWLL/7YwmXLkmLpaczBjr33pY+L4dgen tqsir5yHYS8w5/GCrj17cCTTwYZ3Yi0= X-Received: by 10.200.57.97 with SMTP id t30mr15020601qtb.22.1518439352375; Mon, 12 Feb 2018 04:42:32 -0800 (PST) From: Adhemerval Zanella To: libc-alpha@sourceware.org Subject: [PATCH v4 2/4] Filter out NPTL internal signals (BZ #22391) Date: Mon, 12 Feb 2018 10:42:23 -0200 Message-Id: <1518439345-6013-2-git-send-email-adhemerval.zanella@linaro.org> In-Reply-To: <1518439345-6013-1-git-send-email-adhemerval.zanella@linaro.org> References: <1518439345-6013-1-git-send-email-adhemerval.zanella@linaro.org> 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 Reported-by: Yury Norov --- 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 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 . */ - -/* This is no complete implementation. The file is meant to be - included in the real implementation to provide the wrapper around - __libc_sigaction. */ - -#include - -/* We use the libc implementation but we tell it to not allow - SIGCANCEL or SIGTIMER to be handled. */ -#define LIBC_SIGACTION 1 - +#include 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 #include -#include +#include /* 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 #include -#include +#include /* 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 #include #include +#include /* 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 -#include -#define TEST_FUNCTION do_test () +#include + 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 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 . */ + +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 - . */ - -#include - -#include 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 #include -#include /* For the real memset prototype. */ #include +#include 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 #include +#include +#include /* 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