mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/2] mm: kmsan: fix iounmap metadata teardown
@ 2026-09-15 16:02 Dima Koziuk
  2026-09-15 16:02 ` [PATCH 2/2] mm: kmsan: fix ioremap error cleanup Dima Koziuk
  2026-09-15 22:35 ` [PATCH 1/2] mm: kmsan: fix iounmap metadata teardown Andrew Morton
  0 siblings, 2 replies; 4+ messages in thread
From: Dima Koziuk @ 2026-09-15 16:02 UTC (permalink / raw)
  To: Alexander Potapenko, Andrew Morton
  Cc: Marco Elver, Dmitry Vyukov, kasan-dev, linux-mm, linux-kernel,
	dmytrokoziuk68

While studying the code, I noticed that kmsan_iounmap_page_range() calls
__vunmap_range_noflush(v_shadow, vmalloc_shadow(end)) inside its per-page
loop, and does the same for origin. The first iteration therefore unmaps
the entire metadata range, removing the PTEs for later pages before the
loop can recover their backing pages.

Looking at the page lookup, I found another problem. The only callers of
kmsan_vmalloc_to_page_or_null() pass shadow and origin addresses, but the
helper accepts only regular vmalloc and module addresses. KMSAN metadata
lies outside those ranges, so the helper returns NULL and the backing
blocks are never freed.

I considered extending the helper's range check, but vmalloc_to_page()
also checks is_vmalloc_or_module_addr() through VIRTUAL_BUG_ON() when
CONFIG_DEBUG_VIRTUAL is enabled. Supporting metadata addresses there
would bring KMSAN-specific information of the address layout into generic
vmalloc code. That seemed broader than necessary for this fix and would
make it harder to review and maintain, so I looked at how KASAN handles
metadata teardown.

Follow the existing KASAN approach: walk the existing metadata PTEs, clear
each mapping and free its backing block, then flush the metadata TLB
ranges. Put this in a private kmsan_iounmap_pages() helper and remove the
now-unused kmsan_vmalloc_to_page_or_null() and its declaration.

Fixes: b073d7f8aee4 ("mm: kmsan: maintain KMSAN metadata for page operations")
Signed-off-by: Dima Koziuk <dmytrokoziuk68@gmail.com>

---
 mm/kmsan/core.c  |   14 ------------
 mm/kmsan/hooks.c |   64 +++++++++++++++++++++++++++++++++++++++---------------
 mm/kmsan/kmsan.h |    1 -
 3 files changed, 46 insertions(+), 33 deletions(-)

diff --git a/mm/kmsan/core.c b/mm/kmsan/core.c
index 90f427b95a21..bf8c86fdcc5a 100644
--- a/mm/kmsan/core.c
+++ b/mm/kmsan/core.c
@@ -236,20 +236,6 @@ void kmsan_internal_set_shadow_origin(void *addr, size_t size, int b,
 	}
 }
 
-struct page *kmsan_vmalloc_to_page_or_null(void *vaddr)
-{
-	struct page *page;
-
-	if (!kmsan_internal_is_vmalloc_addr(vaddr) &&
-	    !kmsan_internal_is_module_addr(vaddr))
-		return NULL;
-	page = vmalloc_to_page(vaddr);
-	if (pfn_valid(page_to_pfn(page)))
-		return page;
-	else
-		return NULL;
-}
-
 void kmsan_internal_check_memory(void *addr, size_t size,
 				 const void __user *user_addr, int reason)
 {
diff --git a/mm/kmsan/hooks.c b/mm/kmsan/hooks.c
index 5f1b8053f9fa..084ba667cbf7 100644
--- a/mm/kmsan/hooks.c
+++ b/mm/kmsan/hooks.c
@@ -20,6 +20,8 @@
 #include <linux/uaccess.h>
 #include <linux/usb.h>
 
+#include <asm/tlbflush.h>
+
 #include "../internal.h"
 #include "../vmalloc.h"
 #include "../slab.h"
@@ -142,6 +144,49 @@ void kmsan_vunmap_range_noflush(unsigned long start, unsigned long end)
 	flush_cache_vmap(vmalloc_origin(start), vmalloc_origin(end));
 }
 
