Re: [PATCH 3.2 056/185] mm: ensure get_unmapped_area() returns higher address than mmap_min_addr

Ben Hutchings
2014-01-03T04:26:57Z

[-- Attachment #1: Type: text/plain, Size: 3116 bytes --]

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

Ben.

> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1368,7 +1368,7 @@ arch_get_unmapped_area(struct file *filp
> 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)
> @@ -1377,7 +1377,7 @@ arch_get_unmapped_area(struct file *filp
> 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;
> }
> @@ -1442,9 +1442,10 @@ arch_get_unmapped_area_topdown(struct fi
> struct vm_area_struct *vma;
> struct mm_struct *mm = current->mm;
> unsigned long addr = addr0;
> + 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)
> @@ -1454,7 +1455,7 @@ arch_get_unmapped_area_topdown(struct fi
> 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;
> }
> @@ -1469,14 +1470,14 @@ arch_get_unmapped_area_topdown(struct fi
> addr = mm->free_area_cache;
>
> /* make sure it can fit in the remaining address space */
> - if (addr > len) {
> + if (addr >= low_limit + len) {
> vma = find_vma(mm, addr-len);
> if (!vma || addr <= vma->vm_start)
> /* remember the address as a hint for next time */
> return (mm->free_area_cache = addr-len);
> }
>
> - if (mm->mmap_base < len)
> + if (mm->mmap_base < low_limit + len)
> goto bottomup;
>
> addr = mm->mmap_base-len;
> @@ -1498,7 +1499,7 @@ arch_get_unmapped_area_topdown(struct fi
>
> /* 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);
>
> bottomup:
> /*

--
Ben Hutchings
The program is absolutely right; therefore, the computer must be wrong.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]