Re: Possible bug in do_execve()

[Sonny Rao]

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.
-
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/