+#define KMSAN_IOREMAP_META_ORDER 1
+
+static int kmsan_depopulate_vmalloc_pte(pte_t *ptep, unsigned long addr,
+					void *unused)
+{
+	pte_t pte;
+	int none;
+
+	lazy_mmu_mode_pause();
+
+	spin_lock(&init_mm.page_table_lock);
+	pte = ptep_get(ptep);
+	none = pte_none(pte);
+	if (likely(!none))
+		pte_clear(&init_mm, addr, ptep);
+	spin_unlock(&init_mm.page_table_lock);
+
+	if (likely(!none))
+		__free_pages(pfn_to_page(pte_pfn(pte)), KMSAN_IOREMAP_META_ORDER);
+
+	lazy_mmu_mode_resume();
+
+	return 0;
+}
+
+static void kmsan_iounmap_pages(unsigned long start, unsigned long end)
+{
+	unsigned long shadow_start = vmalloc_shadow(start),
+		      shadow_end = vmalloc_shadow(end);
+	unsigned long origin_start = vmalloc_origin(start),
+		      origin_end = vmalloc_origin(end);
+
+	apply_to_existing_page_range(&init_mm, shadow_start,
+				     shadow_end - shadow_start,
+				     kmsan_depopulate_vmalloc_pte, NULL);
+	apply_to_existing_page_range(&init_mm, origin_start,
+				     origin_end - origin_start,
+				     kmsan_depopulate_vmalloc_pte, NULL);
+
+	flush_tlb_kernel_range(shadow_start, shadow_end);
+	flush_tlb_kernel_range(origin_start, origin_end);
+}
+
 /*
  * This function creates new shadow/origin pages for the physical pages mapped
  * into the virtual memory. If those physical pages already had shadow/origin,
@@ -219,28 +264,11 @@ ret:
 
 void kmsan_iounmap_page_range(unsigned long start, unsigned long end)
 {
-	unsigned long v_shadow, v_origin;
-	struct page *shadow, *origin;
-	int nr;
-
 	if (!kmsan_enabled || kmsan_in_runtime())
 		return;
 
-	nr = (end - start) / PAGE_SIZE;
 	kmsan_enter_runtime();
-	v_shadow = (unsigned long)vmalloc_shadow(start);
-	v_origin = (unsigned long)vmalloc_origin(start);
-	for (int i = 0; i < nr;
-	     i++, v_shadow += PAGE_SIZE, v_origin += PAGE_SIZE) {
-		shadow = kmsan_vmalloc_to_page_or_null((void *)v_shadow);
-		origin = kmsan_vmalloc_to_page_or_null((void *)v_origin);
-		__vunmap_range_noflush(v_shadow, vmalloc_shadow(end));
-		__vunmap_range_noflush(v_origin, vmalloc_origin(end));
-		if (shadow)
-			__free_pages(shadow, 1);
-		if (origin)
-			__free_pages(origin, 1);
-	}
+	kmsan_iounmap_pages(start, end);
 	flush_cache_vmap(vmalloc_shadow(start), vmalloc_shadow(end));
 	flush_cache_vmap(vmalloc_origin(start), vmalloc_origin(end));
 	kmsan_leave_runtime();
diff --git a/mm/kmsan/kmsan.h b/mm/kmsan/kmsan.h
index bc3d1810f352..681594186663 100644
--- a/mm/kmsan/kmsan.h
+++ b/mm/kmsan/kmsan.h
@@ -165,7 +165,6 @@ bool kmsan_metadata_is_contiguous(void *addr, size_t size);
 void kmsan_internal_check_memory(void *addr, size_t size,
 				 const void __user *user_addr, int reason);
 
-struct page *kmsan_vmalloc_to_page_or_null(void *vaddr);
 void kmsan_setup_meta(struct page *page, struct page *shadow,
 		      struct page *origin, int order);
 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] mm: kmsan: fix ioremap error cleanup
  2026-09-15 16:02 [PATCH 1/2] mm: kmsan: fix iounmap metadata teardown Dima Koziuk
@ 2026-09-15 16:02 ` Dima Koziuk
  2026-09-15 22:35 ` [PATCH 1/2] mm: kmsan: fix iounmap metadata teardown Andrew Morton
  1 sibling, 0 replies; 4+ messages in thread
From: Dima Koziuk @ 2026-09-15 16:02 UTC (permalink / raw)
  To: Alexander Potapenko, Andrew Morton
  Cc: Marco Elver, Dmitry Vyukov, kasan-dev, linux-mm, linux-kernel,
	dmytrokoziuk68

Looking further at kmsan_ioremap_page_range(), I found three cases where
error cleanup leaks metadata blocks. Fault-injection testing confirmed
all three:

1. If the first iteration fails, clean is zero and cleanup is skipped,
   leaking any allocations that succeeded in that iteration.

2. If shadow mapping succeeds but origin mapping fails, the shadow
   pointer has already been cleared. Removing its mapping loses the
   backing block.

3. On failures after completed iterations, cleanup removes the earlier
   metadata mappings without freeing their backing blocks.

The cleanup needed here is the same as for iounmap, so it makes sense to
reuse kmsan_iounmap_pages(). Track the end of installed mappings with
mapped_end and advance it after each successful shadow mapping. This
includes the current shadow block if origin mapping subsequently fails,
while the PTE walk skips the missing origin mapping.

Run cleanup whenever err is non-zero. Free allocations that have not been
mapped directly, and use the shared helper to unmap and free the installed
metadata, including blocks from completed iterations.

Fixes: fdea03e12aa2 ("mm: kmsan: handle alloc failures in kmsan_ioremap_page_range()")
Signed-off-by: Dima Koziuk <dmytrokoziuk68@gmail.com>

---
I tested this series on Linux 7.3-rc3 under QEMU with CONFIG_KMSAN=y
and CONFIG_DEBUG_VIRTUAL=n, using ioremap()/iounmap() calls on the QEMU
VGA BAR0. All tested mappings were torn down without metadata leaks.

 mm/kmsan/hooks.c |   35 ++++++++++++-----------------------
 1 file changed, 12 insertions(+), 23 deletions(-)

diff --git a/mm/kmsan/hooks.c b/mm/kmsan/hooks.c
index 084ba667cbf7..02c402f129e1 100644
--- a/mm/kmsan/hooks.c
+++ b/mm/kmsan/hooks.c
@@ -199,16 +199,17 @@ int kmsan_ioremap_page_range(unsigned long start, unsigned long end,
 	gfp_t gfp_mask = GFP_KERNEL | __GFP_ZERO;
 	struct page *shadow, *origin;
 	unsigned long off = 0;
-	int nr, err = 0, clean = 0, mapped;
+	unsigned long mapped_end = start;
+	int nr, err = 0, mapped;
 
 	if (!kmsan_enabled || kmsan_in_runtime())
 		return 0;
 
 	nr = (end - start) / PAGE_SIZE;
 	kmsan_enter_runtime();
-	for (int i = 0; i < nr; i++, off += PAGE_SIZE, clean = i) {
-		shadow = alloc_pages(gfp_mask, 1);
-		origin = alloc_pages(gfp_mask, 1);
+	for (int i = 0; i < nr; i++, off += PAGE_SIZE) {
+		shadow = alloc_pages(gfp_mask, KMSAN_IOREMAP_META_ORDER);
+		origin = alloc_pages(gfp_mask, KMSAN_IOREMAP_META_ORDER);
 		if (!shadow || !origin) {
 			err = -ENOMEM;
 			goto ret;
@@ -222,39 +223,27 @@ int kmsan_ioremap_page_range(unsigned long start, unsigned long end,
 			goto ret;
 		}
 		shadow = NULL;
+		mapped_end = start + off + PAGE_SIZE;
 		mapped = __vmap_pages_range_noflush(
 			vmalloc_origin(start + off),
 			vmalloc_origin(start + off + PAGE_SIZE), prot, &origin,
 			PAGE_SHIFT);
 		if (mapped) {
-			__vunmap_range_noflush(
-				vmalloc_shadow(start + off),
-				vmalloc_shadow(start + off + PAGE_SIZE));
 			err = mapped;
 			goto ret;
 		}
 		origin = NULL;
 	}
-	/* Page mapping loop finished normally, nothing to clean up. */
-	clean = 0;
 
 ret:
