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. -- H.J.