From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x834.google.com (mail-qt1-x834.google.com [IPv6:2607:f8b0:4864:20::834]) by sourceware.org (Postfix) with ESMTPS id D83F83857C73 for ; Fri, 19 Mar 2021 19:56:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org D83F83857C73 Received: by mail-qt1-x834.google.com with SMTP id x9so7712520qto.8 for ; Fri, 19 Mar 2021 12:56:43 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:to:references:from:subject:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=XOg5iQfiGuKGTV6VS+1GuFi0uCsXoPPVH1euIOeic2A=; b=Y4UAHlWWcumYepsAkbmZv3unToIK0OSWJ+ZANmd5Lw12gCHNm3G4xb9aA0h2OUuO5z LdSPw10YmC5vhM4NHwmJQoxdw45BlnqJCawsztS7g5K5bfzkJK2MJoxSGD7SPpx6rG3x W+x29oOr8UswloLGn9IncZWDUqeLp6oIZ8mXbvptouq8YqaRvp35dZc2LsQRfM6wckHX kbNdobFWaFtQHuaZmxu5p/ICuwhH5whzgTYG0OoOTr4GxZNShOIQJ6T1NYTq6m/BnOFa XjoPjuWysIyHuHcPUwgmcnS5xWTTxmtj2rrf7ZlP/P/o+ugD6jLiiQb0rob2pMoTV7yl OcBA== X-Gm-Message-State: AOAM5335QGGsxSYyM9hdwE9YYkRRvbIr0LA6CyHSxC1KSWZYsVzt5y4h 9NeENiP1kNtYnGsic6/HcA1nSN2kE0Tx5rvW X-Google-Smtp-Source: ABdhPJyXtlXLKcnptNKOP8N0aP0BO+Rn7I38MD38bIvPl1XUMsN8Hyo0t+eMe5LztUdUzzc8uZd+UQ== X-Received: by 2002:ac8:7a95:: with SMTP id x21mr267105qtr.209.1616183803228; Fri, 19 Mar 2021 12:56:43 -0700 (PDT) Received: from [192.168.1.4] ([177.194.48.209]) by smtp.googlemail.com with ESMTPSA id r2sm4439979qti.4.2021.03.19.12.56.42 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 19 Mar 2021 12:56:42 -0700 (PDT) To: Florian Weimer , libc-alpha@sourceware.org References: From: Adhemerval Zanella Subject: Re: [PATCH v3 19/37] dlfcn: Failures after dlmopen should not terminate process [BZ #24772] Message-ID: Date: Fri, 19 Mar 2021 16:56:40 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.6 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: Fri, 19 Mar 2021 19:56:46 -0000 On 16/03/2021 14:30, Florian Weimer via Libc-alpha wrote: > 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. LGTM, thanks. Reviewed-by: Adhemerval Zanella > --- > 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; Ok. > diff --git a/elf/Makefile b/elf/Makefile > index 936d4cf276..deb76aed99 100644 > --- a/elf/Makefile > +++ b/elf/Makefile > @@ -222,7 +222,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 \ > @@ -344,6 +345,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. > @@ -1580,6 +1582,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 = \ Ok. > 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 */ Ok, but why change the usual prepend string to 'rtld'? > 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 Ok. > 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. s/2019/2021. > + 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"); > +} Ok. > 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. s/2019/2021. > + 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 Ok. > 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 > Ok.