Re: [tip:x86/urgent] x86/mpx: Fix recursive munmap() corruption

[Thomas Gleixner <tglx@linutronix.de>]

On Thu, 18 Apr 2019, Dave Hansen wrote:

> On 4/18/19 11:29 AM, Sasha Levin wrote:
> > This commit has been processed because it contains a "Fixes:" tag,
> > fixing commit: 1de4fa14ee25 x86, mpx: Cleanup unused bound tables.
> > 
> > The bot has tested the following trees: v5.0.8, v4.19.35, v4.14.112, v4.9.169, v4.4.178.
> > 
> > v5.0.8: Build OK!
> > v4.19.35: Failed to apply! Possible dependencies:
> >     dd2283f2605e ("mm: mmap: zap pages with read mmap_sem in munmap")
> 
> I probably should have looked more closely at the state of the code
> before dd2283f2605e.  A more correct Fixes: would probably have referred
> to dd2283f2605e.  *It* appears to be the root cause rather than the
> original MPX code that I called out.
> 
> The pre-dd2283f2605e code does this:
> 
> >         /*
> >          * Remove the vma's, and unmap the actual pages
> >          */
> >         detach_vmas_to_be_unmapped(mm, vma, prev, end);
> >         unmap_region(mm, vma, prev, start, end);
> > 
> >         arch_unmap(mm, vma, start, end);
> > 
> >         /* Fix up all other VM information */
> >         remove_vma_list(mm, vma);
> 
> But, this is actually safe.  arch_unmap() can't see 'vma' in the rbtree
> because it's been detached, so it can't do anything to 'vma' that might
> be unsafe for remove_vma_list()'s use of 'vma'.	
> 
> The bug in dd2283f2605e was moving unmap_region() to the after arch_unmap().
> 
> I confirmed this by running the reproducer on v4.19.35.  It did not
> trigger anything there, even with a bunch of debugging enabled which
> detected the issue in 5.0.

I'l amend the commit to avoid further confusion.

Thanks,

	tglx