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

[Vegard Nossum <vegard.nossum@oracle.com>]

On 07/26/2016 10:30 PM, Kirill A. Shutemov wrote:
> 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?

Tested-by: Vegard Nossum <vegard.nossum@oracle.com>

Feel free to reuse any or all parts of the commit message I wrote for my
patch.

Thanks!


Vegard

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