From patchwork Wed Feb 14 13:04:40 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 25934 Received: (qmail 64166 invoked by alias); 14 Feb 2018 13:04:51 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 64150 invoked by uid 89); 14 Feb 2018 13:04:51 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.0 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-qt0-f171.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=uVwRY6wOm3jfiLYkoARw2vx12j7YRqsfHm9y/RxbMMM=; b=OEAxK+HkPZggK9bGtNXpo9GGFyM8P1JNxBTpxTq3/v9uiNjnkv8DfmXjBeKLsHNJPG nay3050BdJU8fkOog5j9e77aMdEj8e7jCvCQa4AbUPVCOrQCu4wogL5mAuI875MMMwWE xuN4G6pp/EExnomhuTXV+HcdmL6PyHsmE0U+KnSwPunwx/bKDvJQla7MGpIEfTCHnBxW /+S3GLCch7dDJ3c4cxtkh2d+8dVrgcfMbL8proDAJRWekE/rsQsPnQzUPMr9dYWFQfiy 0V01p2G3uFh2hoCDVXPs5rNLzpr23tdUdQFZOh703GWJPPwDdts8+p3Mo5ql+ABG/nWP aeFQ== X-Gm-Message-State: APf1xPBsfEDoubJ13jtjlEHfQnYP572SYzGUL1nUj3VqOUv830ezANsv WoSRwbYuM8R5xtmh/U50pdI/bQ== X-Google-Smtp-Source: AH8x225NYCjpD6LdixWWAUayLEuvVK5RcsLiopq0CXHtVI/abnl8wwZVqXbeZ/Ly4mREv4KcSK1b2A== X-Received: by 10.200.52.204 with SMTP id x12mr7612162qtb.267.1518613486319; Wed, 14 Feb 2018 05:04:46 -0800 (PST) Subject: Re: [PATCH] aarch64: Add split-stack TCB field To: Szabolcs Nagy , libc-alpha@sourceware.org Cc: nd@arm.com References: <1518026313-22248-1-git-send-email-adhemerval.zanella@linaro.org> From: Adhemerval Zanella Message-ID: Date: Wed, 14 Feb 2018 11:04:40 -0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: On 13/02/2018 14:56, Szabolcs Nagy wrote: > 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 Fixed locally. > >> 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) Indeed, it is an artefact from previous version. I removed it in the patch below. >> 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 >>   # include >>   # include >> +# include >>     #else /* __ASSEMBLER__ */ >>   # include >> @@ -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) I have added a comment about the layout and possible newer member additions on tcbprehead_t. --- * sysdeps/aarch64/Makefile [$(subdir) = elf] (sysdeps-dl-routines): Add tcb-version. * sysdeps/aarch64/Versions [ld] (GLIBC_2.28): 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.28): Add. (__libc_tcb_private_ss): Likewise. --- 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/tls.h b/sysdeps/aarch64/nptl/tls.h index ac39c24..99606d0 100644 --- a/sysdeps/aarch64/nptl/tls.h +++ b/sysdeps/aarch64/nptl/tls.h @@ -26,6 +26,7 @@ # include # include # include +# include #else /* __ASSEMBLER__ */ # include @@ -43,12 +44,32 @@ /* Get the thread descriptor definition. */ # include +/* The resulting TLS layout is: + + |----------------|--------------|-----------|------------| + | struct pthread | tcbprehead_t | tcbhead_t | static tls | + |----------------|--------------|-----------|------------| + \_ Thread pointer + + And tcbprehead_t might overlap the padding at the end of the + struct pthread, otherwise extra space is allocated (check TLS_PRE_TCB_SIZE + definition). + + Additional fields on tcbhead_t must keep member offset constant (so they + should be added at the beggining of the struct). */ + typedef struct { dtv_t *dtv; 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 +79,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 +111,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 +120,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..eaa75dd --- /dev/null +++ b/sysdeps/aarch64/tcb-version.c @@ -0,0 +1,23 @@ +/* TCB field abi advertise symbols. + Copyright (C) 2017-2018 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 + . */ + +#include + +/* 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