linux-kernel - Re: [PATCH] [RFC]hugetlbfs: Get unmapped area below TASK_UNMAPPED

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <68031cc2-e6dc-52ee-9bdc-ffb9e154fb1c@oracle.com>
Date:   Mon, 27 Apr 2020 15:46:28 -0700
From:   Mike Kravetz <mike.kravetz@...cle.com>
To:     Shijie Hu <hushijie3@...wei.com>
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        nixiaoming@...wei.com, wangxu72@...wei.com,
        wangkefeng.wang@...wei.com, yangerkun@...wei.com,
        wangle6@...wei.com, cg.chen@...wei.com
Subject: Re: [PATCH] [RFC]hugetlbfs: Get unmapped area below
 TASK_UNMAPPED_BASE for hugetlbfs

On 4/27/20 4:10 AM, Shijie Hu wrote:
> In 32-bit programs, the address space is limited. When the normal mmap
> consumes the space above TASK_UNMAPPED_BASE on legacy mode, it can still
> successfully obtain unmapped area below TASK_UNMAPPED_BASE, but mmap or
> shmat for huge pages will fail. This seems "not fair".
> 
> When the request for huge pages fails, fall back to reuse mmap_min_addr
> ~ TASK_UNMAPPED_BASE for hugetlbfs.

Just curious.  Have you actually seeing a problem with this code, or is
the reason for the proposed change just the result of code inspection?  I ask
because many architectures have their own version of hugetlb_get_unmapped_area.
So, if you are seeing this issue it would be interesting to know what
architecture you are running.

The routine hugetlb_get_unmapped_area has not changed much since this first
git version.  I suspect this is because it is mostly unused.

I noticed that hugetlb_get_unmapped_area is one of only a few places in arch
independent code calling vm_unmapped_area().  The other callers are arch
independent fall back routines for arch_get_unmapped_area* routines.  If we
move forward with changes to this routine, would it make more sense to use
the arch_get_unmapped_area* routines instead of calling vm_unmapped_area
directly?  This would take advantage of any arch specific if it exists.

-- 
Mike Kravetz

> 
> Signed-off-by: Shijie Hu <hushijie3@...wei.com>
> ---
>  fs/hugetlbfs/inode.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index aff8642f0c2e..0f5997394aaa 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -224,7 +224,21 @@ hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
>  	info.high_limit = TASK_SIZE;
>  	info.align_mask = PAGE_MASK & ~huge_page_mask(h);
>  	info.align_offset = 0;
> -	return vm_unmapped_area(&info);
> +	addr = vm_unmapped_area(&info);
> +
> +	/*
> +	 * A failed request for huge pages very likely causes application
> +	 * failure, so fall back to the top-down function here.
> +	 */
> +	if (unlikely(offset_in_page(addr))) {
> +		VM_BUG_ON(addr != -ENOMEM);
> +		info.flags = VM_UNMAPPED_AREA_TOPDOWN;
> +		info.low_limit = max(PAGE_SIZE, mmap_min_addr);
> +		info.high_limit = TASK_UNMAPPED_BASE;
> +		addr = vm_unmapped_area(&info);
> +	}
> +
> +	return addr;
>  }
>  #endif
>  
>