Re: [PATCH] mm/mmap: Check for RLIMIT_AS before unmapping

[Wanpeng Li <liwanp@linux.vnet.ibm.com>]

On Tue, Apr 02, 2013 at 11:54:03AM +0200, Cyril Hrubis wrote:
>This patch fixes corner case for MAP_FIXED when requested mapping length
>is larger than rlimit for virtual memory. In such case any overlapping
>mappings are unmapped before we check for the limit and return ENOMEM.
>
>The check is moved before the loop that unmaps overlapping parts of
>existing mappings. When we are about to hit the limit (currently mapped
>pages + len > limit) we scan for overlapping pages and check again
>accounting for them.
>
>This fixes situation when userspace program expects that the previous
>mappings are preserved after the mmap() syscall has returned with error.
>(POSIX clearly states that successfull mapping shall replace any
>previous mappings.)
>
>This corner case was found and can be tested with LTP testcase:
>
>testcases/open_posix_testsuite/conformance/interfaces/mmap/24-2.c
>
>In this case the mmap, which is clearly over current limit, unmaps
>dynamic libraries and the testcase segfaults right after returning into
>userspace.
>
>I've also looked at the second instance of the unmapping loop in the
>do_brk(). The do_brk() is called from brk() syscall and from vm_brk().
>The brk() syscall checks for overlapping mappings and bails out when
>there are any (so it can't be triggered from the brk syscall). The
>vm_brk() is called only from binmft handlers so it shouldn't be
>triggered unless binmft handler created overlapping mappings.
>

Reviewed-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>

>Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
>---
> mm/mmap.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 46 insertions(+), 4 deletions(-)
>
>diff --git a/mm/mmap.c b/mm/mmap.c
>index 2664a47..e755080 100644
>--- a/mm/mmap.c
>+++ b/mm/mmap.c
>@@ -33,6 +33,7 @@
> #include <linux/uprobes.h>
> #include <linux/rbtree_augmented.h>
> #include <linux/sched/sysctl.h>
>+#include <linux/kernel.h>
>
> #include <asm/uaccess.h>
> #include <asm/cacheflush.h>
>@@ -543,6 +544,34 @@ static int find_vma_links(struct mm_struct *mm, unsigned long addr,
> 	return 0;
> }
>
>+static unsigned long count_vma_pages_range(struct mm_struct *mm,
>+		unsigned long addr, unsigned long end)
>+{
>+	unsigned long nr_pages = 0;
>+	struct vm_area_struct *vma;
>+
>+	/* Find first overlaping mapping */
>+	vma = find_vma_intersection(mm, addr, end);
>+	if (!vma)
>+		return 0;
>+
>+	nr_pages = (min(end, vma->vm_end) -
>+		max(addr, vma->vm_start)) >> PAGE_SHIFT;
>+
>+	/* Iterate over the rest of the overlaps */
>+	for (vma = vma->vm_next; vma; vma = vma->vm_next) {
>+		unsigned long overlap_len;
>+
>+		if (vma->vm_start > end)
>+			break;
>+
>+		overlap_len = min(end, vma->vm_end) - vma->vm_start;
>+		nr_pages += overlap_len >> PAGE_SHIFT;
>+	}
>+
>+	return nr_pages;
>+}
>+
> void __vma_link_rb(struct mm_struct *mm, struct vm_area_struct *vma,
> 		struct rb_node **rb_link, struct rb_node *rb_parent)
> {
>@@ -1433,6 +1462,23 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
> 	unsigned long charged = 0;
> 	struct inode *inode =  file ? file_inode(file) : NULL;
>
>+	/* Check against address space limit. */
>+	if (!may_expand_vm(mm, len >> PAGE_SHIFT)) {
>+		unsigned long nr_pages;
>+
>+		/*
>+		 * MAP_FIXED may remove pages of mappings that intersects with
>+		 * requested mapping. Account for the pages it would unmap.
>+		 */
>+		if (!(vm_flags & MAP_FIXED))
>+			return -ENOMEM;
>+
>+		nr_pages = count_vma_pages_range(mm, addr, addr + len);
>+
>+		if (!may_expand_vm(mm, (len >> PAGE_SHIFT) - nr_pages))
>+			return -ENOMEM;
>+	}
>+
> 	/* Clear old maps */
> 	error = -ENOMEM;
> munmap_back:
>@@ -1442,10 +1488,6 @@ munmap_back:
> 		goto munmap_back;
> 	}
>
>-	/* Check against address space limit. */
>-	if (!may_expand_vm(mm, len >> PAGE_SHIFT))
>-		return -ENOMEM;
>-
> 	/*
> 	 * Private writable mapping: check memory availability
> 	 */
>-- 
>1.8.1.5
>
>See also a testsuite that exercies the newly added codepaths which is
>attached as a tarball (All testcases minus the second that tests
>that this patch works succeeds both before and after this patch).
>
>-- 
>Cyril Hrubis
>chrubis@suse.cz


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