Re: Possible bug in do_execve()

[Serge E. Hallyn]

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/