public inbox for elfutils@sourceware.org
 help / color / mirror / Atom feed
From: Mark Wielaard <mark@klomp.org>
To: elfutils-devel@sourceware.org
Cc: "Dirk Müller" <dirk@dmllr.de>, "Mark Wielaard" <mark@klomp.org>
Subject: [PATCH] debuginfod, libdwfl: Initialize libcurl and dlopen debuginfod-client lazily
Date: Fri, 22 Apr 2022 23:53:43 +0200	[thread overview]
Message-ID: <20220422215343.270433-1-mark@klomp.org> (raw)

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


             reply	other threads:[~2022-04-22 21:53 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-22 21:53 Mark Wielaard [this message]
2022-04-25 14:18 ` Mark Wielaard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220422215343.270433-1-mark@klomp.org \
    --to=mark@klomp.org \
    --cc=dirk@dmllr.de \
    --cc=elfutils-devel@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).