From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 128185 invoked by alias); 13 Jul 2018 18:51:48 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 128172 invoked by uid 89); 13 Jul 2018 18:51:48 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.8 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,KAM_SHORT,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy=H*M:4884 X-HELO: mail-qt0-f180.google.com Return-Path: Subject: Re: V2: [PATCH 01/24] x86: Rename __glibc_reserved1 to feature_1 in tcbhead_t [BZ #22563] To: "H.J. Lu" , libc-alpha@sourceware.org References: <20180613153207.57232-1-hjl.tools@gmail.com> <20180613153207.57232-2-hjl.tools@gmail.com> <20180713131908.GB2606@gmail.com> From: Carlos O'Donell Message-ID: <663c719e-febe-4884-f814-bf79e2e553ec@redhat.com> Date: Fri, 13 Jul 2018 18:51:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <20180713131908.GB2606@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-07/txt/msg00393.txt.bz2 On 07/13/2018 09:19 AM, H.J. Lu wrote: > On Wed, Jun 13, 2018 at 08:31:44AM -0700, H.J. Lu wrote: >> This will be used by CET run-time control. >> >> [BZ #22563] >> * nptl/pthread_create.c (__pthread_create_2_1): Use >> THREAD_COPY_ADDITONAL_INFO to copy additonal info if defined. >> * sysdeps/i386/nptl/tcb-offsets.sym (FEATURE_1_OFFSET): New. >> * sysdeps/x86_64/nptl/tcb-offsets.sym (FEATURE_1_OFFSET): >> Likewise. >> * sysdeps/i386/nptl/tls.h (tcbhead_t): Rename __glibc_reserved1 >> to feature_1. >> * sysdeps/x86_64/nptl/tls.h (tcbhead_t): Likewise. >> * sysdeps/unix/sysv/linux/x86/pthreaddef.h: New file. > > Here is the updated patch to add feature_1 to tcbhead_t and > introduce macros for CET enabling. OK for master? Fix the typo-prone macro API and post a v3 please. Thank you. > > H.J. > ---- > feature_1 has X86_FEATURE_1_IBT and X86_FEATURE_1_SHSTK bits for CET > run-time control. > > CET_ENABLED, IBT_ENABLED and SHSTK_ENABLED are defined to 1 or 0 to > indicate that if CET, IBT and SHSTK are enabled. OK. > > [BZ #22563] > * nptl/pthread_create.c (__pthread_create_2_1): Use > THREAD_COPY_ADDITONAL_INFO to copy additonal info if defined. > * sysdeps/i386/nptl/tcb-offsets.sym (FEATURE_1_OFFSET): New. > * sysdeps/x86_64/nptl/tcb-offsets.sym (FEATURE_1_OFFSET): > Likewise. > * sysdeps/i386/nptl/tls.h (tcbhead_t): Rename __glibc_reserved1 > to feature_1. > * sysdeps/x86_64/nptl/tls.h (tcbhead_t): Likewise. > * sysdeps/unix/sysv/linux/x86/pthreaddef.h: New file. > * sysdeps/x86/sysdep.h (X86_FEATURE_1_IBT): New. > (X86_FEATURE_1_SHSTK): Likewise. > (CET_ENABLED): Likewise. > (IBT_ENABLED): Likewise. > (SHSTK_ENABLED): Likewise. > --- > nptl/pthread_create.c | 5 +++++ > sysdeps/i386/nptl/tcb-offsets.sym | 1 + > sysdeps/i386/nptl/tls.h | 5 ++++- > sysdeps/unix/sysv/linux/x86/pthreaddef.h | 24 +++++++++++++++++++++ > sysdeps/x86/sysdep.h | 27 ++++++++++++++++++++++++ > sysdeps/x86_64/nptl/tcb-offsets.sym | 1 + > sysdeps/x86_64/nptl/tls.h | 5 ++++- > 7 files changed, 66 insertions(+), 2 deletions(-) > create mode 100644 sysdeps/unix/sysv/linux/x86/pthreaddef.h > > diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c > index 92c945b12b..16998f4bbd 100644 > --- a/nptl/pthread_create.c > +++ b/nptl/pthread_create.c > @@ -712,6 +712,11 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr, > THREAD_COPY_POINTER_GUARD (pd); > #endif > > + /* Copy additonal info. */ > +#ifdef THREAD_COPY_ADDITONAL_INFO > + THREAD_COPY_ADDITONAL_INFO (pd); > +#endif This is a typo-prone macro API. Please find a way to define this unconditionally. > + > /* Verify the sysinfo bits were copied in allocate_stack if needed. */ > #ifdef NEED_DL_SYSINFO > CHECK_THREAD_SYSINFO (pd); > diff --git a/sysdeps/i386/nptl/tcb-offsets.sym b/sysdeps/i386/nptl/tcb-offsets.sym > index 7d7fe5e71c..fbac241c45 100644 > --- a/sysdeps/i386/nptl/tcb-offsets.sym > +++ b/sysdeps/i386/nptl/tcb-offsets.sym > @@ -12,3 +12,4 @@ CLEANUP offsetof (struct pthread, cleanup) > CLEANUP_PREV offsetof (struct _pthread_cleanup_buffer, __prev) > MUTEX_FUTEX offsetof (pthread_mutex_t, __data.__lock) > POINTER_GUARD offsetof (tcbhead_t, pointer_guard) > +FEATURE_1_OFFSET offsetof (tcbhead_t, feature_1) > diff --git a/sysdeps/i386/nptl/tls.h b/sysdeps/i386/nptl/tls.h > index afb71ce431..21e23cd809 100644 > --- a/sysdeps/i386/nptl/tls.h > +++ b/sysdeps/i386/nptl/tls.h > @@ -41,7 +41,10 @@ typedef struct > uintptr_t stack_guard; > uintptr_t pointer_guard; > int gscope_flag; > - int __glibc_reserved1; > + /* Bit 0: X86_FEATURE_1_IBT. > + Bit 1: X86_FEATURE_1_SHSTK. > + */ > + unsigned int feature_1; OK. > /* Reservation of some values for the TM ABI. */ > void *__private_tm[3]; > /* GCC split stack support. */ > diff --git a/sysdeps/unix/sysv/linux/x86/pthreaddef.h b/sysdeps/unix/sysv/linux/x86/pthreaddef.h > new file mode 100644 > index 0000000000..539f6540d0 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/x86/pthreaddef.h > @@ -0,0 +1,24 @@ > +/* Pthread macros. Linux/x86 version. > + Copyright (C) 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_next > + > +/* Wee need to copy feature_1 in pthread_create. */ > +#define THREAD_COPY_ADDITONAL_INFO(descr) \ > + ((descr)->header.feature_1 \ > + = THREAD_GETMEM (THREAD_SELF, header.feature_1)) OK. > diff --git a/sysdeps/x86/sysdep.h b/sysdeps/x86/sysdep.h > index afcb7cfd76..976566aa37 100644 > --- a/sysdeps/x86/sysdep.h > +++ b/sysdeps/x86/sysdep.h > @@ -21,6 +21,33 @@ > > #include > > +/* __CET__ is defined by GCC with Control-Flow Protection values: > + > +enum cf_protection_level > +{ > + CF_NONE = 0, > + CF_BRANCH = 1 << 0, > + CF_RETURN = 1 << 1, > + CF_FULL = CF_BRANCH | CF_RETURN, > + CF_SET = 1 << 2 > +}; > +*/ > + > +/* Set if CF_BRANCH (IBT) is enabled. */ > +#define X86_FEATURE_1_IBT (1U << 0) OK. Thanks for defining this, it makes the assembly easier to read. > +/* Set if CF_RETURN (SHSTK) is enabled. */ > +#define X86_FEATURE_1_SHSTK (1U << 1) OK. > + > +#ifdef __CET__ > +# define CET_ENABLED 1 > +# define IBT_ENABLED (__CET__ & X86_FEATURE_1_IBT) > +# define SHSTK_ENABLED (__CET__ & X86_FEATURE_1_SHSTK) > +#else > +# define CET_ENABLED 0 > +# define IBT_ENABLED 0 > +# define SHSTK_ENABLED 0 > +#endif > + > #ifdef __ASSEMBLER__ > > /* Syntactic details of assembler. */ > diff --git a/sysdeps/x86_64/nptl/tcb-offsets.sym b/sysdeps/x86_64/nptl/tcb-offsets.sym > index be63404a16..387621e88c 100644 > --- a/sysdeps/x86_64/nptl/tcb-offsets.sym > +++ b/sysdeps/x86_64/nptl/tcb-offsets.sym > @@ -12,6 +12,7 @@ MUTEX_FUTEX offsetof (pthread_mutex_t, __data.__lock) > MULTIPLE_THREADS_OFFSET offsetof (tcbhead_t, multiple_threads) > POINTER_GUARD offsetof (tcbhead_t, pointer_guard) > VGETCPU_CACHE_OFFSET offsetof (tcbhead_t, vgetcpu_cache) > +FEATURE_1_OFFSET offsetof (tcbhead_t, feature_1) OK. > > -- Not strictly offsets, but these values are also used in the TCB. > TCB_CANCELSTATE_BITMASK CANCELSTATE_BITMASK > diff --git a/sysdeps/x86_64/nptl/tls.h b/sysdeps/x86_64/nptl/tls.h > index 65c0051dcf..f042a0250a 100644 > --- a/sysdeps/x86_64/nptl/tls.h > +++ b/sysdeps/x86_64/nptl/tls.h > @@ -51,7 +51,10 @@ typedef struct > uintptr_t stack_guard; > uintptr_t pointer_guard; > unsigned long int vgetcpu_cache[2]; > - int __glibc_reserved1; > + /* Bit 0: X86_FEATURE_1_IBT. > + Bit 1: X86_FEATURE_1_SHSTK. > + */ > + unsigned int feature_1; OK. > int __glibc_unused1; > /* Reservation of some values for the TM ABI. */ > void *__private_tm[4]; > Cheers, Carlos.