Re: Possible bug in do_execve()

From: Serge E. Hallyn
Date: Thu Jun 22 2006 - 07:58:09 EST


Quoting Sonny Rao (sonny@xxxxxxxxxxx):
> > > It seems to assume that mm->context is valid before doing a check.
> > >
> > > Since I don't have a sparc64 box, I can't check to see if this
> > > actually breaks things or not.
> >
> > So we can either go through all arch's and make sure destroy_context is
> > safe for invalid context, or split mmput() and destroy_context()...
> >
> > The former seems easier, but the latter seems more robust in the face of
> > future code changes I guess.
>
> Yes, the former does seem easier, and perhaps easiest is to do that
> and document what the pre-conditions are so future developers at least
> have a clue.

Hmm, but document it where, since there is no single destroy_context()
definition? At the mmput() and __mmdrop() definitions in kernel/fork.c?

(like so: ?)

-serge

From: Serge E. Hallyn <hallyn@sergelap.(none)>
Date: Wed, 21 Jun 2006 13:37:27 -0500
Subject: [PATCH] powerpc: check for proper mm->context before destroying

arch/powerpc/mm/mmu_context_64.c:destroy_context() can be called
from __mmput() in do_execve() if init_new_context() failed. This
can result in idr_remove() being called for an invalid context.

So, don't call idr_remove if there is no context.

Signed-off-by: Serge E. Hallyn <serue@xxxxxxxxxx>

---

arch/powerpc/mm/mmu_context_64.c | 3 +++
kernel/fork.c | 4 ++++
2 files changed, 7 insertions(+), 0 deletions(-)

d4fdebfda2170615db87f5aaf45b8478e223824a
diff --git a/arch/powerpc/mm/mmu_context_64.c b/arch/powerpc/mm/mmu_context_64.c
index 714a84d..552d590 100644
--- a/arch/powerpc/mm/mmu_context_64.c
+++ b/arch/powerpc/mm/mmu_context_64.c
@@ -55,6 +55,9 @@ again:

void destroy_context(struct mm_struct *mm)
{
+ if (mm->context.id == NO_CONTEXT)
+ return;
+
spin_lock(&mmu_context_lock);
idr_remove(&mmu_context_idr, mm->context.id);
spin_unlock(&mmu_context_lock);
diff --git a/kernel/fork.c b/kernel/fork.c
index ac8100e..0fe51aa 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -354,6 +354,10 @@ struct mm_struct * mm_alloc(void)
* Called when the last reference to the mm
* is dropped: either by a lazy thread or by
* mmput. Free the page directory and the mm.
+ *
+ * Arch-specific destroy_context() implementations
+ * should be aware that this can be called when
+ * the mm->context initialization has failed.
*/
void fastcall __mmdrop(struct mm_struct *mm)
{
--
1.3.3

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