aarch64: Add split-stack TCB field

Message ID 1518026313-22248-1-git-send-email-adhemerval.zanella@linaro.org
State Dropped
Headers

Commit Message

Adhemerval Zanella Netto Feb. 7, 2018, 5:58 p.m. UTC
  This patch adds split-stack support pointer guard on glibc for aarch64.
Different from other architectures (powerpc, s390) where the memory is
placed on TCB, aarch64 one is placed before thread pointer initial
position.  It has an advantage over extending TCB because for aarch64
TLS variable placement take in consideration tcbhead_t size and by
changing its value would require to also update the static linker
(and it would also add incompatibility with glibc and older linkers).

For aarch64 tcb direct access is fastest for thread local variable on
all mode and related TLS access.  It requires just a direct load with
displacement of -8 (since thread pointer points to tcbhead_t).

It also adds a loader symbol (__tcb_private_ss) to signal the existence
of the split stack guard area.

Checked on aarch64-linux-gnu.

	* sysdeps/aarch64/Makefile [$(subdir) = elf] (sysdeps-dl-routines):
	Add tcb-version.
	* sysdeps/aarch64/Versions [ld] (GLIBC_2.26): Add
	__libc_tcb_private_ss.
	* sysdeps/aarch64/nptl/tls.h (tcbprehead_t): New struct.
	(TLS_PRE_TCB_SIZE): Take tcbprehead_t in consideration.
	(TLS_DEFINE_INIT_TP): Likewise.
	(THREAD_SELF): Likewise.
	(DB_THREAD_SELF): Likewise.
	* sysdeps/aarch64/tcb-version.c: New file.
	* sysdeps/unix/sysv/linux/aarch64/ld.abilist (GLIBC_2.26): Add.
	(__libc_tcb_private_ss): Likewise.
	* sysdeps/aarch64/nptl/tcb-offsets.sym (PTHREAD_SIZEOF): Rename to
	PTHREAD_PRE_TCB_SIZE.
---
 ChangeLog                                  | 17 +++++++++++++++++
 sysdeps/aarch64/Makefile                   |  2 +-
 sysdeps/aarch64/Versions                   |  8 ++++++++
 sysdeps/aarch64/nptl/tcb-offsets.sym       |  2 +-
 sysdeps/aarch64/nptl/tls.h                 | 25 ++++++++++++++++++++-----
 sysdeps/aarch64/tcb-version.c              | 23 +++++++++++++++++++++++
 sysdeps/unix/sysv/linux/aarch64/ld.abilist |  2 ++
 7 files changed, 72 insertions(+), 7 deletions(-)
 create mode 100644 sysdeps/aarch64/tcb-version.c
  

Comments

Joseph Myers Feb. 7, 2018, 6:04 p.m. UTC | #1
On Wed, 7 Feb 2018, Adhemerval Zanella wrote:

> 	* sysdeps/aarch64/Versions [ld] (GLIBC_2.26): Add
> 	__libc_tcb_private_ss.

This ChangeLog entry is out of date compared to current symbol versions.

