This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] x86: Use pad in pthread_unwind_buf to preserve shadow stack register
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: "Carlos O'Donell" <carlos at redhat dot com>
- Cc: Florian Weimer <fw at deneb dot enyo dot de>, Joseph Myers <joseph at codesourcery dot com>, "Tsimbalist, Igor V" <igor dot v dot tsimbalist at intel dot com>, GNU C Library <libc-alpha at sourceware dot org>
- Date: Thu, 12 Apr 2018 16:50:34 -0700
- Subject: Re: [PATCH] x86: Use pad in pthread_unwind_buf to preserve shadow stack register
- References: <CAMe9rOpj6nYdP33emmepZ+KWFG2JDVNsHn1bve3oo5WQyFmduA@mail.gmail.com> <61a5b452-e59e-dfef-4530-a94a60480961@redhat.com> <CAMe9rOr68P6Z=rX=xeFG1PbQE1fZ14iN3NmZEpChjP6QmUfS8g@mail.gmail.com> <2dc14d06-a06c-1aeb-dca3-f97f4ee68415@redhat.com>
On Thu, Apr 12, 2018 at 2:36 PM, Carlos O'Donell <carlos@redhat.com> wrote:
> On 04/06/2018 07:59 AM, H.J. Lu wrote:
>> On Thu, Apr 5, 2018 at 9:46 PM, Carlos O'Donell <carlos@redhat.com> wrote:
>>> On 03/30/2018 12:41 PM, H.J. Lu wrote:
>>>> On Thu, Mar 29, 2018 at 1:20 PM, Florian Weimer <fw@deneb.enyo.de> wrote:
>>>>> * H. J. Lu:
>>>>>
>>>>>> On Thu, Mar 29, 2018 at 1:15 PM, Florian Weimer <fw@deneb.enyo.de> 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" <hjl.tools@gmail.com>
>>>> 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 <bits/types/__sigset_t.h>
>>>>
>>>> /* 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 <bits/types/__sigset_t.h>
> +#include <libc-pointer-arith.h>
>
> -/* 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
>>>> + <http://www.gnu.org/licenses/>. */
>>>> +
>>>> +#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
>>>> + <http://www.gnu.org/licenses/>. */
>>>> +
>>>> +#define __libc_longjmp __redirect___libc_longjmp
>>>> +#include <setjmp/longjmp.c>
>>>> +#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
>>>> + <http://www.gnu.org/licenses/>. */
>>>> +
>>>> +/* <nptl/descr.h> 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.
From 1ce2b4f199070a63d9a60bd9b7ca9e33013d4208 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
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
<libc-pointer-arith.h>.
(_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 <bits/types/__sigset_t.h>
+#include <libc-pointer-arith.h>
-/* 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
+ <http://www.gnu.org/licenses/>. */
+
+#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
+ <http://www.gnu.org/licenses/>. */
+
+#define __libc_longjmp __redirect___libc_longjmp
+#include <setjmp/longjmp.c>
+#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
+ <http://www.gnu.org/licenses/>. */
+
+/* <nptl/descr.h> 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 <pthreadP.h>
+#include <jmp_buf-ssp.h>
+
+#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 <shlib-compat.h>
+
+/* 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