Bug 23403

Summary: Wrong alignment of TLS variables
Product: glibc Reporter: Stefan Liebler <stli>
Component: nptlAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED FIXED    
Severity: normal CC: adhemerval.zanella, codonell, drepper.fsp, fweimer, stli
Priority: P2 Flags: fweimer: security-
Version: 2.28   
Target Milestone: 2.30   
Host: Target:
Build: Last reconfirmed:
Attachments: Testcase shows the false alignment

Description Stefan Liebler 2018-07-11 15:49:25 UTC
Created attachment 11121 [details]
Testcase shows the false alignment

The alignment of TLS variables is wrong if accessed from within a thread.
The attached testcase is testing different alignments from 0x10, 0x20, ..., 0x2000.

On e.g. s390x it fails starting with an alignment of 0x20.
Where it starts failing with an alignment of 0x200 on x86_64.

The fails occur on architectures where TLS_TCB_AT_TP is defined to 1.
The TLS variables are located in the .tdata section and it seems as the variables would be correctly aligned if the section itself would be aligned.
The section is located relative to the thread-pointer.
But it is not aligned to __static_tls_align.
See nptl/allocatestack.c:
	  /* Place the thread descriptor at the end of the stack.  */
#if TLS_TCB_AT_TP
	  pd = (struct pthread *) ((char *) mem + size) - 1;

Note:
On s390x, sizeof (struct pthread) is 0x6f0. Thus it starts failing with an alignment of 0x20.
On x86_64, sizeof (struct pthread) is 0x900. Thus it starts failing with an alignment of 0x200.
Comment 1 Adhemerval Zanella 2018-07-11 16:32:25 UTC
It fails on sparc64 (and sparcv9) as well:

thread 0xffffffffffffffff:   &tls_int_10: ptr=0xffff800100042000; val=0x0010 => OK
thread 0xffffffffffffffff:   &tls_int_20: ptr=0xffff800100042020; val=0x0020 => OK
thread 0xffffffffffffffff:   &tls_int_40: ptr=0xffff800100042040; val=0x0040 => OK
thread 0xffffffffffffffff:   &tls_int_80: ptr=0xffff800100042080; val=0x0080 => OK
thread 0xffffffffffffffff:  &tls_int_100: ptr=0xffff800100042100; val=0x0100 => OK
thread 0xffffffffffffffff:  &tls_int_200: ptr=0xffff800100042200; val=0x0200 => OK
thread 0xffffffffffffffff:  &tls_int_400: ptr=0xffff800100042400; val=0x0400 => OK
thread 0xffffffffffffffff:  &tls_int_800: ptr=0xffff800100042800; val=0x0800 => OK
thread 0xffffffffffffffff: &tls_int_1000: ptr=0xffff800100043000; val=0x1000 => OK
thread 0xffffffffffffffff: &tls_int_2000: ptr=0xffff800100044000; val=0x2000 => OK
thread 0x0000000000000000:   &tls_int_10: ptr=0xffff800100caf910; val=0x0010 => OK
thread 0x0000000000000000:   &tls_int_20: ptr=0xffff800100caf930; val=0x0020 => FAIL
thread 0x0000000000000000:   &tls_int_40: ptr=0xffff800100caf950; val=0x0040 => FAIL
thread 0x0000000000000000:   &tls_int_80: ptr=0xffff800100caf990; val=0x0080 => FAIL
thread 0x0000000000000000:  &tls_int_100: ptr=0xffff800100cafa10; val=0x0100 => FAIL
thread 0x0000000000000000:  &tls_int_200: ptr=0xffff800100cafb10; val=0x0200 => FAIL
thread 0x0000000000000000:  &tls_int_400: ptr=0xffff800100cafd10; val=0x0400 => FAIL
thread 0x0000000000000000:  &tls_int_800: ptr=0xffff800100cb0110; val=0x0800 => FAIL
thread 0x0000000000000000: &tls_int_1000: ptr=0xffff800100cb0910; val=0x1000 => FAIL
thread 0x0000000000000000: &tls_int_2000: ptr=0xffff800100cb1910; val=0x2000 => FAIL
thread 0x0000000000000002:   &tls_int_10: ptr=0xffff800101cb3910; val=0x0010 => OK
thread 0x0000000000000002:   &tls_int_20: ptr=0xffff800101cb3930; val=0x0020 => FAIL
thread 0x0000000000000002:   &tls_int_40: ptr=0xffff800101cb3950; val=0x0040 => FAIL
thread 0x0000000000000002:   &tls_int_80: ptr=0xffff800101cb3990; val=0x0080 => FAIL
thread 0x0000000000000002:  &tls_int_100: ptr=0xffff800101cb3a10; val=0x0100 => FAIL
thread 0x0000000000000002:  &tls_int_200: ptr=0xffff800101cb3b10; val=0x0200 => FAIL
thread 0x0000000000000002:  &tls_int_400: ptr=0xffff800101cb3d10; val=0x0400 => FAIL
thread 0x0000000000000002:  &tls_int_800: ptr=0xffff800101cb4110; val=0x0800 => FAIL
thread 0x0000000000000002: &tls_int_1000: ptr=0xffff800101cb4910; val=0x1000 => FAIL
thread 0x0000000000000002: &tls_int_2000: ptr=0xffff800101cb5910; val=0x2000 => FAIL
thread 0x0000000000000001:   &tls_int_10: ptr=0xffff8001014b1910; val=0x0010 => OK
thread 0x0000000000000001:   &tls_int_20: ptr=0xffff8001014b1930; val=0x0020 => FAIL
thread 0x0000000000000001:   &tls_int_40: ptr=0xffff8001014b1950; val=0x0040 => FAIL
thread 0x0000000000000001:   &tls_int_80: ptr=0xffff8001014b1990; val=0x0080 => FAIL
thread 0x0000000000000001:  &tls_int_100: ptr=0xffff8001014b1a10; val=0x0100 => FAIL
thread 0x0000000000000001:  &tls_int_200: ptr=0xffff8001014b1b10; val=0x0200 => FAIL
thread 0x0000000000000001:  &tls_int_400: ptr=0xffff8001014b1d10; val=0x0400 => FAIL
thread 0x0000000000000001:  &tls_int_800: ptr=0xffff8001014b2110; val=0x0800 => FAIL
thread 0x0000000000000001: &tls_int_1000: ptr=0xffff8001014b2910; val=0x1000 => FAIL
thread 0x0000000000000001: &tls_int_2000: ptr=0xffff8001014b3910; val=0x2000 => FAIL
Comment 2 Sourceware Commits 2019-02-06 08:27:00 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, master has been updated
       via  94eeeec04be8efae1a85d7c224e0026ee18ee6e7 (commit)
       via  bc79db3fd487daea36e7c130f943cfb9826a41b4 (commit)
      from  f1ac7455831546e5dca0ed98fe8af2686fae7ce6 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=94eeeec04be8efae1a85d7c224e0026ee18ee6e7

