faf38262bf
Resolves: #2012169
467 lines
16 KiB
Diff
467 lines
16 KiB
Diff
commit 024a7640ab9ecea80e527f4e4d7f7a1868e952c5
|
|
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
|
|
Date: Wed Sep 15 15:16:19 2021 +0100
|
|
|
|
elf: Avoid deadlock between pthread_create and ctors [BZ #28357]
|
|
|
|
The fix for bug 19329 caused a regression such that pthread_create can
|
|
deadlock when concurrent ctors from dlopen are waiting for it to finish.
|
|
Use a new GL(dl_load_tls_lock) in pthread_create that is not taken
|
|
around ctors in dlopen.
|
|
|
|
The new lock is also used in __tls_get_addr instead of GL(dl_load_lock).
|
|
|
|
The new lock is held in _dl_open_worker and _dl_close_worker around
|
|
most of the logic before/after the init/fini routines. When init/fini
|
|
routines are running then TLS is in a consistent, usable state.
|
|
In _dl_open_worker the new lock requires catching and reraising dlopen
|
|
failures that happen in the critical section.
|
|
|
|
The new lock is reinitialized in a fork child, to keep the existing
|
|
behaviour and it is kept recursive in case malloc interposition or TLS
|
|
access from signal handlers can retake it. It is not obvious if this
|
|
is necessary or helps, but avoids changing the preexisting behaviour.
|
|
|
|
The new lock may be more appropriate for dl_iterate_phdr too than
|
|
GL(dl_load_write_lock), since TLS state of an incompletely loaded
|
|
module may be accessed. If the new lock can replace the old one,
|
|
that can be a separate change.
|
|
|
|
Fixes bug 28357.
|
|
|
|
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
|
|
(cherry picked from commit 83b5323261bb72313bffcf37476c1b8f0847c736)
|
|
|
|
diff --git a/elf/dl-close.c b/elf/dl-close.c
|
|
index f39001cab981b723..cd7b9c9fe83a1a44 100644
|
|
--- a/elf/dl-close.c
|
|
+++ b/elf/dl-close.c
|
|
@@ -549,6 +549,9 @@ _dl_close_worker (struct link_map *map, bool force)
|
|
size_t tls_free_end;
|
|
tls_free_start = tls_free_end = NO_TLS_OFFSET;
|
|
|
|
+ /* Protects global and module specitic TLS state. */
|
|
+ __rtld_lock_lock_recursive (GL(dl_load_tls_lock));
|
|
+
|
|
/* We modify the list of loaded objects. */
|
|
__rtld_lock_lock_recursive (GL(dl_load_write_lock));
|
|
|
|
@@ -784,6 +787,9 @@ _dl_close_worker (struct link_map *map, bool force)
|
|
GL(dl_tls_static_used) = tls_free_start;
|
|
}
|
|
|
|
+ /* TLS is cleaned up for the unloaded modules. */
|
|
+ __rtld_lock_unlock_recursive (GL(dl_load_tls_lock));
|
|
+
|
|
#ifdef SHARED
|
|
/* Auditing checkpoint: we have deleted all objects. */
|
|
if (__glibc_unlikely (do_audit))
|
|
diff --git a/elf/dl-open.c b/elf/dl-open.c
|
|
index 41c7250bf630f978..bc68e2c376debd71 100644
|
|
--- a/elf/dl-open.c
|
|
+++ b/elf/dl-open.c
|
|
@@ -66,6 +66,9 @@ struct dl_open_args
|
|
libc_map value in the namespace in case of a dlopen failure. */
|
|
bool libc_already_loaded;
|
|
|
|
+ /* Set to true if the end of dl_open_worker_begin was reached. */
|
|
+ bool worker_continue;
|
|
+
|
|
/* Original parameters to the program and the current environment. */
|
|
int argc;
|
|
char **argv;
|
|
@@ -482,7 +485,7 @@ call_dl_init (void *closure)
|
|
}
|
|
|
|
static void
|
|
-dl_open_worker (void *a)
|
|
+dl_open_worker_begin (void *a)
|
|
{
|
|
struct dl_open_args *args = a;
|
|
const char *file = args->file;
|
|
@@ -774,6 +777,36 @@ dl_open_worker (void *a)
|
|
_dl_call_libc_early_init (libc_map, false);
|
|
}
|
|
|
|
+ args->worker_continue = true;
|
|
+}
|
|
+
|
|
+static void
|
|
+dl_open_worker (void *a)
|
|
+{
|
|
+ struct dl_open_args *args = a;
|
|
+
|
|
+ args->worker_continue = false;
|
|
+
|
|
+ {
|
|
+ /* Protects global and module specific TLS state. */
|
|
+ __rtld_lock_lock_recursive (GL(dl_load_tls_lock));
|
|
+
|
|
+ struct dl_exception ex;
|
|
+ int err = _dl_catch_exception (&ex, dl_open_worker_begin, args);
|
|
+
|
|
+ __rtld_lock_unlock_recursive (GL(dl_load_tls_lock));
|
|
+
|
|
+ if (__glibc_unlikely (ex.errstring != NULL))
|
|
+ /* Reraise the error. */
|
|
+ _dl_signal_exception (err, &ex, NULL);
|
|
+ }
|
|
+
|
|
+ if (!args->worker_continue)
|
|
+ return;
|
|
+
|
|
+ int mode = args->mode;
|
|
+ struct link_map *new = args->map;
|
|
+
|
|
/* Run the initializer functions of new objects. Temporarily
|
|
disable the exception handler, so that lazy binding failures are
|
|
fatal. */
|
|
diff --git a/elf/dl-support.c b/elf/dl-support.c
|
|
index 01557181753fb38d..d8c06ba7eb4c76ea 100644
|
|
--- a/elf/dl-support.c
|
|
+++ b/elf/dl-support.c
|
|
@@ -229,6 +229,13 @@ __rtld_lock_define_initialized_recursive (, _dl_load_lock)
|
|
list of loaded objects while an object is added to or removed from
|
|
that list. */
|
|
__rtld_lock_define_initialized_recursive (, _dl_load_write_lock)
|
|
+ /* This lock protects global and module specific TLS related data.
|
|
+ E.g. it is held in dlopen and dlclose when GL(dl_tls_generation),
|
|
+ GL(dl_tls_max_dtv_idx) or GL(dl_tls_dtv_slotinfo_list) are
|
|
+ accessed and when TLS related relocations are processed for a
|
|
+ module. It was introduced to keep pthread_create accessing TLS
|
|
+ state that is being set up. */
|
|
+__rtld_lock_define_initialized_recursive (, _dl_load_tls_lock)
|
|
|
|
|
|
#ifdef HAVE_AUX_VECTOR
|
|
diff --git a/elf/dl-tls.c b/elf/dl-tls.c
|
|
index 423e380f7ca654fe..40263cf586e74c64 100644
|
|
--- a/elf/dl-tls.c
|
|
+++ b/elf/dl-tls.c
|
|
@@ -532,7 +532,7 @@ _dl_allocate_tls_init (void *result)
|
|
size_t maxgen = 0;
|
|
|
|
/* Protects global dynamic TLS related state. */
|
|
- __rtld_lock_lock_recursive (GL(dl_load_lock));
|
|
+ __rtld_lock_lock_recursive (GL(dl_load_tls_lock));
|
|
|
|
/* Check if the current dtv is big enough. */
|
|
if (dtv[-1].counter < GL(dl_tls_max_dtv_idx))
|
|
@@ -606,7 +606,7 @@ _dl_allocate_tls_init (void *result)
|
|
listp = listp->next;
|
|
assert (listp != NULL);
|
|
}
|
|
- __rtld_lock_unlock_recursive (GL(dl_load_lock));
|
|
+ __rtld_lock_unlock_recursive (GL(dl_load_tls_lock));
|
|
|
|
/* The DTV version is up-to-date now. */
|
|
dtv[0].counter = maxgen;
|
|
@@ -745,7 +745,7 @@ _dl_update_slotinfo (unsigned long int req_modid)
|
|
|
|
Here the dtv needs to be updated to new_gen generation count.
|
|
|
|
- This code may be called during TLS access when GL(dl_load_lock)
|
|
+ This code may be called during TLS access when GL(dl_load_tls_lock)
|
|
is not held. In that case the user code has to synchronize with
|
|
dlopen and dlclose calls of relevant modules. A module m is
|
|
relevant if the generation of m <= new_gen and dlclose of m is
|
|
@@ -867,11 +867,11 @@ tls_get_addr_tail (GET_ADDR_ARGS, dtv_t *dtv, struct link_map *the_map)
|
|
if (__glibc_unlikely (the_map->l_tls_offset
|
|
!= FORCED_DYNAMIC_TLS_OFFSET))
|
|
{
|
|
- __rtld_lock_lock_recursive (GL(dl_load_lock));
|
|
+ __rtld_lock_lock_recursive (GL(dl_load_tls_lock));
|
|
if (__glibc_likely (the_map->l_tls_offset == NO_TLS_OFFSET))
|
|
{
|
|
the_map->l_tls_offset = FORCED_DYNAMIC_TLS_OFFSET;
|
|
- __rtld_lock_unlock_recursive (GL(dl_load_lock));
|
|
+ __rtld_lock_unlock_recursive (GL(dl_load_tls_lock));
|
|
}
|
|
else if (__glibc_likely (the_map->l_tls_offset
|
|
!= FORCED_DYNAMIC_TLS_OFFSET))
|
|
@@ -883,7 +883,7 @@ tls_get_addr_tail (GET_ADDR_ARGS, dtv_t *dtv, struct link_map *the_map)
|
|
#else
|
|
# error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined"
|
|
#endif
|
|
- __rtld_lock_unlock_recursive (GL(dl_load_lock));
|
|
+ __rtld_lock_unlock_recursive (GL(dl_load_tls_lock));
|
|
|
|
dtv[GET_ADDR_MODULE].pointer.to_free = NULL;
|
|
dtv[GET_ADDR_MODULE].pointer.val = p;
|
|
@@ -891,7 +891,7 @@ tls_get_addr_tail (GET_ADDR_ARGS, dtv_t *dtv, struct link_map *the_map)
|
|
return (char *) p + GET_ADDR_OFFSET;
|
|
}
|
|
else
|
|
- __rtld_lock_unlock_recursive (GL(dl_load_lock));
|
|
+ __rtld_lock_unlock_recursive (GL(dl_load_tls_lock));
|
|
}
|
|
struct dtv_pointer result = allocate_and_init (the_map);
|
|
dtv[GET_ADDR_MODULE].pointer = result;
|
|
@@ -962,7 +962,7 @@ _dl_tls_get_addr_soft (struct link_map *l)
|
|
return NULL;
|
|
|
|
dtv_t *dtv = THREAD_DTV ();
|
|
- /* This may be called without holding the GL(dl_load_lock). Reading
|
|
+ /* This may be called without holding the GL(dl_load_tls_lock). Reading
|
|
arbitrary gen value is fine since this is best effort code. */
|
|
size_t gen = atomic_load_relaxed (&GL(dl_tls_generation));
|
|
if (__glibc_unlikely (dtv[0].counter != gen))
|
|
diff --git a/elf/rtld.c b/elf/rtld.c
|
|
index d733359eaf808b8a..08cf50145a1c01ce 100644
|
|
--- a/elf/rtld.c
|
|
+++ b/elf/rtld.c
|
|
@@ -322,6 +322,7 @@ struct rtld_global _rtld_global =
|
|
#ifdef _LIBC_REENTRANT
|
|
._dl_load_lock = _RTLD_LOCK_RECURSIVE_INITIALIZER,
|
|
._dl_load_write_lock = _RTLD_LOCK_RECURSIVE_INITIALIZER,
|
|
+ ._dl_load_tls_lock = _RTLD_LOCK_RECURSIVE_INITIALIZER,
|
|
#endif
|
|
._dl_nns = 1,
|
|
._dl_ns =
|
|
diff --git a/posix/fork.c b/posix/fork.c
|
|
index c471f7b15fe4290d..021691b9b7441f15 100644
|
|
--- a/posix/fork.c
|
|
+++ b/posix/fork.c
|
|
@@ -99,6 +99,9 @@ __libc_fork (void)
|
|
/* Reset the lock the dynamic loader uses to protect its data. */
|
|
__rtld_lock_initialize (GL(dl_load_lock));
|
|
|
|
+ /* Reset the lock protecting dynamic TLS related data. */
|
|
+ __rtld_lock_initialize (GL(dl_load_tls_lock));
|
|
+
|
|
reclaim_stacks ();
|
|
|
|
/* Run the handlers registered for the child. */
|
|
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
|
|
index 9c15259236adab43..1ceb9c3212c148ba 100644
|
|
--- a/sysdeps/generic/ldsodefs.h
|
|
+++ b/sysdeps/generic/ldsodefs.h
|
|
@@ -372,6 +372,13 @@ struct rtld_global
|
|
list of loaded objects while an object is added to or removed
|
|
from that list. */
|
|
__rtld_lock_define_recursive (EXTERN, _dl_load_write_lock)
|
|
+ /* This lock protects global and module specific TLS related data.
|
|
+ E.g. it is held in dlopen and dlclose when GL(dl_tls_generation),
|
|
+ GL(dl_tls_max_dtv_idx) or GL(dl_tls_dtv_slotinfo_list) are
|
|
+ accessed and when TLS related relocations are processed for a
|
|
+ module. It was introduced to keep pthread_create accessing TLS
|
|
+ state that is being set up. */
|
|
+ __rtld_lock_define_recursive (EXTERN, _dl_load_tls_lock)
|
|
|
|
/* Incremented whenever something may have been added to dl_loaded. */
|
|
EXTERN unsigned long long _dl_load_adds;
|
|
@@ -1261,7 +1268,7 @@ extern int _dl_scope_free (void *) attribute_hidden;
|
|
|
|
/* Add module to slot information data. If DO_ADD is false, only the
|
|
required memory is allocated. Must be called with GL
|
|
- (dl_load_lock) acquired. If the function has already been called
|
|
+ (dl_load_tls_lock) acquired. If the function has already been called
|
|
for the link map L with !do_add, then this function will not raise
|
|
an exception, otherwise it is possible that it encounters a memory
|
|
allocation failure. */
|
|
diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile
|
|
index 0af9c59b425aefb1..df8943f4860a39d8 100644
|
|
--- a/sysdeps/pthread/Makefile
|
|
+++ b/sysdeps/pthread/Makefile
|
|
@@ -152,15 +152,17 @@ tests += tst-cancelx2 tst-cancelx3 tst-cancelx6 tst-cancelx8 tst-cancelx9 \
|
|
tst-cleanupx0 tst-cleanupx1 tst-cleanupx2 tst-cleanupx3
|
|
|
|
ifeq ($(build-shared),yes)
|
|
-tests += tst-atfork2 tst-pt-tls4 tst-_res1 tst-fini1
|
|
+tests += tst-atfork2 tst-pt-tls4 tst-_res1 tst-fini1 tst-create1
|
|
tests-nolibpthread += tst-fini1
|
|
endif
|
|
|
|
modules-names += tst-atfork2mod tst-tls4moda tst-tls4modb \
|
|
- tst-_res1mod1 tst-_res1mod2 tst-fini1mod
|
|
+ tst-_res1mod1 tst-_res1mod2 tst-fini1mod \
|
|
+ tst-create1mod
|
|
test-modules = $(addprefix $(objpfx),$(addsuffix .so,$(modules-names)))
|
|
|
|
tst-atfork2mod.so-no-z-defs = yes
|
|
+tst-create1mod.so-no-z-defs = yes
|
|
|
|
ifeq ($(build-shared),yes)
|
|
# Build all the modules even when not actually running test programs.
|
|
@@ -279,4 +281,8 @@ LDFLAGS-tst-join7mod.so = -Wl,-soname,tst-join7mod.so
|
|
|
|
CFLAGS-tst-unwind-thread.c += -funwind-tables
|
|
|
|
+LDFLAGS-tst-create1 = -Wl,-export-dynamic
|
|
+$(objpfx)tst-create1: $(shared-thread-library)
|
|
+$(objpfx)tst-create1.out: $(objpfx)tst-create1mod.so
|
|
+
|
|
endif
|
|
diff --git a/sysdeps/pthread/tst-create1.c b/sysdeps/pthread/tst-create1.c
|
|
new file mode 100644
|
|
index 0000000000000000..932586c30990d1d4
|
|
--- /dev/null
|
|
+++ b/sysdeps/pthread/tst-create1.c
|
|
@@ -0,0 +1,119 @@
|
|
+/* Verify that pthread_create does not deadlock when ctors take locks.
|
|
+ Copyright (C) 2021 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 <stdio.h>
|
|
+#include <support/xdlfcn.h>
|
|
+#include <support/xthread.h>
|
|
+
|
|
+/*
|
|
+Check if ctor and pthread_create deadlocks in
|
|
+
|
|
+thread 1: dlopen -> ctor -> lock(user_lock)
|
|
+thread 2: lock(user_lock) -> pthread_create
|
|
+
|
|
+or in
|
|
+
|
|
+thread 1: dlclose -> dtor -> lock(user_lock)
|
|
+thread 2: lock(user_lock) -> pthread_create
|
|
+*/
|
|
+
|
|
+static pthread_barrier_t bar_ctor;
|
|
+static pthread_barrier_t bar_dtor;
|
|
+static pthread_mutex_t user_lock = PTHREAD_MUTEX_INITIALIZER;
|
|
+
|
|
+void
|
|
+ctor (void)
|
|
+{
|
|
+ xpthread_barrier_wait (&bar_ctor);
|
|
+ dprintf (1, "thread 1: in ctor: started.\n");
|
|
+ xpthread_mutex_lock (&user_lock);
|
|
+ dprintf (1, "thread 1: in ctor: locked user_lock.\n");
|
|
+ xpthread_mutex_unlock (&user_lock);
|
|
+ dprintf (1, "thread 1: in ctor: unlocked user_lock.\n");
|
|
+ dprintf (1, "thread 1: in ctor: done.\n");
|
|
+}
|
|
+
|
|
+void
|
|
+dtor (void)
|
|
+{
|
|
+ xpthread_barrier_wait (&bar_dtor);
|
|
+ dprintf (1, "thread 1: in dtor: started.\n");
|
|
+ xpthread_mutex_lock (&user_lock);
|
|
+ dprintf (1, "thread 1: in dtor: locked user_lock.\n");
|
|
+ xpthread_mutex_unlock (&user_lock);
|
|
+ dprintf (1, "thread 1: in dtor: unlocked user_lock.\n");
|
|
+ dprintf (1, "thread 1: in dtor: done.\n");
|
|
+}
|
|
+
|
|
+static void *
|
|
+thread3 (void *a)
|
|
+{
|
|
+ dprintf (1, "thread 3: started.\n");
|
|
+ dprintf (1, "thread 3: done.\n");
|
|
+ return 0;
|
|
+}
|
|
+
|
|
+static void *
|
|
+thread2 (void *a)
|
|
+{
|
|
+ pthread_t t3;
|
|
+ dprintf (1, "thread 2: started.\n");
|
|
+
|
|
+ xpthread_mutex_lock (&user_lock);
|
|
+ dprintf (1, "thread 2: locked user_lock.\n");
|
|
+ xpthread_barrier_wait (&bar_ctor);
|
|
+ t3 = xpthread_create (0, thread3, 0);
|
|
+ xpthread_mutex_unlock (&user_lock);
|
|
+ dprintf (1, "thread 2: unlocked user_lock.\n");
|
|
+ xpthread_join (t3);
|
|
+
|
|
+ xpthread_mutex_lock (&user_lock);
|
|
+ dprintf (1, "thread 2: locked user_lock.\n");
|
|
+ xpthread_barrier_wait (&bar_dtor);
|
|
+ t3 = xpthread_create (0, thread3, 0);
|
|
+ xpthread_mutex_unlock (&user_lock);
|
|
+ dprintf (1, "thread 2: unlocked user_lock.\n");
|
|
+ xpthread_join (t3);
|
|
+
|
|
+ dprintf (1, "thread 2: done.\n");
|
|
+ return 0;
|
|
+}
|
|
+
|
|
+static void
|
|
+thread1 (void)
|
|
+{
|
|
+ dprintf (1, "thread 1: started.\n");
|
|
+ xpthread_barrier_init (&bar_ctor, NULL, 2);
|
|
+ xpthread_barrier_init (&bar_dtor, NULL, 2);
|
|
+ pthread_t t2 = xpthread_create (0, thread2, 0);
|
|
+ void *p = xdlopen ("tst-create1mod.so", RTLD_NOW | RTLD_GLOBAL);
|
|
+ dprintf (1, "thread 1: dlopen done.\n");
|
|
+ xdlclose (p);
|
|
+ dprintf (1, "thread 1: dlclose done.\n");
|
|
+ xpthread_join (t2);
|
|
+ dprintf (1, "thread 1: done.\n");
|
|
+}
|
|
+
|
|
+static int
|
|
+do_test (void)
|
|
+{
|
|
+ thread1 ();
|
|
+ return 0;
|
|
+}
|
|
+
|
|
+#include <support/test-driver.c>
|
|
diff --git a/sysdeps/pthread/tst-create1mod.c b/sysdeps/pthread/tst-create1mod.c
|
|
new file mode 100644
|
|
index 0000000000000000..62c9006961683177
|
|
--- /dev/null
|
|
+++ b/sysdeps/pthread/tst-create1mod.c
|
|
@@ -0,0 +1,41 @@
|
|
+/* Verify that pthread_create does not deadlock when ctors take locks.
|
|
+ Copyright (C) 2021 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 <stdio.h>
|
|
+
|
|
+/* Require TLS setup for the module. */
|
|
+__thread int tlsvar;
|
|
+
|
|
+void ctor (void);
|
|
+void dtor (void);
|
|
+
|
|
+static void __attribute__ ((constructor))
|
|
+do_init (void)
|
|
+{
|
|
+ dprintf (1, "constructor started: %d.\n", tlsvar++);
|
|
+ ctor ();
|
|
+ dprintf (1, "constructor done: %d.\n", tlsvar++);
|
|
+}
|
|
+
|
|
+static void __attribute__ ((destructor))
|
|
+do_end (void)
|
|
+{
|
|
+ dprintf (1, "destructor started: %d.\n", tlsvar++);
|
|
+ dtor ();
|
|
+ dprintf (1, "destructor done: %d.\n", tlsvar++);
|
|
+}
|