[PATCH] debuginfod, libdwfl: Initialize libcurl and dlopen debuginfod-client lazily

[PATCH] debuginfod, libdwfl: Initialize libcurl and dlopen debuginfod-client lazily

[Mark Wielaard]

We used to go out of our way to initialize libcurl early before any other
thread/code was running. But this meant that we might pay startup cost,
which under FIPS is significant, even for code that never uses libdebuginfod
or TLS libcurl connections. Although curl_global_init itself isn't thread-safe
we can use pthread_once to make sure we don't race against ourselves. This
still means we might race against any application code that might use
libcurl. But we can assume they will have called curl_global_init before
calling dwfl_begin or debuginfod_begin.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 debuginfod/ChangeLog           | 10 ++++++++++
 debuginfod/Makefile.am         |  4 ++--
 debuginfod/debuginfod-client.c | 29 ++++++++++++++---------------
 libdwfl/ChangeLog              |  7 +++++++
 libdwfl/debuginfod-client.c    | 14 +++++++++++---
 5 files changed, 44 insertions(+), 20 deletions(-)

diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index d6f7b282..7d790bf9 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,13 @@
+2022-04-22  Mark Wielaard  <mark@klomp.org>
+
+	* Makefile.am (libdebuginfod): Add -lpthread.
+	(libdebuginfod_so_LDLIBS): Likewise.
+	* debuginfod-client.c (init_control): New static pthread_once_t.
+	(libcurl_init): New static function.
+	(debuginfod_begin): Use ptrace_once to call libcurl_init.
+	(libdebuginfod_ctor): Removed.
+	(libdebuginfod_dtor): Likewise.
+
 2022-04-13  Aaron Merey  <amerey@redhat.com>
 
 	* debuginfod-client.c (debuginfod_query_server):
diff --git a/debuginfod/Makefile.am b/debuginfod/Makefile.am
index 3adb2755..435cb8a6 100644
--- a/debuginfod/Makefile.am
+++ b/debuginfod/Makefile.am
@@ -47,7 +47,7 @@ libelf = ../libelf/libelf.a -lz
 if DUMMY_LIBDEBUGINFOD
 libdebuginfod = ./libdebuginfod.a
 else
-libdebuginfod = ./libdebuginfod.a $(libcurl_LIBS)
+libdebuginfod = ./libdebuginfod.a -lpthread $(libcurl_LIBS)
 endif
 else
 libasm = ../libasm/libasm.so
@@ -97,7 +97,7 @@ libdebuginfod_so_LIBS = libdebuginfod_pic.a
 if DUMMY_LIBDEBUGINFOD
 libdebuginfod_so_LDLIBS =
 else
-libdebuginfod_so_LDLIBS = $(libcurl_LIBS) $(fts_LIBS)
+libdebuginfod_so_LDLIBS = -lpthread $(libcurl_LIBS) $(fts_LIBS)
 endif
 $(LIBDEBUGINFOD_SONAME): $(srcdir)/libdebuginfod.map $(libdebuginfod_so_LIBS)
 	$(AM_V_CCLD)$(LINK) $(dso_LDFLAGS) -o $@ \
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index 58ef6442..45a27b5e 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -1,6 +1,6 @@
 /* Retrieve ELF / DWARF / source files from the debuginfod.
    Copyright (C) 2019-2021 Red Hat, Inc.
-   Copyright (C) 2021 Mark J. Wielaard <mark@klomp.org>
+   Copyright (C) 2021, 2022 Mark J. Wielaard <mark@klomp.org>
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -98,6 +98,16 @@ void debuginfod_end (debuginfod_client *c) { }
   #include <fts.h>
 #endif
 
+#include <pthread.h>
+
+static pthread_once_t init_control = PTHREAD_ONCE_INIT;
+
+static void
+libcurl_init(void)
+{
+  curl_global_init(CURL_GLOBAL_DEFAULT);
+}
+
 struct debuginfod_client
 {
   /* Progress/interrupt callback function. */
