From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x82d.google.com (mail-qt1-x82d.google.com [IPv6:2607:f8b0:4864:20::82d]) by sourceware.org (Postfix) with ESMTPS id 2AE84385701F for ; Tue, 23 Mar 2021 14:47:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 2AE84385701F Received: by mail-qt1-x82d.google.com with SMTP id x9so15067573qto.8 for ; Tue, 23 Mar 2021 07:47:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=Y1gsyUSAb2vYAFm4cA3OpoDHMeRGFXps92u1Ye6r7h0=; b=gzBslHO//MMnwhOuIMdv3cwO8Ji0VV8imieoU1pQNwhtXnrRdor4bUiFF/S+nxi5wD vh9/kjHXM6pWZQN3pMed95nEVM29JxPXFtx4cQUBVK5Q/ANq+nfUqtWbP/6qp/wj7jty 6OJ6FGwu/p9nN4IssxCx6pPoTWSQArZwU6Pg6SMb8yXEx6JEmRksi50sA9XAN9t/0KFS un9iYfIsSbSedMgXz7qISTsPobM2EhRZeGNTszqoKpOqZBIxAIIhawh/1ZRR61mz/pON IEY8tG/DqPy/tdvv6smGyAKox36GYMvjEpZHtf71+DsxCefezDJTD4m6TIk6noeiL884 ts5A== X-Gm-Message-State: AOAM531RU9NP0zDHkETOqjZJ2WBaJZT30hVJHnCzIKSf3OrZJjwF/+Ey CZWYBB71NyYKrrciCNgc3QILaA== X-Google-Smtp-Source: ABdhPJzTUD0oFRNw0IQVOXXk7lmbMkIiaQnjBK48SaC4z3cC6zZdqjSCReuy76KFYeqYqvPoe8LSzw== X-Received: by 2002:ac8:45d7:: with SMTP id e23mr4636211qto.107.1616510878247; Tue, 23 Mar 2021 07:47:58 -0700 (PDT) Received: from [192.168.1.132] ([177.194.41.149]) by smtp.gmail.com with ESMTPSA id k4sm13709826qke.13.2021.03.23.07.47.56 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 23 Mar 2021 07:47:57 -0700 (PDT) Subject: Re: [PATCH v3 20/37] dlfcn: dlerror needs to call free from the base namespace [BZ #24773] To: libc-alpha@sourceware.org, Florian Weimer References: From: Adhemerval Zanella Message-ID: <55d65ab2-235b-12e6-667b-10aa8b4c125f@linaro.org> Date: Tue, 23 Mar 2021 11:47:55 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, 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-List-Received-Date: Tue, 23 Mar 2021 14:48:02 -0000 On 16/03/2021 14:30, Florian Weimer via Libc-alpha wrote: > 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 > Ok. > 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; > } > } Ok. > 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); > - > - Ok. > 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"; > } Ok. > > - /* 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; > } Isn't this essentially '__libc_dlerror_result_free()' ? > - 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; Shouldn't we handle asprintf error here? > + > + 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; Ok. > @@ -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; > } > } > Ok. > - 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; > -} > - Ok. > - > -/* 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; > } Ok. > -} > - > - > -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 > Ok. > -/* 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) > Ok. > 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 @@ Missing header? I am never sure if it is required for internal headers. > +#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; > + } > +} > + Ok. > +static inline struct dl_action_result * > +dl_action_result_malloc_failed (void) > +{ > + return (struct dl_action_result *) (intptr_t) -1; > +} > + Why not use a static const variable as suggested by Joseph? > +/* 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 */ k. > 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); > -} Ok. > 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; > + } > +} Ok. > 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 Ok. > 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 Pl. > 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); > } > Ok. > 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); > } Ok. > 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; > } Ok. > 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 Ok. > 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 Ok. > @@ -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) Ok. > @@ -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); Ok. > 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. */ Ok. > 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. */ > Ok.