On Fri, Sep 24, 2021 at 04:28:58PM -0500, Eric W. Biederman wrote: > So everything should work from a locking perspective as I am not > changing the locking I am simply moving the call from exit_mm earlier. > > To explain how the locking works. > > Coredumps are not handled in complete_signal, so when > a thread dequeues the signal the other threads are all running. > > If two threads dequeue core dumping signals at the same time both will > contend for mmap_write_lock one will get it and set core_state the > second will return -EBUSY from coredump_wait and return from do_coredump > and proceed to do_exit. > > There similar set of races that zap_threads called from coredump_wait > resolves by testing for signal_group_exit inside of sighand lock. > > The normal case is one thread runs through do_coredump, coredump_wait, > and zap_threads, counts the threads and waits in coredump_wait for > all of the other threads to stop. > > The other threads proceed to do_exit and coredump_task_exit. > From their the discover that core_state is set. And holding > the mmap_read_lock is enough to know ensure that either > no coredump is in progress or all of the setup is complete in > coredump_wait. > > If core_state is found it is known that there is a waiter waiting in > coredump_wait. So the tasks add themselves to the list. Decrement > the count to indicate they don't need to be waited for any longer > and the last one waits up the waiter in coredump_wait. > > The threads then spin in TASK_UNINTERRUPTLE until the coredump > completes. > > The dumping thread takes the coredump then calls coredump_finish. > In coredump_finish the linked list is torn down, and each element > of the linked list has sets ".task = NULL" Then the task is woken > up. > > The task waiting in TASK_UNINTERRUPTIBLE wakes up. Sees that .task = > NULL and proceeds with the rest of do_exit. > > > The only change this patch makes to that entire process is > "task->mm = NULL" is replaced by setting PF_POSTCOREDUMP. > > Does that help? That does! Thanks very much for the run-down on the flow there. I'm convinced. :) Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx> -Kees -- Kees Cook
