From patchwork Mon Mar 15 11:18:27 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 42541 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A796C3858D33; Mon, 15 Mar 2021 11:18:30 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A796C3858D33 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1615807110; bh=LS0tR27OxITpU2za5GnkrMT8+5dHLlclh2bNf5q+2/k=; h=To:Subject:In-Reply-To:References:Date:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=FEskepruxVpYgQnQyKYaLs14QYCqeHzfBcUVckOT3flXyPbUq4a9gdeJR41iWIrGX Sx5si1Awg3wBHPSYKH5RN7pdRAs9BOhcWJZX6knNgGqZbRwf45tMWOvLUIrTq5lUs0 v1k3zvgUg+SZHDT8Xmu7Y3nkRRg8i3l3GPrZYChU= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 932C23858D29 for ; Mon, 15 Mar 2021 11:18:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 932C23858D29 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-407-WtUnj29FNjKGarvnzqn_FQ-1; Mon, 15 Mar 2021 07:18:23 -0400 X-MC-Unique: WtUnj29FNjKGarvnzqn_FQ-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 0CBBCCC622 for ; Mon, 15 Mar 2021 11:18:22 +0000 (UTC) Received: from oldenburg.str.redhat.com (ovpn-112-163.ams2.redhat.com [10.36.112.163]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 2FBC1100164C for ; Mon, 15 Mar 2021 11:18:21 +0000 (UTC) To: libc-alpha@sourceware.org Subject: [PATCH 1/2] dlfcn: Failures after dlmopen should not terminate process [BZ #24772] In-Reply-To: References: Message-Id: <038ee59a44a4546933eea91a3db974e6e1d2ac91.1615806966.git.fweimer@redhat.com> Date: Mon, 15 Mar 2021 12:18:27 +0100 User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP, URIBL_BLACK autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Florian Weimer via Libc-alpha From: Florian Weimer Reply-To: Florian Weimer Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" Commit 9e78f6f6e7134a5f299cc8de77370218f8019237 ("Implement _dl_catch_error, _dl_signal_error in libc.so [BZ #16628]") has the side effect that distinct namespaces, as created by dlmopen, now have separate implementations of the rtld exception mechanism. This means that the call to _dl_catch_error from libdl in a secondary namespace does not actually install an exception handler because the thread-local variable catch_hook in the libc.so copy in the secondary namespace is distinct from that of the base namepace. As a result, a dlsym/dlopen/… failure in a secondary namespace terminates the process with a dynamic linker error because it looks to the exception handler mechanism as if no handler has been installed. This commit restores GLRO (dl_catch_error) and uses it to set the handler in the base namespace. --- dlfcn/dlerror.c | 6 +++-- elf/Makefile | 8 ++++++- elf/dl-error-skeleton.c | 12 ++++++++++ elf/rtld.c | 1 + elf/tst-dlmopen-dlerror-mod.c | 41 +++++++++++++++++++++++++++++++++++ elf/tst-dlmopen-dlerror.c | 37 +++++++++++++++++++++++++++++++ sysdeps/generic/ldsodefs.h | 9 ++++++++ 7 files changed, 111 insertions(+), 3 deletions(-) create mode 100644 elf/tst-dlmopen-dlerror-mod.c create mode 100644 elf/tst-dlmopen-dlerror.c diff --git a/dlfcn/dlerror.c b/dlfcn/dlerror.c index 48b4c25bea..947b7c10c6 100644 --- a/dlfcn/dlerror.c +++ b/dlfcn/dlerror.c @@ -167,8 +167,10 @@ _dlerror_run (void (*operate) (void *), void *args) result->errstring = NULL; } - result->errcode = _dl_catch_error (&result->objname, &result->errstring, - &result->malloced, operate, args); + result->errcode = GLRO (dl_catch_error) (&result->objname, + &result->errstring, + &result->malloced, + operate, args); /* If no error we mark that no error string is available. */ result->returned = result->errstring == NULL; diff --git a/elf/Makefile b/elf/Makefile index 4c9e63dac9..7e0a85df49 100644 --- a/elf/Makefile +++ b/elf/Makefile @@ -226,7 +226,8 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \ tst-audit14 tst-audit15 tst-audit16 \ tst-single_threaded tst-single_threaded-pthread \ tst-tls-ie tst-tls-ie-dlmopen argv0test \ - tst-glibc-hwcaps tst-glibc-hwcaps-prepend tst-glibc-hwcaps-mask + tst-glibc-hwcaps tst-glibc-hwcaps-prepend tst-glibc-hwcaps-mask \ + tst-dlmopen-dlerror # reldep9 tests-internal += loadtest unload unload2 circleload1 \ neededtest neededtest2 neededtest3 neededtest4 \ @@ -348,6 +349,7 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \ libmarkermod2-1 libmarkermod2-2 \ libmarkermod3-1 libmarkermod3-2 libmarkermod3-3 \ libmarkermod4-1 libmarkermod4-2 libmarkermod4-3 libmarkermod4-4 \ + tst-ldconfig-ld-mod tst-dlmopen-dlerror-mod \ # Most modules build with _ISOMAC defined, but those filtered out # depend on internal headers. @@ -1581,6 +1583,10 @@ $(objpfx)tst-sonamemove-dlopen.out: \ $(objpfx)tst-sonamemove-runmod1.so \ $(objpfx)tst-sonamemove-runmod2.so +$(objpfx)tst-dlmopen-dlerror: $(libdl) +$(objpfx)tst-dlmopen-dlerror-mod.so: $(libdl) $(libsupport) +$(objpfx)tst-dlmopen-dlerror.out: $(objpfx)tst-dlmopen-dlerror-mod.so + # Override -z defs, so that we can reference an undefined symbol. # Force lazy binding for the same reason. LDFLAGS-tst-latepthreadmod.so = \ diff --git a/elf/dl-error-skeleton.c b/elf/dl-error-skeleton.c index 2fd62777cf..b699936c6e 100644 --- a/elf/dl-error-skeleton.c +++ b/elf/dl-error-skeleton.c @@ -248,4 +248,16 @@ _dl_receive_error (receiver_fct fct, void (*operate) (void *), void *args) catch_hook = old_catch; receiver = old_receiver; } + +/* Forwarder used for initializing GLRO (_dl_catch_error). */ +int +_rtld_catch_error (const char **objname, const char **errstring, + bool *mallocedp, void (*operate) (void *), + void *args) +{ + /* The reference to _dl_catch_error will eventually be relocated to + point to the implementation in libc.so. */ + return _dl_catch_error (objname, errstring, mallocedp, operate, args); +} + #endif /* DL_ERROR_BOOTSTRAP */ diff --git a/elf/rtld.c b/elf/rtld.c index 94a00e2049..fd02438936 100644 --- a/elf/rtld.c +++ b/elf/rtld.c @@ -368,6 +368,7 @@ struct rtld_global_ro _rtld_global_ro attribute_relro = ._dl_lookup_symbol_x = _dl_lookup_symbol_x, ._dl_open = _dl_open, ._dl_close = _dl_close, + ._dl_catch_error = _rtld_catch_error, ._dl_tls_get_addr_soft = _dl_tls_get_addr_soft, #ifdef HAVE_DL_DISCOVER_OSVERSION ._dl_discover_osversion = _dl_discover_osversion diff --git a/elf/tst-dlmopen-dlerror-mod.c b/elf/tst-dlmopen-dlerror-mod.c new file mode 100644 index 0000000000..dcb94320b4 --- /dev/null +++ b/elf/tst-dlmopen-dlerror-mod.c @@ -0,0 +1,41 @@ +/* Check that dlfcn errors are reported properly after dlmopen. Test module. + 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 + +/* Note: This object is not linked into the main program, so we cannot + use delayed test failure reporting via TEST_VERIFY etc., and have + to use FAIL_EXIT1 (or something else that calls exit). */ + +void +call_dlsym (void) +{ + void *ptr = dlsym (NULL, "does not exist"); + if (ptr != NULL) + FAIL_EXIT1 ("dlsym did not fail as expected"); +} + +void +call_dlopen (void) +{ + void *handle = dlopen ("tst-dlmopen-dlerror does not exist", RTLD_NOW); + if (handle != NULL) + FAIL_EXIT1 ("dlopen did not fail as expected"); +} diff --git a/elf/tst-dlmopen-dlerror.c b/elf/tst-dlmopen-dlerror.c new file mode 100644 index 0000000000..65638f7f38 --- /dev/null +++ b/elf/tst-dlmopen-dlerror.c @@ -0,0 +1,37 @@ +/* Check that dlfcn errors are reported properly after dlmopen. + 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 + +static int +do_test (void) +{ + void *handle = xdlmopen (LM_ID_NEWLM, "tst-dlmopen-dlerror-mod.so", + RTLD_NOW); + void (*call_dlsym) (void) = xdlsym (handle, "call_dlsym"); + void (*call_dlopen) (void) = xdlsym (handle, "call_dlopen"); + + call_dlsym (); + call_dlopen (); + + return 0; +} + +#include diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h index ea3f7a69d0..b207f229c3 100644 --- a/sysdeps/generic/ldsodefs.h +++ b/sysdeps/generic/ldsodefs.h @@ -662,6 +662,12 @@ struct rtld_global_ro void *(*_dl_open) (const char *file, int mode, const void *caller_dlopen, Lmid_t nsid, int argc, char *argv[], char *env[]); void (*_dl_close) (void *map); + /* libdl in a secondary namespace (after dlopen) must use + _dl_catch_error from the main namespace, so it has to be + exported in some way. */ + int (*_dl_catch_error) (const char **objname, const char **errstring, + bool *mallocedp, void (*operate) (void *), + void *args); void *(*_dl_tls_get_addr_soft) (struct link_map *); #ifdef HAVE_DL_DISCOVER_OSVERSION int (*_dl_discover_osversion) (void); @@ -900,6 +906,9 @@ extern int _dl_catch_error (const char **objname, const char **errstring, void *args); libc_hidden_proto (_dl_catch_error) +/* Used for initializing GLRO (_dl_catch_error). */ +extern __typeof__ (_dl_catch_error) _rtld_catch_error attribute_hidden; + /* Call OPERATE (ARGS). If no error occurs, set *EXCEPTION to zero. Otherwise, store a copy of the raised exception in *EXCEPTION, which has to be freed by _dl_exception_free. As a special case, if From patchwork Mon Mar 15 11:18:32 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 42542 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 3ABA1385803F; Mon, 15 Mar 2021 11:18:40 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 3ABA1385803F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1615807120; bh=pfCvQX9UHvS5GWyEwcgLw2wvzOvew/sPCCtE3ZIPsb8=; h=To:Subject:In-Reply-To:References:Date:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=oVrj/UBXfKORKtTVCoJgYza4DjMSU75xrvWLppaoXmDAnvI+mZv/vxegm4PbLwHXR PpuFdb2+DgaVMnYqDMpz0FSOdb6CQ9i6TZjiELtVj2Beqha3VaaV3nFFknwNprQB0v izghCctnCdmiUHCpkSJBSmF0Tz3UeFadD5rGl59o= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 4FD0B385803F for ; Mon, 15 Mar 2021 11:18:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 4FD0B385803F Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-385-hfzuRacrPQiPvyuoefWD8A-1; Mon, 15 Mar 2021 07:18:28 -0400 X-MC-Unique: hfzuRacrPQiPvyuoefWD8A-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id BEA528030D6 for ; Mon, 15 Mar 2021 11:18:27 +0000 (UTC) Received: from oldenburg.str.redhat.com (ovpn-112-163.ams2.redhat.com [10.36.112.163]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7D4E32B8BF for ; Mon, 15 Mar 2021 11:18:26 +0000 (UTC) To: libc-alpha@sourceware.org Subject: [PATCH 2/2] dlfcn: dlerror needs to call free from the base namespace [BZ #24773] In-Reply-To: References: Message-Id: Date: Mon, 15 Mar 2021 12:18:32 +0100 User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP, URIBL_BLACK autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Florian Weimer via Libc-alpha From: Florian Weimer Reply-To: Florian Weimer Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" Calling free directly may end up freeing a pointer allocated by the dynamic loader using malloc from libc.so in the base namespace using the allocator from libc.so in a secondary namespace, which results in crashes. This commit redirects the free call through GLRO and the dynamic linker, to reach the correct namespace. It also cleans up the dlerror handling along the way, so that pthread_setspecific is no longer needed (which avoids triggering bug 24774). --- dlfcn/Makefile | 3 +- dlfcn/Versions | 6 +- dlfcn/dlerror.c | 299 ++++++++++++++-------------------- dlfcn/dlerror.h | 74 +++++++++ dlfcn/dlfreeres.c | 29 ---- dlfcn/libc_dlerror_result.c | 39 +++++ elf/dl-exception.c | 11 ++ elf/rtld.c | 1 + elf/tst-dlmopen-dlerror-mod.c | 29 +++- elf/tst-dlmopen-dlerror.c | 22 ++- include/dlfcn.h | 2 - malloc/set-freeres.c | 10 +- malloc/thread-freeres.c | 2 + sysdeps/generic/ldsodefs.h | 7 + 14 files changed, 307 insertions(+), 227 deletions(-) create mode 100644 dlfcn/dlerror.h delete mode 100644 dlfcn/dlfreeres.c create mode 100644 dlfcn/libc_dlerror_result.c diff --git a/dlfcn/Makefile b/dlfcn/Makefile index d51fd08c33..994a3afee6 100644 --- a/dlfcn/Makefile +++ b/dlfcn/Makefile @@ -22,9 +22,10 @@ include ../Makeconfig headers := bits/dlfcn.h dlfcn.h extra-libs := libdl libdl-routines := dlopen dlclose dlsym dlvsym dlerror dladdr dladdr1 dlinfo \ - dlmopen dlfcn dlfreeres + dlmopen dlfcn routines := $(patsubst %,s%,$(filter-out dlfcn,$(libdl-routines))) elide-routines.os := $(routines) +routines += libc_dlerror_result extra-libs-others := libdl diff --git a/dlfcn/Versions b/dlfcn/Versions index 1df6925a92..f07cb929aa 100644 --- a/dlfcn/Versions +++ b/dlfcn/Versions @@ -1,3 +1,8 @@ +libc { + GLIBC_PRIVATE { + __libc_dlerror_result; + } +} libdl { GLIBC_2.0 { dladdr; dlclose; dlerror; dlopen; dlsym; @@ -13,6 +18,5 @@ libdl { } GLIBC_PRIVATE { _dlfcn_hook; - __libdl_freeres; } } diff --git a/dlfcn/dlerror.c b/dlfcn/dlerror.c index 947b7c10c6..e0dedbe3a6 100644 --- a/dlfcn/dlerror.c +++ b/dlfcn/dlerror.c @@ -25,6 +25,8 @@ #include #include #include +#include +#include #if !defined SHARED && IS_IN (libdl) @@ -36,89 +38,70 @@ dlerror (void) #else -/* Type for storing results of dynamic loading actions. */ -struct dl_action_result - { - int errcode; - int returned; - bool malloced; - const char *objname; - const char *errstring; - }; -static struct dl_action_result last_result; -static struct dl_action_result *static_buf; - -/* This is the key for the thread specific memory. */ -static __libc_key_t key; -__libc_once_define (static, once); - -/* Destructor for the thread-specific data. */ -static void init (void); -static void free_key_mem (void *mem); - - char * __dlerror (void) { - char *buf = NULL; - struct dl_action_result *result; - # ifdef SHARED if (!rtld_active ()) return _dlfcn_hook->dlerror (); # endif - /* If we have not yet initialized the buffer do it now. */ - __libc_once (once, init); + struct dl_action_result *result = __libc_dlerror_result; - /* Get error string. */ - if (static_buf != NULL) - result = static_buf; - else + /* No libdl function has been called. No error is possible. */ + if (result == NULL) + return NULL; + + /* For an early malloc failure, clear the error flag and return the + error message. This marks the error as delivered. */ + if (result == dl_action_result_malloc_failed ()) { - /* init () has been run and we don't use the static buffer. - So we have a valid key. */ - result = (struct dl_action_result *) __libc_getspecific (key); - if (result == NULL) - result = &last_result; + __libc_dlerror_result = NULL; + return (char *) "out of memory"; } - /* Test whether we already returned the string. */ - if (result->returned != 0) + /* Placeholder object. This can be observed in a recursive call, + e.g. from an ELF constructor. */ + if (result->errstring == NULL) + return NULL; + + /* If we have already reported the error, we can free the result + and return NULL. */ + if (result->returned) { - /* We can now free the string. */ - if (result->errstring != NULL) - { - if (strcmp (result->errstring, "out of memory") != 0) - free ((char *) result->errstring); - result->errstring = NULL; - } + __libc_dlerror_result = NULL; + dl_action_result_errstring_free (result); + free (result); + return NULL; } - else if (result->errstring != NULL) - { - buf = (char *) result->errstring; - int n; - if (result->errcode == 0) - n = __asprintf (&buf, "%s%s%s", - result->objname, - result->objname[0] == '\0' ? "" : ": ", - _(result->errstring)); - else - n = __asprintf (&buf, "%s%s%s: %s", - result->objname, - result->objname[0] == '\0' ? "" : ": ", - _(result->errstring), - strerror (result->errcode)); - if (n != -1) - { - /* We don't need the error string anymore. */ - if (strcmp (result->errstring, "out of memory") != 0) - free ((char *) result->errstring); - result->errstring = buf; - } - /* Mark the error as returned. */ - result->returned = 1; + assert (result->errstring != NULL); + + /* Create the combined error message. */ + char *buf; + int n; + if (result->errcode == 0) + n = __asprintf (&buf, "%s%s%s", + result->objname, + result->objname[0] == '\0' ? "" : ": ", + _(result->errstring)); + else + n = __asprintf (&buf, "%s%s%s: %s", + result->objname, + result->objname[0] == '\0' ? "" : ": ", + _(result->errstring), + strerror (result->errcode)); + + /* Mark the error as delivered. */ + result->returned = true; + + if (n >= 0) + { + /* Replace the error string with the newly allocated one. */ + dl_action_result_errstring_free (result); + result->errstring = buf; + result->errstring_source = dl_action_result_errstring_local; + return buf; } return buf; @@ -130,130 +113,94 @@ strong_alias (__dlerror, dlerror) int _dlerror_run (void (*operate) (void *), void *args) { - struct dl_action_result *result; - - /* If we have not yet initialized the buffer do it now. */ - __libc_once (once, init); - - /* Get error string and number. */ - if (static_buf != NULL) - result = static_buf; - else + struct dl_action_result *result = __libc_dlerror_result; + if (result != NULL) { - /* We don't use the static buffer and so we have a key. Use it - to get the thread-specific buffer. */ - result = __libc_getspecific (key); - if (result == NULL) + if (result == dl_action_result_malloc_failed ()) { - result = (struct dl_action_result *) calloc (1, sizeof (*result)); - if (result == NULL) - /* We are out of memory. Since this is no really critical - situation we carry on by using the global variable. - This might lead to conflicts between the threads but - they soon all will have memory problems. */ - result = &last_result; - else - /* Set the tsd. */ - __libc_setspecific (key, result); + /* Clear the previous error. */ + __libc_dlerror_result = NULL; + result = NULL; + } + else + { + /* There is an existing object. Free its error string, but + keep the object. */ + dl_action_result_errstring_free (result); + /* Mark the object as not containing an error. This ensures + that call to dlerror from, for example, an ELF + constructor will not notice this result object. */ + result->errstring = NULL; } } - if (result->errstring != NULL) - { - /* Free the error string from the last failed command. This can - happen if `dlerror' was not run after an error was found. */ - if (result->malloced) - free ((char *) result->errstring); - result->errstring = NULL; - } - - result->errcode = GLRO (dl_catch_error) (&result->objname, - &result->errstring, - &result->malloced, - operate, args); + const char *objname; + const char *errstring; + bool malloced; + int errcode = GLRO (dl_catch_error) (&objname, &errstring, &malloced, + operate, args); - /* If no error we mark that no error string is available. */ - result->returned = result->errstring == NULL; + /* ELF constructors or destructors may have indirectly altered the + value of __libc_dlerror_result, therefore reload it. */ + result = __libc_dlerror_result; - return result->errstring != NULL; -} - - -/* Initialize buffers for results. */ -static void -init (void) -{ - if (__libc_key_create (&key, free_key_mem)) - /* Creating the key failed. This means something really went - wrong. In any case use a static buffer which is better than - nothing. */ - static_buf = &last_result; -} - - -static void -check_free (struct dl_action_result *rec) -{ - if (rec->errstring != NULL - && strcmp (rec->errstring, "out of memory") != 0) + if (errstring == NULL) { - /* We can free the string only if the allocation happened in the - C library used by the dynamic linker. This means, it is - always the C library in the base namespace. When we're statically - linked, the dynamic linker is part of the program and so always - uses the same C library we use here. */ -#ifdef SHARED - struct link_map *map = NULL; - Dl_info info; - if (_dl_addr (check_free, &info, &map, NULL) != 0 && map->l_ns == 0) -#endif + /* There is no error. We no longer need the result object if it + does not contain an error. However, a recursive call may + have added an error even if this call did not cause it. Keep + the other error. */ + if (result != NULL && result->errstring == NULL) { - free ((char *) rec->errstring); - rec->errstring = NULL; + __libc_dlerror_result = NULL; + free (result); } + return 0; } -} - - -static void -__attribute__ ((destructor)) -fini (void) -{ - check_free (&last_result); -} - - -/* Free the thread specific data, this is done if a thread terminates. */ -static void -free_key_mem (void *mem) -{ - check_free ((struct dl_action_result *) mem); + else + { + /* A new error occurred. Check if a result object has to be + allocated. */ + if (result == NULL || result == dl_action_result_malloc_failed ()) + { + /* Allocating storage for the error message after the fact + is not ideal. But this avoids an infinite recursion in + case malloc itself calls libdl functions (without + triggering errors). */ + result = malloc (sizeof (*result)); + if (result == NULL) + { + /* Assume that the dlfcn failure was due to a malloc + failure, too. */ + if (malloced) + dl_error_free ((char *) errstring); + __libc_dlerror_result = dl_action_result_malloc_failed (); + return 1; + } + __libc_dlerror_result = result; + } + else + /* Deallocate the existing error message from a recursive + call, but reuse the result object. */ + dl_action_result_errstring_free (result); + + result->errcode = errcode; + result->objname = objname; + result->errstring = (char *) errstring; + result->returned = false; + /* In case of an error, the malloced flag indicates whether the + error string is constant or not. */ + if (malloced) + result->errstring_source = dl_action_result_errstring_rtld; + else + result->errstring_source = dl_action_result_errstring_constant; - free (mem); - __libc_setspecific (key, NULL); + return 1; + } } # ifdef SHARED -/* Free the dlerror-related resources. */ -void -__dlerror_main_freeres (void) -{ - /* Free the global memory if used. */ - check_free (&last_result); - - if (__libc_once_get (once) && static_buf == NULL) - { - /* init () has been run and we don't use the static buffer. - So we have a valid key. */ - void *mem; - /* Free the TSD memory if used. */ - mem = __libc_getspecific (key); - if (mem != NULL) - free_key_mem (mem); - } -} - struct dlfcn_hook *_dlfcn_hook __attribute__((nocommon)); libdl_hidden_data_def (_dlfcn_hook) diff --git a/dlfcn/dlerror.h b/dlfcn/dlerror.h new file mode 100644 index 0000000000..ae07495139 --- /dev/null +++ b/dlfcn/dlerror.h @@ -0,0 +1,74 @@ +#ifndef _DLERROR_H +#define _DLERROR_H + +#include +#include +#include +#include +#include + +/* Source of the errstring member in struct dl_action_result, for + finding the right deallocation routine. */ +enum dl_action_result_errstring_source + { + dl_action_result_errstring_constant, /* String literal, no deallocation. */ + dl_action_result_errstring_rtld, /* libc in the primary namespace. */ + dl_action_result_errstring_local, /* libc in the current namespace. */ + }; + +struct dl_action_result +{ + int errcode; + char errstring_source; + bool returned; + const char *objname; + char *errstring; +}; + +/* Used to free the errstring member of struct dl_action_result in the + dl_action_result_errstring_rtld case. */ +static inline void +dl_error_free (void *ptr) +{ +#ifdef SHARED + /* In the shared case, ld.so may use a different malloc than this + namespace. */ + GLRO (dl_error_free (ptr)); +#else + /* Call the implementation directly. It still has to check for + pointers which cannot be freed, so do not call free directly + here. */ + _dl_error_free (ptr); +#endif +} + +/* Deallocate RESULT->errstring, leaving *RESULT itself allocated. */ +static inline void +dl_action_result_errstring_free (struct dl_action_result *result) +{ + switch (result->errstring_source) + { + case dl_action_result_errstring_constant: + break; + case dl_action_result_errstring_rtld: + dl_error_free (result->errstring); + break; + case dl_action_result_errstring_local: + free (result->errstring); + break; + } +} + +static inline struct dl_action_result * +dl_action_result_malloc_failed (void) +{ + return (struct dl_action_result *) (intptr_t) -1; +} + +/* Thread-local variable for storing dlfcn failures for subsequent + reporting via dlerror. */ +extern __thread struct dl_action_result *__libc_dlerror_result + attribute_tls_model_ie; +void __libc_dlerror_result_free (void) attribute_hidden; + +#endif /* _DLERROR_H */ diff --git a/dlfcn/dlfreeres.c b/dlfcn/dlfreeres.c deleted file mode 100644 index 856b76416d..0000000000 --- a/dlfcn/dlfreeres.c +++ /dev/null @@ -1,29 +0,0 @@ -/* Clean up allocated libdl memory on demand. - Copyright (C) 2018-2021 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 - -/* Free libdl.so resources. - Note: Caller ensures we are called only once. */ -void -__libdl_freeres (void) -{ - call_function_static_weak (__dlerror_main_freeres); -} diff --git a/dlfcn/libc_dlerror_result.c b/dlfcn/libc_dlerror_result.c new file mode 100644 index 0000000000..11468937a2 --- /dev/null +++ b/dlfcn/libc_dlerror_result.c @@ -0,0 +1,39 @@ +/* Thread-local variable holding the dlerror result. + 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 + +/* This pointer is either NULL, dl_action_result_malloc_failed (), or + has been allocated using malloc by the namespace that also contains + this instance of the thread-local variable. */ +__thread struct dl_action_result *__libc_dlerror_result attribute_tls_model_ie; + +/* Called during thread shutdown to free resources. */ +void +__libc_dlerror_result_free (void) +{ + if (__libc_dlerror_result != NULL) + { + if (__libc_dlerror_result != dl_action_result_malloc_failed ()) + { + dl_action_result_errstring_free (__libc_dlerror_result); + free (__libc_dlerror_result); + } + __libc_dlerror_result = NULL; + } +} diff --git a/elf/dl-exception.c b/elf/dl-exception.c index 30adb7d1dc..8eaad418cb 100644 --- a/elf/dl-exception.c +++ b/elf/dl-exception.c @@ -30,6 +30,17 @@ a pointer comparison. See below and in dlfcn/dlerror.c. */ static const char _dl_out_of_memory[] = "out of memory"; +/* Call free in the main libc.so. This allows other namespaces to + free pointers on the main libc heap, via GLRO (dl_error_free). It + also avoids calling free on the special, pre-allocated + out-of-memory error message. */ +void +_dl_error_free (void *ptr) +{ + if (ptr != _dl_out_of_memory) + free (ptr); +} + /* Dummy allocation object used if allocating the message buffer fails. */ static void diff --git a/elf/rtld.c b/elf/rtld.c index fd02438936..c2ca4b7ce3 100644 --- a/elf/rtld.c +++ b/elf/rtld.c @@ -369,6 +369,7 @@ struct rtld_global_ro _rtld_global_ro attribute_relro = ._dl_open = _dl_open, ._dl_close = _dl_close, ._dl_catch_error = _rtld_catch_error, + ._dl_error_free = _dl_error_free, ._dl_tls_get_addr_soft = _dl_tls_get_addr_soft, #ifdef HAVE_DL_DISCOVER_OSVERSION ._dl_discover_osversion = _dl_discover_osversion diff --git a/elf/tst-dlmopen-dlerror-mod.c b/elf/tst-dlmopen-dlerror-mod.c index dcb94320b4..25188750fb 100644 --- a/elf/tst-dlmopen-dlerror-mod.c +++ b/elf/tst-dlmopen-dlerror-mod.c @@ -18,6 +18,8 @@ #include #include +#include +#include #include /* Note: This object is not linked into the main program, so we cannot @@ -25,17 +27,32 @@ to use FAIL_EXIT1 (or something else that calls exit). */ void -call_dlsym (void) +call_dlsym (const char *name) { - void *ptr = dlsym (NULL, "does not exist"); + void *ptr = dlsym (NULL, name); if (ptr != NULL) - FAIL_EXIT1 ("dlsym did not fail as expected"); + FAIL_EXIT1 ("dlsym did not fail as expected for: %s", name); + const char *message = dlerror (); + if (strstr (message, ": undefined symbol: does not exist X") == NULL) + FAIL_EXIT1 ("invalid dlsym error message for [[%s]]: %s", name, message); + message = dlerror (); + if (message != NULL) + FAIL_EXIT1 ("second dlsym for [[%s]]: %s", name, message); } void -call_dlopen (void) +call_dlopen (const char *name) { - void *handle = dlopen ("tst-dlmopen-dlerror does not exist", RTLD_NOW); + void *handle = dlopen (name, RTLD_NOW); if (handle != NULL) - FAIL_EXIT1 ("dlopen did not fail as expected"); + FAIL_EXIT1 ("dlopen did not fail as expected for: %s", name); + const char *message = dlerror (); + if (strstr (message, "X: cannot open shared object file:" + " No such file or directory") == NULL + && strstr (message, "X: cannot open shared object file:" + " File name too long") == NULL) + FAIL_EXIT1 ("invalid dlopen error message for [[%s]]: %s", name, message); + message = dlerror (); + if (message != NULL) + FAIL_EXIT1 ("second dlopen for [[%s]]: %s", name, message); } diff --git a/elf/tst-dlmopen-dlerror.c b/elf/tst-dlmopen-dlerror.c index 65638f7f38..d02993847d 100644 --- a/elf/tst-dlmopen-dlerror.c +++ b/elf/tst-dlmopen-dlerror.c @@ -17,6 +17,7 @@ . */ #include +#include #include #include @@ -25,11 +26,22 @@ do_test (void) { void *handle = xdlmopen (LM_ID_NEWLM, "tst-dlmopen-dlerror-mod.so", RTLD_NOW); - void (*call_dlsym) (void) = xdlsym (handle, "call_dlsym"); - void (*call_dlopen) (void) = xdlsym (handle, "call_dlopen"); - - call_dlsym (); - call_dlopen (); + void (*call_dlsym) (const char *name) = xdlsym (handle, "call_dlsym"); + void (*call_dlopen) (const char *name) = xdlsym (handle, "call_dlopen"); + + /* Iterate over various name lengths. This changes the size of + error messages allocated by ld.so and has been shown to trigger + detectable heap corruption if malloc/free calls in different + namespaces are mixed. */ + char buffer[2048]; + char *buffer_end = &buffer[sizeof (buffer) - 2]; + for (char *p = stpcpy (buffer, "does not exist "); p < buffer_end; ++p) + { + p[0] = 'X'; + p[1] = '\0'; + call_dlsym (buffer); + call_dlopen (buffer); + } return 0; } diff --git a/include/dlfcn.h b/include/dlfcn.h index a1816e4991..a8d48bdada 100644 --- a/include/dlfcn.h +++ b/include/dlfcn.h @@ -156,7 +156,5 @@ extern void __libc_register_dlfcn_hook (struct link_map *map) attribute_hidden; #endif -extern void __dlerror_main_freeres (void) attribute_hidden; - #endif #endif diff --git a/malloc/set-freeres.c b/malloc/set-freeres.c index 817fbea8b8..d404250151 100644 --- a/malloc/set-freeres.c +++ b/malloc/set-freeres.c @@ -20,6 +20,7 @@ #include #include #include +#include #include "../nss/nsswitch.h" #include "../libio/libioP.h" @@ -28,8 +29,6 @@ DEFINE_HOOK (__libc_subfreeres, (void)); symbol_set_define (__libc_freeres_ptrs); -extern __attribute__ ((weak)) void __libdl_freeres (void); - extern __attribute__ ((weak)) void __libpthread_freeres (void); void __libc_freeres_fn_section @@ -52,11 +51,6 @@ __libc_freeres (void) /* We run the resource freeing after IO cleanup. */ RUN_HOOK (__libc_subfreeres, ()); - /* Call the libdl list of cleanup functions - (weak-ref-and-check). */ - if (&__libdl_freeres != NULL) - __libdl_freeres (); - /* Call the libpthread list of cleanup functions (weak-ref-and-check). */ if (&__libpthread_freeres != NULL) @@ -66,6 +60,8 @@ __libc_freeres (void) __libc_unwind_link_freeres (); #endif + call_function_static_weak (__libc_dlerror_result_free); + for (p = symbol_set_first_element (__libc_freeres_ptrs); !symbol_set_end_p (__libc_freeres_ptrs, p); ++p) free (*p); diff --git a/malloc/thread-freeres.c b/malloc/thread-freeres.c index da76a3dca7..77a204f9fa 100644 --- a/malloc/thread-freeres.c +++ b/malloc/thread-freeres.c @@ -16,6 +16,7 @@ License along with the GNU C Library; if not, see . */ +#include #include #include #include @@ -36,6 +37,7 @@ __libc_thread_freeres (void) #endif call_function_static_weak (__res_thread_freeres); __glibc_tls_internal_free (); + call_function_static_weak (__libc_dlerror_result_free); /* This should come last because it shuts down malloc for this thread and the other shutdown functions might well call free. */ diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h index b207f229c3..dfc117a445 100644 --- a/sysdeps/generic/ldsodefs.h +++ b/sysdeps/generic/ldsodefs.h @@ -668,6 +668,9 @@ struct rtld_global_ro int (*_dl_catch_error) (const char **objname, const char **errstring, bool *mallocedp, void (*operate) (void *), void *args); + /* libdl in a secondary namespace must use free from the base + namespace. */ + void (*_dl_error_free) (void *); void *(*_dl_tls_get_addr_soft) (struct link_map *); #ifdef HAVE_DL_DISCOVER_OSVERSION int (*_dl_discover_osversion) (void); @@ -823,6 +826,10 @@ void _dl_exception_create (struct dl_exception *, const char *object, __attribute__ ((nonnull (1, 3))); rtld_hidden_proto (_dl_exception_create) +/* Used internally to implement dlerror message freeing. See + include/dlfcn.h and dlfcn/dlerror.c. */ +void _dl_error_free (void *ptr) attribute_hidden; + /* Like _dl_exception_create, but create errstring from a format string FMT. Currently, only "%s" and "%%" are supported as format directives. */