> diff --git a/sysdeps/aarch64/tcb-version.c b/sysdeps/aarch64/tcb-version.c
> new file mode 100644
> index 0000000..c94e5d3
> --- /dev/null
> +++ b/sysdeps/aarch64/tcb-version.c
> @@ -0,0 +1,23 @@
> +/* TCB field abi advertise symbols.
> +   Copyright (C) 2017 Free Software Foundation, Inc.

Should now be 2017-2018.
  
Adhemerval Zanella Netto Feb. 7, 2018, 6:08 p.m. UTC | #2
On 07/02/2018 16:04, Joseph Myers wrote:
> On Wed, 7 Feb 2018, Adhemerval Zanella wrote:
> 
>> 	* sysdeps/aarch64/Versions [ld] (GLIBC_2.26): Add
>> 	__libc_tcb_private_ss.
> 
> This ChangeLog entry is out of date compared to current symbol versions.
> 
>> diff --git a/sysdeps/aarch64/tcb-version.c b/sysdeps/aarch64/tcb-version.c
>> new file mode 100644
>> index 0000000..c94e5d3
>> --- /dev/null
>> +++ b/sysdeps/aarch64/tcb-version.c
>> @@ -0,0 +1,23 @@
>> +/* TCB field abi advertise symbols.
>> +   Copyright (C) 2017 Free Software Foundation, Inc.
> 
> Should now be 2017-2018.
> 

Thanks I fixed it locally.
  
Szabolcs Nagy Feb. 13, 2018, 4:56 p.m. UTC | #3
On 07/02/18 17:58, Adhemerval Zanella wrote:
> This patch adds split-stack support pointer guard on glibc for aarch64.
> Different from other architectures (powerpc, s390) where the memory is
> placed on TCB, aarch64 one is placed before thread pointer initial
> position.  It has an advantage over extending TCB because for aarch64
> TLS variable placement take in consideration tcbhead_t size and by
> changing its value would require to also update the static linker
> (and it would also add incompatibility with glibc and older linkers).
> 
> For aarch64 tcb direct access is fastest for thread local variable on
> all mode and related TLS access.  It requires just a direct load with
> displacement of -8 (since thread pointer points to tcbhead_t).
> 
> It also adds a loader symbol (__tcb_private_ss) to signal the existence

__libc_tcb_private_ss

> of the split stack guard area.
> 
> Checked on aarch64-linux-gnu.
> 
> 	* sysdeps/aarch64/Makefile [$(subdir) = elf] (sysdeps-dl-routines):
> 	Add tcb-version.
> 	* sysdeps/aarch64/Versions [ld] (GLIBC_2.26): Add
> 	__libc_tcb_private_ss.
> 	* sysdeps/aarch64/nptl/tls.h (tcbprehead_t): New struct.
> 	(TLS_PRE_TCB_SIZE): Take tcbprehead_t in consideration.
> 	(TLS_DEFINE_INIT_TP): Likewise.
> 	(THREAD_SELF): Likewise.
> 	(DB_THREAD_SELF): Likewise.
> 	* sysdeps/aarch64/tcb-version.c: New file.
> 	* sysdeps/unix/sysv/linux/aarch64/ld.abilist (GLIBC_2.26): Add.
> 	(__libc_tcb_private_ss): Likewise.
> 	* sysdeps/aarch64/nptl/tcb-offsets.sym (PTHREAD_SIZEOF): Rename to
> 	PTHREAD_PRE_TCB_SIZE.

i don't see where PTHREAD_PRE_TCB_SIZE is used
(or PTHREAD_SIZEOF)

> ---
>   ChangeLog                                  | 17 +++++++++++++++++
>   sysdeps/aarch64/Makefile                   |  2 +-
>   sysdeps/aarch64/Versions                   |  8 ++++++++
>   sysdeps/aarch64/nptl/tcb-offsets.sym       |  2 +-
>   sysdeps/aarch64/nptl/tls.h                 | 25 ++++++++++++++++++++-----
>   sysdeps/aarch64/tcb-version.c              | 23 +++++++++++++++++++++++
>   sysdeps/unix/sysv/linux/aarch64/ld.abilist |  2 ++
>   7 files changed, 72 insertions(+), 7 deletions(-)
>   create mode 100644 sysdeps/aarch64/tcb-version.c
> 
> diff --git a/sysdeps/aarch64/Makefile b/sysdeps/aarch64/Makefile
> index 5f3e203..141442a 100644
> --- a/sysdeps/aarch64/Makefile
> +++ b/sysdeps/aarch64/Makefile
> @@ -1,7 +1,7 @@
>   long-double-fcts = yes
>   
>   ifeq ($(subdir),elf)
> -sysdep-dl-routines += tlsdesc dl-tlsdesc
> +sysdep-dl-routines += tlsdesc dl-tlsdesc tcb-version
>   gen-as-const-headers += dl-link.sym
>   endif
>   
> diff --git a/sysdeps/aarch64/Versions b/sysdeps/aarch64/Versions
> index e1aa44f..dcc395c 100644
> --- a/sysdeps/aarch64/Versions
> +++ b/sysdeps/aarch64/Versions
> @@ -3,3 +3,11 @@ libc {
>       _mcount;
>     }
>   }
> +
> +ld {
> +  GLIBC_2.28 {
> +    # Symbol used to version control the private GLIBC TCB split-stack
> +    # field.
> +    __libc_tcb_private_ss;
> +  }
> +}
> diff --git a/sysdeps/aarch64/nptl/tcb-offsets.sym b/sysdeps/aarch64/nptl/tcb-offsets.sym
> index 238647d..6004379 100644
> --- a/sysdeps/aarch64/nptl/tcb-offsets.sym
> +++ b/sysdeps/aarch64/nptl/tcb-offsets.sym
> @@ -3,4 +3,4 @@
>   
>   PTHREAD_MULTIPLE_THREADS_OFFSET		offsetof (struct pthread, header.multiple_threads)
>   PTHREAD_TID_OFFSET			offsetof (struct pthread, tid)
> -PTHREAD_SIZEOF				sizeof (struct pthread)
> +PTHREAD_PRE_TCB_SIZE			TLS_PRE_TCB_SIZE
> diff --git a/sysdeps/aarch64/nptl/tls.h b/sysdeps/aarch64/nptl/tls.h
> index ac39c24..2d88310 100644
> --- a/sysdeps/aarch64/nptl/tls.h
> +++ b/sysdeps/aarch64/nptl/tls.h
> @@ -26,6 +26,7 @@
>   # include <stddef.h>
>   # include <stdint.h>
>   # include <dl-dtv.h>
> +# include <libc-pointer-arith.h>
>   
>   #else /* __ASSEMBLER__ */
>   # include <tcb-offsets.h>
> @@ -49,6 +50,12 @@ typedef struct
>     void *private;
>   } tcbhead_t;
>   
> +typedef struct
> +{
> +  /* GCC split stack support.  */
> +  void *__private_ss;
> +} tcbprehead_t;
> +
>   /* This is the size of the initial TCB.  */
>   # define TLS_INIT_TCB_SIZE	sizeof (tcbhead_t)
>   
> @@ -58,8 +65,14 @@ typedef struct
>   /* This is the size of the TCB.  */
>   # define TLS_TCB_SIZE		sizeof (tcbhead_t)
>   
> -/* This is the size we need before TCB.  */
> -# define TLS_PRE_TCB_SIZE	sizeof (struct pthread)
> +/* This is the size we need before TCB.  Check if there is room for
> +   tcbprehead_t in struct pthread's final padding and if not add it on
> +   required pre-tcb size.  */
> +# define TLS_PRE_TCB_SIZE \
> +  (sizeof (struct pthread)						\
> +   + (PTHREAD_STRUCT_END_PADDING < sizeof (tcbprehead_t)		\
> +      ? ALIGN_UP (sizeof (tcbprehead_t), __alignof__ (struct pthread))	\
> +      : 0))
>   