commit 94eeeec04be8efae1a85d7c224e0026ee18ee6e7
Author: Stefan Liebler <stli@linux.ibm.com>
Date:   Wed Feb 6 09:10:31 2019 +0100

    S390: Fix introduction of __wmemcmp and weak wmemcmp symbols.
    
    The recent commit 65f7767a914144ae303f7b9ae81865061793dcb9
    has introduced __wmemcmp and the weak alias wmemcmp.
    This patch also introduces those symbols if glibc is build
    with CFLAGS="-march=z13" where the ifunc is omitted.
    
    ChangeLog:
    
    	* sysdeps/s390/wmemcmp-vx.S: Add strong alias to
    	__wmemcmp and weak alias to wmemcmp.

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=bc79db3fd487daea36e7c130f943cfb9826a41b4

commit bc79db3fd487daea36e7c130f943cfb9826a41b4
Author: Stefan Liebler <stli@linux.ibm.com>
Date:   Wed Feb 6 09:06:34 2019 +0100

    Fix alignment of TLS variables for tls variant TLS_TCB_AT_TP [BZ #23403]
    
    The alignment of TLS variables is wrong if accessed from within a thread
    for architectures with tls variant TLS_TCB_AT_TP.
    For the main thread the static tls data is properly aligned.
    For other threads the alignment depends on the alignment of the thread
    pointer as the static tls data is located relative to this pointer.
    
    This patch adds this alignment for TLS_TCB_AT_TP variants in the same way
    as it is already done for TLS_DTV_AT_TP. The thread pointer is also already
    properly aligned if the user provides its own stack for the new thread.
    
    This patch extends the testcase nptl/tst-tls1.c in order to check the
    alignment of the tls variables and it adds a pthread_create invocation
    with a user provided stack.
    The test itself is migrated from test-skeleton.c to test-driver.c
    and the missing support functions xpthread_attr_setstack and xposix_memalign
    are added.
    
    ChangeLog:
    
    	[BZ #23403]
    	* nptl/allocatestack.c (allocate_stack): Align pointer pd for
    	TLS_TCB_AT_TP tls variant.
    	* nptl/tst-tls1.c: Migrate to support/test-driver.c.
    	Add alignment checks.
    	* support/Makefile (libsupport-routines): Add xposix_memalign and
    	xpthread_setstack.
    	* support/support.h: Add xposix_memalign.
    	* support/xthread.h: Add xpthread_attr_setstack.
    	* support/xposix_memalign.c: New File.
    	* support/xpthread_attr_setstack.c: Likewise.

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog                                          |   19 ++++
 nptl/allocatestack.c                               |    4 +-
 nptl/tst-tls1.c                                    |   90 +++++++++++---------
 support/Makefile                                   |    2 +
 support/support.h                                  |    1 +
 support/{xcalloc.c => xposix_memalign.c}           |   21 +++--
 ...d_rwlock_destroy.c => xpthread_attr_setstack.c} |    8 +-
 support/xthread.h                                  |    2 +
 sysdeps/s390/wmemcmp-vx.S                          |    3 +-
 9 files changed, 93 insertions(+), 57 deletions(-)
 copy support/{xcalloc.c => xposix_memalign.c} (72%)
 copy support/{xpthread_rwlock_destroy.c => xpthread_attr_setstack.c} (77%)
Comment 3 Stefan Liebler 2019-02-06 08:31:51 UTC
Closed with commit in comment 2
Comment 4 Sourceware Commits 2019-11-07 22:13:33 UTC
The release/2.28/master branch has been updated by DJ Delorie <dj@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=cedb3e47c68d319607736a820da2d5b3b8ddff6f

commit cedb3e47c68d319607736a820da2d5b3b8ddff6f
Author: Stefan Liebler <stli@linux.ibm.com>
Date:   Wed Feb 6 09:06:34 2019 +0100

    Fix alignment of TLS variables for tls variant TLS_TCB_AT_TP [BZ #23403]
    
    The alignment of TLS variables is wrong if accessed from within a thread
    for architectures with tls variant TLS_TCB_AT_TP.
    For the main thread the static tls data is properly aligned.
    For other threads the alignment depends on the alignment of the thread
    pointer as the static tls data is located relative to this pointer.
    
    This patch adds this alignment for TLS_TCB_AT_TP variants in the same way
    as it is already done for TLS_DTV_AT_TP. The thread pointer is also already
    properly aligned if the user provides its own stack for the new thread.
    
    This patch extends the testcase nptl/tst-tls1.c in order to check the
    alignment of the tls variables and it adds a pthread_create invocation
    with a user provided stack.
    The test itself is migrated from test-skeleton.c to test-driver.c
    and the missing support functions xpthread_attr_setstack and xposix_memalign
    are added.
    
    ChangeLog:
    
    	[BZ #23403]
    	* nptl/allocatestack.c (allocate_stack): Align pointer pd for
    	TLS_TCB_AT_TP tls variant.
    	* nptl/tst-tls1.c: Migrate to support/test-driver.c.
    	Add alignment checks.
    	* support/Makefile (libsupport-routines): Add xposix_memalign and
    	xpthread_setstack.
    	* support/support.h: Add xposix_memalign.
    	* support/xthread.h: Add xpthread_attr_setstack.
    	* support/xposix_memalign.c: New File.
    	* support/xpthread_attr_setstack.c: Likewise.
    
    (cherry picked from commit bc79db3fd487daea36e7c130f943cfb9826a41b4)
Comment 5 Sourceware Commits 2019-11-07 22:13:44 UTC
The release/2.29/master branch has been updated by DJ Delorie <dj@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=8646009efd1df151cc796055c7f6306495835577

commit 8646009efd1df151cc796055c7f6306495835577
Author: Stefan Liebler <stli@linux.ibm.com>
Date:   Wed Feb 6 09:06:34 2019 +0100

    Fix alignment of TLS variables for tls variant TLS_TCB_AT_TP [BZ #23403]
    
    The alignment of TLS variables is wrong if accessed from within a thread
    for architectures with tls variant TLS_TCB_AT_TP.
    For the main thread the static tls data is properly aligned.
    For other threads the alignment depends on the alignment of the thread
    pointer as the static tls data is located relative to this pointer.
    
    This patch adds this alignment for TLS_TCB_AT_TP variants in the same way
    as it is already done for TLS_DTV_AT_TP. The thread pointer is also already
    properly aligned if the user provides its own stack for the new thread.
    
    This patch extends the testcase nptl/tst-tls1.c in order to check the
    alignment of the tls variables and it adds a pthread_create invocation
    with a user provided stack.
    The test itself is migrated from test-skeleton.c to test-driver.c
    and the missing support functions xpthread_attr_setstack and xposix_memalign
    are added.
    
    ChangeLog:
    
    	[BZ #23403]
    	* nptl/allocatestack.c (allocate_stack): Align pointer pd for
    	TLS_TCB_AT_TP tls variant.
    	* nptl/tst-tls1.c: Migrate to support/test-driver.c.
    	Add alignment checks.
    	* support/Makefile (libsupport-routines): Add xposix_memalign and
    	xpthread_setstack.
    	* support/support.h: Add xposix_memalign.
    	* support/xthread.h: Add xpthread_attr_setstack.
    	* support/xposix_memalign.c: New File.
    	* support/xpthread_attr_setstack.c: Likewise.
    
    (cherry picked from commit bc79db3fd487daea36e7c130f943cfb9826a41b4)