From patchwork Tue Nov 18 19:57:36 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexandre Oliva X-Patchwork-Id: 412151 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id A809F14010B for ; Wed, 19 Nov 2014 06:58:10 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id:mime-version :content-type; q=dns; s=default; b=c9CO4aMv9hodmVkbc/FaJBmztfg5f 1F9hthvtp/56g0bEZBUBAq1iIthyTIlmxoa+zeUyYbYiWYn7eTVYTYb6Q5KF7AhL dm/h9u7fD/d/tpk8zVN1tGjZKR2VMPY5KSpMrTYyHrvzE/2G1DNcCZrh0Vr+LBjx FukW+oQpgpkNus= 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:from:to:subject:date:message-id:mime-version :content-type; s=default; bh=AH2QU/Br2OgWsYjHHVF7TQG1PLk=; b=njr 1gK86MbjKDKmyc3kFDGVMWofnP/2kwi2XpUy9Jn43tBloCK/18bxY4GDMIld2RAV ZPPhEiQt9vBxO1fPmQUxBvFRm1KTHjoKtB+8mtwA6jlWTeAL4xqpdMFg/8BVILcC Tef7A1SzECQd+YWgdfoD/giuoCXaEpSmjO+MyHdo= Received: (qmail 12950 invoked by alias); 18 Nov 2014 19:58:04 -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 12932 invoked by uid 89); 18 Nov 2014 19:58:03 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL, BAYES_50, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com From: Alexandre Oliva To: libc-alpha@sourceware.org Subject: [BZ#17090/17620/17621]: fix DTV race, assert, and DTV_SURPLUS Static TLS limit Date: Tue, 18 Nov 2014 17:57:36 -0200 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 This patch fixes 3 bugs, 2 of which I've just filed to have all of the issues covered in the bug database. The original goal was to fix #17620, the problem with Static TLS overflowing the DTV_SURPLUS range of the DTV, causing unnecessary dlopen failures, more visible on AArch64 due to TLS Descriptor use by default. In the process of fixing it, I noticed the race condition reported in #17621. The fix for the latter is to move Static TLS DTV entry initialization to DTV updates by the thread itself, since the DTV entry is never used except in GD/LD access models. This enables us to get rid of the limit imposed in dlopen, that arises precisely from the former need to ensure that all threads' DTVs are big enough. If we leave it for each thread to update its own DTV, the check becomes unnecessary and it can be safely dropped. The assert in bug 17090 was pretty obvious, and since I found it while looking for dupes of the bugs I filed, and I was touching neighbor code, I figured I'd just bring it into this patch. Regression-tested on x86_64-linux-gnu. Ok to install? for ChangeLog [BZ #17090] [BZ #17620] [BZ #17621] * NEWS: Update. * elf/dl-tls.c (_dl_update_slotinfo): Clean up outdated DTV entries with Static TLS too. (tls_get_addr_tail): Move Static TLS DTV entry set up from... (_dl_allocate_tls_init): ... here (fix modid assertion), ... * elf/dl-reloc.c (_dl_nothread_init_static_tls): ... here... * nptl/allocatestack.c (init_one_static_tls): ... and here... * elf/dlopen.c (dl_open_worker): Drop l_tls_modid upper bound for Static TLS. --- NEWS | 8 ++++---- elf/dl-open.c | 12 +---------- elf/dl-reloc.c | 6 ------ elf/dl-tls.c | 53 ++++++++++++++++++++++++-------------------------- nptl/allocatestack.c | 9 +++----- 5 files changed, 33 insertions(+), 55 deletions(-) diff --git a/NEWS b/NEWS index b152488..4f208a2 100644 --- a/NEWS +++ b/NEWS @@ -9,10 +9,10 @@ Version 2.21 * The following bugs are resolved with this release: - 6652, 12926, 14132, 14138, 14171, 15215, 15884, 17266, 17344, 17363, - 17370, 17371, 17411, 17460, 17475, 17485, 17501, 17506, 17508, 17522, - 17555, 17570, 17571, 17572, 17573, 17574, 17582, 17583, 17584, 17585, - 17589, 17594, 17616. + 6652, 12926, 14132, 14138, 14171, 15215, 15884, 17090, 17266, 17344, + 17363, 17370, 17371, 17411, 17460, 17475, 17485, 17501, 17506, 17508, + 17522, 17555, 17570, 17571, 17572, 17573, 17574, 17582, 17583, 17584, + 17585, 17589, 17594, 17616, 17620, 17621. * The minimum GCC version that can be used to build this version of the GNU C Library is GCC 4.6. Older GCC versions, and non-GNU compilers, can diff --git a/elf/dl-open.c b/elf/dl-open.c index 7cc4cc1..9d6006b 100644 --- a/elf/dl-open.c +++ b/elf/dl-open.c @@ -531,17 +531,7 @@ TLS generation counter wrapped! Please report this.")); && imap->l_tls_blocksize > 0) { /* For static TLS we have to allocate the memory here and - now. This includes allocating memory in the DTV. But we - cannot change any DTV other than our own. So, if we - cannot guarantee that there is room in the DTV we don't - even try it and fail the load. - - XXX We could track the minimum DTV slots allocated in - all threads. */ - if (! RTLD_SINGLE_THREAD_P && imap->l_tls_modid > DTV_SURPLUS) - _dl_signal_error (0, "dlopen", NULL, N_("\ -cannot load any more object with static TLS")); - + now, but we can delay updating the DTV. */ imap->l_need_tls_init = 0; #ifdef SHARED /* Update the slot information data for at least the diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c index 97a7119..1d66f79 100644 --- a/elf/dl-reloc.c +++ b/elf/dl-reloc.c @@ -136,12 +136,6 @@ _dl_nothread_init_static_tls (struct link_map *map) # error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined" #endif - /* Fill in the DTV slot so that a later LD/GD access will find it. */ - dtv_t *dtv = THREAD_DTV (); - assert (map->l_tls_modid <= dtv[-1].counter); - dtv[map->l_tls_modid].pointer.val = dest; - dtv[map->l_tls_modid].pointer.is_static = true; - /* Initialize the memory. */ memset (__mempcpy (dest, map->l_tls_initimage, map->l_tls_initimage_size), '\0', map->l_tls_blocksize - map->l_tls_initimage_size); diff --git a/elf/dl-tls.c b/elf/dl-tls.c index 5204fda..2408384 100644 --- a/elf/dl-tls.c +++ b/elf/dl-tls.c @@ -437,17 +437,14 @@ _dl_allocate_tls_init (void *result) assert (listp->slotinfo[cnt].gen <= GL(dl_tls_generation)); maxgen = MAX (maxgen, listp->slotinfo[cnt].gen); + dtv[map->l_tls_modid].pointer.val = TLS_DTV_UNALLOCATED; + dtv[map->l_tls_modid].pointer.is_static = false; + if (map->l_tls_offset == NO_TLS_OFFSET || map->l_tls_offset == FORCED_DYNAMIC_TLS_OFFSET) - { - /* For dynamically loaded modules we simply store - the value indicating deferred allocation. */ - dtv[map->l_tls_modid].pointer.val = TLS_DTV_UNALLOCATED; - dtv[map->l_tls_modid].pointer.is_static = false; - continue; - } + continue; - assert (map->l_tls_modid == cnt); + assert (map->l_tls_modid == total + cnt); assert (map->l_tls_blocksize >= map->l_tls_initimage_size); #if TLS_TCB_AT_TP assert ((size_t) map->l_tls_offset >= map->l_tls_blocksize); @@ -459,8 +456,6 @@ _dl_allocate_tls_init (void *result) #endif /* Copy the initialization image and clear the BSS part. */ - dtv[map->l_tls_modid].pointer.val = dest; - dtv[map->l_tls_modid].pointer.is_static = true; memset (__mempcpy (dest, map->l_tls_initimage, map->l_tls_initimage_size), '\0', map->l_tls_blocksize - map->l_tls_initimage_size); @@ -632,10 +627,9 @@ _dl_update_slotinfo (unsigned long int req_modid) might still be allocated. */ if (! dtv[total + cnt].pointer.is_static && dtv[total + cnt].pointer.val != TLS_DTV_UNALLOCATED) - { - free (dtv[total + cnt].pointer.val); - dtv[total + cnt].pointer.val = TLS_DTV_UNALLOCATED; - } + free (dtv[total + cnt].pointer.val); + dtv[total + cnt].pointer.val = TLS_DTV_UNALLOCATED; + dtv[total + cnt].pointer.is_static = false; continue; } @@ -698,10 +692,8 @@ _dl_update_slotinfo (unsigned long int req_modid) memalign and not malloc. */ free (dtv[modid].pointer.val); - /* This module is loaded dynamically- We defer memory - allocation. */ - dtv[modid].pointer.is_static = false; dtv[modid].pointer.val = TLS_DTV_UNALLOCATED; + dtv[modid].pointer.is_static = false; if (modid == req_modid) the_map = map; @@ -739,7 +731,6 @@ tls_get_addr_tail (GET_ADDR_ARGS, dtv_t *dtv, struct link_map *the_map) the_map = listp->slotinfo[idx].map; } - again: /* Make sure that, if a dlopen running in parallel forces the variable into static storage, we'll wait until the address in the static TLS block is set up, and use that. If we're undecided @@ -753,22 +744,28 @@ tls_get_addr_tail (GET_ADDR_ARGS, dtv_t *dtv, struct link_map *the_map) the_map->l_tls_offset = FORCED_DYNAMIC_TLS_OFFSET; __rtld_lock_unlock_recursive (GL(dl_load_lock)); } - else + else if (__builtin_expect (the_map->l_tls_offset + != FORCED_DYNAMIC_TLS_OFFSET, 1)) { +#if TLS_TCB_AT_TP + void *p = (char *) THREAD_SELF - the_map->l_tls_offset; +#elif TLS_DTV_AT_TP + void *p = (char *) THREAD_SELF + the_map->l_tls_offset + TLS_PRE_TCB_SIZE; +#else +# error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined" +#endif __rtld_lock_unlock_recursive (GL(dl_load_lock)); - if (__builtin_expect (the_map->l_tls_offset - != FORCED_DYNAMIC_TLS_OFFSET, 1)) - { - void *p = dtv[GET_ADDR_MODULE].pointer.val; - if (__glibc_unlikely (p == TLS_DTV_UNALLOCATED)) - goto again; - return (char *) p + GET_ADDR_OFFSET; - } + dtv[GET_ADDR_MODULE].pointer.is_static = true; + dtv[GET_ADDR_MODULE].pointer.val = p; + + return (char *) p + GET_ADDR_OFFSET; } + else + __rtld_lock_unlock_recursive (GL(dl_load_lock)); } void *p = dtv[GET_ADDR_MODULE].pointer.val = allocate_and_init (the_map); - dtv[GET_ADDR_MODULE].pointer.is_static = false; + assert (!dtv[GET_ADDR_MODULE].pointer.is_static); return (char *) p + GET_ADDR_OFFSET; } diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index 8cf0274..8b053c1 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -1190,7 +1190,6 @@ __nptl_setxid (struct xid_command *cmdp) static inline void __attribute__((always_inline)) init_one_static_tls (struct pthread *curp, struct link_map *map) { - dtv_t *dtv = GET_DTV (TLS_TPADJ (curp)); # if TLS_TCB_AT_TP void *dest = (char *) curp - map->l_tls_offset; # elif TLS_DTV_AT_TP @@ -1199,11 +1198,9 @@ init_one_static_tls (struct pthread *curp, struct link_map *map) # error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined" # endif - /* Fill in the DTV slot so that a later LD/GD access will find it. */ - dtv[map->l_tls_modid].pointer.val = dest; - dtv[map->l_tls_modid].pointer.is_static = true; - - /* Initialize the memory. */ + /* We cannot delay the initialization of the Static TLS area, since + it can be accessed with LE or IE, but since the DTV is only used + by GD and LD, we can delay its update to avoid a race. */ memset (__mempcpy (dest, map->l_tls_initimage, map->l_tls_initimage_size), '\0', map->l_tls_blocksize - map->l_tls_initimage_size); }