@@ -1475,6 +1485,9 @@ debuginfod_query_server (debuginfod_client *c,
 debuginfod_client  *
 debuginfod_begin (void)
 {
+  /* Initialize libcurl lazily, but only once.  */
+  pthread_once (&init_control, libcurl_init);
+
   debuginfod_client *client;
   size_t size = sizeof (struct debuginfod_client);
   client = calloc (1, size);
@@ -1608,18 +1621,4 @@ debuginfod_set_verbose_fd(debuginfod_client *client, int fd)
   client->verbose_fd = fd;
 }
 
-
-/* NB: these are thread-unsafe. */
-__attribute__((constructor)) attribute_hidden void libdebuginfod_ctor(void)
-{
-  curl_global_init(CURL_GLOBAL_DEFAULT);
-}
-
-/* NB: this is very thread-unsafe: it breaks other threads that are still in libcurl */
-__attribute__((destructor)) attribute_hidden void libdebuginfod_dtor(void)
-{
-  /* ... so don't do this: */
-  /* curl_global_cleanup(); */
-}
-
 #endif /* DUMMY_LIBDEBUGINFOD */
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 9c5c8517..76053039 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,10 @@
+2022-04-22  Mark Wielaard  <mark@klomp.org>
+
+	* debuginfod-client.c (init_control): New static pthread_once_t.
+	(get_client): Use pthread_once to call __libdwfl_debuginfod_init.
+	(__libdwfl_debuginfod_init): Make static, remove attribute
+	constructor.
+
 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..153260c3 100644
--- a/libdwfl/debuginfod-client.c
+++ b/libdwfl/debuginfod-client.c
@@ -1,5 +1,6 @@
 /* Try to get an ELF or debug file through the debuginfod.
    Copyright (C) 2019 Red Hat, Inc.
+   Copyright (C) 2022 Mark J. Wielaard <mark@klomp.org>
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -31,6 +32,7 @@
 #endif
 
 #include "libdwflP.h"
+#include <pthread.h>
 #include <dlfcn.h>
 
 static __typeof__ (debuginfod_begin) *fp_debuginfod_begin;
@@ -38,6 +40,10 @@ static __typeof__ (debuginfod_find_executable) *fp_debuginfod_find_executable;
 static __typeof__ (debuginfod_find_debuginfo) *fp_debuginfod_find_debuginfo;
 static __typeof__ (debuginfod_end) *fp_debuginfod_end;
 
+static void __libdwfl_debuginfod_init (void);
+
+static pthread_once_t init_control = PTHREAD_ONCE_INIT;
+
 /* NB: this is slightly thread-unsafe */
 
 static debuginfod_client *
@@ -46,6 +52,8 @@ get_client (Dwfl *dwfl)
   if (dwfl->debuginfod != NULL)
     return dwfl->debuginfod;
 
+  pthread_once (&init_control, __libdwfl_debuginfod_init);
+
   if (fp_debuginfod_begin != NULL)
     {
       dwfl->debuginfod = (*fp_debuginfod_begin) ();
@@ -96,9 +104,9 @@ __libdwfl_debuginfod_end (debuginfod_client *c)
     (*fp_debuginfod_end) (c);
 }
 
-/* Try to get the libdebuginfod library functions to make sure
-   everything is initialized early.  */
-void __attribute__ ((constructor))
+/* Try to get the libdebuginfod library functions.
+   Only needs to be called once from get_client.  */
+static void
 __libdwfl_debuginfod_init (void)
 {
   void *debuginfod_so = dlopen(DEBUGINFOD_SONAME, RTLD_LAZY);
-- 
2.30.2

Re: [PATCH] debuginfod, libdwfl: Initialize libcurl and dlopen debuginfod-client lazily

[Mark Wielaard]

On Fri, Apr 22, 2022 at 11:53:43PM +0200, Mark Wielaard wrote:
> We used to go out of our way to initialize libcurl early before any other
> thread/code was running. But this meant that we might pay startup cost,
> which under FIPS is significant, even for code that never uses libdebuginfod
> or TLS libcurl connections. Although curl_global_init itself isn't thread-safe
> we can use pthread_once to make sure we don't race against ourselves. This
> still means we might race against any application code that might use
> libcurl. But we can assume they will have called curl_global_init before
> calling dwfl_begin or debuginfod_begin.

Pushed.

Mark