[PATCH] Avoid dlopen on debuginfod-client when not configured

2022-03-28 Thread Dirk Müller
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 
---
 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 
+
+   * debuginfod-client.c (__libdwfl_debuginfod_init): Skip dlopen
+   if debuginfod url is unset.
+
 2022-02-18  Mark Wielaard  
 
* 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



Re: [PATCH] Avoid dlopen on debuginfod-client when not configured

2022-03-29 Thread Dirk Müller
Hi Mark,

Am Di., 29. März 2022 um 09:42 Uhr schrieb Mark Wielaard :

> 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 
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