From patchwork Fri Apr 6 20:26:41 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: 26630 Received: (qmail 90069 invoked by alias); 6 Apr 2018 20:26:50 -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 90058 invoked by uid 89); 6 Apr 2018 20:26:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-23.3 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, UNSUBSCRIBE_BODY autolearn=ham version=3.3.2 spammy= X-HELO: mail-ot0-f194.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=0IVlSGTWQu5vIdS7waXJEp05/BwbzBoC8qwjWHhPPOs=; b=kH8Vuy/SWk5ux1RfwkJfnhpSLjKWjgnoBtvTcdtS1V7WgqsxAjozBztvWuFrUoREIg otesfiqkI1EAiC5T9kxkxv7GLz3C3PnE+po8hH/YaSHB3Nw2/Lwo/SMKLfx13eY+CrE7 +5rK+j9Wi3bwaJgxHjqSwZCxx0jiiw+PTXJBcrYSJ0SP5NHKLKVSEbyA+50l9Wk2hdAT M25lCEzBUAlDAGX8e1YUrUICaYQIFiyG3m6i4p3MnMHi5xH0UCtQE+bO6+5Lr+ok/FjH UILOyzYQcDQPPjbj+SsfxOXIU7Bn2n5irebhZf40NLwimiKnH6X5OUS6A+Jmm1jwwY8A IGig== X-Gm-Message-State: ALQs6tAmiISeOjhPgApUU8wmhkXU/f9wXzTr36lHs0Tj6tkNz5+hCWZK dL6MIKznZKeNpPHYlQPLNF7q0IZ7NKzDXNMUzBE= X-Google-Smtp-Source: AIpwx49QyciBKpQAIirYzC/gFwrWbVhnLyBYaByolzBBPhEU5ogOfHmcXMYyhyBDVNN0PUjZnn8E5Z0hlL7tVgD5q8g= X-Received: by 2002:a9d:5186:: with SMTP id y6-v6mr3791868otg.125.1523046402234; Fri, 06 Apr 2018 13:26:42 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <61a5b452-e59e-dfef-4530-a94a60480961@redhat.com> From: "H.J. Lu" Date: Fri, 6 Apr 2018 13:26:41 -0700 Message-ID: Subject: Re: [PATCH] x86: Use pad in pthread_unwind_buf to preserve shadow stack register To: "Carlos O'Donell" Cc: Florian Weimer , Joseph Myers , "Tsimbalist, Igor V" , GNU C Library On Fri, Apr 6, 2018 at 5:59 AM, H.J. Lu wrote: > On Thu, Apr 5, 2018 at 9:46 PM, Carlos O'Donell wrote: >> On 03/30/2018 12:41 PM, H.J. Lu wrote: >>> On Thu, Mar 29, 2018 at 1:20 PM, Florian Weimer wrote: >>>> * H. J. Lu: >>>> >>>>> On Thu, Mar 29, 2018 at 1:15 PM, Florian Weimer wrote: >>>>>> * H. J. Lu: >>>>>> >>>>>>> You need to make a choice. You either don't introduce a new symbol >>>>>>> version or don't save shadow stack for thread cancellation. You >>>>>>> can't have both. >>>>>> I don't understand. We have room to save the shadow stack pointer in >>>>>> the existing struct. >>>>> No, we don't have room in struct pthread_unwind_buf: >>>>> >>>>> Note: There is an unused pointer space in pthread_unwind_buf_data. But >>>>> it isn't suitable for saving and restoring shadow stack register since >>>>> x32 is a 64-bit process with 32-bit software pointer and kernel may >>>>> place x32 shadow stack above 4GB. We need to save and restore 64-bit >>>>> shadow stack register for x32. >>>> We have for void * fields. They are subsequently overwritten by >>>> __pthread_register_cancel. But __sigsetjmp can write to them first >>>> without causing any harm. We just need a private __longjmp_cancel >>>> that doesn't restore the shadow stack pointer. >>> Here is the patch which does that. Any comments? >> >> OK, so I have reviewed https://github.com/hjl-tools/glibc/ hjl/cet/master, >> please confirm that this is the correct branch of the required implementation >> for CET. It really helps to review the rest of the patch set, you should be >> preparing this as a patch set instead of having it reviewed one-at-a-time. >> This issue was already raised in the thread with Zack. > > Thanks for your feedbacks. > >> Architecture: >> >> * We avoid a "flag day" with feature_1 TCB flag to switch to a new ABI, >> which we have discussed is a fragile process which should be avoided if >> a supportable alternative solution exists. >> >> * We avoid versioned symbols, this makes CET backportable, and this has a >> bigger benefit for long-term stable distributions. >> >> * A key design problem has been that cancellation jump buffers within glibc >> are truncated to optimize on-stack size, and this means that setjmp could >> write beyond the structure because setjmp now tries to save the shadowstack >> pointer into space that the cancellation jump buffer did not allocate. >> For the record this optimization seems premature and I'm sad we did it, this >> is a lesson we should learn from. >> >> * We have all agreed to the following concepts: >> >> * The cancellation process, in particular the unwinder, never returns from >> any of the functions we call, it just keeps calling into the unwinder to >> jump to the next unwound cancellation function all the way to the thread >> start routine. Therefore because we never return from one of these functions >> we never need to restore the shadow stack, and consequently wherever it is >> stored in the cancellation jump buffer can be overwritten if we need the >> space (it's a dead store). >> >> * The corollary to this is that function calls made from cancellation handlers >> will continue to advance the shadowstack from the deepest point at which >> cancellation is initiated from. This means that the depth of the shadowstack >> doesn't match the depth of the real stack while we are unwinding. I don't >> know if this will have consequences on analysis tooling or not, or debug >> tools during unwinding. It's a fairly advanced situation and corner case, >> and restoring the shadowstack is not useful becuase we don't need it and >> simplifies the implementation. >> >> * The cancellation jump buffer has private data used for chaining the cancel >> jump buffers together such that the custom unwinder can follow them and >> call them in sequence. This space constitutes 4 void *'s which is space >> that setjmp can write to, because we will just overwrite it when we register >> the cancel handler. >> >> * If the new shadowstack-enabled setjmp stores the shadowstack pointer into >> the space taken by the 4 void*'s then we won't overflow the stack, and we >> don't need to change the layout of the cancellation jump buffer. The 4 void*'s >> are sufficient, even for x32 to write a 64-bit shadow stack address. >> >> * After fixing the cancellation jump buffers the following work needs to be reviewed: >> >> * Add feature_1 in tcb head to track CET status and make it easily available >> to runtime for checking. >> >> * Save and restore shadowstack in setjmp/longjmp. >> >> * Add CET support to ld.so et. al. and track runtime status. >> >> * Adjust vfork for shadow stack usage. >> >> * Add ENDBR or NOTRACK where required in assembly. >> >> * CET and makecontext incompatible. >> - Probably need to discuss which default is appropriate. >> - Should the user get CET automatically disabled in makecontext() et. al. silently? >> - Should your current solution, which is to error out during the build, and require >> flag changes, be the default? This forces the user to review the security for their >> application. > > I'd like to reserve 4 slots in ucontext for shadow stack: > > https://github.com/hjl-tools/glibc/commit/9bf6aefa8fb45f8df140d42ce9cf890bb24076e1 > > It should be binary backward compatible. I will investigate if there is a way > to support shadow stack with existing API. Otherwise, we need to add a new > API for ucontext functions with shadow stack. > >> * prctl for CET. > > We have been experimenting different approaches to get the best implementation. > I am expecting that this patch may change as we collect more data. > >> * The work to review after this patch appears to be less contentious in terms of >> the kinds of changes that are required. Most of the changes are internal details >> of enabling CET and not ABI details, with the exception of the possible pain we >> might cause with makecontext() being unsupported and what default position to take >> there. >> >> Design: >> >> * Overall the implementation looks exactly how I might expect it to look, but some >> of the math that places the shadowstack pointer appears to need either commenting >> or fixing because I don't understand it. You need to make it easy for me to see >> that we have placed the shadowstack pointer into the 4 pad words. >> >> Details: >> >> * One comment needs filling out a bit more, noted below. >> >>> 0001-x86-Use-pad-in-pthread_unwind_buf-to-preserve-shadow.patch >>> >>> >>> From f222537447f5ec879427f318b7c0396362b7453a Mon Sep 17 00:00:00 2001 >>> From: "H.J. Lu" >>> Date: Sat, 24 Feb 2018 17:23:54 -0800 >>> Subject: [PATCH] x86: Use pad in pthread_unwind_buf to preserve shadow stack >>> register >>> >>> 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. >>> >> >> OK. >> >>> 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. >> >> OK. >> >>> __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. >> >> OK. Good. I like the use of a __longjmp_cancel name to call out what's >> going on in the API (clear semantics). >> >>> >>> 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. >> >> OK. >> >>> >>> 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 (_JUMP_BUF_SIGSET_NSIG): >>> Changed to 97. >>> * 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. >> >> This looks much better. >> >>> --- >>> nptl/pthread_create.c | 9 ++-- >>> setjmp/longjmp.c | 2 + >>> sysdeps/unix/sysv/linux/x86/setjmpP.h | 4 +- >>> sysdeps/x86/Makefile | 4 ++ >>> sysdeps/x86/__longjmp_cancel.S | 20 ++++++++ >>> sysdeps/x86/longjmp.c | 45 ++++++++++++++++ >>> sysdeps/x86/nptl/pt-longjmp.c | 97 +++++++++++++++++++++++++++++++++++ >>> 7 files changed, 176 insertions(+), 5 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..1c5b3780c6 100644 >>> --- a/nptl/pthread_create.c >>> +++ b/nptl/pthread_create.c >>> @@ -427,12 +427,15 @@ 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 same space may be used by setjmp to store extra data which >>> + should never be used by __libc_unwind_longjmp. */ >> >> Suggest: >> ~~~ >> 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 >> __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. >> ~~~ > > Will fix it. > >>> 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); >> >> OK. >> >>> 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 >> >> OK. >> >>> 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..90a6bbcf32 100644 >>> --- a/sysdeps/unix/sysv/linux/x86/setjmpP.h >>> +++ b/sysdeps/unix/sysv/linux/x86/setjmpP.h >>> @@ -22,8 +22,8 @@ >>> #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 >>> + Define it to 97 to leave some rooms for future use. */ >> >> OK. >> >>> +#define _JUMP_BUF_SIGSET_NSIG 97 >> >> Please provide proof in the way of a comment or rewriting this constant >> to show that it places the shadow stack pointer on both x86_64 and x32 >> into the range of the private pad. > > sysdeps/x86/nptl/pt-longjmp.c has > > _Static_assert ((sizeof (struct pthread_unwind_buf) >>= (SHADOW_STACK_POINTER_OFFSET > + SHADOW_STACK_POINTER_SIZE)), > "size of struct pthread_unwind_buf < " > "(SHADOW_STACK_POINTER_OFFSET + SHADOW_STACK_POINTER_SIZE)"); > > If shadow stack pointer is saved in the offset bigger than the size of > struct pthread_unwind_buf, assert will trigger at compile-time. > >> Also, from commit f33632ccd1dec3217583fcfdd965afb62954203c, >> where did this math come from? >> >> ((_JUMP_BUF_SIGSET_NSIG - 1 + 7) / (8 * sizeof (unsigned long int))) >> >> Why the +7? > > _JUMP_BUF_SIGSET_NSIG is the biggest signal number + 1. > _JUMP_BUF_SIGSET_NSIG - 1 gives the biggest signal number. > _JUMP_BUF_SIGSET_NSIG - 1 + 7 rounds up to the number of bytes > which are needed to store the biggest signal number. > >>> /* Number of longs to hold all signals. */ >>> #define _JUMP_BUF_SIGSET_NWORDS \ >>> ((_JUMP_BUF_SIGSET_NSIG - 1 + 7) / (8 * sizeof (unsigned long int))) >>> 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 >> >> OK. >> >>> 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> >> >> OK. >> >>> 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); >> >> OK. >> >>> + >>> + if (env[0].__mask_was_saved) >>> + /* Restore the saved signal mask. */ >>> + (void) __sigprocmask (SIG_SETMASK, >>> + (sigset_t *) &env[0].__saved_mask, >>> + (sigset_t *) NULL); >> >> OK. >> >>> + >>> + /* Call the machine-dependent function to restore machine state >>> + without shadow stack. */ >>> + __longjmp_cancel (env[0].__jmpbuf, val ?: 1); >> >> OK. >> >>> +} >>> diff --git a/sysdeps/x86/nptl/pt-longjmp.c b/sysdeps/x86/nptl/pt-longjmp.c >>> new file mode 100644 >>> index 0000000000..7eb8651cfe >>> --- /dev/null >>> +++ b/sysdeps/x86/nptl/pt-longjmp.c >>> @@ -0,0 +1,97 @@ >>> +/* 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 >>> + . */ >>> + >>> +/* 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; >>> +}; >>> + >>> + The pad array in struct pthread_unwind_buf is used by setjmp to save >> >> This appears to be SHADOW_STACK_POINTER_OFFSET in subsequent patches. >> >> However on your hjl/cet/master branch it appears that this offset is not >> defined to be *just after* the mask_was_saved? > > sysdeps/unix/sysv/linux/x86/setjmpP.h has > > typedef struct > { > unsigned long int __val[_JUMP_BUF_SIGSET_NWORDS]; > } __jmp_buf_sigset_t; > > 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; > > __shadow_stack_pointer is placed after __saved_mask, aka mask_was_saved. > >>> + shadow stack register. Assert that size of struct pthread_unwind_buf >>> + is no less than offset of shadow stack pointer plus shadow stack >>> + pointer size. >>> + >>> + NB: setjmp is called in libpthread to save shadow stack register. But >>> + __libc_unwind_longjmp doesn't restore shadow stack register since they >>> + never return after longjmp. */ >>> + >>> +#include >>> +#include >>> + >>> +#ifdef __x86_64__ >>> +# define SHADOW_STACK_POINTER_SIZE 8 >>> +#else >>> +# define SHADOW_STACK_POINTER_SIZE 4 >>> +#endif >>> + >>> +_Static_assert ((sizeof (struct pthread_unwind_buf) >>> + >= (SHADOW_STACK_POINTER_OFFSET >>> + + SHADOW_STACK_POINTER_SIZE)), >>> + "size of struct pthread_unwind_buf < " >>> + "(SHADOW_STACK_POINTER_OFFSET + SHADOW_STACK_POINTER_SIZE)"); >> >> OK. >> >>> + >>> +#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); >> >> OK. >> >>> +} >>> + >>> +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 >>> -- 2.14.3 >> >> Here is the updated patch. OK for master? I will submit a CET patch set after this patch is merged. Thanks. From a1f8a06212dd3c5ed445fdc4e23424b766d41932 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Sat, 24 Feb 2018 17:23:54 -0800 Subject: [PATCH] x86: Use pad in pthread_unwind_buf to preserve shadow stack register 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 (_JUMP_BUF_SIGSET_NSIG): Changed to 97. * 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 | 18 +++++-- setjmp/longjmp.c | 2 + sysdeps/unix/sysv/linux/x86/setjmpP.h | 4 +- sysdeps/x86/Makefile | 4 ++ sysdeps/x86/__longjmp_cancel.S | 20 ++++++++ sysdeps/x86/longjmp.c | 45 ++++++++++++++++ sysdeps/x86/nptl/pt-longjmp.c | 97 +++++++++++++++++++++++++++++++++++ 7 files changed, 185 insertions(+), 5 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..8b1f06599d 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -427,12 +427,24 @@ 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 be 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..90a6bbcf32 100644 --- a/sysdeps/unix/sysv/linux/x86/setjmpP.h +++ b/sysdeps/unix/sysv/linux/x86/setjmpP.h @@ -22,8 +22,8 @@ #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 + Define it to 97 to leave some rooms for future use. */ +#define _JUMP_BUF_SIGSET_NSIG 97 /* Number of longs to hold all signals. */ #define _JUMP_BUF_SIGSET_NWORDS \ ((_JUMP_BUF_SIGSET_NSIG - 1 + 7) / (8 * sizeof (unsigned long int))) 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..7eb8651cfe --- /dev/null +++ b/sysdeps/x86/nptl/pt-longjmp.c @@ -0,0 +1,97 @@ +/* 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 + . */ + +/* 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; +}; + + The pad array in struct pthread_unwind_buf is used by setjmp to save + shadow stack register. Assert that size of struct pthread_unwind_buf + is no less than offset of shadow stack pointer plus shadow stack + pointer size. + + NB: setjmp is called in libpthread to save shadow stack register. But + __libc_unwind_longjmp doesn't restore shadow stack register since they + never return after longjmp. */ + +#include +#include + +#ifdef __x86_64__ +# define SHADOW_STACK_POINTER_SIZE 8 +#else +# define SHADOW_STACK_POINTER_SIZE 4 +#endif + +_Static_assert ((sizeof (struct pthread_unwind_buf) + >= (SHADOW_STACK_POINTER_OFFSET + + SHADOW_STACK_POINTER_SIZE)), + "size of struct pthread_unwind_buf < " + "(SHADOW_STACK_POINTER_OFFSET + SHADOW_STACK_POINTER_SIZE)"); + +#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 -- 2.14.3