diff mbox

x86: Rename __glibc_reserved2 to ssp_base in tcbhead_t

Message ID 20180725113847.GA16211@gmail.com
State Committed
Commit 9aa3113a42d94d7bbf9bb4d50ef0d23b95e66123
Headers

Commit Message

H.J. Lu July 25, 2018, 11:38 a.m. UTC 
  On Tue, Jul 24, 2018 at 08:51:02PM -0400, Carlos O'Donell wrote:
> On 07/24/2018 06:32 PM, H.J. Lu wrote:
> > On Tue, Jul 24, 2018 at 1:49 PM, Carlos O'Donell <carlos@redhat.com> wrote:
> >> On 07/21/2018 10:20 AM, H.J. Lu wrote:
> >>> This will be used to implement shadow stack switching by getcontext,
> >>> makecontext, setcontext and swapcontext.
> >>>
> >>>       * sysdeps/i386/nptl/tcb-offsets.sym (SSP_BASE_OFFSET): New.
> >>>       * sysdeps/i386/nptl/tls.h (tcbhead_t): Replace __glibc_reserved2
> >>>       with ssp_base.
> >>>       * sysdeps/x86_64/nptl/tcb-offsets.sym (SSP_BASE_OFFSET): New.
> >>>       * sysdeps/x86_64/nptl/tls.h (tcbhead_t): Replace __glibc_reserved2
> >>>       with ssp_base.
> >> Looks good to me.
> >>
> >> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> >>
> > We find a testcase where setcontext failed when there are gaps above
> > and below the newly allocated shadow stack.  Here is the updated patch
> > to add shadow stack base and limit to tcbhead_t.
> > 
> > We need to find room for shadow stack base and upper limit in i386
> > tcbhead_t.  I have some ideas.   For now, I'd like to get x86-64 working
> > first.
> 
> Why can't you grow the tcbhead_t? Is it because the sanitizers depend upon it?

Yes.

> 
> > OK for master branch?
> > 

It turns out that ssp_base is sufficient.  I am checking in the original
patch with the updated commit message.


H.J.
---
This will be used to record the current shadow stack base for shadow
stack switching by getcontext, makecontext, setcontext and swapcontext.
If the target shadow stack base is the same as the current shadow stack
base, we unwind the shadow stack.  Otherwise it is a stack switch and
we look for a restore token to restore the target shadow stack.

	* sysdeps/i386/nptl/tcb-offsets.sym (SSP_BASE_OFFSET): New.
	* sysdeps/i386/nptl/tls.h (tcbhead_t): Replace __glibc_reserved2
	with ssp_base.
	* sysdeps/x86_64/nptl/tcb-offsets.sym (SSP_BASE_OFFSET): New.
	* sysdeps/x86_64/nptl/tls.h (tcbhead_t): Replace __glibc_reserved2
	with ssp_base.
---
 sysdeps/i386/nptl/tcb-offsets.sym   |  1 +
 sysdeps/i386/nptl/tls.h             |  3 ++-
 sysdeps/x86_64/nptl/tcb-offsets.sym |  1 +
 sysdeps/x86_64/nptl/tls.h           | 10 +++++++++-
 4 files changed, 13 insertions(+), 2 deletions(-)

Comments

Carlos O'Donell July 25, 2018, 12:49 p.m. UTC | #1 
On 07/25/2018 07:38 AM, H.J. Lu wrote:
> On Tue, Jul 24, 2018 at 08:51:02PM -0400, Carlos O'Donell wrote:
>> On 07/24/2018 06:32 PM, H.J. Lu wrote:
>>> On Tue, Jul 24, 2018 at 1:49 PM, Carlos O'Donell <carlos@redhat.com> wrote:
>>>> On 07/21/2018 10:20 AM, H.J. Lu wrote:
>>>>> This will be used to implement shadow stack switching by getcontext,
>>>>> makecontext, setcontext and swapcontext.
>>>>>
>>>>>       * sysdeps/i386/nptl/tcb-offsets.sym (SSP_BASE_OFFSET): New.
>>>>>       * sysdeps/i386/nptl/tls.h (tcbhead_t): Replace __glibc_reserved2
>>>>>       with ssp_base.
>>>>>       * sysdeps/x86_64/nptl/tcb-offsets.sym (SSP_BASE_OFFSET): New.
>>>>>       * sysdeps/x86_64/nptl/tls.h (tcbhead_t): Replace __glibc_reserved2
>>>>>       with ssp_base.
>>>> Looks good to me.
>>>>
>>>> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
>>>>
>>> We find a testcase where setcontext failed when there are gaps above
>>> and below the newly allocated shadow stack.  Here is the updated patch
>>> to add shadow stack base and limit to tcbhead_t.
>>>
>>> We need to find room for shadow stack base and upper limit in i386
>>> tcbhead_t.  I have some ideas.   For now, I'd like to get x86-64 working
>>> first.
>>
>> Why can't you grow the tcbhead_t? Is it because the sanitizers depend upon it?
> 
> Yes.
> 
>>
>>> OK for master branch?
>>>
> 
> It turns out that ssp_base is sufficient.  I am checking in the original
> patch with the updated commit message.
> 

