From patchwork Wed Nov 27 15:56:01 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Adhemerval Zanella (Code Review)" X-Patchwork-Id: 1201679 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=libc-alpha-return-107420-incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=gnutoolchain-gerrit.osci.io Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="o+r43TUZ"; dkim-atps=neutral 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 47NQNx5msCz9sRf for ; Thu, 28 Nov 2019 02:56:17 +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:date:from:to:subject:in-reply-to:references :reply-to:mime-version:content-transfer-encoding:content-type :message-id; q=dns; s=default; b=COmzziKyExusYo3QZkWQ1n95Sv0p3g0 71zc6PAkzwsY6PgZIQHgHLwLZRkSvhWHHaKbnrf89RM/eQqLB2Uh6GUs0FX6VMXC As2RyxUvACVoqHNT436DfT5ii3MCOnY9hlKlcQrbRDmcnDByn1jhEYNCUjiQB5pv /QcIk19mxyKE= 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:date:from:to:subject:in-reply-to:references :reply-to:mime-version:content-transfer-encoding:content-type :message-id; s=default; bh=gQ5DrxceO5OJxsLoLaYQYWt/s8A=; b=o+r43 TUZsyzY/n4bK80fGMq7M6eRZXl1URKk8o1G+VZ9jiJ7J+xC0dT4zWwjIHi0ihkxM xP0W9EDzE4LvJuJDdCTC8rJspxUyd3bbhigQTd9S7+DA66oAuJcsy9iL2lh+g6Xn yJo4LugbSI6msVW7qtQY8HJ8mCQpoj+sw0uHYY= Received: (qmail 9227 invoked by alias); 27 Nov 2019 15:56:10 -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 9219 invoked by uid 89); 27 Nov 2019 15:56:10 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-21.5 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT autolearn=ham version=3.3.1 spammy=captured, funwind-tables, funwindtables X-HELO: mx1.osci.io X-Gerrit-PatchSet: 4 Date: Wed, 27 Nov 2019 10:56:01 -0500 From: "Florian Weimer (Code Review)" To: Florian Weimer , Carlos O'Donell , libc-alpha@sourceware.org Auto-Submitted: auto-generated X-Gerrit-MessageType: newpatchset Subject: [review v4] Lazy binding failures during dlopen/dlclose must be fatal [BZ #24304] X-Gerrit-Change-Id: I6b1addfe2e30f50a1781595f046f44173db9491a X-Gerrit-Change-Number: 467 X-Gerrit-ChangeURL: X-Gerrit-Commit: 69980f28457c6c330795b5db633db2769d4d8e0c In-Reply-To: References: Reply-To: fweimer@redhat.com, fweimer@redhat.com, libc-alpha@sourceware.org, carlos@redhat.com MIME-Version: 1.0 Content-Disposition: inline User-Agent: Gerrit/3.0.3-79-g83ff7f88f1 Message-Id: <20191127155603.4398720AF6@gnutoolchain-gerrit.osci.io> Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/467 ...................................................................... Lazy binding failures during dlopen/dlclose must be fatal [BZ #24304] If a lazy binding failure happens during the execution of an ELF constructor or destructor, the dynamic loader catches the error and reports it using the dlerror mechanism. This is undesirable because there could be other constructors and destructors that need processing (which are skipped), and the process is in an inconsistent state at this point. Therefore, we have to issue a fatal dynamic loader error error and terminate the process. Note that the _dl_catch_exception in _dl_open is just an inner catch, to roll back some state locally. If called from dlopen, there is still an outer catch, which is why calling _dl_init via call_dl_init and a no-exception is required and cannot be avoiding by moving the _dl_init call directly into _dl_open. _dl_fini does not need changes because it does not install an error handler, so errors are already fatal there. Change-Id: I6b1addfe2e30f50a1781595f046f44173db9491a --- M NEWS M elf/Makefile M elf/dl-close.c M elf/dl-open.c A elf/tst-finilazyfailmod.c A elf/tst-initfinilazyfail.c A elf/tst-initlazyfailmod.c 7 files changed, 216 insertions(+), 21 deletions(-) diff --git a/NEWS b/NEWS index df03f4d..936fc14 100644 --- a/NEWS +++ b/NEWS @@ -80,6 +80,12 @@ * The obsolete functions ftime has been deprecated and will be removed from a future version of glibc. Application should use clock_gettime instead. +* If a lazy binding failure happens during dlopen, during the execution of + an ELF constructor, the process is now terminated. Previously, the + dynamic loader would return NULL from dlopen, with the lazy binding error + captured in a dlerror message. In general, this is unsafe because + resetting the stack in an arbitrary function call is not possible. + Changes to build and runtime requirements: [Add changes to build and runtime requirements here] diff --git a/elf/Makefile b/elf/Makefile index b05af5c..30c3896 100644 --- a/elf/Makefile +++ b/elf/Makefile @@ -199,7 +199,7 @@ tst-debug1 tst-main1 tst-absolute-sym tst-absolute-zero tst-big-note \ tst-unwind-ctor tst-unwind-main tst-audit13 \ tst-sonamemove-link tst-sonamemove-dlopen tst-dlopen-tlsmodid \ - tst-dlopen-self tst-auditmany + tst-dlopen-self tst-auditmany tst-initfinilazyfail # reldep9 tests-internal += loadtest unload unload2 circleload1 \ neededtest neededtest2 neededtest3 neededtest4 \ @@ -290,7 +290,8 @@ tst-sonamemove-runmod1 tst-sonamemove-runmod2 \ tst-auditmanymod1 tst-auditmanymod2 tst-auditmanymod3 \ tst-auditmanymod4 tst-auditmanymod5 tst-auditmanymod6 \ - tst-auditmanymod7 tst-auditmanymod8 tst-auditmanymod9 + tst-auditmanymod7 tst-auditmanymod8 tst-auditmanymod9 \ + tst-initlazyfailmod tst-finilazyfailmod # Most modules build with _ISOMAC defined, but those filtered out # depend on internal headers. modules-names-tests = $(filter-out ifuncmod% tst-libc_dlvsym-dso tst-tlsmod%,\ @@ -1586,3 +1587,13 @@ $(objpfx)tst-unwind-ctor: $(objpfx)tst-unwind-ctor-lib.so CFLAGS-tst-unwind-main.c += -funwind-tables -DUSE_PTHREADS=0 + +$(objpfx)tst-initfinilazyfail: $(libdl) +$(objpfx)tst-initfinilazyfail.out: \ + $(objpfx)tst-initlazyfailmod.so $(objpfx)tst-finilazyfailmod.so +# Override -z defs, so that we can reference an undefined symbol. +# Force lazy binding for the same reason. +LDFLAGS-tst-initlazyfailmod.so = \ + -Wl,-z,lazy -Wl,--unresolved-symbols=ignore-all +LDFLAGS-tst-finilazyfailmod.so = \ + -Wl,-z,lazy -Wl,--unresolved-symbols=ignore-all diff --git a/elf/dl-close.c b/elf/dl-close.c index c32e647..33486b9 100644 --- a/elf/dl-close.c +++ b/elf/dl-close.c @@ -106,6 +106,30 @@ return false; } +/* Invoke dstructors for CLOSURE (a struct link_map *). Called with + exception handling temporarily disabled, to make errors fatal. */ +static void +call_destructors (void *closure) +{ + struct link_map *map = closure; + + if (map->l_info[DT_FINI_ARRAY] != NULL) + { + ElfW(Addr) *array = + (ElfW(Addr) *) (map->l_addr + + map->l_info[DT_FINI_ARRAY]->d_un.d_ptr); + unsigned int sz = (map->l_info[DT_FINI_ARRAYSZ]->d_un.d_val + / sizeof (ElfW(Addr))); + + while (sz-- > 0) + ((fini_t) array[sz]) (); + } + + /* Next try the old-style destructor. */ + if (map->l_info[DT_FINI] != NULL) + DL_CALL_DT_FINI (map, ((void *) map->l_addr + + map->l_info[DT_FINI]->d_un.d_ptr)); +} void _dl_close_worker (struct link_map *map, bool force) @@ -267,7 +291,8 @@ && (imap->l_flags_1 & DF_1_NODELETE) == 0); /* Call its termination function. Do not do it for - half-cooked objects. */ + half-cooked objects. Temporarily disable exception + handling, so that errors are fatal. */ if (imap->l_init_called) { /* When debugging print a message first. */ @@ -276,22 +301,9 @@ _dl_debug_printf ("\ncalling fini: %s [%lu]\n\n", imap->l_name, nsid); - if (imap->l_info[DT_FINI_ARRAY] != NULL) - { - ElfW(Addr) *array = - (ElfW(Addr) *) (imap->l_addr - + imap->l_info[DT_FINI_ARRAY]->d_un.d_ptr); - unsigned int sz = (imap->l_info[DT_FINI_ARRAYSZ]->d_un.d_val - / sizeof (ElfW(Addr))); - - while (sz-- > 0) - ((fini_t) array[sz]) (); - } - - /* Next try the old-style destructor. */ - if (imap->l_info[DT_FINI] != NULL) - DL_CALL_DT_FINI (imap, ((void *) imap->l_addr - + imap->l_info[DT_FINI]->d_un.d_ptr)); + if (imap->l_info[DT_FINI_ARRAY] != NULL + || imap->l_info[DT_FINI] != NULL) + _dl_catch_exception (NULL, call_destructors, imap); } #ifdef SHARED diff --git a/elf/dl-open.c b/elf/dl-open.c index 8d699d3..533fb96 100644 --- a/elf/dl-open.c +++ b/elf/dl-open.c @@ -177,6 +177,23 @@ } rtld_hidden_def (_dl_find_dso_for_object); +/* struct dl_init_args and call_dl_init are used to call _dl_init with + exception handling disabled. */ +struct dl_init_args +{ + struct link_map *new; + int argc; + char **argv; + char **env; +}; + +static void +call_dl_init (void *closure) +{ + struct dl_init_args *args = closure; + _dl_init (args->new, args->argc, args->argv, args->env); +} + static void dl_open_worker (void *a) { @@ -509,8 +526,19 @@ DL_STATIC_INIT (new); #endif - /* Run the initializer functions of new objects. */ - _dl_init (new, args->argc, args->argv, args->env); + /* Run the initializer functions of new objects. Temporarily + disable the exception handler, so that lazy binding failures are + fatal. */ + { + struct dl_init_args init_args = + { + .new = new, + .argc = args->argc, + .argv = args->argv, + .env = args->env + }; + _dl_catch_exception (NULL, call_dl_init, &init_args); + } /* Now we can make the new map available in the global scope. */ if (mode & RTLD_GLOBAL) diff --git a/elf/tst-finilazyfailmod.c b/elf/tst-finilazyfailmod.c new file mode 100644 index 0000000..2670bd1 --- /dev/null +++ b/elf/tst-finilazyfailmod.c @@ -0,0 +1,27 @@ +/* Helper module for tst-initfinilazyfail: lazy binding failure in destructor. + Copyright (C) 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 + . */ + +/* An undefined function. Calling it will cause a lazy binding + failure. */ +void undefined_function (void); + +static void __attribute__ ((destructor)) +fini (void) +{ + undefined_function (); +} diff --git a/elf/tst-initfinilazyfail.c b/elf/tst-initfinilazyfail.c new file mode 100644 index 0000000..9b4a3d0 --- /dev/null +++ b/elf/tst-initfinilazyfail.c @@ -0,0 +1,84 @@ +/* Test that lazy binding failures in constructors and destructors are fatal. + Copyright (C) 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 +#include +#include +#include +#include + +static void +test_constructor (void *closure) +{ + void *handle = dlopen ("tst-initlazyfailmod.so", RTLD_LAZY); + if (handle == NULL) + FAIL_EXIT (2, "dlopen did not terminate the process: %s", dlerror ()); + else + FAIL_EXIT (2, "dlopen did not terminate the process (%p)", handle); +} + +static void +test_destructor (void *closure) +{ + void *handle = xdlopen ("tst-finilazyfailmod.so", RTLD_LAZY); + int ret = dlclose (handle); + const char *message = dlerror (); + if (message != NULL) + FAIL_EXIT (2, "dlclose did not terminate the process: %d, %s", + ret, message); + else + FAIL_EXIT (2, "dlopen did not terminate the process: %d", ret); +} + +static int +do_test (void) +{ + { + struct support_capture_subprocess proc + = support_capture_subprocess (test_constructor, NULL); + support_capture_subprocess_check (&proc, "constructor", 127, + sc_allow_stderr); + printf ("info: constructor failure output: [[%s]]\n", proc.err.buffer); + TEST_VERIFY (strstr (proc.err.buffer, + "tst-initfinilazyfail: symbol lookup error: ") + != NULL); + TEST_VERIFY (strstr (proc.err.buffer, + "tst-initlazyfailmod.so: undefined symbol:" + " undefined_function\n") != NULL); + support_capture_subprocess_free (&proc); + } + + { + struct support_capture_subprocess proc + = support_capture_subprocess (test_destructor, NULL); + support_capture_subprocess_check (&proc, "destructor", 127, + sc_allow_stderr); + printf ("info: destructor failure output: [[%s]]\n", proc.err.buffer); + TEST_VERIFY (strstr (proc.err.buffer, + "tst-initfinilazyfail: symbol lookup error: ") + != NULL); + TEST_VERIFY (strstr (proc.err.buffer, + "tst-finilazyfailmod.so: undefined symbol:" + " undefined_function\n") != NULL); + support_capture_subprocess_free (&proc); + } + + return 0; +} + +#include diff --git a/elf/tst-initlazyfailmod.c b/elf/tst-initlazyfailmod.c new file mode 100644 index 0000000..36348b5 --- /dev/null +++ b/elf/tst-initlazyfailmod.c @@ -0,0 +1,27 @@ +/* Helper module for tst-initfinilazyfail: lazy binding failure in constructor. + Copyright (C) 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 + . */ + +/* An undefined function. Calling it will cause a lazy binding + failure. */ +void undefined_function (void); + +static void __attribute__ ((constructor)) +init (void) +{ + undefined_function (); +}