Re: [PATCH v11 10/10] mm/vmalloc: align vm_area so vmap() can batch mappings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] mm/vmalloc: out-of-bounds physical mapping in vmap() across PMD boundary
--

commit bba985c9d433d420f66d2ffcfeb47090bbcf7552
Author: Barry Song (Xiaomi) <baohua@kernel.org>

mm/vmalloc: align vm_area so vmap() can batch mappings

This commit aligns the virtual address area allocated for vmap() to PMD_SIZE
or the architecture-supported PTE batch shift so that contiguous pages can be
mapped in batches.

> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 42b3addd20f98..e15bcef804fc3 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c

[ ... ]

> @@ -3692,6 +3700,30 @@ static int vmap_pages_range_batched(unsigned long addr, unsigned long end,
>  	return err;
>  }
>  
> +static struct vm_struct *vmap_get_aligned_vm_area(unsigned long size,
> +		unsigned long flags, pgprot_t prot, const void *caller)
> +{
> +	struct vm_struct *vm_area;
> +	unsigned int shift;
> +
> +	if (arch_vmap_pmd_supported(prot) && size >= PMD_SIZE) {
> +		vm_area = __get_vm_area_node_aligned_caller(size, PMD_SIZE,
> +				flags, caller);
> +		if (vm_area)
> +			return vm_area;
> +	}
> +
> +	shift = arch_vmap_pte_supported_shift(size);
> +	if (shift > PAGE_SHIFT) {
> +		vm_area = __get_vm_area_node_aligned_caller(size, 1UL << shift,
> +				flags, caller);
> +		if (vm_area)
> +			return vm_area;
> +	}

[Severity: Critical]
Can this cause vmap() to map out-of-bounds physical pages across a PMD
boundary on arm64 when pages[] contains multiple physically discontiguous
chunks that share a mapping shift larger than CONT_PTE_SHIFT?

In vmap_get_aligned_vm_area(), when size < PMD_SIZE (for example, a 1MB vmap
composed of two 512KB order-7 chunks on 4K-page arm64, or an 8MB vmap composed
of two 4MB order-8 chunks on 16K-page arm64 where PMD_SIZE is 32MB), or when
the PMD_SIZE-aligned allocation falls back, the virtual range is only aligned
to 1UL << arch_vmap_pte_supported_shift(size), which is CONT_PTE_SIZE on
arm64. The same alignment also occurs for batch_start if a PMD-aligned vmap
starts with a CONT_PTE_SIZE chunk followed by larger chunks.

In vmap_pages_range_batched(), get_vmap_mapping_order() can return an order
where shift > CONT_PTE_SHIFT, and consecutive discontiguous chunks with the
same shift are coalesced into a single walk:

mm/vmalloc.c:vmap_pages_range_batched() {
	for (unsigned int i = 0; i < nr_pages; ) {
		unsigned int shift = PAGE_SHIFT +
			get_vmap_mapping_order(pages + i, prot, nr_pages - i);

		if (!i)
			prev_shift = shift;

		if (shift != prev_shift) {
			err = vmap_pages_range_noflush_walk(batch_start, batch_end,
					prot, pages + batch_idx, prev_shift);
			...
		}
		...
		batch_end += 1UL << shift;
		i += 1U << (shift - PAGE_SHIFT);
	}
}

When [batch_start, batch_end) crosses a PMD boundary at an offset M into the
first chunk (where 0 < M < (1UL << prev_shift) and M is a multiple of
CONT_PTE_SIZE), vmap_pages_pmd_range() splits the walk at pmd_addr_end(addr,
end). The first PMD maps the initial M bytes of chunk 0 and advances *nr to
M >> PAGE_SHIFT.

At the start of the next PMD (addr = PMD_BOUNDARY), *nr points M bytes into
chunk 0 (physical address PA_0 + M), leaving only (1UL << prev_shift) - M
contiguous bytes in chunk 0 before pages[] switches to the discontiguous
physical address of chunk 1. However, vmap_pages_pte_range() still passes the
full shift = prev_shift to vmap_set_ptes():

mm/vmalloc.c:vmap_pages_pte_range() {
	do {
		struct page *page = pages[*nr];
		...
		pfn = page_to_pfn(page);
		size = vmap_set_ptes(pte, addr, end, pfn, prot, shift);
		nr_pages = PFN_DOWN(size);
	} while (pte += nr_pages, *nr += nr_pages, addr += size, addr != end);
}

Because addr (at PMD_BOUNDARY) and PFN_PHYS(pfn) (at PA_0 + M) are both
aligned to CONT_PTE_SIZE, and end - addr covers the coalesced second chunk,
arch_vmap_pte_range_map_size() returns the full 1UL << prev_shift:

arch/arm64/include/asm/vmalloc.h:arch_vmap_pte_range_map_size() {
	if (!IS_ALIGNED(addr, CONT_PTE_SIZE))
		return PAGE_SIZE;

	if (!IS_ALIGNED(PFN_PHYS(pfn), CONT_PTE_SIZE))
		return PAGE_SIZE;

	size = min(end - addr, 1UL << max_page_shift);
	size = round_down(size, CONT_PTE_SIZE);
	return size;
}

Doesn't vmap_pte_set_huge() then map 1UL << prev_shift contiguous physical
bytes starting at PA_0 + M, overrunning the end of chunk 0 by M bytes into
unowned physical memory while advancing *nr past the first M bytes of chunk 1?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103101658.766905-1-jiangwenxiaomi@gmail.com?part=10