OK for 2.28.

These are internal details of the CET implementation which won't largely
impact any other architecture, so I'm OK to have these change right now.

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

> 
> H.J.
> ---
> This will be used to record the current shadow stack base for shadow
> stack switching by getcontext, makecontext, setcontext and swapcontext.
> If the target shadow stack base is the same as the current shadow stack
> base, we unwind the shadow stack.  Otherwise it is a stack switch and
> we look for a restore token to restore the target shadow stack.
> 
> 	* sysdeps/i386/nptl/tcb-offsets.sym (SSP_BASE_OFFSET): New.
> 	* sysdeps/i386/nptl/tls.h (tcbhead_t): Replace __glibc_reserved2
> 	with ssp_base.
> 	* sysdeps/x86_64/nptl/tcb-offsets.sym (SSP_BASE_OFFSET): New.
> 	* sysdeps/x86_64/nptl/tls.h (tcbhead_t): Replace __glibc_reserved2
> 	with ssp_base.
> ---
>  sysdeps/i386/nptl/tcb-offsets.sym   |  1 +
>  sysdeps/i386/nptl/tls.h             |  3 ++-
>  sysdeps/x86_64/nptl/tcb-offsets.sym |  1 +
>  sysdeps/x86_64/nptl/tls.h           | 10 +++++++++-
>  4 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/sysdeps/i386/nptl/tcb-offsets.sym b/sysdeps/i386/nptl/tcb-offsets.sym
> index fbac241c45..2ec9e787c1 100644
> --- a/sysdeps/i386/nptl/tcb-offsets.sym
> +++ b/sysdeps/i386/nptl/tcb-offsets.sym
> @@ -13,3 +13,4 @@ CLEANUP_PREV		offsetof (struct _pthread_cleanup_buffer, __prev)
>  MUTEX_FUTEX		offsetof (pthread_mutex_t, __data.__lock)
>  POINTER_GUARD		offsetof (tcbhead_t, pointer_guard)
>  FEATURE_1_OFFSET	offsetof (tcbhead_t, feature_1)
> +SSP_BASE_OFFSET		offsetof (tcbhead_t, ssp_base)
> diff --git a/sysdeps/i386/nptl/tls.h b/sysdeps/i386/nptl/tls.h
> index 21e23cd809..12285d3217 100644
> --- a/sysdeps/i386/nptl/tls.h
> +++ b/sysdeps/i386/nptl/tls.h
> @@ -49,7 +49,8 @@ typedef struct
>    void *__private_tm[3];
>    /* GCC split stack support.  */
>    void *__private_ss;
> -  void *__glibc_reserved2;
> +  /* The lowest address of shadow stack,  */
> +  unsigned long ssp_base;

OK.

>  } tcbhead_t;
>  
>  /* morestack.S in libgcc uses offset 0x30 to access __private_ss,   */
> diff --git a/sysdeps/x86_64/nptl/tcb-offsets.sym b/sysdeps/x86_64/nptl/tcb-offsets.sym
> index 387621e88c..ae8034743b 100644
> --- a/sysdeps/x86_64/nptl/tcb-offsets.sym
> +++ b/sysdeps/x86_64/nptl/tcb-offsets.sym
> @@ -13,6 +13,7 @@ MULTIPLE_THREADS_OFFSET	offsetof (tcbhead_t, multiple_threads)
>  POINTER_GUARD		offsetof (tcbhead_t, pointer_guard)
>  VGETCPU_CACHE_OFFSET	offsetof (tcbhead_t, vgetcpu_cache)
>  FEATURE_1_OFFSET	offsetof (tcbhead_t, feature_1)
> +SSP_BASE_OFFSET		offsetof (tcbhead_t, ssp_base)

OK.

>  
>  -- 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 f042a0250a..e88561c934 100644
> --- a/sysdeps/x86_64/nptl/tls.h
> +++ b/sysdeps/x86_64/nptl/tls.h
> @@ -60,7 +60,8 @@ typedef struct
>    void *__private_tm[4];
>    /* GCC split stack support.  */
>    void *__private_ss;
> -  long int __glibc_reserved2;
> +  /* The lowest address of shadow stack,  */
> +  unsigned long long int ssp_base;

OK.

>    /* Must be kept even if it is no longer used by glibc since programs,
>       like AddressSanitizer, depend on the size of tcbhead_t.  */
>    __128bits __glibc_unused2[8][4] __attribute__ ((aligned (32)));
> @@ -72,10 +73,17 @@ typedef struct
>  /* morestack.S in libgcc uses offset 0x40 to access __private_ss,   */
>  _Static_assert (offsetof (tcbhead_t, __private_ss) == 0x40,
>  		"offset of __private_ss != 0x40");
> +/* NB: ssp_base used to be "long int __glibc_reserved2", which was
> +   changed from 32 bits to 64 bits.  Make sure that the offset of the
> +   next field, __glibc_unused2, is unchanged.  */
> +_Static_assert (offsetof (tcbhead_t, __glibc_unused2) == 0x60,
> +		"offset of __glibc_unused2 != 0x60");

