summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorRich Felker <dalias@aerifal.cx>2022-10-05 10:41:30 -0400
committerRich Felker <dalias@aerifal.cx>2022-10-19 14:01:32 -0400
commit36b72cd6fdfed2cac6b6ff1ed58a96d8265785cf (patch)
treeb47d2641194ff8bbaea800495a6206a3a104748c /src
parent833a469167521040c7ae94f3c990e258e29445f9 (diff)
downloadmusl-36b72cd6fdfed2cac6b6ff1ed58a96d8265785cf.tar.gz
fix potential deadlock in dlerror buffer handling at thread exit
ever since commit 8f11e6127fe93093f81a52b15bb1537edc3fc8af introduced the thread list lock, this has been wrong. initially, it was wrong via calling free from the context with the thread list lock held. commit aa5a9d15e09851f7b4a1668e9dbde0f6234abada deferred the unsafe free but added a lock, which was also unsafe. in particular, it could deadlock if code holding freebuf_queue_lock was interrupted by a signal handler that takes the thread list lock. commit 4d5aa20a94a2d3fae3e69289dc23ecafbd0c16c4 observed that there was a lock here but failed to notice that it's invalid. there is no easy solution to this problem with locks; any attempt at solving it while still using locks would require the lock to be an AS-safe one (blocking signals on each access to the dlerror buffer list to check if there's deferred free work to be done) which would be excessively costly, and there are also lock order considerations with respect to how the lock would be handled at fork. instead, just use an atomic list.
Diffstat (limited to 'src')
-rw-r--r--src/internal/fork_impl.h1
-rw-r--r--src/ldso/dlerror.c34
-rw-r--r--src/process/fork.c2
3 files changed, 18 insertions, 19 deletions
diff --git a/src/internal/fork_impl.h b/src/internal/fork_impl.h
index 5892c13b..ae3a79e5 100644
--- a/src/internal/fork_impl.h
+++ b/src/internal/fork_impl.h
@@ -2,7 +2,6 @@
extern hidden volatile int *const __at_quick_exit_lockptr;
extern hidden volatile int *const __atexit_lockptr;
-extern hidden volatile int *const __dlerror_lockptr;
extern hidden volatile int *const __gettext_lockptr;
extern hidden volatile int *const __locale_lockptr;
extern hidden volatile int *const __random_lockptr;
diff --git a/src/ldso/dlerror.c b/src/ldso/dlerror.c
index afe59253..dae0f3a9 100644
--- a/src/ldso/dlerror.c
+++ b/src/ldso/dlerror.c
@@ -3,8 +3,7 @@
#include <stdarg.h>
#include "pthread_impl.h"
#include "dynlink.h"
-#include "lock.h"
-#include "fork_impl.h"
+#include "atomic.h"
#define malloc __libc_malloc
#define calloc __libc_calloc
@@ -23,28 +22,31 @@ char *dlerror()
return s;
}
-static volatile int freebuf_queue_lock[1];
-static void **freebuf_queue;
-volatile int *const __dlerror_lockptr = freebuf_queue_lock;
+/* Atomic singly-linked list, used to store list of thread-local dlerror
+ * buffers for deferred free. They cannot be freed at thread exit time
+ * because, by the time it's known they can be freed, the exiting thread
+ * is in a highly restrictive context where it cannot call (even the
+ * libc-internal) free. It also can't take locks; thus the atomic list. */
+
+static void *volatile freebuf_queue;
void __dl_thread_cleanup(void)
{
pthread_t self = __pthread_self();
- if (self->dlerror_buf && self->dlerror_buf != (void *)-1) {
- LOCK(freebuf_queue_lock);
- void **p = (void **)self->dlerror_buf;
- *p = freebuf_queue;
- freebuf_queue = p;
- UNLOCK(freebuf_queue_lock);
- }
+ if (!self->dlerror_buf || self->dlerror_buf == (void *)-1)
+ return;
+ void *h;
+ do {
+ h = freebuf_queue;
+ *(void **)self->dlerror_buf = h;
+ } while (a_cas_p(&freebuf_queue, h, self->dlerror_buf) != h);
}
hidden void __dl_vseterr(const char *fmt, va_list ap)
{
- LOCK(freebuf_queue_lock);
- void **q = freebuf_queue;
- freebuf_queue = 0;
- UNLOCK(freebuf_queue_lock);
+ void **q;
+ do q = freebuf_queue;
+ while (q && a_cas_p(&freebuf_queue, q, 0) != q);
while (q) {
void **p = *q;
diff --git a/src/process/fork.c b/src/process/fork.c
index 54bc2892..ff71845c 100644
--- a/src/process/fork.c
+++ b/src/process/fork.c
@@ -9,7 +9,6 @@ static volatile int *const dummy_lockptr = 0;
weak_alias(dummy_lockptr, __at_quick_exit_lockptr);
weak_alias(dummy_lockptr, __atexit_lockptr);
-weak_alias(dummy_lockptr, __dlerror_lockptr);
weak_alias(dummy_lockptr, __gettext_lockptr);
weak_alias(dummy_lockptr, __locale_lockptr);
weak_alias(dummy_lockptr, __random_lockptr);
@@ -24,7 +23,6 @@ weak_alias(dummy_lockptr, __vmlock_lockptr);
static volatile int *const *const atfork_locks[] = {
&__at_quick_exit_lockptr,
&__atexit_lockptr,
- &__dlerror_lockptr,
&__gettext_lockptr,
&__locale_lockptr,
&__random_lockptr,