niansa-misc/musl
1
0
Fork 0
mirror of git://git.musl-libc.org/musl synced 2025-03-06 20:48:29 +01:00
Code Issues Projects Releases Packages Wiki Activity

fix potential deadlock in dlerror buffer handling at thread exit

Browse source 
ever since commit 8f11e6127f 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
aa5a9d15e0 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 4d5aa20a94 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.
This commit is contained in:
Rich Felker 2022-10-05 10:41:30 -04:00
parent 833a469167
commit 36b72cd6fd
3 changed files with 18 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
src/internal/fork_impl.h
View file

@ -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;

34
src/ldso/dlerror.c
View file

@ -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;

2
src/process/fork.c
View file

@ -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,