From patchwork Thu Apr 12 23:50:34 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: 26706 Received: (qmail 36768 invoked by alias); 12 Apr 2018 23:50:42 -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 36753 invoked by uid 89); 12 Apr 2018 23:50:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-23.5 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-oi0-f67.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=CHH2KwwDnhYNDzCIzsWwqKQCrRZpbA4j1GGsDoVtI5M=; b=FRrDM4v2OhaSfNB72mMDmdiGAql6SIuLQlpatlx5Ivt3s9eBXTzCxwsFNRLpUrI2V/ tZjCvS9MjZ+veopEtDc0uun/B1z2jkfSddxzpfVrhrU22B6/CwMI3T+g8GHOv85IvBrD hyG71IdDlx7y2O2w4prpKKXSCNgTbJN/9uxqurKansuQ1ZH1IrZg8B/4ndF8avRY4Gjy 4Kwm/mLa34zaE0tlkvDAPtv/FmAjTsi2ZgYygLfBB/BXTNYvA7WRE06dOFopN+tPZx6O wVHu4YhZsDLELbXC9CAgSKXc+L10XT2pwSvScE3Flr3M1S4ZTEVAQ6Gi7DlQJiquAuxU pb/g== X-Gm-Message-State: ALQs6tAuDxHISe9EE+QF9YsBHi4/IT0Y8RmN5Duao1LZCppwYkTpFtL5 pZT3ipbwMZL3P8xyRpM6Auh7VYWyFSZTusa2JZE= X-Google-Smtp-Source: AIpwx4/E9rLPPnsUAtLc1hbuA/UaLKJPtEHQ6f3E3ByWneuLwbG25BjEu5Ls9cNwZjDjObSFvGtlreq/DrpntMgQJuk= X-Received: by 2002:aca:478a:: with SMTP id u132-v6mr7460875oia.227.1523577034610; Thu, 12 Apr 2018 16:50:34 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <2dc14d06-a06c-1aeb-dca3-f97f4ee68415@redhat.com> References: <61a5b452-e59e-dfef-4530-a94a60480961@redhat.com> <2dc14d06-a06c-1aeb-dca3-f97f4ee68415@redhat.com> From: "H.J. Lu" Date: Thu, 12 Apr 2018 16:50:34 -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 Thu, Apr 12, 2018 at 2:36 PM, Carlos O'Donell wrote: > On 04/06/2018 07: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. > > Could you explain in detail how this is binary backwards compatible? > > Is the assumption that if you extend ucontext_t, that the kernel will just write > more to the stack, and users who accesss it via the void* in a signal handler > setup via sigaction + SA_SIGINFO, will not read past the size they expect? > > How is the shadow stack information moved between a getcontext -> setcontext > set of API calls? The user ucontext_t in existing binaries is too small to hold the > shadow stack? > > Would we again have a "flag day" where CET enablement must be coordinated with > the definition of a new ucontext_t? > >>> * 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. >> > > OK. > >>> * 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. >> > > Thanks, I'll review this in the patch you posted. > >>> 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. > > Agreed. > >> _JUMP_BUF_SIGSET_NSIG - 1 gives the biggest signal number. > > Agreed. > >> _JUMP_BUF_SIGSET_NSIG - 1 + 7 rounds up to the number of bytes >> which are needed to store the biggest signal number. > > It does not. I changed to /* 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. */ #define _JUMP_BUF_SIGSET_NSIG 96 /* Number of longs to hold all signals. */ #define _JUMP_BUF_SIGSET_NWORDS \ (ALIGN_UP (_JUMP_BUF_SIGSET_NSIG, _JUMP_BUF_SIGSET_BITS_PER_WORD) \ / _JUMP_BUF_SIGSET_BITS_PER_WORD) > Result # of signals # of bits Current # Expected # ... > Luckily for 512 signals (513) the math works out. > > For 96 signals it does not. > > (96 - 1 + 7) = 102 > 102 / 64 = 1 True. > That's only a signal word, that only supports 64 signals, not 96. Lucky for me. Since unsigned long int __shadow_stack_pointer is aligned as unsigned long, there is padding before __shadow_stack_pointer. > Why doesn't the static assert trigger? Because you a < the size of the > pthread_unwind_buf, and too small actually, writing into other parts of > the buffer! Assert _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)"); is correct. > I would expect us to use something like this: > > diff --git a/sysdeps/unix/sysv/linux/x86/setjmpP.h b/sysdeps/unix/sysv/linux/x86/setjmpP.h > index c0ed767a0d..6e1e8f901c 100644 > --- a/sysdeps/unix/sysv/linux/x86/setjmpP.h > +++ b/sysdeps/unix/sysv/linux/x86/setjmpP.h > @@ -20,13 +20,15 @@ > #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 > +/* As of kernel 4.14, x86 _NSIG is 64. > + Define it to 512 to leave some rooms for future use. */ > +#define _JUMP_BUF_SIGSET_NSIG 512 > /* 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, (8 * sizeof (unsigned long int))) \ > + / (8 * sizeof (unsigned long int))) > > typedef struct > { > --- > ... but with the size you need and explain *why* it's 96. > > You need a comment like this: > /* The layout looks like this: > - N words for this. > - N words for that. > - N words for shadow stack. > Total 96 signals. */ > > Align the number of signals up to multiple of signals you can store in > a hardware word, and then figure out how many words that takes up. I put comments in sysdeps/x86/nptl/pt-longjmp.c together with static assert. > Please review my math. > >>>> /* 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. > > OK, I'll re-review. > > > I look forward to a v2 with correct rounding and a new comment. > Here is the updated patch. Please see if sysdeps/unix/sysv/linux/x86/setjmpP.h sysdeps/x86/nptl/pt-longjmp.c address your concerns. Thanks. From 1ce2b4f199070a63d9a60bd9b7ca9e33013d4208 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: 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 | 18 +++++-- setjmp/longjmp.c | 2 + sysdeps/unix/sysv/linux/x86/setjmpP.h | 12 +++-- 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, 191 insertions(+), 7 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..24f87da204 100644 --- a/sysdeps/unix/sysv/linux/x86/setjmpP.h +++ b/sysdeps/unix/sysv/linux/x86/setjmpP.h @@ -20,13 +20,17 @@ #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 +/* 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. */ +#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 { 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