On Mon, 6 Jan 2014 12:32:07 +0000 Luis Henriques <luis.henriques@xxxxxxxxxxxxx> wrote: > On Mon, Jan 06, 2014 at 07:19:10PM +0900, Akira Takeuchi wrote: > > On Fri, 03 Jan 2014 04:26:43 +0000 > > Ben Hutchings <ben@xxxxxxxxxxxxxxx> wrote: > > > > > On Sun, 2013-12-29 at 03:08 +0100, Ben Hutchings wrote: > > > > 3.2.54-rc1 review patch. If anyone has any objections, please let me know. > > > > > > > > ------------------ > > > > > > > > From: Akira Takeuchi <takeuchi.akr@xxxxxxxxxxxxxxxx> > > > > > > > > commit 2afc745f3e3079ab16c826be4860da2529054dd2 upstream. > > > [...] > > > > [bwh: Backported to 3.2: > > > > As we do not have vm_unmapped_area(), make arch_get_unmapped_area_topdown() > > > > calculate the lower limit for the new area's end address and then compare > > > > addresses with this instead of with len. In the process, fix an off-by-one > > > > error which could result in returning 0 if mm->mmap_base == len.] > > > > > > I'm dropping this as I have no good way to test the backport (it's not > > > used on x86) and I didn't get any confirmation that it's right. > > > > I'm sorry for delayed reply. > > > > Your backport seems right. > > Additionally, I've confirmed the problem is resolved by your backport patch. > > Sorry I'm also late for this review. > > I guess this means the backport I made for the 3.5 kernel (and released on > 3.5.7.26) is incorrect: > > http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=commitdiff;h=745545489d25d1b9ecf2d78a8f9a31a362806d2d > > Akira, could you please confirm if this is the case so that I revert it in > next release? The backport for the 3.5 kernel is insufficient to solve the problem, as you are concered about. I've created the patch for 3.5 kernel based on Ben's patch. Please review and use it if there is no problem. Regads, Akira Takeuchi >From 70b8066b5a8bdbfd9000eb886f864923450dce9c Mon Sep 17 00:00:00 2001 From: Akira Takeuchi <takeuchi.akr@xxxxxxxxxxxxxxxx> Date: Tue, 7 Jan 2014 11:02:16 +0900 Subject: [PATCH] mm: ensure get_unmapped_area() returns higher address than mmap_min_addr commit 2afc745f3e3079ab16c826be4860da2529054dd2 upstream. This patch fixes the problem that get_unmapped_area() can return illegal address and result in failing mmap(2) etc. In case that the address higher than PAGE_SIZE is set to /proc/sys/vm/mmap_min_addr, the address lower than mmap_min_addr can be returned by get_unmapped_area(), even if you do not pass any virtual address hint (i.e. the second argument). This is because the current get_unmapped_area() code does not take into account mmap_min_addr. This leads to two actual problems as follows: 1. mmap(2) can fail with EPERM on the process without CAP_SYS_RAWIO, although any illegal parameter is not passed. 2. The bottom-up search path after the top-down search might not work in arch_get_unmapped_area_topdown(). Note: The first and third chunk of my patch, which changes "len" check, are for more precise check using mmap_min_addr, and not for solving the above problem. [How to reproduce] --- test.c ------------------------------------------------- #include <stdio.h> #include <unistd.h> #include <sys/mman.h> #include <sys/errno.h> int main(int argc, char *argv[]) { void *ret = NULL, *last_map; size_t pagesize = sysconf(_SC_PAGESIZE); do { last_map = ret; ret = mmap(0, pagesize, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0); // printf("ret=%p\n", ret); } while (ret != MAP_FAILED); if (errno != ENOMEM) { printf("ERR: unexpected errno: %d (last map=%p)\n", errno, last_map); } return 0; } --------------------------------------------------------------- $ gcc -m32 -o test test.c $ sudo sysctl -w vm.mmap_min_addr=65536 vm.mmap_min_addr = 65536 $ ./test (run as non-priviledge user) ERR: unexpected errno: 1 (last map=0x10000) Signed-off-by: Akira Takeuchi <takeuchi.akr@xxxxxxxxxxxxxxxx> Signed-off-by: Kiyoshi Owada <owada.kiyoshi@xxxxxxxxxxxxxxxx> Reviewed-by: Naoya Horiguchi <n-horiguchi@xxxxxxxxxxxxx> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> Signed-off-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx> [bwh: Backported to 3.2: As we do not have vm_unmapped_area(), make arch_get_unmapped_area_topdown() calculate the lower limit for the new area's end address and then compare addresses with this instead of with len. In the process, fix an off-by-one error which could result in returning 0 if mm->mmap_base == len.] Signed-off-by: Akira Takeuchi <takeuchi.akr@xxxxxxxxxxxxxxxx> [akira: Backported to 3.5: Based on Ben's backport for 3.2-stable kernel. ] --- mm/mmap.c | 13 +++++++------ 1 files changed, 7 insertions(+), 6 deletions(-) diff --git a/mm/mmap.c b/mm/mmap.c index 7e24763..529f72c 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -1443,7 +1443,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr, struct vm_area_struct *vma; unsigned long start_addr; - if (len > TASK_SIZE) + if (len > TASK_SIZE - mmap_min_addr) return -ENOMEM; if (flags & MAP_FIXED) @@ -1452,7 +1452,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr, if (addr) { addr = PAGE_ALIGN(addr); vma = find_vma(mm, addr); - if (TASK_SIZE - len >= addr && + if (TASK_SIZE - len >= addr && addr >= mmap_min_addr && (!vma || addr + len <= vma->vm_start)) return addr; } @@ -1515,9 +1515,10 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0, struct vm_area_struct *vma; struct mm_struct *mm = current->mm; unsigned long addr = addr0, start_addr; + unsigned long low_limit = max(PAGE_SIZE, mmap_min_addr); /* requested length too big for entire address space */ - if (len > TASK_SIZE) + if (len > TASK_SIZE - mmap_min_addr) return -ENOMEM; if (flags & MAP_FIXED) @@ -1527,7 +1528,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0, if (addr) { addr = PAGE_ALIGN(addr); vma = find_vma(mm, addr); - if (TASK_SIZE - len >= addr && + if (TASK_SIZE - len >= addr && addr >= mmap_min_addr && (!vma || addr + len <= vma->vm_start)) return addr; } @@ -1542,7 +1543,7 @@ try_again: /* either no address requested or can't fit in requested address hole */ start_addr = addr = mm->free_area_cache; - if (addr < len) + if (addr < low_limit + len) goto fail; addr -= len; @@ -1563,7 +1564,7 @@ try_again: /* try just below the current vma->vm_start */ addr = vma->vm_start-len; - } while (len < vma->vm_start); + } while (vma->vm_start >= low_limit + len); fail: /* -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html