From patchwork Tue Dec 10 18:32:20 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 181149 Delivered-To: patch@linaro.org Received: by 2002:a92:3001:0:0:0:0:0 with SMTP id x1csp6242262ile; Tue, 10 Dec 2019 10:33:25 -0800 (PST) X-Google-Smtp-Source: APXvYqwj5EO8smWakpF0VCtzHFHS2Uz6WkDmDKH+F2twL8r6eqHhOB6Nr9GzydeuCfAKM/sfHl/Y X-Received: by 2002:a05:6830:1e2d:: with SMTP id t13mr27548471otr.128.1576002805528; Tue, 10 Dec 2019 10:33:25 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1576002805; cv=none; d=google.com; s=arc-20160816; b=V+S1L7p60TP/NML6abAGQ4Zbdv1nsCyDUzvIV7C6CThAP1TXaA3U7x/QPVpLh2YCc4 NLP4onLRUP3UFj9CAKArTr+072mqHgyxLQt169obthXeJaO6fZZkk1PsJCnU7Mt5JuOJ 2T06cFJFsZXbDxOWMCTTVImcMh46za5bNP4N3LmX9cwSSrB7NA06S62rsXizoyAuqwxu NMxA9kCi4Ik89jh3dnzYJC5tjkBKqWiOe7iyFYhtFuXRp9TcIs58jmcaty1qytjDTyBh yJcimvzMI62kyifNwtscwqfc0cxGgO/YG0XAYuC7dbOjOPSrRbpruR6Tw/IKf9ev9FLz ow7A== 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=CwL6r6Xelg5Uw+QCbPHeqZQ849t9ehkMMBaRI7CovAw=; b=MJY+a9czbJ7UTXf8WCP98ZZhBYcLlt4SluU3+sDE065YBp4Cs2nqykP25nxqC4DN6W VyUo6wyVqLmbuNOjMSp42KHgmFAmKfo9bR4M63om0UNJqqSWoFxwBqYPmTM12/oLX8gX x5fE1zGwkFLMuPI7bp+//DSt197qY4JXeYakMuwPLRNbKPo3o4+KLREDDHXUtW9vN1T3 07RQjACROgVigcq3qTxuDgIVpv+hciejHQtrrqBiAjIxx8J1FNG4DQrYQcE3T582JJ58 aTIGUL1RNPH++ByaERgsy+LrWTjVKE7H8utj5OeBjmAp6WdxNXCqxmbmyCDBVMXouXH2 996Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b="CB2e6l3/"; dkim=pass header.i=@linaro.org header.s=google header.b=eEyMTnwl; spf=pass (google.com: domain of libc-alpha-return-107900-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom="libc-alpha-return-107900-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 x24si2373368otk.188.2019.12.10.10.33.25 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 10 Dec 2019 10:33:25 -0800 (PST) Received-SPF: pass (google.com: domain of libc-alpha-return-107900-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="CB2e6l3/"; dkim=pass header.i=@linaro.org header.s=google header.b=eEyMTnwl; spf=pass (google.com: domain of libc-alpha-return-107900-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom="libc-alpha-return-107900-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=kw56MiHjLno8jC+hLtBRo+KBnE9VrmI nmv6mN4qKRCsGBvuTGCeEASTrgzLv9Yk1PXqd/eZ3dH7J/8EyNR3EhOmyqb670d7 UY9IKGX8eRvbyC5hUBTrrah1gNU3TIkWyh9khcMUpU/ZbBakf19le1CKkYBFIbU6 fXoTicJjwsiQ= 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=AWSFPXltg4zRdMchCYH6KmzqlpQ=; b=CB2e6 l3/Z1HYCc7ZdWpb61JfP4EsjzuuUH7Mvic9Qu0vuPo/stu3gnsDdOiQxjl+8SrtG d68ckVm4WGsiVqx3bnUBB4fm/8ajiLR9cuo/+RZbCYcV5QjtcA/2AtZijTOqOUF9 9pB1gGidbh9PF2pjnSSpCCVWydmiwMEtKPMlKk= Received: (qmail 40134 invoked by alias); 10 Dec 2019 18:32:40 -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 40060 invoked by uid 89); 10 Dec 2019 18:32:40 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-21.8 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, thoroughly X-HELO: mail-vk1-f176.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=CwL6r6Xelg5Uw+QCbPHeqZQ849t9ehkMMBaRI7CovAw=; b=eEyMTnwlzUjtFrA9neZ2ND8LsGSxwABFgfjtJyOdzgykm2O7WVMwaO2nOH0Nxd1PrI PiKvp5J7gWUqFXB1pzSFOQeFh8Z1raRwLKfI8mueadbEhjWTiXSGn4y1427OxiQ/OFmM P5CxB/V2Nh4xXKEPP4+SSDM0pt8U4U1+K+Y0B/bCelmt9YoFrnle7aWVoRtOgfYC3D2c X3xe2SAYW5JqwLZtCtGKh6nhEYMnRVlZKzz+lEq/Xas19+bSIjxRy8V45VATbqf5hzFd uoPfRvil7P5pXo9vYoTShKEsrL6oe5sxgrJZr1kKwqZR9jHl2MTfKY1mlh8W6g2NFEs9 cyOw== Return-Path: From: Adhemerval Zanella To: libc-alpha@sourceware.org Subject: [PATCH 6/7] Block all signals on timer_create thread (BZ#10815) Date: Tue, 10 Dec 2019 15:32:20 -0300 Message-Id: <20191210183221.26912-6-adhemerval.zanella@linaro.org> In-Reply-To: <20191210183221.26912-1-adhemerval.zanella@linaro.org> References: <20191210183221.26912-1-adhemerval.zanella@linaro.org> 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 the signal while executing the timer threads created by 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 by only unblock the SIGSETXID (used on set*uid). The SIGTIMER is the same as SIGCANCEL and it will be handled by sigwaitinfo on the helper thread (thus it can be masked as well). Checked on x86_64-linux-gnu and i686-linux-gnu. --- rt/Makefile | 7 +- rt/tst-timer-sigmask.c | 96 +++++++++++++++++++ sysdeps/unix/sysv/linux/internal-signals.h | 14 +++ sysdeps/unix/sysv/linux/kernel-posix-timers.h | 1 - sysdeps/unix/sysv/linux/timer_routines.c | 96 +++++++------------ 5 files changed, 148 insertions(+), 66 deletions(-) create mode 100644 rt/tst-timer-sigmask.c -- 2.17.1 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..1030e4c79f --- /dev/null +++ b/rt/tst-timer-sigmask.c @@ -0,0 +1,96 @@ +/* Check resulting signal mask from POSIX time 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_mutex_t lock = PTHREAD_MUTEX_INITIALIZER; +static pthread_cond_t cond = PTHREAD_COND_INITIALIZER; +static enum +{ + THREAD_SIGNAL_CHECK_INIT = 0, + THREAD_SIGNAL_CHECK_OK, + THREAD_SIGNAL_CHECK_FAIL +} thread_handler_check = THREAD_SIGNAL_CHECK_INIT; + +static void +thread_handler (union sigval sv) +{ + bool failure = false; + + 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 blocks + all signals except SIGKILL, SIGSTOP, and SIGSETXID. */ + if (!sigismember (&ss, sig) + && (sig != SIGSETXID && sig != SIGKILL && sig != SIGSTOP)) + { + failure = true; + } +#endif + if (test_verbose && sigismember (&ss, sig)) + printf ("%d, ", sig); + } + if (test_verbose > 0) + printf ("}\n"); + + pthread_mutex_lock (&lock); + thread_handler_check = failure + ? THREAD_SIGNAL_CHECK_FAIL : THREAD_SIGNAL_CHECK_OK; + pthread_cond_signal (&cond); + pthread_mutex_unlock (&lock); +} + +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); + + struct itimerspec trigger = { 0 }; + trigger.it_value.tv_nsec = 1000000; + TEST_COMPARE (timer_settime (timerid, 0, &trigger, NULL), 0); + + pthread_mutex_lock (&lock); + while (thread_handler_check == THREAD_SIGNAL_CHECK_INIT) + pthread_cond_wait (&cond, &lock); + pthread_mutex_unlock (&lock); + + TEST_COMPARE (thread_handler_check, THREAD_SIGNAL_CHECK_OK); + + return 0; +} + +#include diff --git a/sysdeps/unix/sysv/linux/internal-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h index 04e1ec4f0a..f155a8cd32 100644 --- a/sysdeps/unix/sysv/linux/internal-signals.h +++ b/sysdeps/unix/sysv/linux/internal-signals.h @@ -70,6 +70,11 @@ static const sigset_t sigapp_set = { #endif }; +static const sigset_t sigtimer_set = { + .__val = { [0] = __sigmask (SIGTIMER), + [1 ... _SIGSET_NWORDS-1] = 0 } +}; + /* Block all signals, including internal glibc ones. */ static inline void __libc_signal_block_all (sigset_t *set) @@ -88,6 +93,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/kernel-posix-timers.h b/sysdeps/unix/sysv/linux/kernel-posix-timers.h index 1ded4df51a..d60cb95f80 100644 --- a/sysdeps/unix/sysv/linux/kernel-posix-timers.h +++ b/sysdeps/unix/sysv/linux/kernel-posix-timers.h @@ -43,7 +43,6 @@ extern pthread_mutex_t __active_timer_sigev_thread_lock attribute_hidden; /* Type of timers in the kernel. */ typedef int kernel_timer_t; - /* Internal representation of timer. */ struct timer { diff --git a/sysdeps/unix/sysv/linux/timer_routines.c b/sysdeps/unix/sysv/linux/timer_routines.c index d96d963177..98bbe077fa 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 SIGCANCEL is not blocked throughout. */ sigset_t ss; - sigset_t oss; - sigfillset (&ss); - __sigaddset (&ss, SIGCANCEL); - 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);