it looks ok, but a bit ugly, i don't see a clean way
to express the requirements in c.

if i understand correctly the tls layout is

struct pthread   tcbprehead_t   tcbhead_t   static tls
<--------------><-------------><----------><----------->
                                ^
                                TP

where tcbprehead_t may overlap with paddings at the end
of struct pthread.

and we have to make sure offsets from TP to the members
of tcbprehead_t don't change.

i think there should be a comment about this layout and
the alignment/offset requirements somewhere in tls.h
(so next time it's easier to allocate such a tls slot)

>   /* Alignment requirements for the TCB.  */
>   # define TLS_TCB_ALIGN		__alignof__ (struct pthread)
> @@ -84,7 +97,8 @@ typedef struct
>     ({ __asm __volatile ("msr tpidr_el0, %0" : : "r" (tcbp)); NULL; })
>   
>   /* Value passed to 'clone' for initialization of the thread register.  */
> -# define TLS_DEFINE_INIT_TP(tp, pd) void *tp = (pd) + 1
> +# define TLS_DEFINE_INIT_TP(tp, pd) \
> +  void *tp = (void*)((uintptr_t) (pd) + TLS_PRE_TCB_SIZE)
>   
>   /* Return the address of the dtv for the current thread.  */
>   # define THREAD_DTV() \
> @@ -92,11 +106,12 @@ typedef struct
>   
>   /* Return the thread descriptor for the current thread.  */
>   # define THREAD_SELF \
> - ((struct pthread *)__builtin_thread_pointer () - 1)
> +  ((struct pthread *)((uintptr_t) __builtin_thread_pointer () \
> +		      - TLS_PRE_TCB_SIZE))
>   
>   /* Magic for libthread_db to know how to do THREAD_SELF.  */
>   # define DB_THREAD_SELF \
> -  CONST_THREAD_AREA (64, sizeof (struct pthread))
> +  CONST_THREAD_AREA (64, TLS_PRE_TCB_SIZE)
>   
>   /* Access to data in the thread descriptor is easy.  */
>   # define THREAD_GETMEM(descr, member) \
> diff --git a/sysdeps/aarch64/tcb-version.c b/sysdeps/aarch64/tcb-version.c
> new file mode 100644
> index 0000000..c94e5d3
> --- /dev/null
> +++ b/sysdeps/aarch64/tcb-version.c
> @@ -0,0 +1,23 @@
> +/* TCB field abi advertise symbols.
> +   Copyright (C) 2017 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 <libc-internal.h>
> +
> +/* Symbol used to version control the private GLIBC TCB split-stack
> +   field.  */
> +long int attribute_relro __libc_tcb_private_ss;
> diff --git a/sysdeps/unix/sysv/linux/aarch64/ld.abilist b/sysdeps/unix/sysv/linux/aarch64/ld.abilist
> index ec7f617..aaddb24 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/aarch64/ld.abilist
> @@ -8,3 +8,5 @@ GLIBC_2.17 calloc F
>   GLIBC_2.17 free F
>   GLIBC_2.17 malloc F
>   GLIBC_2.17 realloc F
> +GLIBC_2.28 GLIBC_2.28 A
> +GLIBC_2.28 __libc_tcb_private_ss D 0x8
>
  

Patch

diff --git a/sysdeps/aarch64/Makefile b/sysdeps/aarch64/Makefile
index 5f3e203..141442a 100644
--- a/sysdeps/aarch64/Makefile
+++ b/sysdeps/aarch64/Makefile
@@ -1,7 +1,7 @@ 
 long-double-fcts = yes
 
 ifeq ($(subdir),elf)
-sysdep-dl-routines += tlsdesc dl-tlsdesc
+sysdep-dl-routines += tlsdesc dl-tlsdesc tcb-version
 gen-as-const-headers += dl-link.sym
 endif
 
diff --git a/sysdeps/aarch64/Versions b/sysdeps/aarch64/Versions
index e1aa44f..dcc395c 100644
--- a/sysdeps/aarch64/Versions
+++ b/sysdeps/aarch64/Versions
@@ -3,3 +3,11 @@  libc {
     _mcount;
   }
 }
