From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from insect.birch.relay.mailchannels.net (insect.birch.relay.mailchannels.net [23.83.209.93]) by sourceware.org (Postfix) with ESMTPS id 5EAF63858D3C for ; Wed, 8 Dec 2021 18:04:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 5EAF63858D3C Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=gotplt.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gotplt.org X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from relay.mailchannels.net (localhost [127.0.0.1]) by relay.mailchannels.net (Postfix) with ESMTP id C3DB16C1049; Wed, 8 Dec 2021 18:04:11 +0000 (UTC) Received: from pdx1-sub0-mail-a304.dreamhost.com (unknown [127.0.0.6]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id 9D1506C173F; Wed, 8 Dec 2021 18:04:10 +0000 (UTC) X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from pdx1-sub0-mail-a304.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384) by 100.120.81.174 (trex/6.4.3); Wed, 08 Dec 2021 18:04:11 +0000 X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Bottle-Glossy: 0d1d49824ef64b08_1638986651301_2025327396 X-MC-Loop-Signature: 1638986651300:437897814 X-MC-Ingress-Time: 1638986651300 Received: from [192.168.52.116] (unknown [223.185.2.127]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) (Authenticated sender: siddhesh@gotplt.org) by pdx1-sub0-mail-a304.dreamhost.com (Postfix) with ESMTPSA id 4J8Q5w6W7Lz18q; Wed, 8 Dec 2021 10:04:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gotplt.org; s=gotplt.org; t=1638986647; bh=RM1BSpEGsK1tvjQb6nmPj0P3QdQ=; h=Date:Subject:To:From:Content-Type:Content-Transfer-Encoding; b=qBknkKYYVhlguB4kdBy5c4Da9gy/P8Mk14WzkuOMm2Cq0VWmICbSn9uxHhOgZWYhx P0zlptMlmAd06pNkJ3ivmBKqrUxlT8bhHns/tFFGnoLcyCABvlYCIOi17PJf7S6QnG IOogRkdna7NRNkk9YQiPx9cg1l4gRWlP4OmyDKc4= Message-ID: Date: Wed, 8 Dec 2021 23:33:59 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.0 Subject: Re: [PATCH v2 6/8] nptl: Add glibc.pthread.rseq tunable to control rseq registration Content-Language: en-US To: Florian Weimer , libc-alpha@sourceware.org References: From: Siddhesh Poyarekar In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3039.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Dec 2021 18:04:16 -0000 On 12/7/21 18:32, Florian Weimer via Libc-alpha wrote: > This tunable allows applications to register the rseq area instead > of glibc. > --- > v2: Unchanged. > > manual/tunables.texi | 10 +++ > nptl/pthread_create.c | 10 ++- > sysdeps/nptl/dl-tls_init_tp.c | 11 ++- > sysdeps/nptl/dl-tunables.list | 6 ++ > sysdeps/nptl/internaltypes.h | 1 + > sysdeps/unix/sysv/linux/Makefile | 8 ++ > sysdeps/unix/sysv/linux/rseq-internal.h | 19 +++-- > sysdeps/unix/sysv/linux/tst-rseq-disable.c | 89 ++++++++++++++++++++++ > 8 files changed, 145 insertions(+), 9 deletions(-) > create mode 100644 sysdeps/unix/sysv/linux/tst-rseq-disable.c > > diff --git a/manual/tunables.texi b/manual/tunables.texi > index 10f4d75993..5d50b90f64 100644 > --- a/manual/tunables.texi > +++ b/manual/tunables.texi > @@ -424,6 +424,16 @@ The value is measured in bytes. The default is @samp{41943040} > (fourty mibibytes). > @end deftp > > +@deftp Tunable glibc.pthread.rseq > +The @code{glibc.pthread.rseq} tunable can be set to @samp{0}, to disable > +restartable sequences support in @theglibc{}. This enables applications > +to perform direct restartable sequence registration with the kernel. > +The default is @samp{1}, which means that @theglibc{} performs > +registration on behalf of the application. > + > +Restartable sequences are a Linux-specific extension. > +@end deftp > + OK. > @node Hardware Capability Tunables > @section Hardware Capability Tunables > @cindex hardware capability tunables > diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c > index ea0d79341e..4608fd9068 100644 > --- a/nptl/pthread_create.c > +++ b/nptl/pthread_create.c > @@ -368,7 +368,10 @@ start_thread (void *arg) > __ctype_init (); > > /* Register rseq TLS to the kernel. */ > - rseq_register_current_thread (pd); > + { > + bool do_rseq = THREAD_GETMEM (pd, flags) & ATTR_FLAG_DO_RSEQ; > + rseq_register_current_thread (pd, do_rseq); > + } > OK, the flag is set... > #ifndef __ASSUME_SET_ROBUST_LIST > if (__nptl_set_robust_list_avail) > @@ -677,6 +680,11 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr, > pd->flags = ((iattr->flags & ~(ATTR_FLAG_SCHED_SET | ATTR_FLAG_POLICY_SET)) > | (self->flags & (ATTR_FLAG_SCHED_SET | ATTR_FLAG_POLICY_SET))); > > + /* Inherit rseq registration state. Without seccomp filters, rseq > + registration will either always fail or always succeed. */ > + if ((int) THREAD_GETMEM_VOLATILE (self, rseq_area.cpu_id) >= 0) > + pd->flags |= ATTR_FLAG_DO_RSEQ; > + ... here, as is inherited from the calling thread, which should eventually be inherited from the main thread and through the tunable. Further to my comment in 4/8, if we leave it as UNINITIALIZED, we could simply modify the check to != -1. > /* Initialize the field for the ID of the thread which is waiting > for us. This is a self-reference in case the thread is created > detached. */ > diff --git a/sysdeps/nptl/dl-tls_init_tp.c b/sysdeps/nptl/dl-tls_init_tp.c > index fedb876fdb..b39dfbff2c 100644 > --- a/sysdeps/nptl/dl-tls_init_tp.c > +++ b/sysdeps/nptl/dl-tls_init_tp.c > @@ -23,6 +23,9 @@ > #include > #include > > +#define TUNABLE_NAMESPACE pthread > +#include > + > #ifndef __ASSUME_SET_ROBUST_LIST > bool __nptl_set_robust_list_avail; > rtld_hidden_data_def (__nptl_set_robust_list_avail) > @@ -92,7 +95,13 @@ __tls_init_tp (void) > } > } > > - rseq_register_current_thread (pd); > + { > + bool do_rseq = true; > +#if HAVE_TUNABLES > + do_rseq = TUNABLE_GET (rseq, int, NULL); > +#endif > + rseq_register_current_thread (pd, do_rseq); > + } rseq registration for the main thread. OK. > > /* Set initial thread's stack block from 0 up to __libc_stack_end. > It will be bigger than it actually is, but for unwind.c/pt-longjmp.c > diff --git a/sysdeps/nptl/dl-tunables.list b/sysdeps/nptl/dl-tunables.list > index ac5d053298..d24f4be0d0 100644 > --- a/sysdeps/nptl/dl-tunables.list > +++ b/sysdeps/nptl/dl-tunables.list > @@ -27,5 +27,11 @@ glibc { > type: SIZE_T > default: 41943040 > } > + rseq { > + type: INT_32 > + minval: 0 > + maxval: 1 > + default: 1 > + } Tunable defaults to 1 and can only have two values. OK. > } > } > diff --git a/sysdeps/nptl/internaltypes.h b/sysdeps/nptl/internaltypes.h > index 6032a6b785..dec8c5b5ff 100644 > --- a/sysdeps/nptl/internaltypes.h > +++ b/sysdeps/nptl/internaltypes.h > @@ -48,6 +48,7 @@ struct pthread_attr > #define ATTR_FLAG_OLDATTR 0x0010 > #define ATTR_FLAG_SCHED_SET 0x0020 > #define ATTR_FLAG_POLICY_SET 0x0040 > +#define ATTR_FLAG_DO_RSEQ 0x0080 > > /* Used to allocate a pthread_attr_t object which is also accessed > internally. */ > diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile > index eb0f5fc021..62a796f214 100644 > --- a/sysdeps/unix/sysv/linux/Makefile > +++ b/sysdeps/unix/sysv/linux/Makefile > @@ -136,6 +136,12 @@ tests-internal += \ > tst-sigcontext-get_pc \ > # tests-internal > > +ifneq (no,$(have-tunables)) > +tests-internal += \ > + tst-rseq-disable \ > + # tests-internal $(have-tunables) > +endif > + Test conditional on tunables. OK. > tests-time64 += \ > tst-adjtimex-time64 \ > tst-clock_adjtime-time64 \ > @@ -227,6 +233,8 @@ $(objpfx)tst-mman-consts.out: ../sysdeps/unix/sysv/linux/tst-mman-consts.py > < /dev/null > $@ 2>&1; $(evaluate-test) > $(objpfx)tst-mman-consts.out: $(sysdeps-linux-python-deps) > > +tst-rseq-disable-ENV = GLIBC_TUNABLES=glibc.pthread.rseq=0 > + OK. > endif # $(subdir) == misc > > ifeq ($(subdir),time) > diff --git a/sysdeps/unix/sysv/linux/rseq-internal.h b/sysdeps/unix/sysv/linux/rseq-internal.h > index 909f547825..15bc7ffd6e 100644 > --- a/sysdeps/unix/sysv/linux/rseq-internal.h > +++ b/sysdeps/unix/sysv/linux/rseq-internal.h > @@ -21,22 +21,27 @@ > #include > #include > #include > +#include > #include > #include > > #ifdef RSEQ_SIG > static inline void > -rseq_register_current_thread (struct pthread *self) > +rseq_register_current_thread (struct pthread *self, bool do_rseq) > { > - int ret = INTERNAL_SYSCALL_CALL (rseq, > - &self->rseq_area, sizeof (self->rseq_area), > - 0, RSEQ_SIG); > - if (INTERNAL_SYSCALL_ERROR_P (ret)) > - THREAD_SETMEM (self, rseq_area.cpu_id, RSEQ_CPU_ID_REGISTRATION_FAILED); > + if (do_rseq) > + { > + int ret = INTERNAL_SYSCALL_CALL (rseq, &self->rseq_area, > + sizeof (self->rseq_area), > + 0, RSEQ_SIG); > + if (!INTERNAL_SYSCALL_ERROR_P (ret)) > + return; > + } > + THREAD_SETMEM (self, rseq_area.cpu_id, RSEQ_CPU_ID_REGISTRATION_FAILED); Register only if do_rseq. OK, but if we get rid of RSEQ_CPU_ID_REGISTRATION_FAILED and leave cpu_id untouched, this function could simply be an INTERNAL_SYSCALL_CALL and the do_rseq condition could be hoisted into the caller as the tunable check and ATTR_FLAG_DO_RSEQ flag check for the main thread and children threads respectively. > } > #else /* RSEQ_SIG */ > static inline void > -rseq_register_current_thread (struct pthread *self) > +rseq_register_current_thread (struct pthread *self, bool do_rseq) > { > THREAD_SETMEM (self, rseq_area.cpu_id, RSEQ_CPU_ID_REGISTRATION_FAILED); > } > diff --git a/sysdeps/unix/sysv/linux/tst-rseq-disable.c b/sysdeps/unix/sysv/linux/tst-rseq-disable.c > new file mode 100644 > index 0000000000..000e351872 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/tst-rseq-disable.c > @@ -0,0 +1,89 @@ > +/* Test disabling of rseq registration via tunable. > + Copyright (C) 2021 Free Software Foundation, Inc. > + > + 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 > + > +#ifdef RSEQ_SIG > + > +/* Check that rseq can be registered and has not been taken by glibc. */ > +static void > +check_rseq_disabled (void) > +{ > + struct pthread *pd = THREAD_SELF; > + TEST_COMPARE ((int) pd->rseq_area.cpu_id, RSEQ_CPU_ID_REGISTRATION_FAILED); If we use UNINITIALIZED, then that's what we would check for. > + > + int ret = syscall (__NR_rseq, &pd->rseq_area, sizeof (pd->rseq_area), > + 0, RSEQ_SIG); > + if (ret == 0) > + { > + ret = syscall (__NR_rseq, &pd->rseq_area, sizeof (pd->rseq_area), > + RSEQ_FLAG_UNREGISTER, RSEQ_SIG); > + TEST_COMPARE (ret, 0); > + pd->rseq_area.cpu_id = RSEQ_CPU_ID_REGISTRATION_FAILED; Is this needed because the kernel sets cpu_id to UNINITIALIZED? > + } > + else > + { > + TEST_VERIFY (errno != -EINVAL); > + TEST_VERIFY (errno != -EBUSY); > + } > +} > + > +static void * > +thread_func (void *ignored) > +{ > + check_rseq_disabled (); > + return NULL; > +} Checking threads. OK. > + > +static void > +proc_func (void *ignored) > +{ > + check_rseq_disabled (); > +} Checking at process level. OK. > + > +static int > +do_test (void) > +{ > + puts ("info: checking main thread"); > + check_rseq_disabled (); > + > + puts ("info: checking main thread (2)"); > + check_rseq_disabled (); > + > + puts ("info: checking new thread"); > + xpthread_join (xpthread_create (NULL, thread_func, NULL)); > + > + puts ("info: checking subprocess"); > + support_isolate_in_subprocess (proc_func, NULL); > + > + return 0; > +} > +#else /* !RSEQ_SIG */ > +static int > +do_test (void) > +{ > + FAIL_UNSUPPORTED ("glibc does not define RSEQ_SIG, skipping test"); > +} > +#endif > + > +#include > Thanks, Siddhesh