[2/2] nptl: Update struct pthread_unwind_buf [BZ #22743]

Message ID CAMe9rOqvReLz4ZBV4z7dkWKe53TWF_rmPZ7zXrRULN3ntsXa3g@mail.gmail.com
State New, archived
Headers

Commit Message

H.J. Lu Feb. 8, 2018, 6:19 p.m. UTC
  On Thu, Feb 8, 2018 at 5:27 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Feb 8, 2018 at 1:25 AM, Carlos O'Donell <carlos@redhat.com> wrote:

>
>> like that to indicate the intent is to make the structure match the layout
>> used by setjmp jump buffers.
>>
>> Both UNWIND_BUF_PRIV and THREAD_COPY_ADDITONAL_INFO are exmaples of typo-prone
>> macro APIs. We should work hard to define defaults and have unconditional
>> users of these macros such that -Wundef can warn us if we make mistakes
>> e.g. https://sourceware.org/glibc/wiki/Wundef
>
> Will do.
>

Here is the updated patch for struct pthread_unwind_buf .  Now UNWIND_BUF_PRIV
is defined unconditionally and THREAD_COPY_ADDITONAL_INFO is renamed to
THREAD_COPY_CANCEL_JMP_BUF_INFO which is also defined unconditionally.

Tested with build-many-glibcs.py.  OK for master?

Thanks.
  

Patch

From d53c32a723c6f2249ffe2b6f75b4425afb70ebda Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 24 Jan 2018 15:27:49 -0800
Subject: [PATCH 2/2] nptl: Update struct pthread_unwind_buf [BZ #22743]

In glibc 2.28, the size of cancel_jmp_buf in struct pthread_unwind_buf
has been increased to match the size of __jmp_buf_tag on Linux/x86 in
order to save and restore shadow stack register.  pthread_unwind_buf is
used in <pthread.h>, whose address is passed from applications to
libpthread.  To access the private data in struct pthread_unwind_buf,
which is placed after cancel_jmp_buf, in libpthread, we must know which
struct pthread_unwind_buf, before glibc 28 and after glibc 2.28, is used
in caller.  If the size of caller's struct pthread_unwind_buf is smaller
than what libpthread expects, libpthread will override caller's stack
since struct pthread_unwind_buf is placed on caller's stack.

We enable shadow stack at run-time only if program and all used shared
objects, including dlopened ones, are shadow stack enabled, which means
that they must be compiled with GCC 8 or above and glibc 2.28 or above.
Since we need to save and restore shadow stack register only if shadow
stack is enabled, we can safely assume that caller is compiled with
smaller struct pthread_unwind_buf on stack if shadow stack isn't enabled
at run-time.  For callers with larger struct pthread_unwind_buf, but
shadow stack isn't enabled, we just have some unused space on caller's
stack.

struct pthread_unwind_buf is changed to union of

1. struct cancel_jmp_buf[1], which contains the common fields of struct
full and struct compat_pthread_unwind_buf.
2. struct full_pthread_unwind_buf, which is the full layout of the
cleanup buffer.
3. struct compat_pthread_unwind_buf, which is the compatible layout of
the cleanup buffer.

A macro, UNWIND_BUF_PRIV, is added to get the pointer to the priv field.
By default, it uses the priv field of struct compat_pthread_unwind_buf.
On Linux/x86, it uses the priv field of struct compat_pthread_unwind_buf
if shadow stack is disabled and struct full_pthread_unwind_buf if shadow
stack is enabled. The overhead of in __pthread_register_cancel on x86-64
is:

Without UNWIND_BUF_PRIV:

	movq	%fs:768,%rax
	movq	%rax, 72(%rdi)
	movq	%fs:760,%rax
	movq	%rax, 80(%rdi)
	movq	%rdi,%fs:768
	ret

With UNWIND_BUF_PRIV:

	movl	%fs:76,%ecx
	leaq	72(%rdi), %rdx
	leaq	200(%rdi), %rax
	andl	$2, %ecx
	cmove	%rdx, %rax
	movq	%fs:768,%rdx
	movq	%rdx, (%rax)
	movq	%fs:760,%rdx
	movq	%rdx, 8(%rax)
	movq	%rdi,%fs:768
	ret

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.

Tested with build-many-glibcs.py.

	[BZ #22743]
	* csu/libc-start.c (LIBC_START_MAIN): Use the full version of
	the cleanup buffer.
	* nptl/cleanup.c (__pthread_register_cancel): Use UNWIND_BUF_PRIV
	to access the priv field in the cleanup buffer.
	(__pthread_unregister_cancel): Likewise.
	* nptl/cleanup_defer.c (__pthread_register_cancel_defer):
	Likewise.
	(__pthread_unregister_cancel_restore): Likewise.
	* nptl/unwind.c (unwind_stop): Likewise.
	(__pthread_unwind_next): Likewise.
	* nptl/descr.h (pthread_unwind_buf_data): New.
	(full_pthread_unwind_buf): Likewise.
	(compat_pthread_unwind_buf): Likewise.
	(pthread_unwind_buf): Updated to use full_pthread_unwind_buf
	and compat_pthread_unwind_buf.
	* nptl/pthread_create.c (START_THREAD_DEFN): Use the full
	version of the cleanup buffer.
	(__pthread_create_2_1): Use THREAD_COPY_CANCEL_JMP_BUF_INFO.
	* sysdeps/nptl/cancel_jmp_buf.h (THREAD_COPY_CANCEL_JMP_BUF_INFO):
	New.
	(UNWIND_BUF_PRIV): New.  Macro
	to get pointer to the priv field in the cleanup buffer.
	* sysdeps/unix/sysv/linux/x86/nptl/pthreadP.h: Use the full
	version of the cleanup buffer to check cancel_jmp_buf size.
	* sysdeps/unix/sysv/linux/x86/cancel_jmp_buf.h
	(THREAD_COPY_CANCEL_JMP_BUF_INFO): New.
	(UNWIND_BUF_PRIV): Likewise.
---
 csu/libc-start.c                             |  6 ++-
 nptl/cleanup.c                               |  9 ++--
 nptl/cleanup_defer.c                         | 16 ++++---
 nptl/descr.h                                 | 66 ++++++++++++++++++++--------
 nptl/pthread_create.c                        |  7 ++-
 nptl/unwind.c                                |  6 ++-
 sysdeps/nptl/cancel_jmp_buf.h                |  8 ++++
 sysdeps/unix/sysv/linux/x86/cancel_jmp_buf.h | 14 ++++++
 sysdeps/unix/sysv/linux/x86/nptl/pthreadP.h  |  2 +-
 9 files changed, 99 insertions(+), 35 deletions(-)

diff --git a/csu/libc-start.c b/csu/libc-start.c
index 605222fa3f..c6bbc97ef0 100644
--- a/csu/libc-start.c
+++ b/csu/libc-start.c
@@ -298,8 +298,10 @@  LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
       struct pthread *self = THREAD_SELF;
 
       /* Store old info.  */
-      unwind_buf.priv.data.prev = THREAD_GETMEM (self, cleanup_jmp_buf);
-      unwind_buf.priv.data.cleanup = THREAD_GETMEM (self, cleanup);
+      unwind_buf.full.priv.data.prev
+	= THREAD_GETMEM (self, cleanup_jmp_buf);
+      unwind_buf.full.priv.data.cleanup
+	= THREAD_GETMEM (self, cleanup);
 
       /* Store the new cleanup handler info.  */
       THREAD_SETMEM (self, cleanup_jmp_buf, &unwind_buf);
diff --git a/nptl/cleanup.c b/nptl/cleanup.c
index d21b86e88b..6403a42d46 100644
--- a/nptl/cleanup.c
+++ b/nptl/cleanup.c
@@ -28,8 +28,9 @@  __pthread_register_cancel (__pthread_unwind_buf_t *buf)
   struct pthread *self = THREAD_SELF;
 
   /* Store old info.  */
-  ibuf->priv.data.prev = THREAD_GETMEM (self, cleanup_jmp_buf);
-  ibuf->priv.data.cleanup = THREAD_GETMEM (self, cleanup);
+  union pthread_unwind_buf_data *priv = UNWIND_BUF_PRIV (self, ibuf);
+  priv->data.prev = THREAD_GETMEM (self, cleanup_jmp_buf);
+  priv->data.cleanup = THREAD_GETMEM (self, cleanup);
 
   /* Store the new cleanup handler info.  */
   THREAD_SETMEM (self, cleanup_jmp_buf, (struct pthread_unwind_buf *) buf);
@@ -42,7 +43,9 @@  __cleanup_fct_attribute
 __pthread_unregister_cancel (__pthread_unwind_buf_t *buf)
 {
   struct pthread_unwind_buf *ibuf = (struct pthread_unwind_buf *) buf;
+  struct pthread *self = THREAD_SELF;
 
-  THREAD_SETMEM (THREAD_SELF, cleanup_jmp_buf, ibuf->priv.data.prev);
+  THREAD_SETMEM (self, cleanup_jmp_buf,
+		 UNWIND_BUF_PRIV (self, ibuf)->data.prev);
 }
 hidden_def (__pthread_unregister_cancel)
diff --git a/nptl/cleanup_defer.c b/nptl/cleanup_defer.c
index 5701ce4213..fddf7434db 100644
--- a/nptl/cleanup_defer.c
+++ b/nptl/cleanup_defer.c
@@ -28,8 +28,9 @@  __pthread_register_cancel_defer (__pthread_unwind_buf_t *buf)
   struct pthread *self = THREAD_SELF;
 
   /* Store old info.  */
-  ibuf->priv.data.prev = THREAD_GETMEM (self, cleanup_jmp_buf);
-  ibuf->priv.data.cleanup = THREAD_GETMEM (self, cleanup);
+  union pthread_unwind_buf_data *priv = UNWIND_BUF_PRIV (self, ibuf);
+  priv->data.prev = THREAD_GETMEM (self, cleanup_jmp_buf);
+  priv->data.cleanup = THREAD_GETMEM (self, cleanup);
 
   int cancelhandling = THREAD_GETMEM (self, cancelhandling);
 
@@ -49,9 +50,9 @@  __pthread_register_cancel_defer (__pthread_unwind_buf_t *buf)
 	cancelhandling = curval;
       }
 
-  ibuf->priv.data.canceltype = (cancelhandling & CANCELTYPE_BITMASK
-				? PTHREAD_CANCEL_ASYNCHRONOUS
-				: PTHREAD_CANCEL_DEFERRED);
+  priv->data.canceltype = (cancelhandling & CANCELTYPE_BITMASK
+			   ? PTHREAD_CANCEL_ASYNCHRONOUS
+			   : PTHREAD_CANCEL_DEFERRED);
 
   /* Store the new cleanup handler info.  */
   THREAD_SETMEM (self, cleanup_jmp_buf, (struct pthread_unwind_buf *) buf);
@@ -64,11 +65,12 @@  __pthread_unregister_cancel_restore (__pthread_unwind_buf_t *buf)
 {
   struct pthread *self = THREAD_SELF;
   struct pthread_unwind_buf *ibuf = (struct pthread_unwind_buf *) buf;
+  union pthread_unwind_buf_data *priv = UNWIND_BUF_PRIV (self, ibuf);
 
-  THREAD_SETMEM (self, cleanup_jmp_buf, ibuf->priv.data.prev);
+  THREAD_SETMEM (self, cleanup_jmp_buf, priv->data.prev);
 
   int cancelhandling;
-  if (ibuf->priv.data.canceltype != PTHREAD_CANCEL_DEFERRED
+  if (priv->data.canceltype != PTHREAD_CANCEL_DEFERRED
       && ((cancelhandling = THREAD_GETMEM (self, cancelhandling))
 	  & CANCELTYPE_BITMASK) == 0)
     {
diff --git a/nptl/descr.h b/nptl/descr.h
index f56ac69379..7e5aa7f6bc 100644
--- a/nptl/descr.h
+++ b/nptl/descr.h
@@ -56,11 +56,30 @@ 
    / PTHREAD_KEY_2NDLEVEL_SIZE)
 
 
+/* Private data in the cleanup buffer.  */
+union pthread_unwind_buf_data
+{
+  /* This is the placeholder of the public version.  */
+  void *pad[4];
 
+  struct
+  {
+    /* Pointer to the previous cleanup buffer.  */
+    struct pthread_unwind_buf *prev;
 
-/* Internal version of the buffer to store cancellation handler
+    /* Backward compatibility: state of the old-style cleanup
+       handler at the time of the previous new-style cleanup handler
+       installment.  */
+    struct _pthread_cleanup_buffer *cleanup;
+
+    /* Cancellation type before the push call.  */
+    int canceltype;
+  } data;
+};
+
+/* Internal full version of the buffer to store cancellation handler
    information.  */
-struct pthread_unwind_buf
+struct full_pthread_unwind_buf
 {
   struct
   {
@@ -71,28 +90,39 @@  struct pthread_unwind_buf
 #endif
   } cancel_jmp_buf[1];
 
-  union
+  union pthread_unwind_buf_data priv;
+};
+
+/* Internal compatible version of the buffer to store cancellation
+   handler information.  */
+struct compat_pthread_unwind_buf
+{
+  struct
   {
-    /* This is the placeholder of the public version.  */
-    void *pad[4];
+    __jmp_buf jmp_buf;
+    int mask_was_saved;
+  } cancel_jmp_buf[1];
 
+  union pthread_unwind_buf_data priv;
+};
+
+/* Internal version of the buffer to store cancellation handler
+   information.  */
+struct pthread_unwind_buf
+{
+  union
+  {
+    /* The common fields of full and compatible versions.  */
     struct
     {
-      /* Pointer to the previous cleanup buffer.  */
-      struct pthread_unwind_buf *prev;
-
-      /* Backward compatibility: state of the old-style cleanup
-	 handler at the time of the previous new-style cleanup handler
-	 installment.  */
-      struct _pthread_cleanup_buffer *cleanup;
-
-      /* Cancellation type before the push call.  */
-      int canceltype;
-    } data;
-  } priv;
+      __jmp_buf jmp_buf;
+      int mask_was_saved;
+    } cancel_jmp_buf[1];
+    struct full_pthread_unwind_buf full;
+    struct compat_pthread_unwind_buf compat;
+  };
 };
 
-
 /* Opcodes and data types for communication with the signal handler to
    change user/group IDs.  */
 struct xid_command
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index caaf07c134..a7ef18a788 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -428,8 +428,8 @@  START_THREAD_DEFN
   struct pthread_unwind_buf unwind_buf;
 
   /* No previous handlers.  */
-  unwind_buf.priv.data.prev = NULL;
-  unwind_buf.priv.data.cleanup = NULL;
+  unwind_buf.full.priv.data.prev = NULL;
+  unwind_buf.full.priv.data.cleanup = NULL;
 
   int not_first_call;
   not_first_call = setjmp ((struct __jmp_buf_tag *) unwind_buf.cancel_jmp_buf);
@@ -701,6 +701,9 @@  __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr,
   THREAD_COPY_POINTER_GUARD (pd);
 #endif
 
+  /* Copy cancel_jmp_buf info.  */
+  THREAD_COPY_CANCEL_JMP_BUF_INFO (pd);
+
   /* Verify the sysinfo bits were copied in allocate_stack if needed.  */
 #ifdef NEED_DL_SYSINFO
   CHECK_THREAD_SYSINFO (pd);
diff --git a/nptl/unwind.c b/nptl/unwind.c
index b37a063c53..f58be0ee5f 100644
--- a/nptl/unwind.c
+++ b/nptl/unwind.c
@@ -66,7 +66,8 @@  unwind_stop (int version, _Unwind_Action actions,
       /* Handle the compatibility stuff.  Execute all handlers
 	 registered with the old method which would be unwound by this
 	 step.  */
-      struct _pthread_cleanup_buffer *oldp = buf->priv.data.cleanup;
+      struct _pthread_cleanup_buffer *oldp
+	= UNWIND_BUF_PRIV (self, buf)->data.cleanup;
       void *cfa = (void *) (_Unwind_Ptr) _Unwind_GetCFA (context);
 
       if (curp != oldp && (do_longjump || FRAME_LEFT (cfa, curp, adj)))
@@ -133,6 +134,7 @@  __pthread_unwind_next (__pthread_unwind_buf_t *buf)
 {
   struct pthread_unwind_buf *ibuf = (struct pthread_unwind_buf *) buf;
 
-  __pthread_unwind ((__pthread_unwind_buf_t *) ibuf->priv.data.prev);
+  __pthread_unwind ((__pthread_unwind_buf_t *)
+		    UNWIND_BUF_PRIV (THREAD_SELF, ibuf)->data.prev);
 }
 hidden_def (__pthread_unwind_next)
diff --git a/sysdeps/nptl/cancel_jmp_buf.h b/sysdeps/nptl/cancel_jmp_buf.h
index a6612bb60b..d3e7de011a 100644
--- a/sysdeps/nptl/cancel_jmp_buf.h
+++ b/sysdeps/nptl/cancel_jmp_buf.h
@@ -18,3 +18,11 @@ 
 
 /* No need to use the same setjmp jmp_buf layout in cancel_jmp_buf.  */
 #define NEED_SETJMP_JMP_BUF_LAYOUT 0
+
+/* No need to copy info for cancel_jmp_buf.  */
+#define THREAD_COPY_CANCEL_JMP_BUF_INFO(descr)
+
+/* Get pointer to the priv field from THREAD_SELF, "self", and pointer
+   to the cleanup buffer, "p".  By default, the compatible version is
+   used.  */
+#define UNWIND_BUF_PRIV(self,p) (&((p)->compat.priv))
diff --git a/sysdeps/unix/sysv/linux/x86/cancel_jmp_buf.h b/sysdeps/unix/sysv/linux/x86/cancel_jmp_buf.h
index 362f12d0ac..cd834c5c3f 100644
--- a/sysdeps/unix/sysv/linux/x86/cancel_jmp_buf.h
+++ b/sysdeps/unix/sysv/linux/x86/cancel_jmp_buf.h
@@ -18,3 +18,17 @@ 
 
 /* Need the same setjmp jmp_buf layout in cancel_jmp_buf.  */
 #define NEED_SETJMP_JMP_BUF_LAYOUT 1
+
+/* Wee need to copy feature_1 in pthread_create.  */
+#define THREAD_COPY_CANCEL_JMP_BUF_INFO(descr)				\
+  ((descr)->header.feature_1						\
+   = THREAD_GETMEM (THREAD_SELF, header.feature_1))
+
+/* Get pointer to the priv field from THREAD_SELF, "self", and pointer
+   to the cleanup buffer, "p".  If shadow stack is disabled, use the
+   compatible struct __cancel_jmp_buf_tag.  */
+#define UNWIND_BUF_PRIV(self,p) \
+  (__extension__ ({							\
+     unsigned int feature_1 = THREAD_GETMEM (self, header.feature_1);	\
+     (((feature_1 & (1 << 1)) == 0)					\
+      ? &((p)->compat.priv) : &((p)->full.priv));}))
diff --git a/sysdeps/unix/sysv/linux/x86/nptl/pthreadP.h b/sysdeps/unix/sysv/linux/x86/nptl/pthreadP.h
index 247a62e9a0..ff9ea4cb6d 100644
--- a/sysdeps/unix/sysv/linux/x86/nptl/pthreadP.h
+++ b/sysdeps/unix/sysv/linux/x86/nptl/pthreadP.h
@@ -23,7 +23,7 @@ 
 
 extern struct pthread_unwind_buf ____pthread_unwind_buf_private;
 
-_Static_assert (sizeof (____pthread_unwind_buf_private.cancel_jmp_buf)
+_Static_assert (sizeof (____pthread_unwind_buf_private.full.cancel_jmp_buf)
 		>= sizeof (struct __jmp_buf_tag),
 		"size of cancel_jmp_buf < sizeof __jmp_buf_tag");
 
-- 
2.14.3