Messages in this thread | | | Date | Wed, 15 Oct 2003 14:46:11 +0200 | From | Andrea Arcangeli <> | Subject | Re: [PATCH][2.4.23-pre7] alternate fix for BUG() in exec_mmap() |
| |
On Mon, Oct 13, 2003 at 09:11:27PM -0400, Ernie Petrides wrote: > Yes, this in fact necessary. I have retested with the locking sequence > shown above and verified that the problem is still fixed. > > Further, I've discovered that in my original version, with the mmap_sem > held across mm_release() and exit_aio(), there are potential deadlocks > in at least the following hypothetical calling trees: > > mm_release() > put_user() > direct_put_user() > __put_user_check() > __put_user_size() > __put_user_asm() > [page fault] > do_page_fault() > down_read(&mm->mmap_sem) > > exit_aio() > aio_cancel_all() > async_poll_cancel() > aio_put_req() > put_ioctx() > __put_ioctx() > aio_free_ring() > down_write(&ctx->mm->mmap_sem) > > The corrected patch against 2.4.23-pre7, which restores the fast path in > exec_mmap() and adds the holding of "mmap_sem" across only the exit_mmap() > call, is attached below. Since "mmap_sem" locking is conceptually higher > than file system locks in the locking hierarchy, the whole calling tree > from exit_mmap() on down (including potential fput() calls) should be > safe to run with "mmap_sem" owned. > > Thanks for the help. > > Cheers. -ernie > > > > --- linux-2.4.23-pre7/fs/exec.c.orig > +++ linux-2.4.23-pre7/fs/exec.c > @@ -426,6 +426,13 @@ static int exec_mmap(void) > struct mm_struct * mm, * old_mm; > > old_mm = current->mm; > + if (old_mm && atomic_read(&old_mm->mm_users) == 1) { > + mm_release(); > + down_write(&old_mm->mmap_sem); > + exit_mmap(old_mm); > + up_write(&old_mm->mmap_sem); > + return 0; > + } > > mm = mm_alloc(); > if (mm) {
looks fine thanks.
Andrea - 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/
| |