Re: [PATCH 2/2] x86: Add feature_1 to tcbhead_t [BZ #22563]

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

On Thu, Dec 7, 2017 at 9:41 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
> On x86, padding in struct __jmp_buf_tag is used for shadow stack pointer
> to support Shadow Stack in Intel Control-flow Enforcemen Technology.
> cancel_jmp_buf has been updated to include saved_mask so that it is as
> large as struct __jmp_buf_tag.  We must suport the old cancel_jmp_buf
> in existing binaries.  Since symbol versioning doesn't work on
> cancel_jmp_buf, feature_1 is added to tcbhead_t so that setjmp and
> longjmp can check if shadow stack is enabled.  NB: Shadow stack is
> enabled only if all modules are shadow stack enabled.
>
> Any comments?
>
> H.J.
> ---
>         [BZ #22563]
>         * sysdeps/i386/nptl/tcb-offsets.sym (FEATURE_1_OFFSET): New.
>         * sysdeps/i386/nptl/tls.h (tcbhead_t): Add feature_1.
>         * sysdeps/x86_64/nptl/tcb-offsets.sym (FEATURE_1_OFFSET): New.
>         * sysdeps/x86_64/nptl/tls.h (tcbhead_t): Rename __glibc_unused1
>         to feature_1.
> ---
>  sysdeps/i386/nptl/tcb-offsets.sym   | 1 +
>  sysdeps/i386/nptl/tls.h             | 4 ++++
>  sysdeps/x86_64/nptl/tcb-offsets.sym | 1 +
>  sysdeps/x86_64/nptl/tls.h           | 5 ++++-
>  4 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/sysdeps/i386/nptl/tcb-offsets.sym b/sysdeps/i386/nptl/tcb-offsets.sym
> index 695a810386..250f1a6e13 100644
> --- a/sysdeps/i386/nptl/tcb-offsets.sym
> +++ b/sysdeps/i386/nptl/tcb-offsets.sym
> @@ -15,3 +15,4 @@ POINTER_GUARD         offsetof (tcbhead_t, pointer_guard)
>  #ifndef __ASSUME_PRIVATE_FUTEX
>  PRIVATE_FUTEX          offsetof (tcbhead_t, private_futex)
>  #endif
> +FEATURE_1_OFFSET       offsetof (tcbhead_t, feature_1)
> diff --git a/sysdeps/i386/nptl/tls.h b/sysdeps/i386/nptl/tls.h
> index f9a6b11ecf..7d4b18c16a 100644
> --- a/sysdeps/i386/nptl/tls.h
> +++ b/sysdeps/i386/nptl/tls.h
> @@ -50,6 +50,10 @@ typedef struct
>    void *__private_tm[4];
>    /* GCC split stack support.  */
>    void *__private_ss;
> +  /* Bit 0: IBT.
> +     Bit 1: SHSTK.
> +   */
> +  unsigned int feature_1;
>  } tcbhead_t;
>
>  # define TLS_MULTIPLE_THREADS_IN_TCB 1
> diff --git a/sysdeps/x86_64/nptl/tcb-offsets.sym b/sysdeps/x86_64/nptl/tcb-offsets.sym
> index 8a25c482cb..03b6dba5c3 100644
> --- a/sysdeps/x86_64/nptl/tcb-offsets.sym
> +++ b/sysdeps/x86_64/nptl/tcb-offsets.sym
> @@ -15,6 +15,7 @@ VGETCPU_CACHE_OFFSET  offsetof (tcbhead_t, vgetcpu_cache)
>  #ifndef __ASSUME_PRIVATE_FUTEX
>  PRIVATE_FUTEX          offsetof (tcbhead_t, private_futex)
>  #endif
> +FEATURE_1_OFFSET       offsetof (tcbhead_t, feature_1)
>
>  -- Not strictly offsets, but these values are also used in the TCB.
>  TCB_CANCELSTATE_BITMASK         CANCELSTATE_BITMASK
> diff --git a/sysdeps/x86_64/nptl/tls.h b/sysdeps/x86_64/nptl/tls.h
> index 9b8ad82550..cec864f231 100644
> --- a/sysdeps/x86_64/nptl/tls.h
> +++ b/sysdeps/x86_64/nptl/tls.h
> @@ -56,7 +56,10 @@ typedef struct
>  # else
>    int __glibc_reserved1;
>  # endif
> -  int __glibc_unused1;
> +  /* Bit 0: IBT.
> +     Bit 1: SHSTK.
> +   */
> +  unsigned int feature_1;
>    /* Reservation of some values for the TM ABI.  */
>    void *__private_tm[4];
>    /* GCC split stack support.  */
> --
> 2.14.3
>

This patch turns a previously unused field in x86-64 tcbhead_t
for feature_1.  This shouldn't be an issue.

But it adds a new field to i386 tcbhead_t.  Could it be an problem
for i386 libsanitizer?

If there is no objection, I will check it next Monday.


-- 
H.J.