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 --- 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));