Re: [PATCH] mm: correctly handle errors during VMA merging

["Kirill A. Shutemov" <kirill@shutemov.name>]

On Tue, Jul 26, 2016 at 10:19:48PM +0200, Vegard Nossum wrote:
> On 07/26/2016 01:48 PM, Kirill A. Shutemov wrote:
> >On Tue, Jul 26, 2016 at 08:34:03AM +0200, Vegard Nossum wrote:
> >>Using trinity + fault injection I've been running into this bug a lot:
> >>
> >>     ==================================================================
> >>     BUG: KASAN: out-of-bounds in mprotect_fixup+0x523/0x5a0 at addr ffff8800b9e7d740
> 
> [...]
> 
> >>What's happening is that we're doing an mprotect() on a range that spans
> >>three existing adjacent mappings. The first two are merged fine, but if
> >>we merge the last one and anon_vma_clone() runs out of memory, we return
> >>an error and mprotect_fixup() tries to use the (now stale) pointer. It
> >>goes like this:
> >>
> >>     SyS_mprotect()
> >>       - mprotect_fixup()
> >>          - vma_merge()
> >>             - vma_adjust()
> >>                // first merge
> >>                - kmem_cache_free(vma)
> >>                - goto again;
> >>                // second merge
> >>                - anon_vma_clone()
> >>                   - kmem_cache_alloc()
> >>                      - return NULL
> >>                   - kmem_cache_alloc()
> >>                      - return NULL
> >>                   - return -ENOMEM
> >>                - return -ENOMEM
> >>             - return NULL
> >>          - vma->vm_start // use-after-free
> >>
> >>In other words, it is possible to run into a memory allocation error
> >>*after* part of the merging work has already been done. In this case,
> >>we probably shouldn't return an error back to userspace anyway (since
> >>it would not reflect the partial work that was done).
> >>
> >>I *think* the solution might be to simply ignore the errors from
> >>vma_adjust() and carry on with distinct VMAs for adjacent regions that
> >>might otherwise have been represented with a single VMA.
> >>
> >>I have a reproducer that runs into the bug within a few seconds when
> >>fault injection is enabled -- with the patch I no longer see any
> >>problems.
> >>
> >>The patch and resulting code admittedly look odd and I'm *far* from
> >>an expert on mm internals, so feel free to propose counter-patches and
> >>I can give the reproducer a spin.
> >
> >Could you give this a try (barely tested):
> 
> No apparent problems using either the quick reproducer or trinity (used
> to take 1-5 hours) after ~8 hours of testing :-)

Good. I'll prepare proper patch tomorrow.

I assume I can use your Tested-by, right?

-- 
 Kirill A. Shutemov

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>