Message ID | 20220328213641.8667-1-dirk@dmllr.de |
---|---|
State | Superseded |
Headers |
Return-Path: <elfutils-devel-bounces+patchwork=sourceware.org@sourceware.org> 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 9A1E53858C50 for <patchwork@sourceware.org>; Mon, 28 Mar 2022 21:36:55 +0000 (GMT) X-Original-To: elfutils-devel@sourceware.org Delivered-To: elfutils-devel@sourceware.org Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id 88C8A3858C50 for <elfutils-devel@sourceware.org>; Mon, 28 Mar 2022 21:36:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 88C8A3858C50 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=dmllr.de Authentication-Results: sourceware.org; spf=none smtp.mailfrom=dmllr.de Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id B02691FDAC; Mon, 28 Mar 2022 21:36:44 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 9AC081332D; Mon, 28 Mar 2022 21:36:44 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id dPWtJGwqQmIZZQAAMHmgww (envelope-from <dirk@dmllr.de>); Mon, 28 Mar 2022 21:36:44 +0000 From: =?utf-8?q?Dirk_M=C3=BCller?= <dirk@dmllr.de> To: elfutils-devel@sourceware.org Subject: [PATCH] Avoid dlopen on debuginfod-client when not configured Date: Mon, 28 Mar 2022 23:36:41 +0200 Message-Id: <20220328213641.8667-1-dirk@dmllr.de> X-Mailer: git-send-email 2.35.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.7 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: elfutils-devel@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Elfutils-devel mailing list <elfutils-devel.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/elfutils-devel>, <mailto:elfutils-devel-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/elfutils-devel/> List-Help: <mailto:elfutils-devel-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/elfutils-devel>, <mailto:elfutils-devel-request@sourceware.org?subject=subscribe> Cc: =?utf-8?q?Dirk_M=C3=BCller?= <dirk@dmllr.de> Errors-To: elfutils-devel-bounces+patchwork=sourceware.org@sourceware.org Sender: "Elfutils-devel" <elfutils-devel-bounces+patchwork=sourceware.org@sourceware.org> |
Series |
Avoid dlopen on debuginfod-client when not configured
|
|
Commit Message
Dirk Müller
March 28, 2022, 9:36 p.m. UTC
Loading debuginfod-client is expensive, especially when
FIPS is enabled, as it goes through time intensive self-checks
on load. Avoid dlopen() when no debuginfo url is set.
Signed-off-by: Dirk Müller <dirk@dmllr.de>
---
libdwfl/ChangeLog | 5 +++++
libdwfl/debuginfod-client.c | 12 +++++++++++-
2 files changed, 16 insertions(+), 1 deletion(-)
Comments
Hi Dirk, On Mon, Mar 28, 2022 at 11:36:41PM +0200, Dirk Müller wrote: > Loading debuginfod-client is expensive, especially when > FIPS is enabled, as it goes through time intensive self-checks > on load. I assume this is because debuginfod-client loads and initializes libcurl, which triggers crypto checks for https support? > Avoid dlopen() when no debuginfo url is set. The patch itself looks right. But I am slightly afraid this (theoretically?) will break some programs which set DEBUGINFOD_URLS themselves. We currently have no other way to make libdwfl use debuginfod-client. Thoughts? Cheers, Mark > Signed-off-by: Dirk Müller <dirk@dmllr.de> > --- > libdwfl/ChangeLog | 5 +++++ > libdwfl/debuginfod-client.c | 12 +++++++++++- > 2 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog > index 9c5c8517..1ec13f30 100644 > --- a/libdwfl/ChangeLog > +++ b/libdwfl/ChangeLog > @@ -1,3 +1,8 @@ > +2022-03-28 Dirk Müller <dirk@dmllr.de> > + > + * debuginfod-client.c (__libdwfl_debuginfod_init): Skip dlopen > + if debuginfod url is unset. > + > 2022-02-18 Mark Wielaard <mark@klomp.org> > > * image-header.c (__libdw_image_header): Assign header values for > diff --git a/libdwfl/debuginfod-client.c b/libdwfl/debuginfod-client.c > index 99b66b6e..af9d1363 100644 > --- a/libdwfl/debuginfod-client.c > +++ b/libdwfl/debuginfod-client.c > @@ -101,7 +101,17 @@ __libdwfl_debuginfod_end (debuginfod_client *c) > void __attribute__ ((constructor)) > __libdwfl_debuginfod_init (void) > { > - void *debuginfod_so = dlopen(DEBUGINFOD_SONAME, RTLD_LAZY); > + void *debuginfod_so; > + > + /* Is there any server we can query? If not, don't do any work, > + just return with ENOSYS. Don't even access the cache. */ > + char *urls_envvar = getenv(DEBUGINFOD_URLS_ENV_VAR); > + if (urls_envvar == NULL || urls_envvar[0] == '\0') > + { > + return; > + } > + > + debuginfod_so = dlopen(DEBUGINFOD_SONAME, RTLD_LAZY); > > if (debuginfod_so != NULL) > { > -- > 2.35.1 >
Hi - > > Avoid dlopen() when no debuginfo url is set. > > The patch itself looks right. But I am slightly afraid this > (theoretically?) will break some programs which set DEBUGINFOD_URLS > themselves. We currently have no other way to make libdwfl use > debuginfod-client. Thoughts? I've been thinking that doing all this dlopen work in the real dwfl function(s) rather than the solib ctor would be fine. - FChE
Hi Mark, Am Di., 29. März 2022 um 09:42 Uhr schrieb Mark Wielaard <mark@klomp.org>: > I assume this is because debuginfod-client loads and initializes > libcurl, which triggers crypto checks for https support? Yes, the dependencies of the DSO contain libcurl and other libraries including the openssl library, which, when patched with FIPS support, take quite a while to initialize. > The patch itself looks right. But I am slightly afraid this > (theoretically?) will break some programs which set DEBUGINFOD_URLS > themselves. We currently have no other way to make libdwfl use > debuginfod-client. Thoughts? I was concerned about that as well, but I couldn't google a case where this is the case. What we can also do is to do the initialization lazily upon first use rather than in a constructor function, but the intention of initializing early has been specified as avoiding issues in multiple threads with libcurl: commit fa0226a78a101d26fd80c7e9e70a5f0aa6f9d1cc Author: Mark Wielaard <mark@klomp.org> Date: Sun Nov 17 22:17:26 2019 +0100 debuginfod: add client context Add a mandatory debuginfod_begin()/_end() call pair to manage a client object that represents persistent but non-global state. From libdwfl, dlopen the debuginfod.so client library early on. This hopefully makes sure that the code (and the libcurl.so dependency) is loaded before the program goes into multi-threaded mode. What we can do is both, do the reinit at a later point in time if the environment variable happens to be set at that point in time. then you are risking the thread-safety issues only when there is no other way. Or just do it lazily on demand, and work through the libcurl or whatever the unsafety issue is. Greetings, Dirk
Hi Dirk, On Tue, 2022-03-29 at 13:06 +0200, Dirk Müller wrote: > Am Di., 29. März 2022 um 09:42 Uhr schrieb Mark Wielaard < > mark@klomp.org>: > > The patch itself looks right. But I am slightly afraid this > > (theoretically?) will break some programs which set DEBUGINFOD_URLS > > themselves. We currently have no other way to make libdwfl use > > debuginfod-client. Thoughts? > > I was concerned about that as well, but I couldn't google a case where > this is the case. What we can also do is to do the initialization > lazily upon first use rather than in a > constructor function, but the intention of initializing early has been > specified as avoiding issues in multiple threads with libcurl: > > commit fa0226a78a101d26fd80c7e9e70a5f0aa6f9d1cc > Author: Mark Wielaard <mark@klomp.org> > Date: Sun Nov 17 22:17:26 2019 +0100 > > debuginfod: add client context > > Add a mandatory debuginfod_begin()/_end() call pair to manage a client > object that represents persistent but non-global state. From libdwfl, > dlopen the debuginfod.so client library early on. This hopefully > makes sure that the code (and the libcurl.so dependency) is loaded > before the program goes into multi-threaded mode. > > > What we can do is both, do the reinit at a later point in time if the > environment variable happens to be set at that point in time. then you > are risking the thread-safety issues only when there is no other way. > > Or just do it lazily on demand, and work through the libcurl or > whatever the unsafety issue is. This is indeed the (safety) issue I am concerned about. I just sent an email to the libcurl mailinglist (CCed to the elfutils-devel list) to ask about how/when/if a library should call curl_global_init. https://curl.se/mail/lib-2022-03/0118.html Lets see what the curl developers say and then decide what to do. Cheers, Mark
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog index 9c5c8517..1ec13f30 100644 --- a/libdwfl/ChangeLog +++ b/libdwfl/ChangeLog @@ -1,3 +1,8 @@ +2022-03-28 Dirk Müller <dirk@dmllr.de> + + * debuginfod-client.c (__libdwfl_debuginfod_init): Skip dlopen + if debuginfod url is unset. + 2022-02-18 Mark Wielaard <mark@klomp.org> * image-header.c (__libdw_image_header): Assign header values for diff --git a/libdwfl/debuginfod-client.c b/libdwfl/debuginfod-client.c index 99b66b6e..af9d1363 100644 --- a/libdwfl/debuginfod-client.c +++ b/libdwfl/debuginfod-client.c @@ -101,7 +101,17 @@ __libdwfl_debuginfod_end (debuginfod_client *c) void __attribute__ ((constructor)) __libdwfl_debuginfod_init (void) { - void *debuginfod_so = dlopen(DEBUGINFOD_SONAME, RTLD_LAZY); + void *debuginfod_so; + + /* Is there any server we can query? If not, don't do any work, + just return with ENOSYS. Don't even access the cache. */ + char *urls_envvar = getenv(DEBUGINFOD_URLS_ENV_VAR); + if (urls_envvar == NULL || urls_envvar[0] == '\0') + { + return; + } + + debuginfod_so = dlopen(DEBUGINFOD_SONAME, RTLD_LAZY); if (debuginfod_so != NULL) {