-	if (clean > 0) {
-		/*
-		 * Something went wrong. Clean up shadow/origin pages allocated
-		 * on the last loop iteration, then delete mappings created
-		 * during the previous iterations.
-		 */
+	if (err) {
 		if (shadow)
-			__free_pages(shadow, 1);
+			__free_pages(shadow, KMSAN_IOREMAP_META_ORDER);
 		if (origin)
-			__free_pages(origin, 1);
-		__vunmap_range_noflush(
-			vmalloc_shadow(start),
-			vmalloc_shadow(start + clean * PAGE_SIZE));
-		__vunmap_range_noflush(
-			vmalloc_origin(start),
-			vmalloc_origin(start + clean * PAGE_SIZE));
+			__free_pages(origin, KMSAN_IOREMAP_META_ORDER);
+
+		if (mapped_end > start)
+			kmsan_iounmap_pages(start, mapped_end);
 	}
 	flush_cache_vmap(vmalloc_shadow(start), vmalloc_shadow(end));
 	flush_cache_vmap(vmalloc_origin(start), vmalloc_origin(end));

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] mm: kmsan: fix iounmap metadata teardown
  2026-09-15 16:02 [PATCH 1/2] mm: kmsan: fix iounmap metadata teardown Dima Koziuk
  2026-09-15 16:02 ` [PATCH 2/2] mm: kmsan: fix ioremap error cleanup Dima Koziuk
@ 2026-09-15 22:35 ` Andrew Morton
  2026-09-16  7:22   ` Dmytro Koziuk
  1 sibling, 1 reply; 4+ messages in thread
From: Andrew Morton @ 2026-09-15 22:35 UTC (permalink / raw)
  To: Dima Koziuk
  Cc: Alexander Potapenko, Marco Elver, Dmitry Vyukov, kasan-dev,
	linux-mm, linux-kernel

On Tue, 15 Sep 2026 19:02:06 +0300 Dima Koziuk <dmytrokoziuk68@gmail.com> wrote:

> While studying the code, I noticed that kmsan_iounmap_page_range() calls
> __vunmap_range_noflush(v_shadow, vmalloc_shadow(end)) inside its per-page
> loop, and does the same for origin. The first iteration therefore unmaps
> the entire metadata range, removing the PTEs for later pages before the
> loop can recover their backing pages.

Thanks.

AI review might have a found a couple of issues - please check?

https://sashiko.dev/#/patchset/20260915160207.2952-1-dmytrokoziuk68@gmail.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] mm: kmsan: fix iounmap metadata teardown
  2026-09-15 22:35 ` [PATCH 1/2] mm: kmsan: fix iounmap metadata teardown Andrew Morton
