Re: Possible bug in do_execve()
From: Serge E. Hallyn
Date: Wed Jun 21 2006 - 15:08:12 EST
Quoting Sonny Rao (sonny@xxxxxxxxxxx):
> On Wed, Jun 21, 2006 at 01:41:29PM -0500, Serge E. Hallyn wrote:
> <snip>
> > > Is the behavior in do_execve() correct?
> >
> > Well, I assume the intent is for out_mm: to clean up from mm_alloc(),
> > not from 'init_new_context'. So I think that code is correct.
> > This bug appears to be powerpc-specific, so would the following patch
> > be reasonable?
> >
> > Note it is entirely untested, just to show where i think this should
> > be solved. But I could try compile+boot test tonight.
> >
> > thanks,
> > -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 +++
> > 1 files changed, 3 insertions(+), 0 deletions(-)
> >
> > ee74da9d3c122b92541dd6b7670731bd4a033f04
> > 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);
>
> Yeah, I proposed a similar patch to Anton, and it would quiet the
> warning on powerpc, but that's not the point. It happens that powerpc
> doesn't use 0 as a context id, but that may not be true on another
> architecture. That's really what I'm concerned about.
FWIW, ppc and cris do the NO_CONTEXT check, while others don't
even have a arch-specific 'mm->context.id'.
-serge
-
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/