Re: [PATCH stable 5.15] 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 357809efe869317caba6039aa012c441fb76a0c6
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
callers and into unmapped_area() and unmapped_area_topdown() so that
vm_unmapped_area() never returns an address below mmap_min_addr.

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

[ ... ]

> diff --git a/mm/mmap.c b/mm/mmap.c
> index 53fa4f5167dd0..2c22f4bee04ee 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1917,6 +1917,7 @@ static unsigned long unmapped_area(struct vm_unmapped_area_info *info)
>  	struct mm_struct *mm = current->mm;
>  	struct vm_area_struct *vma;
>  	unsigned long length, low_limit, high_limit, gap_start, gap_end;
> +	unsigned long min_addr = max(info->low_limit, mmap_min_addr);
>  
>  	/* Adjust search length to account for worst case alignment overhead */
>  	length = info->length + info->align_mask;
> @@ -1928,9 +1929,9 @@ static unsigned long unmapped_area(struct vm_unmapped_area_info *info)
>  		return -ENOMEM;
>  	high_limit = info->high_limit - length;
>  
> -	if (info->low_limit > high_limit)
> +	if (min_addr > high_limit)
>  		return -ENOMEM;
> -	low_limit = info->low_limit + length;
> +	low_limit = min_addr + length;
>  
>  	/* Check if rbtree root looks promising */
>  	if (RB_EMPTY_ROOT(&mm->mm_rb))
> @@ -1996,8 +1997,8 @@ static unsigned long unmapped_area(struct vm_unmapped_area_info *info)
>  
>  found:
>  	/* We found a suitable gap. Clip it with the original low_limit. */
> -	if (gap_start < info->low_limit)
> -		gap_start = info->low_limit;
> +	if (gap_start < min_addr)
> +		gap_start = min_addr;
>  
>  	/* Adjust gap address to the desired alignment */
>  	gap_start += (info->align_offset - gap_start) & info->align_mask;

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

In security/min_addr.c, mmap_min_addr_handler() updates mmap_min_addr via
proc_doulongvec_minmax() without enforcing page alignment, which is why
round_hint_to_min() in mm/mmap.c explicitly uses PAGE_ALIGN(mmap_min_addr):

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

In the upstream commit 6b008640db73 ("mm: move 'mmap_min_addr' logic from
callers into vm_unmapped_area()"), vm_unmapped_area() also aligned
mmap_min_addr to a page boundary:

	info->low_limit = max(info->low_limit, PAGE_ALIGN(mmap_min_addr));

Here in unmapped_area(), min_addr is computed using the raw mmap_min_addr. If
mmap_min_addr is set to a non-page-aligned value above info->low_limit and a
bottom-up search finds a gap with gap_start < min_addr, gap_start is clipped
to the unaligned min_addr. Because info->align_mask has its low page-offset
bits cleared (or is 0), adjusting gap_start with info->align_mask leaves the
non-zero sub-page bits intact (and can also borrow from bit PAGE_SHIFT when
computing higher-order alignment).

Callers of vm_unmapped_area() then see offset_in_page(addr) or
(addr & ~PAGE_MASK) as non-zero and treat the returned address as an error,
causing get_unmapped_area() to fail with -EINVAL or callers checking
VM_BUG_ON(addr != -ENOMEM) to trigger.

Should unmapped_area() and unmapped_area_topdown() use
PAGE_ALIGN(mmap_min_addr) when computing min_addr?

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