[PATCH] stdlib: Make __cxa_thread_atexit_impl lock-free (BZ 15686)

Adhemerval Zanella <[email protected]> Thu, 30 Jul 2026 15:26:44 -0300
Newsgroups gmane.comp.lib.glibc.alpha
Message-ID <[email protected]>
__cxa_thread_atexit_impl takes dl_load_lock to protect the
_dl_find_dso_for_object lookup and the l_tls_dtor_count increment
against a racing dlclose.  This deadlocks when the function is
reached from a thread spawned by an ELF constructor, because dlopen
runs constructors with dl_load_lock held.

Instead of releasing dl_load_lock around constructor execution
(which requires per-map serialization of the constructor calls and
re-auditing all the state accessed by dl_open_worker after the
initializers run), remove the lock acquisition by using
_dl_find_object instead.

The lock is not required for correctness, DSO_SYMBOL is the address
of the caller's __dso_handle, so the calling thread is executing
code of the object being looked up.  A dlclose that unloaded the
object concurrently would unmap the running code itself, which is
undefined regardless of the lock.

The per-thread dso_symbol_cache/lm_cache is also removed, the cache
key was never updated.

Using _dl_find_object also shows a slight better performance, it
replaces a O(n) by a lock-free O(log n) lookup.

This does not fully fix BZ 15686.  dlopen still runs ELF
constructors with dl_load_lock held, so a thread spawned by a
constructor that the constructor then joins still deadlocks if it
calls dlopen and related functions; or if it lazily binds to a
symbol defined in another dlopen'ed object (which reaches
add_dependency in elf/dl-lookup.c).  Only the TLS destructor
registration path is addressed here.

Checked on aarch64-linux-gnu, x86_64-linux-gnu, i686-linux-gnu, and
arm-linux-gnueabihf.
---
 stdlib/cxa_thread_atexit_impl.c  | 81 +++++++++++++++++---------------
 sysdeps/pthread/Makefile         |  6 +++
 sysdeps/pthread/tst-create2.c    | 52 ++++++++++++++++++++
 sysdeps/pthread/tst-create2mod.c | 51 ++++++++++++++++++++
 4 files changed, 152 insertions(+), 38 deletions(-)
 create mode 100644 sysdeps/pthread/tst-create2.c
 create mode 100644 sysdeps/pthread/tst-create2mod.c

diff --git a/stdlib/cxa_thread_atexit_impl.c b/stdlib/cxa_thread_atexit_impl.c
index c4382bc9efe..c162461afb5 100644
--- a/stdlib/cxa_thread_atexit_impl.c
+++ b/stdlib/cxa_thread_atexit_impl.c
@@ -25,15 +25,24 @@
    combinations of all three functions are the link map list, a link map for a
    DSO and the link map member l_tls_dtor_count.
 
-   __cxa_thread_atexit_impl acquires the dl_load_lock before accessing any
-   shared state and hence multiple of its instances can safely execute
-   concurrently.
+   __cxa_thread_atexit_impl does not take dl_load_lock (taking it deadlocks
+   if this function is reached from a thread spawned by an ELF constructor,
+   because dlopen runs constructors with dl_load_lock held).  It locates the
+   caller's link map with _dl_find_object, which is async-signal-safe and
+   lock-free, and then increments l_tls_dtor_count atomically.
 
-   _dl_close_worker acquires the dl_load_lock before accessing any shared state
-   as well and hence can concurrently execute multiple of its own instances as
-   well as those of __cxa_thread_atexit_impl safely.  Not all accesses to
-   l_tls_dtor_count are protected by the dl_load_lock, so we need to
-   synchronize using atomics.
+   Not taking the lock is safe because DSO_SYMBOL is the address of the
+   caller's __dso_handle, so the calling thread is executing code of the very
+   object.  A concurrent dlclose that unloads the object while this function
+   runs would unmap the caller's code as well, which is undefined.  A
+   conforming program must ensure, via its own synchronization, that the
+   object stays loaded across this call, and that same synchronization
+   publishes the l_tls_dtor_count increment to any subsequent
+   _dl_close_worker.
+
+   _dl_close_worker acquires the dl_load_lock before accessing any shared
+   state.  Not all accesses to l_tls_dtor_count are protected by the
+   dl_load_lock, so we need to synchronize using atomics.
 
    __call_tls_dtors accesses the l_tls_dtor_count without taking the lock; it
    decrements the value by one.  It does not need the big lock because it does
