Re: [PATCH stable 6.1] mm: move 'mmap_min_addr' logic from callers into vm_unmapped_area()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm: unaligned return from unmapped_area() when mmap_min_addr is unaligned
--

commit dd218bb48a347d4efd5770f53b201eae3a9f2336
Author: Linus Torvalds <torvalds@linux-foundation.org>

mm: move 'mmap_min_addr' logic from callers into vm_unmapped_area()

This commit moves the mmap_min_addr lower-bound clamping out of individual
get_unmapped_area() callers and into unmapped_area() and
unmapped_area_topdown().

Link: https://lkml.kernel.org/r/20230418214009.1142926-1-Liam.Howlett@oracle.com

> diff --git a/mm/mmap.c b/mm/mmap.c
> index 1eb86aba7de56..17ec4d40edc11 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1598,7 +1598,8 @@ static inline int accountable_mapping(struct file *file, vm_flags_t vm_flags)
>   */
>  static unsigned long unmapped_area(struct vm_unmapped_area_info *info)
>  {
> -	unsigned long length, gap, low_limit;
> +	unsigned long length, gap;
> +	unsigned long low_limit, high_limit;
>  	struct vm_area_struct *tmp;
>  
>  	MA_STATE(mas, &current->mm->mm_mt, 0, 0);
> @@ -1609,8 +1610,11 @@ static unsigned long unmapped_area(struct vm_unmapped_area_info *info)
>  		return -ENOMEM;
>  
>  	low_limit = info->low_limit;
> +	if (low_limit < mmap_min_addr)
> +		low_limit = mmap_min_addr;
> +	high_limit = info->high_limit;
>  retry:
> -	if (mas_empty_area(&mas, low_limit, info->high_limit - 1, length))
> +	if (mas_empty_area(&mas, low_limit, high_limit - 1, length))
>  		return -ENOMEM;
>  
>  	gap = mas.index;
> @@ -1646,7 +1650,8 @@ static unsigned long unmapped_area(struct vm_unmapped_area_info *info)
>   */
>  static unsigned long unmapped_area_topdown(struct vm_unmapped_area_info *info)
>  {
> -	unsigned long length, gap, high_limit, gap_end;
> +	unsigned long length, gap, gap_end;
> +	unsigned long low_limit, high_limit;
>  	struct vm_area_struct *tmp;
>  
>  	MA_STATE(mas, &current->mm->mm_mt, 0, 0);
> @@ -1655,10 +1660,12 @@ static unsigned long unmapped_area_topdown(struct vm_unmapped_area_info *info)
>  	if (length < info->length)
>  		return -ENOMEM;
>  
> +	low_limit = info->low_limit;
> +	if (low_limit < mmap_min_addr)
> +		low_limit = mmap_min_addr;

[Severity: Medium]
Can unmapped_area() return a non-page-aligned address when mmap_min_addr is
not a multiple of PAGE_SIZE?

In security/min_addr.c, mmap_min_addr_handler() updates dac_mmap_min_addr and
mmap_min_addr via proc_doulongvec_minmax() without enforcing page alignment,
which is why round_hint_to_min() in mm/mmap.c aligns it:

mm/mmap.c:round_hint_to_min() {
	if (((void *)hint != NULL) &&
	    (hint < mmap_min_addr))
		return PAGE_ALIGN(mmap_min_addr);
	return hint;
}

When a caller invokes vm_unmapped_area() with info->low_limit < mmap_min_addr
and mmap_min_addr is not page-aligned, unmapped_area() sets low_limit to the
unaligned mmap_min_addr. If no VMA covers low_limit, mas_empty_area() sets
mas.index to low_limit, and unmapped_area() computes:

	gap = mas.index;
	gap += (info->align_offset - gap) & info->align_mask;

Because info->align_mask is 0 for normal mappings and has its low PAGE_SHIFT
bits masked off via PAGE_MASK for colour-aligned and hugetlb mappings, the
non-zero page offset bits of gap are never cleared (and when info->align_mask
is non-zero, borrowing from the unaligned low bits during
info->align_offset - gap also corrupts the upper alignment bits).

This causes get_unmapped_area() to reject the returned address with -EINVAL:

mm/mmap.c:get_unmapped_area() {
	...
	if (offset_in_page(addr))
		return -EINVAL;
}

and triggers VM_BUG_ON(addr != -ENOMEM) in callers such as
hugetlb_get_unmapped_area_bottomup() and sparc64 arch_get_unmapped_area()
that check offset_in_page(addr) or addr & ~PAGE_MASK for errors.

Should unmapped_area() and unmapped_area_topdown() clamp low_limit to
PAGE_ALIGN(mmap_min_addr) instead of raw mmap_min_addr?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030172216.368052-4-florian.fainelli@broadcom.com?part=1