reorder thread list unlink in pthread_exit after all locks
authorRich Felker <dalias@aerifal.cx>
Fri, 22 May 2020 21:35:14 +0000 (17:35 -0400)
committerRich Felker <dalias@aerifal.cx>
Fri, 22 May 2020 21:39:57 +0000 (17:39 -0400)
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

index 5f491092597eca1fe5554357cead7b1b73192be3..6a3b0c21659a4e7bb1a822f9fea070bdc9de2f91 100644 (file)
@@ -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);