From patchwork Sat Jul 28 01:50:24 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Carlos O'Donell X-Patchwork-Id: 28666 Received: (qmail 25841 invoked by alias); 28 Jul 2018 01:50:31 -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 25771 invoked by uid 89); 28 Jul 2018 01:50:30 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, KAM_SHORT, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy=!shared X-HELO: mail-qt0-f176.google.com Return-Path: To: GNU C Library From: Carlos O'Donell Subject: [PATCH] Fix DSTs with static applications (Bug 23462). Message-ID: Date: Fri, 27 Jul 2018 21:50:24 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 After commit d1d5471579eb0426671bf94f2d71e61dfb204c30 we no longer check to see if the link map passed to DL_DST_REQUIRED is NULL or not. This is a problem becuase in _dl_init_paths we do not set the link map to anything in the !SHARED case, and this is a bug. With the work done to setup link maps in both static and shared builds we can always rely on the link map. The fix is to always set the link map, and assert that it is not NULL. Setting the link map allows the use of _dl_main_map link map to be used and results in DSTs being correctly processed. We add one regression test tst-dst-static.c just to look for the crash, but we need to add a larger set of tests for DSTs, but as noted in the DST cleanup changes this needs the test-in-container framework to go in first. No regressions on x86_64. Signed-off-by: Carlos O'Donell OK for 2.29 when it opens? I don't want to touch 2.28 with this change, and it's a long standing bug. I'd like to test this change out in Fedora Rawhide for a while to make sure I didn't miss anything. From 4383cb3f50ccddd2af45f21b78ea2d519689d34c Mon Sep 17 00:00:00 2001 From: Carlos O'Donell Date: Fri, 27 Jul 2018 21:40:37 -0400 Subject: [PATCH] Fix DSTs with static applications (Bug 23462). After commit d1d5471579eb0426671bf94f2d71e61dfb204c30 we no longer check to see if the link map passed to DL_DST_REQUIRED is NULL or not. This is a problem becuase in _dl_init_paths we do not set the link map to anything in the !SHARED case, and this is a bug. With the work done to setup link maps in both static and shared builds we can always rely on the link map. The fix is to always set the link map, and assert that it is not NULL. Setting the link map allows the use of _dl_main_map link map to be used and results in DSTs being correctly processed. We add one regression test tst-dst-static.c just to look for the crash, but we need to add a larger set of tests for DSTs, but as noted in the DST cleanup changes this needs the test-in-container framework to go in first. No regressions on x86_64. Signed-off-by: Carlos O'Donell --- ChangeLog | 7 ++++++ elf/Makefile | 7 +++++- elf/dl-load.c | 68 ++++++++++++++++++++++++++-------------------------- elf/tst-dst-static.c | 27 +++++++++++++++++++++ 4 files changed, 74 insertions(+), 35 deletions(-) create mode 100644 elf/tst-dst-static.c diff --git a/ChangeLog b/ChangeLog index e5abf96411..03fbd9dd63 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,12 @@ 2018-07-27 Carlos O'Donell + [BZ# 23462] + * elf/Makefile (tests-static-normal): tst-dst-static. + (tst-dst-static-ENV): Define. + * elf/tst-dst-static.c: New file. + * elf/dl-load.c (_dl_init_paths): Always set l. Assume l is never + NULL, assert that. + * po/uk.po: Update translations. * po/cs.po: Likewise. * po/pl.po: Likewise. diff --git a/elf/Makefile b/elf/Makefile index cd0771307f..dac5599756 100644 --- a/elf/Makefile +++ b/elf/Makefile @@ -147,7 +147,8 @@ endif tests-static-normal := tst-leaks1-static tst-array1-static tst-array5-static \ tst-dl-iter-static \ tst-tlsalign-static tst-tlsalign-extern-static \ - tst-linkall-static tst-env-setuid tst-env-setuid-tunables + tst-linkall-static tst-env-setuid tst-env-setuid-tunables \ + tst-dst-static tests-static-internal := tst-tls1-static tst-tls2-static \ tst-ptrguard1-static tst-stackguard1-static \ tst-tls1-static-non-pie tst-libc_dlvsym-static @@ -1484,3 +1485,7 @@ tst-libc_dlvsym-static-ENV = \ $(objpfx)tst-libc_dlvsym-static.out: $(objpfx)tst-libc_dlvsym-dso.so $(objpfx)tst-big-note: $(objpfx)tst-big-note-lib.so + +# We run tst-dst-static with an LD_LIBRARY_PATH of '$LIB', which before +# the fix for bug 23462 would cause an immediately segfault. +tst-dst-static-ENV = LD_LIBRARY_PATH='$$LIB' diff --git a/elf/dl-load.c b/elf/dl-load.c index c51e4b3718..5d5953da95 100644 --- a/elf/dl-load.c +++ b/elf/dl-load.c @@ -748,48 +748,48 @@ _dl_init_paths (const char *llp) max_dirnamelen = SYSTEM_DIRS_MAX_LEN; *aelem = NULL; -#ifdef SHARED - /* This points to the map of the main object. */ + /* Map of the main object (even in static executables). */ l = GL(dl_ns)[LM_ID_BASE]._ns_loaded; - if (l != NULL) + assert (l != NULL); + +#ifdef SHARED + /* Static executable startup should never execute this code. */ + assert (l->l_type != lt_loaded); + + if (l->l_info[DT_RUNPATH]) + { + /* Allocate room for the search path and fill in information + from RUNPATH. */ + decompose_rpath (&l->l_runpath_dirs, + (const void *) (D_PTR (l, l_info[DT_STRTAB]) + + l->l_info[DT_RUNPATH]->d_un.d_val), + l, "RUNPATH"); + /* During rtld init the memory is allocated by the stub malloc, + prevent any attempt to free it by the normal malloc. */ + l->l_runpath_dirs.malloced = 0; + + /* The RPATH is ignored. */ + l->l_rpath_dirs.dirs = (void *) -1; + } + else { - assert (l->l_type != lt_loaded); + l->l_runpath_dirs.dirs = (void *) -1; - if (l->l_info[DT_RUNPATH]) + if (l->l_info[DT_RPATH]) { /* Allocate room for the search path and fill in information - from RUNPATH. */ - decompose_rpath (&l->l_runpath_dirs, + from RPATH. */ + decompose_rpath (&l->l_rpath_dirs, (const void *) (D_PTR (l, l_info[DT_STRTAB]) - + l->l_info[DT_RUNPATH]->d_un.d_val), - l, "RUNPATH"); - /* During rtld init the memory is allocated by the stub malloc, - prevent any attempt to free it by the normal malloc. */ - l->l_runpath_dirs.malloced = 0; - - /* The RPATH is ignored. */ - l->l_rpath_dirs.dirs = (void *) -1; + + l->l_info[DT_RPATH]->d_un.d_val), + l, "RPATH"); + /* During rtld init the memory is allocated by the stub + malloc, prevent any attempt to free it by the normal + malloc. */ + l->l_rpath_dirs.malloced = 0; } else - { - l->l_runpath_dirs.dirs = (void *) -1; - - if (l->l_info[DT_RPATH]) - { - /* Allocate room for the search path and fill in information - from RPATH. */ - decompose_rpath (&l->l_rpath_dirs, - (const void *) (D_PTR (l, l_info[DT_STRTAB]) - + l->l_info[DT_RPATH]->d_un.d_val), - l, "RPATH"); - /* During rtld init the memory is allocated by the stub - malloc, prevent any attempt to free it by the normal - malloc. */ - l->l_rpath_dirs.malloced = 0; - } - else - l->l_rpath_dirs.dirs = (void *) -1; - } + l->l_rpath_dirs.dirs = (void *) -1; } #endif /* SHARED */ diff --git a/elf/tst-dst-static.c b/elf/tst-dst-static.c new file mode 100644 index 0000000000..5ad28ebbdc --- /dev/null +++ b/elf/tst-dst-static.c @@ -0,0 +1,27 @@ +/* Verify that statically compiled executables with DSTs don't crash. + Copyright (C) 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 + . */ + +static int +do_test (void) +{ + /* Before the fix for bug 23462 a static application run with + LD_LIBRARY_PATH='$LIB' would crash at startup. */ + return 0; +} + +#include -- 2.14.4