Re: [PATCH v4 3/9] mm/vmalloc: BUG_ON() on an unaligned range in vmap_range_noflush()

[Hao Ge <hao.ge@linux.dev>]

On 2026/11/10 19:25, Hao Ge wrote:
> commit a739e6b557af ("mm: vmalloc: WARN_ON if mapping size is not
> PAGE_SIZE aligned") put the check in vmap_pte_range(), which is too
> late, because the upper levels may already have installed page
> tables for a range that is rejected anyway.
> 
> The pte loop maps whole pages and stops only when addr lands
> exactly on end.  An unaligned end lets addr jump over it and the
> loop keeps mapping past the range.  An unaligned start maps the
> page below the range and usually misses end too.  Such a range is
> never a legitimate request, and the corruption it leaves behind is
> hard to attribute to the caller afterwards, so bug out on addr and
> end at the entry of vmap_range_noflush() and drop the check from
> vmap_pte_range().
> 
> Signed-off-by: Hao Ge <hao.ge@linux.dev>
> ---
>  mm/vmalloc.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 093b8d5f7b0e..27f2674a5556 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -125,9 +125,6 @@ static int vmap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
>  	unsigned long size;
>  	unsigned int nr_pages;
>  
> -	if (WARN_ON_ONCE(!PAGE_ALIGNED(end - addr)))
> -		return -EINVAL;
> -
>  	pfn = phys_addr >> PAGE_SHIFT;
>  	pte = pte_alloc_kernel_track(pmd, addr, mask);
>  	if (!pte)
> @@ -347,6 +344,7 @@ static int vmap_range_noflush(unsigned long addr, unsigned long end,
>  	 */
>  	might_sleep();
>  	BUG_ON(addr >= end);
> +	BUG_ON(!PAGE_ALIGNED(addr) || !PAGE_ALIGNED(end));

I know BUG_ON() is ugly and it was not my first choice either, but I
have not figured out a better way to handle __vunmap_range_noflush().

If an unaligned range is passed to it, the pte loop may clear PTEs
adjacent to the range. Say a caller passes [3, 12) while [0, 3)
belongs to somebody else. The loop operates on whole pages, so it
starts at the page containing 3 and clears [0, 3) with it. That
silently corrupts a live mapping, and the corruption is not detected
until the owner touches the page. Bugs like that are nearly
impossible to debug afterwards.

It is not feasible for __vunmap_range_noflush() to just return an
invalid-argument error upfront. Its job is to clear PTEs, and an
early return leaves the residual PTEs behind.

I also thought about warning and returning -EINVAL on the map side
and keeping BUG_ON() only in __vunmap_range_noflush(), but the
rollback kills that idea.

int __vmap_pages_range_noflush(unsigned long addr, unsigned long end,
                pgprot_t prot, struct page **pages, unsigned int page_shift,
                gfp_t gfp_mask)
{
        int err;

        if (WARN_ON_ONCE(page_shift < PAGE_SHIFT))
                return -EINVAL;

        if (!IS_ENABLED(CONFIG_HAVE_ARCH_HUGE_VMALLOC))
                page_shift = PAGE_SHIFT;

        err = vmap_pages_range_noflush_walk(addr, end, prot, pages, page_shift);

        /* Undo the PTEs installed before the failure. */
        if (err)
                __vunmap_range_noflush(addr, end,
                                       gfpflags_allow_blocking(gfp_mask));

        return err;
}

If the walk WARNs and returns -EINVAL for a bad range, the undo
above passes that same range to __vunmap_range_noflush(), and we
BUG right after the WARN. vmap_page_range() runs into the same
thing on its error path.

So the map side cannot warn while the unmap side BUGs, the rollback
drags the rejected range into the unmap anyway. And the unmap side
cannot warn and skip either, that leaves the stale PTEs.

Also, these checks are the last line of defense. All in-tree callers
pass page aligned, non-empty ranges, and I am not aware of anyone ever
hitting this line. So for correct code the BUG_ON never fires at all,
no user will ever see it. The only one who can trip it is the developer
who just passed the bad range, and for him a stack trace right at the
spot is much better than chasing a silent corruption afterwards.

I settled on BUG_ON here. It may not be an ideal solution, but if anyone
has a better idea or alternative approach, please let me know.
I would greatly appreciate it.

Thanks
Best Regards
Hao

>  
>  	start = addr;
>  	pgd = pgd_offset_k(addr);