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

[Uladzislau Rezki <urezki@gmail.com>]

On Tue, Sep 29, 2026 at 04:20:09PM +0800, Hao Ge wrote:
> __vmap_pages_range_noflush() and friends can install some PTEs before
> failing and leave them mapped, and the kernel callers do not agree on
> who cleans them up. For example, pcpu_map_pages() and
> kmsan_ioremap_page_range() unmap the leftovers themselves, while
> vm_module_tags_populate() and the __GFP_NOFAIL retry loop in
> __vmalloc_area_node() relied on the mapping functions cleaning up and
> did not call anything like vunmap_range() themselves. When the same
> range is mapped again, the attempt hits the leftovers and fails, with
> BUG() in vmap_pte_range() for huge mappings.
> 
> After discussing with Suren and Ulad, we decided the cleanup belongs
> to __vmap_pages_range_noflush() and friends, so the callers no longer
> need to unmap the partial mappings themselves. Each function now
> undoes the PTEs it installed itself.
> 
> The rollback calls the low-level __vunmap_range_noflush(), it just
> clears the PTEs of the range it is given, which is all a rollback
> needs. It cannot use vunmap_range_noflush() because these mapping
> functions also map the KMSAN shadow and origin, and for a metadata
> range its hook would look up the metadata of the metadata, get 0
> and BUG() on addr >= end. The failed mappings were never accessed,
> no TLB flush needed.
> 
> Fixes: 9376130c390a ("mm/vmalloc: add support for __GFP_NOFAIL")
> Fixes: 0f9b685626da ("alloc_tag: populate memory for module tags as needed")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Cc: stable@vger.kernel.org
> Signed-off-by: Hao Ge <hao.ge@linux.dev>
> ---
>  mm/kmsan/shadow.c |  4 ++++
>  mm/vmalloc.c      | 31 +++++++++++++++++++++++++++++--
>  2 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/kmsan/shadow.c b/mm/kmsan/shadow.c
> index 0c88d89bf0d6..2166086d3dc3 100644
> --- a/mm/kmsan/shadow.c
> +++ b/mm/kmsan/shadow.c
> @@ -258,6 +258,10 @@ int kmsan_vmap_pages_range_noflush(unsigned long start, unsigned long end,
>  					    o_pages, page_shift);
>  	kmsan_leave_runtime();
>  	if (mapped) {
> +		/* Undo the shadow mapping set up above. */
> +		kmsan_enter_runtime();
> +		__vunmap_range_noflush(shadow_start, shadow_end);
> +		kmsan_leave_runtime();
>  		err = mapped;
>  		goto ret;
>  	}
>
Can we just do that inside the vmalloc? For example in the
__vmap_pages_range_noflush() as entry function for all(?) helpers?
So we do not need do it manually?

> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 859e6d2d57a3..9bbf75706627 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;
>  }
>  
> @@ -363,6 +367,9 @@ int vmap_page_range(unsigned long addr, unsigned long end,
>  	if (!err)
>  		err = kmsan_ioremap_page_range(addr, end, phys_addr, prot,
>  					       ioremap_max_page_shift);
> +	if (err)
> +		__vunmap_range_noflush(addr, end);
> +
>  	return err;
>  }
>  
> @@ -667,6 +674,10 @@ static int vmap_small_pages_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;
>  }
>  
The only one user of that function is __vmap_pages_range_noflush()
so we can cover both cases there small and huge path.

Is that enough just to do unroll in the below:

__vmap_pages_range_noflush()
vmap_page_range()

functions? It will also fix NOFAIL case:

<snip>
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 89c327a6ce7d..fa1f357ecf3f 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -359,10 +359,19 @@ int vmap_page_range(unsigned long addr, unsigned long end,
 
 	err = vmap_range_noflush(addr, end, phys_addr, pgprot_nx(prot),
 				 ioremap_max_page_shift);
+	if (err)
+		goto error_cleanup_range;
+
 	flush_cache_vmap(addr, end);
-	if (!err)
-		err = kmsan_ioremap_page_range(addr, end, phys_addr, prot,
+	err = kmsan_ioremap_page_range(addr, end, phys_addr, prot,
 					       ioremap_max_page_shift);
+	if (err)
+		goto error_cleanup_range;
+
+	return 0;
+
+error_cleanup_range:
+	__vunmap_range_noflush(addr, end);
 	return err;
 }
 
@@ -683,26 +692,35 @@ 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;
+	int err;
 
-	WARN_ON(page_shift < PAGE_SHIFT);
-
-	if (!IS_ENABLED(CONFIG_HAVE_ARCH_HUGE_VMALLOC) ||
-			page_shift == PAGE_SHIFT)
-		return vmap_small_pages_range_noflush(addr, end, prot, pages);
+	if (WARN_ON(addr >= end))
+		return -EINVAL;
 
-	for (i = 0; i < nr; i += 1U << (page_shift - PAGE_SHIFT)) {
-		int err;
+	if (WARN_ON(page_shift < PAGE_SHIFT))
+		return -EINVAL;
 
-		err = vmap_range_noflush(addr, addr + (1UL << page_shift),
+	if (!IS_ENABLED(CONFIG_HAVE_ARCH_HUGE_VMALLOC) ||
+		page_shift == PAGE_SHIFT) {
+		err = vmap_small_pages_range_noflush(addr, end, prot, pages);
+	} else {
+		for (i = 0; i < nr; i += 1U << (page_shift - PAGE_SHIFT)) {
+			err = vmap_range_noflush(addr, addr + (1UL << page_shift),
 					page_to_phys(pages[i]), prot,
 					page_shift);
-		if (err)
-			return err;
+			if (err)
+				break;
 
-		addr += 1UL << page_shift;
+			addr += 1UL << page_shift;
+		}
 	}
 
-	return 0;
+	if (err)
+		__vunmap_range_noflush(start, end);
+
+	/* 0 on success. */
+	return err;
 }
 
 int vmap_pages_range_noflush(unsigned long addr, unsigned long end,
@@ -714,7 +732,13 @@ 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);
+	if (ret)
+		/* Cleanup KMSAN metadata. */
+		kmsan_vunmap_range_noflush(addr, end);
+
+	return ret;
 }
 
 static int __vmap_pages_range(unsigned long addr, unsigned long end,
<snip>

--
Uladzislau Rezki