Re: [PATCH v11 2/7] mm/vmalloc: undo partial mappings inside the mapping functions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mm: BUG_ON() triggered in __vunmap_range_noflush() on first chunk failure
- [High] mm: stale TLB entries left on mapping rollback cause UAF
--

commit 8d2eaa273026d4be467acd8256954d343a66fc4c
Author: Hao Ge <hao.ge@linux.dev>

mm/vmalloc: undo partial mappings inside the mapping functions

This patch modifies __vmap_pages_range_noflush() and friends to undo the
PTEs they installed themselves on failure. Callers no longer need to clean up
partial mappings.

> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index a9fce7efe4f49..436393aba7eb7 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -349,6 +349,10 @@ static int vmap_range_noflush(unsigned long addr, unsigned long end,
>  	if (mask & ARCH_PAGE_TABLE_SYNC_MASK)
>  		arch_sync_kernel_mappings(start, end);
>  
> +	/* Undo the PTEs installed before the failure. */
> +	if (err)
> +		__vunmap_range_noflush(start, end);
> +
>  	return err;
>  }

[Severity: High]
Is it safe to omit a TLB flush on rollback here?

By clearing the PTEs using __vunmap_range_noflush() without flushing the TLB,
the CPU might retain stale entries.

If a caller like vm_module_tags_populate() in mm/alloc_tag.c frees the
physical pages immediately after the mapping attempt fails:

    if (vmap_pages_range(...) < 0) {
        ...
        release_pages(arg, nr);
    }

could speculative or adjacent accesses hit the freed memory, leading to a
use-after-free and state corruption?

[ ... ]

> @@ -683,6 +694,7 @@ int __vmap_pages_range_noflush(unsigned long addr, unsigned long end,
>  		pgprot_t prot, struct page **pages, unsigned int page_shift)
>  {
>  	unsigned int i, nr = (end - addr) >> PAGE_SHIFT;
> +	unsigned long start = addr;
>  
>  	WARN_ON(page_shift < PAGE_SHIFT);
>  
> @@ -696,8 +708,14 @@ int __vmap_pages_range_noflush(unsigned long addr, unsigned long end,
>  		err = vmap_range_noflush(addr, addr + (1UL << page_shift),
>  					page_to_phys(pages[i]), prot,
>  					page_shift);
> -		if (err)
> +		if (err) {
> +			/*
> +			 * vmap_range_noflush() undoes its own partial work,
> +			 * discard the chunks that already mapped.
> +			 */
> +			__vunmap_range_noflush(start, addr);
>  			return err;
> +		}

[Severity: High]
What happens if vmap_range_noflush() fails on the very first iteration of
this loop?

In that case, the variable addr is still equal to start, so this calls
__vunmap_range_noflush() with a zero-length range.

Because __vunmap_range_noflush() contains:
    BUG_ON(addr >= end);

will this immediately panic the kernel on an early allocation failure?

>  
>  		addr += 1UL << page_shift;
>  	}
> @@ -714,7 +732,16 @@ int vmap_pages_range_noflush(unsigned long addr, unsigned long end,
>  
>  	if (ret)
>  		return ret;
> -	return __vmap_pages_range_noflush(addr, end, prot, pages, page_shift);
> +
> +	ret = __vmap_pages_range_noflush(addr, end, prot, pages, page_shift);
> +	/*
> +	 * The page tables undo themselves on failure. Tear down the
> +	 * metadata that was fully set up before the mapping failed.
> +	 */
> +	if (ret)
> +		kmsan_vunmap_range_noflush(addr, end);

[Severity: High]
Are we also leaving behind stale TLB entries for the KMSAN metadata here?

kmsan_vmap_pages_range_noflush() successfully maps the metadata and
explicitly flushes the TLB for shadow/origin mappings.

If the subsequent main mapping fails, this rollback clears the KMSAN PTEs but
skips the TLB flush, which might leave stale TLB entries pointing to freed
KMSAN metadata.

> +
> +	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929082014.160587-1-hao.ge@linux.dev?part=2