On Tue, Jan 07, 2014 at 11:25:30AM +0900, Akira Takeuchi wrote: > 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 > Thank you Akira. If there are no objections, I'll just revert the previous backport from 3.5 and apply this one instead. Cheers, -- Luis > > 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 -- 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