@@ -63,7 +72,9 @@
 
    Concurrent executions of __call_tls_dtors should only ensure that the value
    is accessed atomically; no reordering constraints need to be considered.
-   Likewise for the increment of l_tls_dtor_count in __cxa_thread_atexit_impl.
+   The same holds for the increment in __cxa_thread_atexit_impl, whose
+   ordering against _dl_close_worker is provided by the caller as described
+   above.
 
    There is still a possibility on concurrent execution of _dl_close_worker and
    __call_tls_dtors where _dl_close_worker reads the value of l_tls_dtor_count
@@ -72,6 +83,7 @@
    is not very different from a case where __call_tls_dtors is called after
    _dl_close_worker on the DSO and hence is an accepted execution.  */
 
+#include <dlfcn.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <ldsodefs.h>
@@ -88,8 +100,6 @@ struct dtor_list
 };
 
 static __thread struct dtor_list *tls_dtor_list;
-static __thread void *dso_symbol_cache;
-static __thread struct link_map *lm_cache;
 
 /* Register a destructor for TLS variables declared with the 'thread_local'
    keyword.  This function is only called from code generated by the C++
@@ -102,42 +112,37 @@ __cxa_thread_atexit_impl (dtor_func func, void *obj, void *dso_symbol)
 {
   PTR_MANGLE (func);
 
-  /* Prepend.  */
   struct dtor_list *new = calloc (1, sizeof (struct dtor_list));
   if (__glibc_unlikely (new == NULL))
     __libc_fatal ("Fatal glibc error: failed to register TLS destructor: "
 		  "out of memory\n");
+
+  /* A concurrent dlclose may already have reset the link map of a matching
+     entry, so check for it as well.  Either way the object is being unloaded
+     from underneath the caller, which is undefined; assume the main program
+     as for an unrecognized address.  */
+  struct link_map *map;
+  struct dl_find_object dfo;
+  if (GLRO (dl_find_object) (dso_symbol, &dfo) == 0
+      && dfo.dlfo_link_map != NULL)
+    map = dfo.dlfo_link_map;
+  else
+    map = GL(dl_ns)[LM_ID_BASE]._ns_loaded;
+
   new->func = func;
   new->obj = obj;
+  new->map = map;
+
+  /* This increment is only concurrently observed by the decrement in
+     __call_tls_dtors and by the load in _dl_close_worker.  For the latter,
+     the caller's own synchronization with dlclose provides the required
+     ordering (see CONCURRENCY NOTES), so Relaxed MO is sufficient.  */
+  atomic_fetch_add_relaxed (&map->l_tls_dtor_count, 1);
+
+  /* Prepend.  */
   new->next = tls_dtor_list;
   tls_dtor_list = new;
 
-  /* We have to acquire the big lock to prevent a racing dlclose from pulling
-     our DSO from underneath us while we're setting up our destructor.  */
-  __rtld_lock_lock_recursive (GL(dl_load_lock));
-
-  /* See if we already encountered the DSO.  */
-  if (__glibc_unlikely (dso_symbol_cache != dso_symbol))
-    {
-      ElfW(Addr) caller = (ElfW(Addr)) dso_symbol;
-
-      struct link_map *l = _dl_find_dso_for_object (caller);
-
-      /* If the address is not recognized the call comes from the main
-	 program (we hope).  */
-      lm_cache = l ? l : GL(dl_ns)[LM_ID_BASE]._ns_loaded;
-    }
-
-  /* This increment may only be concurrently observed either by the decrement
-     in __call_tls_dtors since the other l_tls_dtor_count access in
-     _dl_close_worker is protected by the dl_load_lock.  The execution in
-     __call_tls_dtors does not really depend on this value beyond the fact that
-     it should be atomic, so Relaxed MO should be sufficient.  */
-  atomic_fetch_add_relaxed (&lm_cache->l_tls_dtor_count, 1);
-  __rtld_lock_unlock_recursive (GL(dl_load_lock));
-
-  new->map = lm_cache;
-
   return 0;
 }
 
diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile
index d0f3cd59ac6..5b062094ec4 100644
--- a/sysdeps/pthread/Makefile
+++ b/sysdeps/pthread/Makefile
@@ -349,6 +349,7 @@ tests += \
   tst-atfork3 \
   tst-atfork4 \
   tst-create1 \
