Messages in this thread | | | Date | Thu, 11 Oct 2007 14:25:07 +0400 | From | Oleg Nesterov <> | Subject | Re: [PATCH] core dump: remain dumpable |
| |
On 10/10, Roland McGrath wrote: > > As far as I know, set_dumpable was only used in do_coredump for > synchronization and not intended for any security purpose.
I don't understand why do_coredump() clears DUMPABLE too.
> This changes do_coredump to use a separate bit for its > synchronization, so the "dumpable" bits remain the same. > ... > > @@ -1727,7 +1727,8 @@ int do_coredump(long signr, int exit_code, struct pt_regs * regs) > if (!binfmt || !binfmt->core_dump) > goto fail; > down_write(&mm->mmap_sem); > - if (!get_dumpable(mm)) { > + if (!get_dumpable(mm) || > + test_and_set_bit(MMF_DUMP_STARTED, &mm->flags)) { > up_write(&mm->mmap_sem); > goto fail;
Do we really need the new MMF_DUMP_STARTED flag? We are holding ->mmap_sem, can't we check "mm->core_waiters != 0" instead?
Hmm. Actually, do we need any check? If another thread (or CLONE_VM task) already started do_coredump(), we must see SIGNAL_GROUP_EXIT when we take ->mmap_sem. In that case coredump_wait() does nothing but returns -EAGAIN.
> - set_dumpable(mm, 0);
Looks like this change is all we need, no? The only problem is that we return -EAGAIN if we race with another thread (the current code returns 0), but nobody checks the returned value.
What do you think?
Oleg.
- 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/
| |