Re: [PATCH] powerpc/mm: Handle mmap_min_addr correctly in get_unmapped_area callback

[Laurent Dufour <ldufour@linux.vnet.ibm.com>]

Le 15/02/2019 à 09:16, Aneesh Kumar K.V a écrit :
> After we ALIGN up the address we need to make sure we didn't overflow
> and resulted in zero address. In that case, we need to make sure that
> the returned address is greater than mmap_min_addr.
> 
> Also when doing top-down search the low_limit is not PAGE_SIZE but rather
> max(PAGE_SIZE, mmap_min_addr). This handle cases in which mmap_min_addr >
> PAGE_SIZE.
> 
> This fixes selftest va_128TBswitch --run-hugetlb reporting failures when
> run as non root user for
> 
> mmap(-1, MAP_HUGETLB)
> mmap(-1, MAP_HUGETLB)
> 
> We also avoid the first mmap(-1, MAP_HUGETLB) returning NULL address as mmap address
> with this change

FWIW:
Reviewed-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>

> CC: Laurent Dufour <ldufour@linux.vnet.ibm.com>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
>   arch/powerpc/mm/hugetlbpage-radix.c |  5 +++--
>   arch/powerpc/mm/slice.c             | 10 ++++++----
>   2 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/powerpc/mm/hugetlbpage-radix.c b/arch/powerpc/mm/hugetlbpage-radix.c
> index 2486bee0f93e..97c7a39ebc00 100644
> --- a/arch/powerpc/mm/hugetlbpage-radix.c
> +++ b/arch/powerpc/mm/hugetlbpage-radix.c
> @@ -1,6 +1,7 @@
>   // SPDX-License-Identifier: GPL-2.0
>   #include <linux/mm.h>
>   #include <linux/hugetlb.h>
> +#include <linux/security.h>
>   #include <asm/pgtable.h>
>   #include <asm/pgalloc.h>
>   #include <asm/cacheflush.h>
> @@ -73,7 +74,7 @@ radix__hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
>   	if (addr) {
>   		addr = ALIGN(addr, huge_page_size(h));
>   		vma = find_vma(mm, addr);
> -		if (high_limit - len >= addr &&
> +		if (high_limit - len >= addr && addr >= mmap_min_addr &&
>   		    (!vma || addr + len <= vm_start_gap(vma)))
>   			return addr;
>   	}
> @@ -83,7 +84,7 @@ radix__hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
>   	 */
>   	info.flags = VM_UNMAPPED_AREA_TOPDOWN;
>   	info.length = len;
> -	info.low_limit = PAGE_SIZE;
> +	info.low_limit = max(PAGE_SIZE, mmap_min_addr);
>   	info.high_limit = mm->mmap_base + (high_limit - DEFAULT_MAP_WINDOW);
>   	info.align_mask = PAGE_MASK & ~huge_page_mask(h);
>   	info.align_offset = 0;
> diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
> index 06898c13901d..aec91dbcdc0b 100644
> --- a/arch/powerpc/mm/slice.c
> +++ b/arch/powerpc/mm/slice.c
> @@ -32,6 +32,7 @@
>   #include <linux/export.h>
>   #include <linux/hugetlb.h>
>   #include <linux/sched/mm.h>
> +#include <linux/security.h>
>   #include <asm/mman.h>
>   #include <asm/mmu.h>
>   #include <asm/copro.h>
> @@ -377,6 +378,7 @@ static unsigned long slice_find_area_topdown(struct mm_struct *mm,
>   	int pshift = max_t(int, mmu_psize_defs[psize].shift, PAGE_SHIFT);
>   	unsigned long addr, found, prev;
>   	struct vm_unmapped_area_info info;
> +	unsigned long min_addr = max(PAGE_SIZE, mmap_min_addr);
> 
>   	info.flags = VM_UNMAPPED_AREA_TOPDOWN;
>   	info.length = len;
> @@ -393,7 +395,7 @@ static unsigned long slice_find_area_topdown(struct mm_struct *mm,
>   	if (high_limit > DEFAULT_MAP_WINDOW)
>   		addr += mm->context.slb_addr_limit - DEFAULT_MAP_WINDOW;
> 
> -	while (addr > PAGE_SIZE) {
> +	while (addr > min_addr) {
>   		info.high_limit = addr;
>   		if (!slice_scan_available(addr - 1, available, 0, &addr))
>   			continue;
> @@ -405,8 +407,8 @@ static unsigned long slice_find_area_topdown(struct mm_struct *mm,
>   		 * Check if we need to reduce the range, or if we can
>   		 * extend it to cover the previous available slice.
>   		 */
> -		if (addr < PAGE_SIZE)
> -			addr = PAGE_SIZE;
> +		if (addr < min_addr)
> +			addr = min_addr;
>   		else if (slice_scan_available(addr - 1, available, 0, &prev)) {
>   			addr = prev;
>   			goto prev_slice;
> @@ -528,7 +530,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
>   		addr = _ALIGN_UP(addr, page_size);
>   		slice_dbg(" aligned addr=%lx\n", addr);
>   		/* Ignore hint if it's too large or overlaps a VMA */
> -		if (addr > high_limit - len ||
> +		if (addr > high_limit - len || addr < mmap_min_addr ||
>   		    !slice_area_is_free(mm, addr, len))
>   			addr = 0;
>   	}
>