From patchwork Fri Dec 2 12:54:42 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 86292 Delivered-To: patch@linaro.org Received: by 10.140.20.101 with SMTP id 92csp240106qgi; Fri, 2 Dec 2016 04:55:08 -0800 (PST) X-Received: by 10.99.251.5 with SMTP id o5mr78958218pgh.152.1480683307947; Fri, 02 Dec 2016 04:55:07 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id m63si5063242pld.73.2016.12.02.04.55.07 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 02 Dec 2016 04:55:07 -0800 (PST) Received-SPF: pass (google.com: domain of libc-alpha-return-75418-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@sourceware.org; spf=pass (google.com: domain of libc-alpha-return-75418-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=libc-alpha-return-75418-patch=linaro.org@sourceware.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:to:references:cc:from:message-id:date :mime-version:in-reply-to:content-type; q=dns; s=default; b=PHeK SUboPr/yvVddZbS/qN7trN9cWis1sWEH/WsDZuBWErOy8mdA/KNlyUHkJC83aqvO fPQA/KP9W26gVRJXj76OpBSEu/eiSveguo70FwM8NNcOWDJ3crUH9iUtrF1sThR1 IaFofATVA8liAtRPmsq5RuTZM+131MbIm1Ayi2g= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:to:references:cc:from:message-id:date :mime-version:in-reply-to:content-type; s=default; bh=2Rke2KAhAI YQzZaW6Ty0tfbXdOg=; b=aYBrLMyskub0HWuNiOEYjZ+NfAElqMIFfqvl0jwWN7 saPiVYBcR2k7bqAHmkHQjuD2RtvsWTxFkQ0KQkqLmu2uA2Mxaw1A+kNPWtSxIvoD CmZhpy3HI0oBmlVonPUsEsMcZBWSN/oipXzlK9yuIoWm3LfETnApx3zIlw/7T8Fr M= Received: (qmail 92479 invoked by alias); 2 Dec 2016 12:54:57 -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 92465 invoked by uid 89); 2 Dec 2016 12:54:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.8 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Enterprise, mrs, n X-HELO: mx1.redhat.com Subject: Re: [PATCH] aarch64: Use explicit offsets in _dl_tlsdesc_dynamic To: Szabolcs Nagy , libc-alpha@sourceware.org References: <20161201144952.1A8AB439942E0@oldenburg.str.redhat.com> <58403C89.1010702@arm.com> <1eb123ff-052d-5382-68cb-80b222129608@redhat.com> Cc: nd@arm.com From: Florian Weimer Message-ID: <723bf9cf-4ad5-ab19-56ab-7807dca75a0d@redhat.com> Date: Fri, 2 Dec 2016 13:54:42 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <1eb123ff-052d-5382-68cb-80b222129608@redhat.com> On 12/02/2016 12:52 PM, Florian Weimer wrote: > I have a test case which triggers a crash on aarch64, but I'm not yet > sure if it actually covers this bug. It fails even with the fix above. > valground still shows an OOB write in TLS data: > > ==16070== Invalid write of size 8 > ==16070== at 0x4897C9C: init_one_static_tls (allocatestack.c:1196) > ==16070== by 0x4897C9C: __pthread_init_static_tls (allocatestack.c:1213) > ==16070== by 0x18A1CB: _dl_try_allocate_static_tls (dl-reloc.c:106) > ==16070== by 0x19383F: _dl_tlsdesc_resolve_rela_fixup (tlsdesc.c:104) > ==16070== by 0x193B03: _dl_tlsdesc_resolve_rela (dl-tlsdesc.S:288) > ==16070== by 0x193B03: _dl_tlsdesc_resolve_rela (dl-tlsdesc.S:288) > ==16070== by 0x193B03: _dl_tlsdesc_resolve_rela (dl-tlsdesc.S:288) > ==16070== by 0x193B03: _dl_tlsdesc_resolve_rela (dl-tlsdesc.S:288) > ==16070== by 0x193B03: _dl_tlsdesc_resolve_rela (dl-tlsdesc.S:288) > ==16070== by 0x193B03: _dl_tlsdesc_resolve_rela (dl-tlsdesc.S:288) > ==16070== by 0x193B03: _dl_tlsdesc_resolve_rela (dl-tlsdesc.S:288) > ==16070== by 0x193B03: _dl_tlsdesc_resolve_rela (dl-tlsdesc.S:288) > ==16070== by 0x193B03: _dl_tlsdesc_resolve_rela (dl-tlsdesc.S:288) > ==16070== Address 0x5240158 is 8 bytes after a block of size 272 alloc'd > ==16070== at 0x4835D4C: calloc (vg_replace_malloc.c:711) > ==16070== by 0x18F9CF: allocate_dtv (dl-tls.c:322) > ==16070== by 0x190117: _dl_allocate_tls (dl-tls.c:570) > ==16070== by 0x4898D4B: allocate_stack (allocatestack.c:578) > ==16070== by 0x4898D4B: pthread_create@@GLIBC_2.17 > (pthread_create.c:539) > ==16070== by 0x401CCB: xpthread_create (test-skeleton.c:691) > ==16070== by 0x401CCB: do_test (tst-tls-manydynamic.c:97) > ==16070== by 0x4018CF: main (test-skeleton.c:539) > > I need to check if this happens before the ILP32 enablement patch, too. It's something else. I do not know yet if this is a bug caused by the Red Hat Enterprise Linux 7 system compiler. The system glibc does not have this issue. I will commit both original bug fix (because it is an obvious improvement) and this new test soon, and separately, unless someone objects. To be on the safe side, I have eliminated cancellation and any potential dlopen/pthread_create race from the test. Thanks, Florian aarch64: Use explicit offsets in _dl_tlsdesc_dynamic Commit 389d1f1b232b3d6b9d73ee2c50e543ace6675621 (“Partial ILP32 support for aarch64”) broke dynamic TLS support because a load offset changed: 0000000000000030 <_dl_tlsdesc_dynamic>: 30: a9bc7bfd stp x29, x30, [sp,#-64]! 34: 910003fd mov x29, sp 38: a9020be1 stp x1, x2, [sp,#32] 3c: a90313e3 stp x3, x4, [sp,#48] 40: d53bd044 mrs x4, tpidr_el0 44: c8dffc1f ldar xzr, [x0] 48: f9400401 ldr x1, [x0,#8] 4c: f9400080 ldr x0, [x4] 50: f9400823 ldr x3, [x1,#16] 54: f9400002 ldr x2, [x0] 58: eb02007f cmp x3, x2 5c: 540001a8 b.hi 90 <_dl_tlsdesc_dynamic+0x60> 60: f9400022 ldr x2, [x1] 64: 8b021000 add x0, x0, x2, lsl #4 68: f9400000 ldr x0, [x0] 6c: b100041f cmn x0, #0x1 70: 54000100 b.eq 90 <_dl_tlsdesc_dynamic+0x60> - 74: f9400421 ldr x1, [x1,#8] + 74: f9400821 ldr x1, [x1,#16] 78: 8b010000 add x0, x0, x1 … This commit introduces explicit struct offsets, generated from the C headers, fixing the regression. 2016-12-02 Florian Weimer * elf/Makefile [build-shared] (tests): Add tst-latepthread. (one-hundred, tst-tls-many-dynamic-modules): Define. (modules-names): Add $(tst-tls-many-dynamic-modules). (tst-tls-manydynamic%mod.os): Build with special preprocessor macros. (tst-tls-manydynamic): Link against libdl, libpthread. (tst-tls-manydynamic.out): The test needs the test modules at run time. * elf/tst-tls-manydynamic.c: New file. * elf/tst-tls-manydynamic.h: Likewise. * elf/tst-tls-manydynamicmod.c: Likewise. 2016-12-01 Florian Weimer * sysdeps/aarch64/tlsdesc.sym (TCBHEAD_DTV, DTV_COUNTER) (TLS_DTV_UNALLOCATED): Add. * sysdeps/aarch64/dl-tlsdesc.S (_dl_tlsdesc_dynamic): Use explicit offsets. diff --git a/elf/Makefile b/elf/Makefile index 33b003b..0d6f691 100644 --- a/elf/Makefile +++ b/elf/Makefile @@ -152,7 +152,7 @@ tests += loadtest restest1 preloadtest loadfail multiload origtest resolvfail \ tst-initorder tst-initorder2 tst-relsort1 tst-null-argv \ tst-ptrguard1 tst-tlsalign tst-tlsalign-extern tst-nodelete-opened \ tst-nodelete2 tst-audit11 tst-audit12 tst-dlsym-error tst-noload \ - tst-latepthread + tst-latepthread tst-tls-manydynamic # reldep9 ifeq ($(build-hardcoded-path-in-tests),yes) tests += tst-dlopen-aout @@ -173,6 +173,10 @@ tlsmod17a-suffixes = 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 tlsmod18a-suffixes = 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 tlsmod17a-modules = $(addprefix tst-tlsmod17a, $(tlsmod17a-suffixes)) tlsmod18a-modules = $(addprefix tst-tlsmod18a, $(tlsmod17a-suffixes)) +one-hundred = $(foreach x,0 1 2 3 4 5 6 7 8 9, \ + 0$x 1$x 2$x 3$x 4$x 5$x 6$x 7$x 8$x 9$x) +tst-tls-many-dynamic-modules := \ + $(foreach n,$(one-hundred),tst-tls-manydynamic$(n)mod) extra-test-objs += $(tlsmod17a-modules:=.os) $(tlsmod18a-modules:=.os) \ tst-tlsalign-vars.o test-extras += tst-tlsmod17a tst-tlsmod18a tst-tlsalign-vars @@ -227,7 +231,7 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \ tst-tlsalign-lib tst-nodelete-opened-lib tst-nodelete2mod \ tst-audit11mod1 tst-audit11mod2 tst-auditmod11 \ tst-audit12mod1 tst-audit12mod2 tst-audit12mod3 tst-auditmod12 \ - tst-latepthreadmod + tst-latepthreadmod $(tst-tls-many-dynamic-modules) ifeq (yes,$(have-mtls-dialect-gnu2)) tests += tst-gnu2-tls1 modules-names += tst-gnu2-tls1mod @@ -1275,6 +1279,15 @@ $(objpfx)tst-latepthreadmod.so: $(shared-thread-library) $(objpfx)tst-latepthread: $(libdl) $(objpfx)tst-latepthread.out: $(objpfx)tst-latepthreadmod.so +# The test modules are parameterized by preprocessor macros. +$(patsubst %,$(objpfx)%.os,$(tst-tls-many-dynamic-modules)): \ + $(objpfx)tst-tls-manydynamic%mod.os : tst-tls-manydynamicmod.c + $(compile-command.c) \ + -DNAME=tls_global_$* -DSETTER=set_value_$* -DGETTER=get_value_$* +$(objpfx)tst-tls-manydynamic: $(libdl) $(shared-thread-library) +$(objpfx)tst-tls-manydynamic.out: \ + $(patsubst %,$(objpfx)%.so,$(tst-tls-many-dynamic-modules)) + tst-prelink-ENV = LD_TRACE_PRELINKING=1 $(objpfx)tst-prelink-conflict.out: $(objpfx)tst-prelink.out diff --git a/elf/tst-tls-manydynamic.c b/elf/tst-tls-manydynamic.c new file mode 100644 index 0000000..29bf139 --- /dev/null +++ b/elf/tst-tls-manydynamic.c @@ -0,0 +1,150 @@ +/* Test with many dynamic TLS variables. + Copyright (C) 2016 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 + . */ + +/* This test intends to exercise dynamic TLS variable allocation. It + achieves this by combining dlopen (to avoid static TLS allocation + after static TLS resizing), many DSOs with a large variable (to + exceed the static TLS reserve), and an already-running thread (to + force full dynamic TLS initialization). */ + +#include "tst-tls-manydynamic.h" + +#include +#include +#include +#include +#include + +static int do_test (void); +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" + +void *handles[COUNT]; +set_value_func set_value_funcs[COUNT]; +get_value_func get_value_funcs[COUNT]; + +static void +init_functions (void) +{ + for (int i = 0; i < COUNT; ++i) + { + /* Open the module. */ + { + char soname[100]; + snprintf (soname, sizeof (soname), "tst-tls-manydynamic%02dmod.so", i); + handles[i] = dlopen (soname, RTLD_LAZY); + if (handles[i] == NULL) + { + printf ("error: dlopen failed: %s\n", dlerror ()); + exit (1); + } + } + + /* Obtain the setter function. */ + { + char fname[100]; + snprintf (fname, sizeof (fname), "set_value_%02d", i); + void *func = dlsym (handles[i], fname); + if (func == NULL) + { + printf ("error: dlsym: %s\n", dlerror ()); + exit (1); + } + set_value_funcs[i] = func; + } + + /* Obtain the getter function. */ + { + char fname[100]; + snprintf (fname, sizeof (fname), "get_value_%02d", i); + void *func = dlsym (handles[i], fname); + if (func == NULL) + { + printf ("error: dlsym: %s\n", dlerror ()); + exit (1); + } + get_value_funcs[i] = func; + } + } +} + +static pthread_barrier_t barrier; + +/* Running thread which forces real TLS initialization. */ +static void * +blocked_thread_func (void *closure) +{ + xpthread_barrier_wait (&barrier); + + /* TLS test runs here in the main thread. */ + + xpthread_barrier_wait (&barrier); + return NULL; +} + +static int +do_test (void) +{ + { + int ret = pthread_barrier_init (&barrier, NULL, 2); + if (ret != 0) + { + errno = ret; + printf ("error: pthread_barrier_init: %m\n"); + exit (1); + } + } + + pthread_t blocked_thread = xpthread_create (NULL, blocked_thread_func, NULL); + xpthread_barrier_wait (&barrier); + + init_functions (); + + struct value values[COUNT]; + /* Initialze the TLS variables. */ + for (int i = 0; i < COUNT; ++i) + { + for (int j = 0; j < PER_VALUE_COUNT; ++j) + values[i].num[j] = rand (); + set_value_funcs[i] (&values[i]); + } + + /* Read back their values to check that they do not overlap. */ + for (int i = 0; i < COUNT; ++i) + { + struct value actual; + get_value_funcs[i] (&actual); + + for (int j = 0; j < PER_VALUE_COUNT; ++j) + if (actual.num[j] != values[i].num[j]) + { + printf ("error: mismatch at variable %d/%d: %d != %d\n", + i, j, actual.num[j], values[i].num[j]); + exit (1); + } + } + + xpthread_barrier_wait (&barrier); + xpthread_join (blocked_thread); + + /* Close the modules. */ + for (int i = 0; i < COUNT; ++i) + dlclose (handles[i]); + + return 0; +} diff --git a/elf/tst-tls-manydynamic.h b/elf/tst-tls-manydynamic.h new file mode 100644 index 0000000..4756ece --- /dev/null +++ b/elf/tst-tls-manydynamic.h @@ -0,0 +1,44 @@ +/* Interfaces for test with many dynamic TLS variables. + Copyright (C) 2016 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 + . */ + +#ifndef TST_TLS_MANYDYNAMIC_H +#define TST_TLS_MANYDYNAMIC_H + +enum + { + /* This many TLS variables (and modules) are defined. */ + COUNT = 100, + + /* Number of elements in the TLS variable. */ + PER_VALUE_COUNT = 1, + }; + +/* The TLS variables are of this type. We use a larger type to ensure + that we can reach the static TLS limit with COUNT variables. */ +struct value +{ + int num[PER_VALUE_COUNT]; +}; + +/* Set the TLS variable defined in the module. */ +typedef void (*set_value_func) (const struct value *); + +/* Read the TLS variable defined in the module. */ +typedef void (*get_value_func) (struct value *); + +#endif /* TST_TLS_MANYDYNAMICMOD_H */ diff --git a/elf/tst-tls-manydynamicmod.c b/elf/tst-tls-manydynamicmod.c new file mode 100644 index 0000000..578f11b --- /dev/null +++ b/elf/tst-tls-manydynamicmod.c @@ -0,0 +1,36 @@ +/* Module for test with many dynamic TLS variables. + Copyright (C) 2016 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 + . */ + +/* This file is parameterized by macros NAME, SETTER, GETTER, which + are set form the Makefile. */ + +#include "tst-tls-manydynamic.h" + +__thread struct value NAME; + +void +SETTER (const struct value *value) +{ + NAME = *value; +} + +void +GETTER (struct value *value) +{ + *value = NAME; +} diff --git a/sysdeps/aarch64/dl-tlsdesc.S b/sysdeps/aarch64/dl-tlsdesc.S index 42fa943..9e557dd 100644 --- a/sysdeps/aarch64/dl-tlsdesc.S +++ b/sysdeps/aarch64/dl-tlsdesc.S @@ -154,7 +154,7 @@ _dl_tlsdesc_undefweak: _dl_tlsdesc_dynamic (struct tlsdesc *tdp) { struct tlsdesc_dynamic_arg *td = tdp->arg; - dtv_t *dtv = *(dtv_t **)((char *)__thread_pointer + DTV_OFFSET); + dtv_t *dtv = *(dtv_t **)((char *)__thread_pointer + TCBHEAD_DTV); if (__builtin_expect (td->gen_count <= dtv[0].counter && (dtv[td->tlsinfo.ti_module].pointer.val != TLS_DTV_UNALLOCATED), @@ -193,18 +193,18 @@ _dl_tlsdesc_dynamic: td->entry in _dl_tlsdesc_resolve_rela_fixup ensuring that the load from [x0,#PTR_SIZE] here happens after the initialization of td->arg. */ ldar PTR_REG (zr), [x0] - ldr PTR_REG (1), [x0,#PTR_SIZE] - ldr PTR_REG (0), [x4] - ldr PTR_REG (3), [x1,#(PTR_SIZE * 2)] - ldr PTR_REG (2), [x0] + ldr PTR_REG (1), [x0,#TLSDESC_ARG] + ldr PTR_REG (0), [x4,#TCBHEAD_DTV] + ldr PTR_REG (3), [x1,#TLSDESC_GEN_COUNT] + ldr PTR_REG (2), [x0,#DTV_COUNTER] cmp PTR_REG (3), PTR_REG (2) b.hi 2f - ldr PTR_REG (2), [x1] + ldr PTR_REG (2), [x1,#TLSDESC_MODID] add PTR_REG (0), PTR_REG (0), PTR_REG (2), lsl #(PTR_LOG_SIZE + 1) - ldr PTR_REG (0), [x0] - cmn x0, #0x1 + ldr PTR_REG (0), [x0] /* Load val member of DTV entry. */ + cmp x0, #TLS_DTV_UNALLOCATED b.eq 2f - ldr PTR_REG (1), [x1,#(PTR_SIZE * 2)] + ldr PTR_REG (1), [x1,#TLSDESC_MODOFF] add PTR_REG (0), PTR_REG (0), PTR_REG (1) sub PTR_REG (0), PTR_REG (0), PTR_REG (4) 1: diff --git a/sysdeps/aarch64/tlsdesc.sym b/sysdeps/aarch64/tlsdesc.sym index 63766af..a06a719 100644 --- a/sysdeps/aarch64/tlsdesc.sym +++ b/sysdeps/aarch64/tlsdesc.sym @@ -13,3 +13,6 @@ TLSDESC_ARG offsetof(struct tlsdesc, arg) TLSDESC_GEN_COUNT offsetof(struct tlsdesc_dynamic_arg, gen_count) TLSDESC_MODID offsetof(struct tlsdesc_dynamic_arg, tlsinfo.ti_module) TLSDESC_MODOFF offsetof(struct tlsdesc_dynamic_arg, tlsinfo.ti_offset) +TCBHEAD_DTV offsetof(tcbhead_t, dtv) +DTV_COUNTER offsetof(dtv_t, counter) +TLS_DTV_UNALLOCATED TLS_DTV_UNALLOCATED