OK.

>  # else
>  /* morestack.S in libgcc uses offset 0x70 to access __private_ss,   */
>  _Static_assert (offsetof (tcbhead_t, __private_ss) == 0x70,
>  		"offset of __private_ss != 0x70");
> +_Static_assert (offsetof (tcbhead_t, __glibc_unused2) == 0x80,
> +		"offset of __glibc_unused2 != 0x80");

OK.

>  # endif
>  
>  #else /* __ASSEMBLER__ */
>
diff mbox

Patch

diff --git a/sysdeps/i386/nptl/tcb-offsets.sym b/sysdeps/i386/nptl/tcb-offsets.sym
index fbac241c45..2ec9e787c1 100644
--- a/sysdeps/i386/nptl/tcb-offsets.sym
+++ b/sysdeps/i386/nptl/tcb-offsets.sym
@@ -13,3 +13,4 @@  CLEANUP_PREV		offsetof (struct _pthread_cleanup_buffer, __prev)
 MUTEX_FUTEX		offsetof (pthread_mutex_t, __data.__lock)
 POINTER_GUARD		offsetof (tcbhead_t, pointer_guard)
 FEATURE_1_OFFSET	offsetof (tcbhead_t, feature_1)
+SSP_BASE_OFFSET		offsetof (tcbhead_t, ssp_base)
diff --git a/sysdeps/i386/nptl/tls.h b/sysdeps/i386/nptl/tls.h
index 21e23cd809..12285d3217 100644
--- a/sysdeps/i386/nptl/tls.h
+++ b/sysdeps/i386/nptl/tls.h
@@ -49,7 +49,8 @@  typedef struct
   void *__private_tm[3];
   /* GCC split stack support.  */
   void *__private_ss;
-  void *__glibc_reserved2;
+  /* The lowest address of shadow stack,  */
+  unsigned long ssp_base;
 } tcbhead_t;
 
 /* morestack.S in libgcc uses offset 0x30 to access __private_ss,   */
diff --git a/sysdeps/x86_64/nptl/tcb-offsets.sym b/sysdeps/x86_64/nptl/tcb-offsets.sym
index 387621e88c..ae8034743b 100644
--- a/sysdeps/x86_64/nptl/tcb-offsets.sym
+++ b/sysdeps/x86_64/nptl/tcb-offsets.sym
@@ -13,6 +13,7 @@  MULTIPLE_THREADS_OFFSET	offsetof (tcbhead_t, multiple_threads)
 POINTER_GUARD		offsetof (tcbhead_t, pointer_guard)
 VGETCPU_CACHE_OFFSET	offsetof (tcbhead_t, vgetcpu_cache)
 FEATURE_1_OFFSET	offsetof (tcbhead_t, feature_1)
+SSP_BASE_OFFSET		offsetof (tcbhead_t, ssp_base)
 
 -- 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 f042a0250a..e88561c934 100644
--- a/sysdeps/x86_64/nptl/tls.h
+++ b/sysdeps/x86_64/nptl/tls.h
@@ -60,7 +60,8 @@  typedef struct
   void *__private_tm[4];
   /* GCC split stack support.  */
   void *__private_ss;
-  long int __glibc_reserved2;
+  /* The lowest address of shadow stack,  */
+  unsigned long long int ssp_base;
   /* Must be kept even if it is no longer used by glibc since programs,
      like AddressSanitizer, depend on the size of tcbhead_t.  */
   __128bits __glibc_unused2[8][4] __attribute__ ((aligned (32)));
@@ -72,10 +73,17 @@  typedef struct
 /* morestack.S in libgcc uses offset 0x40 to access __private_ss,   */
 _Static_assert (offsetof (tcbhead_t, __private_ss) == 0x40,
 		"offset of __private_ss != 0x40");
+/* NB: ssp_base used to be "long int __glibc_reserved2", which was
+   changed from 32 bits to 64 bits.  Make sure that the offset of the
+   next field, __glibc_unused2, is unchanged.  */
+_Static_assert (offsetof (tcbhead_t, __glibc_unused2) == 0x60,
+		"offset of __glibc_unused2 != 0x60");
 # else
 /* morestack.S in libgcc uses offset 0x70 to access __private_ss,   */
 _Static_assert (offsetof (tcbhead_t, __private_ss) == 0x70,
 		"offset of __private_ss != 0x70");
+_Static_assert (offsetof (tcbhead_t, __glibc_unused2) == 0x80,
+		"offset of __glibc_unused2 != 0x80");
 # endif
 
 #else /* __ASSEMBLER__ */