Re: Possible bug in do_execve()

["Serge E. Hallyn" <serue@us.ibm.com>]

Quoting Sonny Rao (sonny@burdell.org):
> 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@us.ibm.com>
> > 
> > ---
> > 
> >  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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/