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

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

On Tue, Jul 26, 2016 at 11:24:21AM +0200, Vegard Nossum wrote:
> On 07/26/2016 10:53 AM, 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
> >>     Read of size 8 by task trinity-c3/6338
> [...]
> >>I can give the reproducer a spin.
> >
> >Could you post your reproducer? I guess it requires kernel instrumentation
> >to make allocation failure more likely.
> 
> I'm sorry but company policy prevents me from posting straight-up
> reproducers.

That's very weird policy. I don't think this kind of reproducer can be
considered exploit.

> But as I said I'm happy to rerun it if you have an alternative patch.
> 
> It should be enough to enable fault injection (echo 1 >
> /proc/self/make-it-fail) for the process doing the mprotect().

That's what I came up with:

	#define _GNU_SOURCE
	#include <stdio.h>
	#include <stdlib.h>
	#include <string.h>
	#include <unistd.h>
	#include <sys/mman.h>

	#define PAGE_SIZE 4096
	#define SIZE (3 * PAGE_SIZE)
	#define BASE ((void *)0x400000000000)

	int main(int argc, char **argv)
	{
		char *p;

		p = mmap(BASE, SIZE, PROT_READ | PROT_WRITE,
				MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);

		mprotect(p + PAGE_SIZE, PAGE_SIZE, PROT_READ | PROT_WRITE | PROT_EXEC);
		memset(p + 2 * PAGE_SIZE, 1, PAGE_SIZE);
		mprotect(p + PAGE_SIZE, PAGE_SIZE, PROT_READ | PROT_WRITE);

		return 0;
	}

Plus kernel modification to make the allocation fail:


index a384c10c7657..35f004676233 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -629,6 +629,7 @@ int vma_adjust(struct vm_area_struct *vma, unsigned long start,
        bool start_changed = false, end_changed = false;
        long adjust_next = 0;
        int remove_next = 0;
+       bool second_iteration = false;
 
        if (next && !insert) {
                struct vm_area_struct *exporter = NULL;
@@ -670,6 +671,8 @@ again:                      remove_next = 1 + (end > next->vm_end);
                        int error;
 
                        importer->anon_vma = exporter->anon_vma;
+                       if (second_iteration)
+                               return -ENOMEM;
                        error = anon_vma_clone(importer, exporter);
                        if (error)
                                return error;
@@ -796,6 +799,7 @@ again:                      remove_next = 1 + (end > next->vm_end);
                 * up the code too much to do both in one go.
                 */
                next = vma->vm_next;
+               second_iteration = true;
                if (remove_next == 2)
                        goto again;
                else if (next)
-- 
 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>