From patchwork Thu Aug 22 16:55:32 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 34247 Received: (qmail 84262 invoked by alias); 22 Aug 2019 16:55:39 -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 84250 invoked by uid 89); 22 Aug 2019 16:55:39 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-18.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, SPF_HELO_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mx1.redhat.com From: Florian Weimer To: libc-alpha@sourceware.org Subject: [PATCH] elf: Assign TLS modid later during dlopen [BZ #24930] Date: Thu, 22 Aug 2019 18:55:32 +0200 Message-ID: <87lfvlw35n.fsf@oldenburg2.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) MIME-Version: 1.0 Commit a42faf59d6d9f82e5293a9ebcc26d9c9e562b12b ("Fix BZ #16634.") attempted to fix a TLS modid consistency issue by adding additional checks to the open_verify function. However, this is fragile because open_verify cannot reliably predict whether _dl_map_object_from_fd will later fail in the more complex cases (such as memory allocation failures). Therefore, this commit assigns the TLS modid as late as possible. At that point, the link map pointer will eventually be passed to _dl_close, which will undo the TLS modid assignment. 2019-08-22 Florian Weimer [BZ #24930] * elf/dl-load.c (_dl_map_object_from_fd): Only assign TLS modid if the link map will be returned to the caller. * elf/Makefile [$(have-fpie) && $(build-shared)] (tests, tests-pie): Add tst-dlopen-aout-pie. (tst-tst-dlopen-aout-no-pie): Set. (CFLAGS-tst-dlopen-aout-pie.c): Build with -fpie. (tst-dlopen-aout-pie): Link with -ldl -lpthread. * elf/tst-dlopen-aout-pie.c: New file. Reviewed-by: Gabriel F. T. Gomes diff --git a/elf/Makefile b/elf/Makefile index d470e41402..a62d620e20 100644 --- a/elf/Makefile +++ b/elf/Makefile @@ -308,8 +308,8 @@ test-xfail-tst-protected1b = yes endif ifeq (yesyes,$(have-fpie)$(build-shared)) modules-names += tst-piemod1 -tests += tst-pie1 tst-pie2 tst-dlopen-pie -tests-pie += tst-pie1 tst-pie2 +tests += tst-pie1 tst-pie2 tst-dlopen-pie tst-dlopen-aout-pie +tests-pie += tst-pie1 tst-pie2 tst-dlopen-aout-pie ifeq (yes,$(have-protected-data)) tests += vismain tests-pie += vismain @@ -1267,7 +1267,11 @@ tst-leaks1-static-ENV = MALLOC_TRACE=$(objpfx)tst-leaks1-static.mtrace $(objpfx)tst-addr1: $(libdl) $(objpfx)tst-thrlock: $(libdl) $(shared-thread-library) + +tst-tst-dlopen-aout-no-pie = yes $(objpfx)tst-dlopen-aout: $(libdl) $(shared-thread-library) +CFLAGS-tst-dlopen-aout-pie.c += $(pie-ccflag) +$(objpfx)tst-dlopen-aout-pie: $(libdl) $(shared-thread-library) $(objpfx)tst-dlopen-aout-container: $(libdl) $(shared-thread-library) LDFLAGS-tst-dlopen-aout-container += -Wl,-rpath,\$$ORIGIN diff --git a/elf/dl-load.c b/elf/dl-load.c index 7bf0ca563e..2cc842f641 100644 --- a/elf/dl-load.c +++ b/elf/dl-load.c @@ -1118,27 +1118,21 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd, offset. We will adjust it later. */ l->l_tls_initimage = (void *) ph->p_vaddr; - /* If not loading the initial set of shared libraries, - check whether we should permit loading a TLS segment. */ - if (__glibc_likely (l->l_type == lt_library) - /* If GL(dl_tls_dtv_slotinfo_list) == NULL, then rtld.c did - not set up TLS data structures, so don't use them now. */ - || __glibc_likely (GL(dl_tls_dtv_slotinfo_list) != NULL)) - { - /* Assign the next available module ID. */ - l->l_tls_modid = _dl_next_tls_modid (); - break; - } + /* l->l_tls_modid is assigned below, once there is no + possibility for failure. */ + if (l->l_type != lt_library + && GL(dl_tls_dtv_slotinfo_list) == NULL) + { #ifdef SHARED - /* We are loading the executable itself when the dynamic - linker was executed directly. The setup will happen - later. Otherwise, the TLS data structures are already - initialized, and we assigned a TLS modid above. */ - assert (l->l_prev == NULL || (mode & __RTLD_AUDIT) != 0); + /* We are loading the executable itself when the dynamic + linker was executed directly. The setup will happen + later. */ + assert (l->l_prev == NULL || (mode & __RTLD_AUDIT) != 0); #else - assert (false && "TLS not initialized in static application"); + assert (false && "TLS not initialized in static application"); #endif + } break; case PT_GNU_STACK: @@ -1379,6 +1373,18 @@ cannot enable executable stack as shared object requires"); add_name_to_object (l, ((const char *) D_PTR (l, l_info[DT_STRTAB]) + l->l_info[DT_SONAME]->d_un.d_val)); + /* _dl_close can only eventually undo the module ID assignment (via + remove_slotinfo) if this function returns a pointer to a link + map. Therefore, delay this step until all possibilities for + failure have been excluded. */ + if (l->l_tls_blocksize > 0 + && (__glibc_likely (l->l_type == lt_library) + /* If GL(dl_tls_dtv_slotinfo_list) == NULL, then rtld.c did + not set up TLS data structures, so don't use them now. */ + || __glibc_likely (GL(dl_tls_dtv_slotinfo_list) != NULL))) + /* Assign the next available module ID. */ + l->l_tls_modid = _dl_next_tls_modid (); + #ifdef DL_AFTER_LOAD DL_AFTER_LOAD (l); #endif @@ -1646,17 +1652,6 @@ open_verify (const char *name, int fd, errstring = N_("only ET_DYN and ET_EXEC can be loaded"); goto call_lose; } - else if (__glibc_unlikely (ehdr->e_type == ET_EXEC - && (mode & __RTLD_OPENEXEC) == 0)) - { - /* BZ #16634. It is an error to dlopen ET_EXEC (unless - __RTLD_OPENEXEC is explicitly set). We return error here - so that code in _dl_map_object_from_fd does not try to set - l_tls_modid for this module. */ - - errstring = N_("cannot dynamically load executable"); - goto call_lose; - } else if (__glibc_unlikely (ehdr->e_phentsize != sizeof (ElfW(Phdr)))) { errstring = N_("ELF file's phentsize not the expected size"); diff --git a/elf/tst-dlopen-aout-pie.c b/elf/tst-dlopen-aout-pie.c new file mode 100644 index 0000000000..8d2009c0f3 --- /dev/null +++ b/elf/tst-dlopen-aout-pie.c @@ -0,0 +1,19 @@ +/* Test case for BZ #16634 and BZ#24900. PIE version. + Copyright (C) 2014-2019 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 "tst-dlopen-aout.c"