+  tst-create2 \
   tst-fini1 \
   tst-pt-tls4 \
   # tests
@@ -365,6 +366,7 @@ modules-names += \
   tst-atfork3mod \
   tst-atfork4mod \
   tst-create1mod \
+  tst-create2mod \
   tst-fini1mod \
   tst-stack2-mod \
   tst-tls4moda \
@@ -540,6 +542,10 @@ LDFLAGS-tst-create1 = -Wl,-export-dynamic
 $(objpfx)tst-create1: $(shared-thread-library)
 $(objpfx)tst-create1.out: $(objpfx)tst-create1mod.so
 
+$(objpfx)tst-create2: $(shared-thread-library)
+$(objpfx)tst-create2mod.so: $(libsupport) $(shared-thread-library)
+$(objpfx)tst-create2.out: $(objpfx)tst-create2mod.so
+
 $(objpfx)tst-stack2.out: $(objpfx)tst-stack2-mod.so
 $(objpfx)tst-stack2-mod.so: $(shared-thread-library)
 LDFLAGS-tst-stack2-mod.so = -Wl,-z,execstack
diff --git a/sysdeps/pthread/tst-create2.c b/sysdeps/pthread/tst-create2.c
new file mode 100644
index 00000000000..b5c81dc64ff
--- /dev/null
+++ b/sysdeps/pthread/tst-create2.c
@@ -0,0 +1,52 @@
+/* Verify that a thread spawned by a dlopen constructor can register a
+   TLS destructor without deadlocking (BZ 15686).
+   Copyright (C) 2026 Free Software Foundation, Inc.
+   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
+   <https://www.gnu.org/licenses/>.  */
+
+/* thread 1: dlopen -> ctor -> pthread_create (worker) -> pthread_join
+   thread 2 (worker): __cxa_thread_atexit_impl -> lock (dl_load_lock)
+
+   dl_load_lock is held by thread 1 across the constructor execution, so if
+   __cxa_thread_atexit_impl acquires it the worker thread blocks forever and
+   pthread_join in the constructor never returns.  */
+
+#include <support/check.h>
+#include <support/xdlfcn.h>
+
+static int
+do_test (void)
+{
+  void *h = xdlopen ("tst-create2mod.so", RTLD_NOW);
+
+  /* The worker thread exited before the constructor's pthread_join
+     returned, so its TLS destructor has already run.  */
+  int *dtor_done = xdlsym (h, "tst_create2mod_dtor_done");
+  TEST_COMPARE (*dtor_done, 1);
+
+  xdlclose (h);
+
+  /* The destructor already ran, so no reference is left on the module's
+     l_tls_dtor_count and dlclose must have unloaded it.  */
+  void *h2 = dlopen ("tst-create2mod.so", RTLD_NOW | RTLD_NOLOAD);
+  TEST_VERIFY (h2 == NULL);
+  if (h2 != NULL)
+    xdlclose (h2);
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/pthread/tst-create2mod.c b/sysdeps/pthread/tst-create2mod.c
new file mode 100644
index 00000000000..3ec31ce15ea
--- /dev/null
+++ b/sysdeps/pthread/tst-create2mod.c
@@ -0,0 +1,51 @@
+/* Verify that a thread spawned by a dlopen constructor can register a
+   TLS destructor without deadlocking (BZ 15686).
+   Copyright (C) 2026 Free Software Foundation, Inc.
+   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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <stdlib.h>
+#include <dso_handle.h>
+#include <support/check.h>
+#include <support/xthread.h>
+
+int tst_create2mod_dtor_done;
+
+static void
+dtor (void *obj)
+{
+  *(int *) obj = 1;
+}
+
+/* The module TLS access mirrors the real-world trigger (a C++ thread_local
+   or Rust thread_local! first access), exercising __tls_get_addr from the
+   spawned thread as well.  */
+static __thread int tls_obj;
+
+static void *
+worker (void *closure)
+{
+  tls_obj = 1;
+  TEST_COMPARE (__cxa_thread_atexit_impl (dtor, &tst_create2mod_dtor_done,
+					  __dso_handle), 0);
+  return NULL;
+}
+
+static void __attribute__ ((constructor))
+do_init (void)
+{
+  xpthread_join (xpthread_create (NULL, worker, NULL));
+}
-- 
2.53.0