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.henriq...@canonical.com> 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 <b...@decadent.org.uk> 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....@jp.panasonic.com>
> > > > >
> > > > > 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....@jp.panasonic.com>
> 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....@jp.panasonic.com>
> Signed-off-by: Kiyoshi Owada <owada.kiyo...@jp.panasonic.com>
> Reviewed-by: Naoya Horiguchi <n-horigu...@ah.jp.nec.com>
> Signed-off-by: Andrew Morton <a...@linux-foundation.org>
> Signed-off-by: Linus Torvalds <torva...@linux-foundation.org>
> Signed-off-by: Ben Hutchings <b...@decadent.org.uk>
> [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....@jp.panasonic.com>
> [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 majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
