Re: [PATCH] x86: Rename __glibc_reserved2 to ssp_base in tcbhead_t

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

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(-)

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__ */
-- 
2.17.1