From 4d5aa20a94a2d3fae3e69289dc23ecafbd0c16c4 Mon Sep 17 00:00:00 2001 From: Rich Felker Date: Fri, 22 May 2020 17:35:14 -0400 Subject: [PATCH 1/4] reorder thread list unlink in pthread_exit after all locks since the backend for LOCK() skips locking if single-threaded, it's unsafe to make the process appear single-threaded before the last use of lock. this fixes potential unsynchronized access to a linked list via __dl_thread_cleanup. --- src/thread/pthread_create.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/thread/pthread_create.c b/src/thread/pthread_create.c index 5f491092..6a3b0c21 100644 --- a/src/thread/pthread_create.c +++ b/src/thread/pthread_create.c @@ -90,14 +90,7 @@ _Noreturn void __pthread_exit(void *result) exit(0); } - /* At this point we are committed to thread termination. Unlink - * the thread from the list. This change will not be visible - * until the lock is released, which only happens after SYS_exit - * has been called, via the exit futex address pointing at the lock. */ - libc.threads_minus_1--; - self->next->prev = self->prev; - self->prev->next = self->next; - self->prev = self->next = self; + /* At this point we are committed to thread termination. */ /* Process robust list in userspace to handle non-pshared mutexes * and the detached thread case where the robust list head will @@ -121,6 +114,16 @@ _Noreturn void __pthread_exit(void *result) __do_orphaned_stdio_locks(); __dl_thread_cleanup(); + /* Last, unlink thread from the list. This change will not be visible + * until the lock is released, which only happens after SYS_exit + * has been called, via the exit futex address pointing at the lock. + * This needs to happen after any possible calls to LOCK() that might + * skip locking if libc.threads_minus_1 is zero. */ + libc.threads_minus_1--; + self->next->prev = self->prev; + self->prev->next = self->next; + self->prev = self->next = self; + /* This atomic potentially competes with a concurrent pthread_detach * call; the loser is responsible for freeing thread resources. */ int state = a_cas(&self->detach_state, DT_JOINABLE, DT_EXITING); -- 2.21.0