Re: [PATCH/v3] x86: Use pad in pthread_unwind_buf to preserve shadow stack register

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Tue, May 1, 2018 at 9:43 PM, Carlos O'Donell <carlos@redhat.com> wrote:
> On 04/21/2018 02:37 PM, H.J. Lu wrote:
>> On Fri, Apr 20, 2018 at 10:28:24PM -0500, Carlos O'Donell wrote:
>>> Lets call this v2. Subject adjusted. Please keep incrementing the version
>>> number on your patches to make the review easier by myself and others.
>>>
>>
>> Done.
>>
>>>> +
>>>> +  /* No previous handlers.  NB: This must be done after setjmp since
>>>> +     the private space in the unwind jump buffer may overlap space
>>>> +     used by setjmp to store extra architecture-specific information
>>>> +     which is never be used by the cancellation-specific
>>>
>>> s/be//g
>>
>>  Done
>>
>>>> +/* Number of bits per long.  */
>>>> +#define _JUMP_BUF_SIGSET_BITS_PER_WORD (8 * sizeof (unsigned long int))
>>>> +/* The biggest signal number.  As of kernel 4.14, x86 _NSIG is 64.
>>>> +   Define it to 96 to leave some rooms for future use.  */
>>>
>>> Why 96?
>>>
>>> Sure on x86_64 the cancel jump buf has 4 x void*'s and each is 64-bits,
>>> so you have 128 signals worth of space and then the shadow stack pointer.
>>>
>>> Does this work on x32?
>>>
>>> On x32 you have only 4 void*'s in the private pad.
>>>
>>> Your presently structured sigset_t looks like this:
>>>
>>> typedef union
>>>   {
>>>     __sigset_t __saved_mask_compat;
>>>     struct
>>>       {
>>>         __jmp_buf_sigset_t __saved_mask;
>>>         /* Used for shadow stack pointer.  */
>>>         unsigned long int __shadow_stack_pointer;
>>>       } __saved;
>>>   } __jmpbuf_arch_t;
>>>
>>> Which means you have a sigset_t *before* the __shadow_stack_pointer.
>>>
>>> On x32 to save a 64-bit shadow stack pointer, you'll only have 2 x 32-bit
>>> words left? That's only space for 64 signals?
>>>
>>> Are you counting on one additional 32-bit word of padding between the
>>> __mask_was_saved and the rest of the long arguments?
>>>
>>> If so, then this still needs spelling out in an a comment why 96 signals
>>> works on both i686, x32, and x86_64.
>>>
>>> Also it should be explained if 96 is the *maximum* we can do with the current
>>> layout, or if more is possible. In which case picking 96 is *not* arbitrary.
>>
>>    The pad array in struct pthread_unwind_buf is used by setjmp to save
>>    shadow stack register.  The usable space in __saved_mask for sigset
>>    and shadow stack pointer:
>>    1. i386: The 4x4 byte pad array which can be used for 4 byte shadow
>>    stack pointer and maximum 12 byte sigset.
>>    2. x32: 4 byte padding + the 4x4 byte pad array which can be used
>>    for 8 byte shadow stack pointer and maximum 12 byte sigset.
>>    3. x86-64: The 4x8 byte pad array which can be used for 8 byte
>>    shadow stack pointer and maximum 24 byte sigset.
>>
>> Please see comments in sysdeps/unix/sysv/linux/x86/setjmpP.h for
>> details.
>>
>>> Suggest:
>>> ~~~
>>> The pad array in struct pthread_unwind_buf is used by setjmp to save
>>> the shadow stack register.  Assert that the size of struct pthread_unwind_buf
>>> is no less than the offset of the shadow stack pointer plus the shadow stack
>>> pointer size.
>>>
>>> NB: We use setjmp in thread cancellation and this saves the shadow stack
>>> register, but __libc_unwind_longjmp doesn't restore the shadow stack register
>>> since cancellation never returns after longjmp.
>>> ~~~
>>>
>>
>> Done.
>>
>>>
>>> This assertion is too loose.
>>>
>>> The assertion you need is that the shadow stack pointer itself falls within
>>> the range of the private padding. This would have caught the previous bug
>>> with the rounded up size.
>>>
>>> Please adjust the assertion to be as tight as possible or add new assertions.
>>>
>>> Completely untested, but just to show what I'm thinking:
>>>
>>> _Static_assert ((offsetof (struct pthread_unwind_buf, cancel_jump_buf[0].mask_was_saved)
>>>               + sizeof (int) < SHADOW_STACK_POINTER_OFFSET
>>>               && (offsetof (struct pthread_unwind_buf, priv.pad[4])
>>>                  > (SHADOW_STACK_POINTER_OFFSET + SHADOW_STACK_POINTER_SIZE),
>>>              "Shadow stack pointer is not within private storage of pthread_unwind_buf.");
>>>
>>
>> I used a slighly different assert.
>>
>>>
>>> Looking forward to a v3.
>>>
>>
>> Here it is.
>>
>> H.J.
>> ---
>> 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                 | 17 +++++++--
>>  setjmp/longjmp.c                      |  2 +
>>  sysdeps/unix/sysv/linux/x86/setjmpP.h | 71 ++++++++++++++++++++++++++++++++---
>>  sysdeps/x86/Makefile                  |  4 ++
>>  sysdeps/x86/__longjmp_cancel.S        | 20 ++++++++++
>>  sysdeps/x86/longjmp.c                 | 45 ++++++++++++++++++++++
>>  sysdeps/x86/nptl/pt-longjmp.c         | 71 +++++++++++++++++++++++++++++++++++
>>  7 files changed, 222 insertions(+), 8 deletions(-)
>>  create mode 100644 sysdeps/x86/__longjmp_cancel.S
>>  create mode 100644 sysdeps/x86/longjmp.c
>>  create mode 100644 sysdeps/x86/nptl/pt-longjmp.c
>>
>> diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
>> index caaf07c134..92c945b12b 100644
>> --- a/nptl/pthread_create.c
>> +++ b/nptl/pthread_create.c
>> @@ -427,12 +427,23 @@ START_THREAD_DEFN
>>       compilers without that support we do use setjmp.  */
>>    struct pthread_unwind_buf unwind_buf;
>>
>> -  /* No previous handlers.  */
>> +  int not_first_call;
>> +  not_first_call = setjmp ((struct __jmp_buf_tag *) unwind_buf.cancel_jmp_buf);
>> +
>> +  /* No previous handlers.  NB: This must be done after setjmp since the
>> +     private space in the unwind jump buffer may overlap space used by
>> +     setjmp to store extra architecture-specific information which is
>> +     never used by the cancellation-specific __libc_unwind_longjmp.
>> +
>> +     The private space is allowed to overlap because the unwinder never
>> +     has to return through any of the jumped-to call frames, and thus
>> +     only a minimum amount of saved data need be stored, and for example,
>> +     need not include the process signal mask information. This is all
>> +     an optimization to reduce stack usage when pushing cancellation
>> +     handlers.  */
>
> OK.
>
>>    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
>
> 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..6b2608453d 100644
>> --- a/sysdeps/unix/sysv/linux/x86/setjmpP.h
>> +++ b/sysdeps/unix/sysv/linux/x86/setjmpP.h
>> @@ -20,13 +20,72 @@
>>  #define      _SETJMPP_H      1
>>
>>  #include <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
>> +/* <setjmp/setjmp.h> has
>> +
>> +struct __jmp_buf_tag
>> +  {
>> +    __jmp_buf __jmpbuf;
>> +    int __mask_was_saved;
>> +    __sigset_t __saved_mask;
>> +  };
>> +
>> +   struct __jmp_buf_tag is 32 bits aligned on i386 and is 64 bits
>> +   aligned on x32 and x86-64.  __saved_mask is aligned to 32 bits
>> +   on i386/x32 without padding and is aligned to 64 bits on x86-64
>> +   with 32 bit padding.
>> +
>> +   and <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;
>> +};
>> +
>> +   struct pthread_unwind_buf is 32 bits aligned on i386 and 64 bits
>> +   aligned on x32/x86-64.  cancel_jmp_buf is aligned to 32 bits on
>> +   i386 and is aligned to 64 bits on x32/x86-64.
>
> OK.
>
>> +
>> +   The pad array in struct pthread_unwind_buf is used by setjmp to save
>> +   shadow stack register.  The usable space in __saved_mask for sigset
>> +   and shadow stack pointer:
>> +   1. i386: The 4x4 byte pad array which can be used for 4 byte shadow
>> +   stack pointer and maximum 12 byte sigset.
>> +   2. x32: 4 byte padding + the 4x4 byte pad array which can be used
>> +   for 8 byte shadow stack pointer and maximum 12 byte sigset.
>> +   3. x86-64: The 4x8 byte pad array which can be used for 8 byte
>> +   shadow stack pointer and maximum 24 byte sigset.
>
> OK.
>
>> +
>> +   NB: We use setjmp in thread cancellation and this saves the shadow
>> +   stack register, but __libc_unwind_longjmp doesn't restore the shadow
>> +   stack register since cancellation never returns after longjmp.  */
>> +
>> +/* Number of bits per long.  */
>> +#define _JUMP_BUF_SIGSET_BITS_PER_WORD (8 * sizeof (unsigned long int))
>> +/* The biggest signal number.  As of kernel 4.14, x86 _NSIG is 64. The
>> +   common maximum sigset for i386, x32 and x86-64 is 12 bytes (96 bits).
>> +   Define it to 96 to leave some rooms for future use.  */
>> +#define _JUMP_BUF_SIGSET_NSIG        96
>>  /* Number of longs to hold all signals.  */
>>  #define _JUMP_BUF_SIGSET_NWORDS \
>> -  ((_JUMP_BUF_SIGSET_NSIG - 1 + 7) / (8 * sizeof (unsigned long int)))
>> +  (ALIGN_UP (_JUMP_BUF_SIGSET_NSIG, _JUMP_BUF_SIGSET_BITS_PER_WORD) \
>> +   / _JUMP_BUF_SIGSET_BITS_PER_WORD)
>
> OK.
>
>>
>>  typedef struct
>>    {
>> @@ -39,7 +98,9 @@ typedef union
>>      struct
>>        {
>>       __jmp_buf_sigset_t __saved_mask;
>> -     /* Used for shadow stack pointer.  */
>> +     /* Used for shadow stack pointer.  NB: Shadow stack pointer
>> +        must have the same alignment as __saved_mask.  Otherwise
>> +        offset of __saved_mask will be changed.  */
>
> s/offset of __saved_mask/offset of __shadow_stack_pointer/g?

__saved_mask here is in the public header:

/* Calling environment, plus possibly a saved signal mask.  */
struct __jmp_buf_tag
  {
    /* NOTE: The machine-dependent definitions of `__sigsetjmp'
       assume that a `jmp_buf' begins with a `__jmp_buf' and that
       `__mask_was_saved' follows it.  Do not move these members
       or add others before it.  */
    __jmp_buf __jmpbuf; /* Calling environment.  */
    int __mask_was_saved; /* Saved the signal mask?  */
    __sigset_t __saved_mask; /* Saved signal mask.  */
  };

and sysdeps/unix/sysv/linux/x86/setjmpP.h has

#undef __sigset_t
#define __sigset_t __jmpbuf_arch_t
#include <setjmp.h>
#undef __saved_mask
#define __saved_mask __saved_mask.__saved.__saved_mask

The offset of  __saved_mask must be unchanged, which is checked by
assert in include/setjmp.h:

/* Check if internal fields in jmp_buf have the expected offsets.  */
TEST_OFFSET (struct __jmp_buf_tag, __mask_was_saved,
     MASK_WAS_SAVED_OFFSET);
TEST_OFFSET (struct __jmp_buf_tag, __saved_mask,
     SAVED_MASK_OFFSET);


> OK with that correction.
>
> It is the __shadow_stack_pointer offset which will change if it doesn't
> have the same alignment as the __saved_mask. The compiler will insert
> padding between the fields.
>
> Perhaps assert there is no padding?

I don't think we need it since there is

/* Assert that the priv field in struct pthread_unwind_buf has space
   to store shadow stack pointer.  */
_Static_assert ((offsetof (struct pthread_unwind_buf, priv)
<= SHADOW_STACK_POINTER_OFFSET)
&& ((offsetof (struct pthread_unwind_buf, priv)
     + sizeof (((struct pthread_unwind_buf *) 0)->priv))
    >= (SHADOW_STACK_POINTER_OFFSET
+ SHADOW_STACK_POINTER_SIZE)),
"Shadow stack pointer is not within private storage "
"of pthread_unwind_buf.");

to ensure that offset and size of the priv field are sufficient.

>>       unsigned long int __shadow_stack_pointer;
>>        } __saved;
>>    } __jmpbuf_arch_t;
>> diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
>> index 0d0326c21a..d25d6f0ae4 100644
>> --- a/sysdeps/x86/Makefile
>> +++ b/sysdeps/x86/Makefile
>> @@ -8,3 +8,7 @@ sysdep-dl-routines += dl-get-cpu-features
>>  tests += tst-get-cpu-features
>>  tests-static += tst-get-cpu-features-static
>>  endif
>> +
>> +ifeq ($(subdir),setjmp)
>> +sysdep_routines += __longjmp_cancel
>> +endif
>
> 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);
>> +
>> +  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);
>> +}
>
> OK.
>
>> diff --git a/sysdeps/x86/nptl/pt-longjmp.c b/sysdeps/x86/nptl/pt-longjmp.c
>> new file mode 100644
>> index 0000000000..6165c7d4a7
>> --- /dev/null
>> +++ b/sysdeps/x86/nptl/pt-longjmp.c
>> @@ -0,0 +1,71 @@
>> +/* ABI compatibility for 'longjmp' and 'siglongjmp' symbols in libpthread ABI.
>> +   X86 version.
>> +   Copyright (C) 18 Free Software Foundation, Inc.
>> +   This file is part of the GNU C Library.
>> +
>> +   The GNU C Library is free software; you can redistribute it and/or
>> +   modify it under the terms of the GNU Lesser General Public
>> +   License as published by the Free Software Foundation; either
>> +   version 2.1 of the License, or (at your option) any later version.
>> +
>> +   The GNU C Library is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> +   Lesser General Public License for more details.
>> +
>> +   You should have received a copy of the GNU Lesser General Public
>> +   License along with the GNU C Library; if not, see
>> +   <http://www.gnu.org/licenses/>.  */
>> +
>> +#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
>> +
>> +/* Assert that the priv field in struct pthread_unwind_buf has space
>> +   to store shadow stack pointer.  */
>> +_Static_assert ((offsetof (struct pthread_unwind_buf, priv)
>> +              <= SHADOW_STACK_POINTER_OFFSET)
>> +             && ((offsetof (struct pthread_unwind_buf, priv)
>> +                  + sizeof (((struct pthread_unwind_buf *) 0)->priv))
>> +                 >= (SHADOW_STACK_POINTER_OFFSET
>> +                     + SHADOW_STACK_POINTER_SIZE)),
>> +             "Shadow stack pointer is not within private storage "
>> +             "of pthread_unwind_buf.");
>
> OK. Thanks for testing this out.
>
>> +
>> +#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
>>
>
> Version 3 looks good to me with the comment correction I mentioned.
>
> Thank you for working with me to improve the quality of the code
> and comments, and for balancing the needs of the implementation
> against the distribution needs e.g. no added symbol for core CET
> enablement.

Thank you for your valuable feedbacks.  I am checking in my patch now.

> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
>
> --
> Cheers,
> Carlos.



-- 
H.J.