Possible race between multi-threaded coredumps and fork

• Previous message: Bartlomiej Zolnierkiewicz: "Re: [PATCH] nForce2 C1halt fixup, again"
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Date:	Thu, 03 Jun 2004 10:00:39 -0500
To: lkml <linux-kernel@vger.kernel.org>

A customer had a field problem with multi-threaded coredumps,
and I think the fix is the exit race (already fixed in 2.6), but I
also noticed a possible race between taking a coredump and fork().

If zap_threads() in fs/exec.c is called while a thread is in do_fork(),
but before the newly created thread is in the thread list, it is possible
to have a running thread during the coredump. This is bad, but not
terribly bad. However, in this situation it is also possible in
__exit_mm() that the final two threads check mm->core_waiters at
the same time and then both decrement mm->core_waiters, causing
it to go negative, and thus causing a BUG() in coredump_wait().

To fix this, I would like to propose the attached patch to 2.6.

Signed-off-by: Corey Minyard <minyard@acm.org>

--- linux.orig/kernel/fork.c 2004-05-21 11:49:07.000000000 -0500
+++ linux/kernel/fork.c 2004-06-03 09:46:00.000000000 -0500
@@ -1011,6 +1011,9 @@
         INIT_LIST_HEAD(&p->ptrace_children);
         INIT_LIST_HEAD(&p->ptrace_list);
 
+ /* Need the mmap_sem lock for coredump synchronization. */
+ if (p->mm)
+ down_read(&p->mm->mmap_sem);
         /* Need tasklist lock for parent etc handling! */
         write_lock_irq(&tasklist_lock);
         /*
@@ -1019,6 +1022,7 @@
          */
         if (sigismember(&current->pending.signal, SIGKILL)) {
                 write_unlock_irq(&tasklist_lock);
+ up_read(&p->mm->mmap_sem);
                 retval = -EINTR;
                 goto bad_fork_cleanup_namespace;
         }
@@ -1040,6 +1044,7 @@
                 if (current->signal->group_exit) {
                         spin_unlock(&current->sighand->siglock);
                         write_unlock_irq(&tasklist_lock);
+ up_read(&p->mm->mmap_sem);
                         retval = -EAGAIN;
                         goto bad_fork_cleanup_namespace;
                 }
@@ -1075,6 +1080,23 @@
 
         nr_threads++;
         write_unlock_irq(&tasklist_lock);
+
+ if (p->mm) {
+ if (p->mm->core_waiters) {
+ /*
+ * The thread group I am in is currently
+ * taking a coredump so add the new thread as
+ * a coredump candidate force the new thread
+ * to die immediately.
+ */
+ up_read(&p->mm->mmap_sem);
+ down_write(&p->mm->mmap_sem);
+ force_sig_specific(SIGKILL, p);
+ p->mm->core_waiters++;
+ up_write(&p->mm->mmap_sem);
+ } else
+ up_read(&p->mm->mmap_sem);
+ }
         retval = 0;
 
 fork_out:

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

• Previous message: Bartlomiej Zolnierkiewicz: "Re: [PATCH] nForce2 C1halt fixup, again"
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]