+
+ld {
+  GLIBC_2.28 {
+    # Symbol used to version control the private GLIBC TCB split-stack
+    # field.
+    __libc_tcb_private_ss;
+  }
+}
diff --git a/sysdeps/aarch64/nptl/tcb-offsets.sym b/sysdeps/aarch64/nptl/tcb-offsets.sym
index 238647d..6004379 100644
--- a/sysdeps/aarch64/nptl/tcb-offsets.sym
+++ b/sysdeps/aarch64/nptl/tcb-offsets.sym
@@ -3,4 +3,4 @@ 
 
 PTHREAD_MULTIPLE_THREADS_OFFSET		offsetof (struct pthread, header.multiple_threads)
 PTHREAD_TID_OFFSET			offsetof (struct pthread, tid)
-PTHREAD_SIZEOF				sizeof (struct pthread)
+PTHREAD_PRE_TCB_SIZE			TLS_PRE_TCB_SIZE
diff --git a/sysdeps/aarch64/nptl/tls.h b/sysdeps/aarch64/nptl/tls.h
index ac39c24..2d88310 100644
--- a/sysdeps/aarch64/nptl/tls.h
+++ b/sysdeps/aarch64/nptl/tls.h
@@ -26,6 +26,7 @@ 
 # include <stddef.h>
 # include <stdint.h>
 # include <dl-dtv.h>
