Created attachment 9195 [details] Source file reproducing the bug. Summary: A race in pthread_detach can trigger a read from deallocated memory (=>SEGV) or can corrupt the state of another thread, if that memory has been reused. When a detached thread exits, it's descriptor and stack does not get freed immediately, but it gets put into a cache. Memory from this cache can later be reused to create new threads, or freed (via munmap) if the cache gets too big. However, it is possible for this reuse/unmap to happen before the actual pthread_detach call returns, while it's still accessing the memory via the descriptor of the now-exited thread. I attach a small test file (a.c) which demonstrates problem. Note that I am running the program under gdb, but I am doing this only to control the relative timings of individual threads, I am messing in no way with the internal state of the library. $ gdb ./a.out (gdb) set non-stop on (gdb) dir /etc/apt/eglibc-2.19/nptl Source directories searched: /etc/apt/eglibc-2.19/nptl:$cdir:$cwd (gdb) b 21 Breakpoint 1 at 0x4008d2: file a.c, line 21. (gdb) b start Breakpoint 2 at 0x4007c5: file a.c, line 6. (gdb) r Starting program: /tmp/a.out [Thread debugging using libthread_db enabled] Using host libthread_db library "/usr/grte/v4/lib64/libthread_db.so.1". [New Thread 0x7ffff77f6700 (LWP 52987)] [New Thread 0x7ffff45f5700 (LWP 52988)] Breakpoint 2, start (arg=0x0) at a.c:6 6 return 0; (gdb) Breakpoint 1, main () at a.c:21 21 assert(pthread_detach(handle2) == 0); Breakpoint 2, start (arg=0x0) at a.c:6 6 return 0; #### Main thread has created two new threads (which we have stopped in the start function). Thread 2 has already been detached, and we are now about to detach Thread 3. #### s pthread_detach (th=140737293276928) at pthread_detach.c:31 31 if (INVALID_NOT_TERMINATED_TD_P (pd)) (gdb) n 38 if (atomic_compare_and_exchange_bool_acq (&pd->joinid, pd, NULL)) (gdb) 50 if ((pd->cancelhandling & EXITING_BITMASK) != 0) (gdb) p pd $1 = (struct pthread *) 0x7ffff45f5700 ##### The main thread is now in pthread_detach. It has already marked the thread as "detached", but it will still access it's memory: pd->cancelhandling and __free_tcb(pd). Before we let it do that, we will let other threads complete. ##### (gdb) info th Id Target Id Frame 3 Thread 0x7ffff45f5700 (LWP 52988) "a.out" start (arg=0x0) at a.c:6 2 Thread 0x7ffff77f6700 (LWP 52987) "a.out" start (arg=0x0) at a.c:6 * 1 Thread 0x7ffff7fd8740 (LWP 52983) "a.out" pthread_detach (th=140737293276928) at pthread_detach.c:50 (gdb) thread 3 [Switching to thread 3 (Thread 0x7ffff45f5700 (LWP 52988))] #0 start (arg=0x0) at a.c:6 6 return 0; (gdb) c Continuing. [Thread 0x7ffff45f5700 (LWP 52988) exited] No unwaited-for children left. ##### Thread 3 has exited, it's memory has been put into the stack_cache (allocatestack.c). ##### (gdb) thread 2 [Switching to thread 2 (Thread 0x7ffff77f6700 (LWP 52987))] #0 start (arg=0x0) at a.c:6 6 return 0; (gdb) c Continuing. [Thread 0x7ffff77f6700 (LWP 52987) exited] No unwaited-for children left. ##### Thread 2 has exited as well. It's exit has triggered a purge of the cache, which unmapped the memory used by Thread 3. Now we let the main thread finish, which will trigger a SIGSEGV. If that memory had been reallocated, or if pthread had reused the cache entry for another thread, it could mess with random memory. ##### (gdb) thread 1 [Switching to thread 1 (Thread 0x7ffff7fd8740 (LWP 52983))] #0 pthread_detach (th=140737293276928) at pthread_detach.c:50 50 if ((pd->cancelhandling & EXITING_BITMASK) != 0) (gdb) c Continuing. Program received signal SIGSEGV, Segmentation fault. pthread_detach (th=140737293276928) at pthread_detach.c:50 50 if ((pd->cancelhandling & EXITING_BITMASK) != 0) (gdb) q Note that even though I have reproduced this bug in gdb, I have observed this happening in the wild (which led me to start investigating).
I can confirm this issue. Fundamentally this code in pthread_detach is flawed: 36 /* Mark the thread as detached. */ 37 if (atomic_compare_and_exchange_bool_acq (&pd->joinid, pd, NULL)) 38 { 39 /* There are two possibilities here. First, the thread might 40 already be detached. In this case we return EINVAL. 41 Otherwise there might already be a waiter. The standard does 42 not mention what happens in this case. */ 43 if (IS_DETACHED (pd)) 44 result = EINVAL; 45 } 46 else 47 /* Check whether the thread terminated meanwhile. In this case we 48 will just free the TCB. */ 49 if ((pd->cancelhandling & EXITING_BITMASK) != 0) 50 /* Note that the code in __free_tcb makes sure each thread 51 control block is freed only once. */ 52 __free_tcb (pd); Once ownership of PD is released on line 37 we may never be touched again. There is a resource leak that we can't prevent in the current implementation. e.g. T1: (a) Check if I'm detached. (b) If detached then free resources. (c) Exit. Any thread T2 may make T1 detached after (a) and create a scenario where T2 doesn't know if T1 was detached before (a) or after (a) and can't check without risk of segfault if PD is unmapped. The detach sequence needs to be rewritten such that (a) is done atomically and is not just a check but writes information back into PD to indicate to T2 that it has already shut down far enough that it will not be freeing it's own resources. In that case T2 can, in pthread_detach, carry out the free of the resources, knowing PD is still around. The only immediate workaround I can suggest is to start the thread detached rather than trying to set the detached status at a later point in time. I do not expect this to get fixed in 2.25 (Feb 1st 2017).
I think the problem is relying the the set_tid_address value to synchronize the thread state (joined, detached, terminated). First there are disjoined fields, so on every state operations (pthread_detach, pthread_join, and pthread_exit/thread exit) we need to check for both fields and handle possible partial results (as for pthread_join which first sets the PD joinid field, wait for kernel, and resets the joinid in case of failure). Second we don't really need the kernel exit execution advertisement, since the synchronization required is to setup which thread would be responsible to cleanup the allocated resources. So I think we need to use a different field with 4 states to synchronize between pthread_create, pthread_join, pthread_detached, and pthread_exit: 1. On thread creation the inital state is JOINABLE or DETACHED (depending of the pthread_attribute). 2. On pthread_detach call, a CAS is done on the state field. If the CAS fails it means that thread is already detached (EINVAL) or the threads is exitng or EXITED and it pthread_detach should be reponsible to join the thread and deallocate the resource. 3. On pthread_create exit phase (reached either thread function has returned, pthread_exit has being called, or cancellation handled has been acted upon) we also issue a CAS over state field to initial the EXITTING mode. If the thread is already on DETACHED mode it will be thread itself responsible to deallocate any resource. In the final pthread exit phase the state is set to EXITED and a futex wake is done (to synchronize with pthread_join). The EXITING mode exists to avoid a pthread_detach to wrongly setup the mode while the thread calling the required exit functions, but has not while deallocate the stack. 4. Finally the pthread_join implementation is much more simpler: the futex wait is done directly on thread state and there is no need to reset it in case of TIMEOUT (since the state is now set either by pthread_detached or pthread_create exit phase). This issue should be fixed because once pthread_join synchronizes the thread state it can exit early in the case of DETACHED since the thread itself will be responsible to cleanup its own memory. Arguably it is undefined behaviour to try join a detached thread, but glibc tries to catch such issues where possible, it would not be possible to catch the isseu trying to join a detached thread that has already exited. I am working a possible fix.
FWIW I couldn't reproduce on glibc-2.40-12.fc41.x86_64 Fixed but not closed? (gdb) set non-stop on (gdb) b 21 No symbol table is loaded. Use the "file" command. (gdb) b start Breakpoint 1 at 0x40116a (gdb) r Starting program: /home/dan/Downloads/a.out [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib64/libthread_db.so.1". [New Thread 0x7ffff7d9f6c0 (LWP 1661717)] [New Thread 0x7ffff4b9e6c0 (LWP 1661718)] Cannot find user-level thread for LWP 1661717: generic error Missing debuginfo, try: dnf debuginfo-install glibc-2.40-12.fc41.x86_64 (gdb) [Thread 0x7ffff4b9e6c0 (LWP 1661718) exited] [Thread 0x7ffff7d9f6c0 (LWP 1661717) exited] [Inferior 1 (process 1661713) exited normally]