From patchwork Sat Apr 21 18:37:14 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "H.J. Lu" X-Patchwork-Id: 26897 Received: (qmail 94654 invoked by alias); 21 Apr 2018 18:37:23 -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 94644 invoked by uid 89); 21 Apr 2018 18:37:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.8 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, 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.2 spammy=H*RU:74.125.83.68, Hx-spam-relays-external:74.125.83.68, 2072 X-HELO: mail-pg0-f68.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=uD73HbgSIiNEg4r4+t0Kok8dyc+Em2x1AYF7tLSQYnM=; b=rwUOH8bZAjffMsU4qvzhXqTgS048Ga6VzTg7aXMKwW9LxNn3DtYbL57PbM/rl3xQR1 XuwiaqZYWpgxMgviUMJcWpqwUTGXfSysE2mC70UjYq+cDXsGeQSG9wmc5C0p7viW4E/C op23V32kvpGHRXnE2K51X/Vq1l4EwOo60B0BqZJaDNHq4/x8s14FQ7Js2IszOa2VqAzY p9NHLAliOa9YZ5RW7hvMd8tE0hAbR5GNjUHaDryzqteDMjipe7bNjJisiIOIeRDKgQts mWZMbkhpKToIvL/2QOxUrzsOiwpHJNQv2V8soR7b2kFrOQqzqR8/ya1jRsegB2Q0aqI8 Crhg== X-Gm-Message-State: ALQs6tAORlpZQxdZQFGe+vf4p4Nd/BDbeYwAxMDwNow5Iuy96bJEqrAi zcH4HYr4gMfDVZOxhALM0qjrVQ== X-Google-Smtp-Source: AIpwx4+JmlYvbIDuaDX+AHnTAg3zUSGfbwXOjBtkjH9clY//GS3Pgp/Z2+T7U0sHz9SfgcPXKTC07Q== X-Received: by 10.101.64.140 with SMTP id t12mr11892113pgp.98.1524335836362; Sat, 21 Apr 2018 11:37:16 -0700 (PDT) Date: Sat, 21 Apr 2018 11:37:14 -0700 From: "H.J. Lu" To: Carlos O'Donell Cc: Florian Weimer , Joseph Myers , "Tsimbalist, Igor V" , GNU C Library Subject: [PATCH/v3] x86: Use pad in pthread_unwind_buf to preserve shadow stack register Message-ID: <20180421183714.GA16402@gmail.com> References: <61a5b452-e59e-dfef-4530-a94a60480961@redhat.com> <2dc14d06-a06c-1aeb-dca3-f97f4ee68415@redhat.com> <3c91d59c-8c1b-2190-0a6a-05fa16d706ee@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <3c91d59c-8c1b-2190-0a6a-05fa16d706ee@redhat.com> User-Agent: Mutt/1.9.2 (2017-12-15) On Fri, Apr 20, 2018 at 10:28:24PM -0500, Carlos O'Donell wrote: > Lets call this v2. Subject adjusted. Please keep incrementing the version > number on your patches to make the review easier by myself and others. > Done. > > + > > + /* No previous handlers. NB: This must be done after setjmp since > > + the private space in the unwind jump buffer may overlap space > > + used by setjmp to store extra architecture-specific information > > + which is never be used by the cancellation-specific > > s/be//g Done > > +/* Number of bits per long. */ > > +#define _JUMP_BUF_SIGSET_BITS_PER_WORD (8 * sizeof (unsigned long int)) > > +/* The biggest signal number. As of kernel 4.14, x86 _NSIG is 64. > > + Define it to 96 to leave some rooms for future use. */ > > Why 96? > > Sure on x86_64 the cancel jump buf has 4 x void*'s and each is 64-bits, > so you have 128 signals worth of space and then the shadow stack pointer. > > Does this work on x32? > > On x32 you have only 4 void*'s in the private pad. > > Your presently structured sigset_t looks like this: > > typedef union > { > __sigset_t __saved_mask_compat; > struct > { > __jmp_buf_sigset_t __saved_mask; > /* Used for shadow stack pointer. */ > unsigned long int __shadow_stack_pointer; > } __saved; > } __jmpbuf_arch_t; > > Which means you have a sigset_t *before* the __shadow_stack_pointer. > > On x32 to save a 64-bit shadow stack pointer, you'll only have 2 x 32-bit > words left? That's only space for 64 signals? > > Are you counting on one additional 32-bit word of padding between the > __mask_was_saved and the rest of the long arguments? > > If so, then this still needs spelling out in an a comment why 96 signals > works on both i686, x32, and x86_64. > > Also it should be explained if 96 is the *maximum* we can do with the current > layout, or if more is possible. In which case picking 96 is *not* arbitrary. The pad array in struct pthread_unwind_buf is used by setjmp to save shadow stack register. The usable space in __saved_mask for sigset and shadow stack pointer: 1. i386: The 4x4 byte pad array which can be used for 4 byte shadow stack pointer and maximum 12 byte sigset. 2. x32: 4 byte padding + the 4x4 byte pad array which can be used for 8 byte shadow stack pointer and maximum 12 byte sigset. 3. x86-64: The 4x8 byte pad array which can be used for 8 byte shadow stack pointer and maximum 24 byte sigset. Please see comments in sysdeps/unix/sysv/linux/x86/setjmpP.h for details. > Suggest: > ~~~ > The pad array in struct pthread_unwind_buf is used by setjmp to save > the shadow stack register. Assert that the size of struct pthread_unwind_buf > is no less than the offset of the shadow stack pointer plus the shadow stack > pointer size. > > NB: We use setjmp in thread cancellation and this saves the shadow stack > register, but __libc_unwind_longjmp doesn't restore the shadow stack register > since cancellation never returns after longjmp. > ~~~ > Done. > > This assertion is too loose. > > The assertion you need is that the shadow stack pointer itself falls within > the range of the private padding. This would have caught the previous bug > with the rounded up size. > > Please adjust the assertion to be as tight as possible or add new assertions. > > Completely untested, but just to show what I'm thinking: > > _Static_assert ((offsetof (struct pthread_unwind_buf, cancel_jump_buf[0].mask_was_saved) > + sizeof (int) < SHADOW_STACK_POINTER_OFFSET > && (offsetof (struct pthread_unwind_buf, priv.pad[4]) > > (SHADOW_STACK_POINTER_OFFSET + SHADOW_STACK_POINTER_SIZE), > "Shadow stack pointer is not within private storage of pthread_unwind_buf."); > I used a slighly different assert. > > Looking forward to a v3. > Here it is. H.J. Reviewed-by: Carlos O'Donell --- The pad array in struct pthread_unwind_buf is used by setjmp to save shadow stack register. We assert that size of struct pthread_unwind_buf is no less than offset of shadow stack pointer + shadow stack pointer size. Since functions, like LIBC_START_MAIN, START_THREAD_DEFN as well as these with thread cancellation, call setjmp, but never return after __libc_unwind_longjmp, __libc_unwind_longjmp, which is defined as __libc_longjmp on x86, doesn't need to restore shadow stack register. __libc_longjmp, which is a private interface for thread cancellation implementation in libpthread, is changed to call __longjmp_cancel, instead of __longjmp. __longjmp_cancel is a new internal function in libc, which is similar to __longjmp, but doesn't restore shadow stack register. The compatibility longjmp and siglongjmp in libpthread.so are changed to call __libc_siglongjmp, instead of __libc_longjmp, so that they will restore shadow stack register. Tested with build-many-glibcs.py. * nptl/pthread_create.c (START_THREAD_DEFN): Clear previous handlers after setjmp. * setjmp/longjmp.c (__libc_longjmp): Don't define alias if defined. * sysdeps/unix/sysv/linux/x86/setjmpP.h: Include . (_JUMP_BUF_SIGSET_BITS_PER_WORD): New. (_JUMP_BUF_SIGSET_NSIG): Changed to 96. (_JUMP_BUF_SIGSET_NWORDS): Changed to use ALIGN_UP and _JUMP_BUF_SIGSET_BITS_PER_WORD. * sysdeps/x86/Makefile (sysdep_routines): Add __longjmp_cancel. * sysdeps/x86/__longjmp_cancel.S: New file. * sysdeps/x86/longjmp.c: Likewise. * sysdeps/x86/nptl/pt-longjmp.c: Likewise. --- nptl/pthread_create.c | 17 +++++++-- setjmp/longjmp.c | 2 + sysdeps/unix/sysv/linux/x86/setjmpP.h | 71 ++++++++++++++++++++++++++++++++--- sysdeps/x86/Makefile | 4 ++ sysdeps/x86/__longjmp_cancel.S | 20 ++++++++++ sysdeps/x86/longjmp.c | 45 ++++++++++++++++++++++ sysdeps/x86/nptl/pt-longjmp.c | 71 +++++++++++++++++++++++++++++++++++ 7 files changed, 222 insertions(+), 8 deletions(-) create mode 100644 sysdeps/x86/__longjmp_cancel.S create mode 100644 sysdeps/x86/longjmp.c create mode 100644 sysdeps/x86/nptl/pt-longjmp.c diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index caaf07c134..92c945b12b 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -427,12 +427,23 @@ START_THREAD_DEFN compilers without that support we do use setjmp. */ struct pthread_unwind_buf unwind_buf; - /* No previous handlers. */ + int not_first_call; + not_first_call = setjmp ((struct __jmp_buf_tag *) unwind_buf.cancel_jmp_buf); + + /* No previous handlers. NB: This must be done after setjmp since the + private space in the unwind jump buffer may overlap space used by + setjmp to store extra architecture-specific information which is + never used by the cancellation-specific __libc_unwind_longjmp. + + The private space is allowed to overlap because the unwinder never + has to return through any of the jumped-to call frames, and thus + only a minimum amount of saved data need be stored, and for example, + need not include the process signal mask information. This is all + an optimization to reduce stack usage when pushing cancellation + handlers. */ unwind_buf.priv.data.prev = NULL; unwind_buf.priv.data.cleanup = NULL; - int not_first_call; - not_first_call = setjmp ((struct __jmp_buf_tag *) unwind_buf.cancel_jmp_buf); if (__glibc_likely (! not_first_call)) { /* Store the new cleanup handler info. */ diff --git a/setjmp/longjmp.c b/setjmp/longjmp.c index a2a7065a85..453889e103 100644 --- a/setjmp/longjmp.c +++ b/setjmp/longjmp.c @@ -40,9 +40,11 @@ __libc_siglongjmp (sigjmp_buf env, int val) } #ifndef __libc_siglongjmp +# ifndef __libc_longjmp /* __libc_longjmp is a private interface for cancellation implementation in libpthread. */ strong_alias (__libc_siglongjmp, __libc_longjmp) +# endif weak_alias (__libc_siglongjmp, _longjmp) weak_alias (__libc_siglongjmp, longjmp) weak_alias (__libc_siglongjmp, siglongjmp) diff --git a/sysdeps/unix/sysv/linux/x86/setjmpP.h b/sysdeps/unix/sysv/linux/x86/setjmpP.h index c0ed767a0d..6b2608453d 100644 --- a/sysdeps/unix/sysv/linux/x86/setjmpP.h +++ b/sysdeps/unix/sysv/linux/x86/setjmpP.h @@ -20,13 +20,72 @@ #define _SETJMPP_H 1 #include +#include -/* The biggest signal number + 1. As of kernel 4.14, x86 _NSIG is 64. - Define it to 513 to leave some rooms for future use. */ -#define _JUMP_BUF_SIGSET_NSIG 513 +/* has + +struct __jmp_buf_tag + { + __jmp_buf __jmpbuf; + int __mask_was_saved; + __sigset_t __saved_mask; + }; + + struct __jmp_buf_tag is 32 bits aligned on i386 and is 64 bits + aligned on x32 and x86-64. __saved_mask is aligned to 32 bits + on i386/x32 without padding and is aligned to 64 bits on x86-64 + with 32 bit padding. + + and has + +struct pthread_unwind_buf +{ + struct + { + __jmp_buf jmp_buf; + int mask_was_saved; + } cancel_jmp_buf[1]; + + union + { + void *pad[4]; + struct + { + struct pthread_unwind_buf *prev; + struct _pthread_cleanup_buffer *cleanup; + int canceltype; + } data; + } priv; +}; + + struct pthread_unwind_buf is 32 bits aligned on i386 and 64 bits + aligned on x32/x86-64. cancel_jmp_buf is aligned to 32 bits on + i386 and is aligned to 64 bits on x32/x86-64. + + The pad array in struct pthread_unwind_buf is used by setjmp to save + shadow stack register. The usable space in __saved_mask for sigset + and shadow stack pointer: + 1. i386: The 4x4 byte pad array which can be used for 4 byte shadow + stack pointer and maximum 12 byte sigset. + 2. x32: 4 byte padding + the 4x4 byte pad array which can be used + for 8 byte shadow stack pointer and maximum 12 byte sigset. + 3. x86-64: The 4x8 byte pad array which can be used for 8 byte + shadow stack pointer and maximum 24 byte sigset. + + NB: We use setjmp in thread cancellation and this saves the shadow + stack register, but __libc_unwind_longjmp doesn't restore the shadow + stack register since cancellation never returns after longjmp. */ + +/* Number of bits per long. */ +#define _JUMP_BUF_SIGSET_BITS_PER_WORD (8 * sizeof (unsigned long int)) +/* The biggest signal number. As of kernel 4.14, x86 _NSIG is 64. The + common maximum sigset for i386, x32 and x86-64 is 12 bytes (96 bits). + Define it to 96 to leave some rooms for future use. */ +#define _JUMP_BUF_SIGSET_NSIG 96 /* Number of longs to hold all signals. */ #define _JUMP_BUF_SIGSET_NWORDS \ - ((_JUMP_BUF_SIGSET_NSIG - 1 + 7) / (8 * sizeof (unsigned long int))) + (ALIGN_UP (_JUMP_BUF_SIGSET_NSIG, _JUMP_BUF_SIGSET_BITS_PER_WORD) \ + / _JUMP_BUF_SIGSET_BITS_PER_WORD) typedef struct { @@ -39,7 +98,9 @@ typedef union struct { __jmp_buf_sigset_t __saved_mask; - /* Used for shadow stack pointer. */ + /* Used for shadow stack pointer. NB: Shadow stack pointer + must have the same alignment as __saved_mask. Otherwise + offset of __saved_mask will be changed. */ unsigned long int __shadow_stack_pointer; } __saved; } __jmpbuf_arch_t; diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile index 0d0326c21a..d25d6f0ae4 100644 --- a/sysdeps/x86/Makefile +++ b/sysdeps/x86/Makefile @@ -8,3 +8,7 @@ sysdep-dl-routines += dl-get-cpu-features tests += tst-get-cpu-features tests-static += tst-get-cpu-features-static endif + +ifeq ($(subdir),setjmp) +sysdep_routines += __longjmp_cancel +endif diff --git a/sysdeps/x86/__longjmp_cancel.S b/sysdeps/x86/__longjmp_cancel.S new file mode 100644 index 0000000000..b57dbfa376 --- /dev/null +++ b/sysdeps/x86/__longjmp_cancel.S @@ -0,0 +1,20 @@ +/* __longjmp_cancel for x86. + 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 + . */ + +#define __longjmp __longjmp_cancel +#include <__longjmp.S> diff --git a/sysdeps/x86/longjmp.c b/sysdeps/x86/longjmp.c new file mode 100644 index 0000000000..a53f31e1dd --- /dev/null +++ b/sysdeps/x86/longjmp.c @@ -0,0 +1,45 @@ +/* __libc_siglongjmp for x86. + 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 + . */ + +#define __libc_longjmp __redirect___libc_longjmp +#include +#undef __libc_longjmp + +extern void __longjmp_cancel (__jmp_buf __env, int __val) + __attribute__ ((__noreturn__)) attribute_hidden; + +/* Since __libc_longjmp is a private interface for cancellation + implementation in libpthread, there is no need to restore shadow + stack register. */ + +void +__libc_longjmp (sigjmp_buf env, int val) +{ + /* Perform any cleanups needed by the frames being unwound. */ + _longjmp_unwind (env, val); + + if (env[0].__mask_was_saved) + /* Restore the saved signal mask. */ + (void) __sigprocmask (SIG_SETMASK, + (sigset_t *) &env[0].__saved_mask, + (sigset_t *) NULL); + + /* Call the machine-dependent function to restore machine state + without shadow stack. */ + __longjmp_cancel (env[0].__jmpbuf, val ?: 1); +} diff --git a/sysdeps/x86/nptl/pt-longjmp.c b/sysdeps/x86/nptl/pt-longjmp.c new file mode 100644 index 0000000000..6165c7d4a7 --- /dev/null +++ b/sysdeps/x86/nptl/pt-longjmp.c @@ -0,0 +1,71 @@ +/* ABI compatibility for 'longjmp' and 'siglongjmp' symbols in libpthread ABI. + X86 version. + Copyright (C) 18 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 + +#ifdef __x86_64__ +# define SHADOW_STACK_POINTER_SIZE 8 +#else +# define SHADOW_STACK_POINTER_SIZE 4 +#endif + +/* Assert that the priv field in struct pthread_unwind_buf has space + to store shadow stack pointer. */ +_Static_assert ((offsetof (struct pthread_unwind_buf, priv) + <= SHADOW_STACK_POINTER_OFFSET) + && ((offsetof (struct pthread_unwind_buf, priv) + + sizeof (((struct pthread_unwind_buf *) 0)->priv)) + >= (SHADOW_STACK_POINTER_OFFSET + + SHADOW_STACK_POINTER_SIZE)), + "Shadow stack pointer is not within private storage " + "of pthread_unwind_buf."); + +#include + +/* libpthread once had its own longjmp (and siglongjmp alias), though there + was no apparent reason for it. There is no use in having a separate + symbol in libpthread, but the historical ABI requires it. For static + linking, there is no need to provide anything here--the libc version + will be linked in. For shared library ABI compatibility, there must be + longjmp and siglongjmp symbols in libpthread.so. + + With an IFUNC resolver, it would be possible to avoid the indirection, + but the IFUNC resolver might run before the __libc_longjmp symbol has + been relocated, in which case the IFUNC resolver would not be able to + provide the correct address. */ + +#if SHLIB_COMPAT (libpthread, GLIBC_2_0, GLIBC_2_22) + +static void __attribute__ ((noreturn, used)) +longjmp_compat (jmp_buf env, int val) +{ + /* NB: We call __libc_siglongjmp, instead of __libc_longjmp, since + __libc_longjmp is a private interface for cancellation which + doesn't restore shadow stack register. */ + __libc_siglongjmp (env, val); +} + +strong_alias (longjmp_compat, longjmp_alias) +compat_symbol (libpthread, longjmp_alias, longjmp, GLIBC_2_0); + +strong_alias (longjmp_alias, siglongjmp_alias) +compat_symbol (libpthread, siglongjmp_alias, siglongjmp, GLIBC_2_0); + +#endif