+# include <libc-pointer-arith.h>
 
 #else /* __ASSEMBLER__ */
 # include <tcb-offsets.h>
@@ -49,6 +50,12 @@  typedef struct
   void *private;
 } tcbhead_t;
 
+typedef struct
+{
+  /* GCC split stack support.  */
+  void *__private_ss;
+} tcbprehead_t;
+
 /* This is the size of the initial TCB.  */
 # define TLS_INIT_TCB_SIZE	sizeof (tcbhead_t)
 
@@ -58,8 +65,14 @@  typedef struct
 /* This is the size of the TCB.  */
 # define TLS_TCB_SIZE		sizeof (tcbhead_t)
 
-/* This is the size we need before TCB.  */
-# define TLS_PRE_TCB_SIZE	sizeof (struct pthread)
+/* This is the size we need before TCB.  Check if there is room for
+   tcbprehead_t in struct pthread's final padding and if not add it on
+   required pre-tcb size.  */
+# define TLS_PRE_TCB_SIZE \
+  (sizeof (struct pthread)						\
+   + (PTHREAD_STRUCT_END_PADDING < sizeof (tcbprehead_t)		\
+      ? ALIGN_UP (sizeof (tcbprehead_t), __alignof__ (struct pthread))	\
+      : 0))
 
 /* Alignment requirements for the TCB.  */
 # define TLS_TCB_ALIGN		__alignof__ (struct pthread)
@@ -84,7 +97,8 @@  typedef struct
   ({ __asm __volatile ("msr tpidr_el0, %0" : : "r" (tcbp)); NULL; })
 
 /* Value passed to 'clone' for initialization of the thread register.  */
-# define TLS_DEFINE_INIT_TP(tp, pd) void *tp = (pd) + 1
+# define TLS_DEFINE_INIT_TP(tp, pd) \
+  void *tp = (void*)((uintptr_t) (pd) + TLS_PRE_TCB_SIZE)
 
 /* Return the address of the dtv for the current thread.  */
 # define THREAD_DTV() \
@@ -92,11 +106,12 @@  typedef struct
 
 /* Return the thread descriptor for the current thread.  */
 # define THREAD_SELF \
- ((struct pthread *)__builtin_thread_pointer () - 1)
+  ((struct pthread *)((uintptr_t) __builtin_thread_pointer () \
+		      - TLS_PRE_TCB_SIZE))
 
 /* Magic for libthread_db to know how to do THREAD_SELF.  */
 # define DB_THREAD_SELF \
-  CONST_THREAD_AREA (64, sizeof (struct pthread))
+  CONST_THREAD_AREA (64, TLS_PRE_TCB_SIZE)
 
 /* Access to data in the thread descriptor is easy.  */
 # define THREAD_GETMEM(descr, member) \
diff --git a/sysdeps/aarch64/tcb-version.c b/sysdeps/aarch64/tcb-version.c
new file mode 100644
index 0000000..c94e5d3
--- /dev/null
+++ b/sysdeps/aarch64/tcb-version.c
@@ -0,0 +1,23 @@ 
+/* TCB field abi advertise symbols.
+   Copyright (C) 2017 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 <libc-internal.h>
+
+/* Symbol used to version control the private GLIBC TCB split-stack
+   field.  */
+long int attribute_relro __libc_tcb_private_ss;
diff --git a/sysdeps/unix/sysv/linux/aarch64/ld.abilist b/sysdeps/unix/sysv/linux/aarch64/ld.abilist
index ec7f617..aaddb24 100644
--- a/sysdeps/unix/sysv/linux/aarch64/ld.abilist
+++ b/sysdeps/unix/sysv/linux/aarch64/ld.abilist
@@ -8,3 +8,5 @@  GLIBC_2.17 calloc F
 GLIBC_2.17 free F
 GLIBC_2.17 malloc F
 GLIBC_2.17 realloc F
+GLIBC_2.28 GLIBC_2.28 A
+GLIBC_2.28 __libc_tcb_private_ss D 0x8