@ 2026-09-16  7:22   ` Dmytro Koziuk
  0 siblings, 0 replies; 4+ messages in thread
From: Dmytro Koziuk @ 2026-09-16  7:22 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Alexander Potapenko, Marco Elver, Dmitry Vyukov, kasan-dev,
	linux-mm, linux-kernel

>  AI review might have a found a couple of issues - please check?

I don't think the patch needs to be changed for these issues. For
normal iounmap(), the
caller must ensure that the mapping is no longer in use. Also,
the x86 implementation keeps the virtual range reserved until after
kmsan_iounmap_page_range() returns and its TLB flush completes.

For the kmsan_ioremap_page_range() error path, the mapping has not yet
been returned to the caller. Cleanup and the TLB flush complete before
the reserved virtual range is released.


ср, 16 сент. 2026 г. в 01:35, Andrew Morton <akpm@linux-foundation.org>:
>
> On Tue, 15 Sep 2026 19:02:06 +0300 Dima Koziuk <dmytrokoziuk68@gmail.com> wrote:
>
> > While studying the code, I noticed that kmsan_iounmap_page_range() calls
> > __vunmap_range_noflush(v_shadow, vmalloc_shadow(end)) inside its per-page
> > loop, and does the same for origin. The first iteration therefore unmaps
> > the entire metadata range, removing the PTEs for later pages before the
> > loop can recover their backing pages.
>
> Thanks.
>
> AI review might have a found a couple of issues - please check?
>
> https://sashiko.dev/#/patchset/20260915160207.2952-1-dmytrokoziuk68@gmail.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-16  7:22 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-15 16:02 [PATCH 1/2] mm: kmsan: fix iounmap metadata teardown Dima Koziuk
2026-09-15 16:02 ` [PATCH 2/2] mm: kmsan: fix ioremap error cleanup Dima Koziuk
2026-09-15 22:35 ` [PATCH 1/2] mm: kmsan: fix iounmap metadata teardown Andrew Morton
2026-09-16  7:22   ` Dmytro Koziuk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®