[PATCH 0/2] iommu: Drop IOMMU_DEBUG_PAGEALLOC refs on iommupt domain teardown

Yuanhe Shu posted 2 patches 1 month ago
drivers/iommu/generic_pt/iommu_pt.h   | 22 +++++++++++++++++---
drivers/iommu/iommu-debug-pagealloc.c | 30 ++++++++++++++++++++++++---
drivers/iommu/iommu-priv.h            | 24 +++++++++++++++++++++
3 files changed, 70 insertions(+), 6 deletions(-)
[PATCH 0/2] iommu: Drop IOMMU_DEBUG_PAGEALLOC refs on iommupt domain teardown
Posted by Yuanhe Shu 1 month ago
Since commit b948a8722848 ("iommu: Fix up map/unmap debugging for
iommupt domains") iommu_map() takes an IOMMU_DEBUG_PAGEALLOC reference
for every page it maps into a domain, and the only path that drops
those references is the IOVA based unmap.  When a generic_pt domain is
freed while mappings are still installed, pt_iommu_deinit() releases
the page table memory without going through that path, so every mapped
page keeps its reference forever and each later allocation or free of
it trips:

    WARNING: drivers/iommu/iommu-debug-pagealloc.c:91 at __iommu_debug_check_unmapped+0x4e/0x70, CPU#0: init/1
    iommu: Detected page leak!

Freeing a domain with mappings still installed is not driver misuse:
the deinit contract in include/linux/generic_pt/iommu.h only requires
the table to be removed from HW access and caches, with no requirement
to unmap first.  iommu_setup_default_domain() frees the old domain with
its IOMMU_RESV_DIRECT mappings still installed (those pages normally
never return to the page allocator, so it does not WARN today), and the
generic_pt kunit suite (CONFIG_IOMMU_PT_KUNIT_TEST) does the same in
pt_kunit_iommu_exit().

Patch 1 adds __iommu_debug_unmap_phys(), the physical address based
counterpart of __iommu_debug_map().  Patch 2 wires it into the deinit
walk: a debug_unmap flag makes __collect_tables() drop the reference of
every OA leaf it destroys, symmetric to how iommu_map() created them.

io-pgtable has the same gap in __arm_lpae_free_pgtable(), but its
cookies cannot reach the struct iommu_domain, so that fix needs an ABI
change or per-driver handling and is left as a follow-up.

The kunit suite doubles as an in-tree reproducer: with
CONFIG_IOMMU_DEBUG_PAGEALLOC=y and CONFIG_IOMMU_PT_KUNIT_TEST=y, boot
with iommu.debug_pagealloc=1
kunit.filter_glob=x86_64_iommu_test.test_pgsize_boundary, then
allocate and free most of memory (the case maps 128K at the hard-coded
OA 0x208b95d000 and never unmaps it, so the machine needs enough RAM
for that address to be online memory - a 150G guest was used here, and
the sweep was a tmpfs filled to 95% of RAM):

    unpatched:          64 page leak WARNINGs
    with this series:   0

The 64 is one WARNING for each of the 32 mapped pages on both its
allocation and free.  On unpatched mainline the suite already fails
test_random_map's NR_SECONDARY_PAGETABLE assertion, which aborts its
cleanup and cascades into the following cases, hence the isolation.  An
out-of-tree module mapping a page into an amdv1 domain and freeing the
domain without unmapping shows the same behaviour, 384 WARNINGs over
64 iterations unpatched and none with the series; each leaked page is
reported again on every later allocation and free, so the count
exceeds the 64 leaked pages.  The control case that unmaps first stays
silent.

The generic_pt format code can be built as a module (e.g.
CONFIG_IOMMU_PT_AMDV1=m), so the new helper and the
iommu_debug_initialized static key are exported GPL.

Yuanhe Shu (2):
  iommu: Add __iommu_debug_unmap_phys() to drop refs by physical address
  iommupt: Drop pagealloc references during domain deinit

 drivers/iommu/generic_pt/iommu_pt.h   | 22 +++++++++++++++++---
 drivers/iommu/iommu-debug-pagealloc.c | 30 ++++++++++++++++++++++++---
 drivers/iommu/iommu-priv.h            | 24 +++++++++++++++++++++
 3 files changed, 70 insertions(+), 6 deletions(-)

base-commit: 45c13f3f9e3bb15fd89ff2864c6f627a3b4b4229
-- 
2.43.5
Re: [PATCH 0/2] iommu: Drop IOMMU_DEBUG_PAGEALLOC refs on iommupt domain teardown
Posted by Mostafa Saleh 1 month ago
Hi Yuanhe,

On Thu, Aug 27, 2026 at 10:58:53PM +0800, Yuanhe Shu wrote:
> Since commit b948a8722848 ("iommu: Fix up map/unmap debugging for
> iommupt domains") iommu_map() takes an IOMMU_DEBUG_PAGEALLOC reference
> for every page it maps into a domain, and the only path that drops
> those references is the IOVA based unmap.  When a generic_pt domain is
> freed while mappings are still installed, pt_iommu_deinit() releases
> the page table memory without going through that path, so every mapped
> page keeps its reference forever and each later allocation or free of
> it trips:
> 
>     WARNING: drivers/iommu/iommu-debug-pagealloc.c:91 at __iommu_debug_check_unmapped+0x4e/0x70, CPU#0: init/1
>     iommu: Detected page leak!
> 
> Freeing a domain with mappings still installed is not driver misuse:
> the deinit contract in include/linux/generic_pt/iommu.h only requires

Which drivers cause this?

AFAICT, users of the DMA-API must unmap the pages, otherwise they
run into bigger issue (as leaking IOVA).
This is stated in Documentation/core-api/dma-api-howto.rst
  Every dma_map_{single,sg}() call should have its dma_unmap_{single,sg}()
  counterpart, because the DMA address space is a shared resource and
  you could render the machine unusable by consuming all DMA addresses.

On the other side, there are very few driver that use the IOMMU API
directly, and from what I can see they do iommu_unmap().

Some page table implementations might tolerate it (because it is simpler
and more efficient to implement instead of descending to last level
tables) but I don't think that makes it right.

Thanks,
Mostafa


> the table to be removed from HW access and caches, with no requirement
> to unmap first.  iommu_setup_default_domain() frees the old domain with
> its IOMMU_RESV_DIRECT mappings still installed (those pages normally
> never return to the page allocator, so it does not WARN today), and the
> generic_pt kunit suite (CONFIG_IOMMU_PT_KUNIT_TEST) does the same in
> pt_kunit_iommu_exit().
> 
> Patch 1 adds __iommu_debug_unmap_phys(), the physical address based
> counterpart of __iommu_debug_map().  Patch 2 wires it into the deinit
> walk: a debug_unmap flag makes __collect_tables() drop the reference of
> every OA leaf it destroys, symmetric to how iommu_map() created them.
> 
> io-pgtable has the same gap in __arm_lpae_free_pgtable(), but its
> cookies cannot reach the struct iommu_domain, so that fix needs an ABI
> change or per-driver handling and is left as a follow-up.
> 
> The kunit suite doubles as an in-tree reproducer: with
> CONFIG_IOMMU_DEBUG_PAGEALLOC=y and CONFIG_IOMMU_PT_KUNIT_TEST=y, boot
> with iommu.debug_pagealloc=1
> kunit.filter_glob=x86_64_iommu_test.test_pgsize_boundary, then
> allocate and free most of memory (the case maps 128K at the hard-coded
> OA 0x208b95d000 and never unmaps it, so the machine needs enough RAM
> for that address to be online memory - a 150G guest was used here, and
> the sweep was a tmpfs filled to 95% of RAM):
> 
>     unpatched:          64 page leak WARNINGs
>     with this series:   0
> 
> The 64 is one WARNING for each of the 32 mapped pages on both its
> allocation and free.  On unpatched mainline the suite already fails
> test_random_map's NR_SECONDARY_PAGETABLE assertion, which aborts its
> cleanup and cascades into the following cases, hence the isolation.  An
> out-of-tree module mapping a page into an amdv1 domain and freeing the
> domain without unmapping shows the same behaviour, 384 WARNINGs over
> 64 iterations unpatched and none with the series; each leaked page is
> reported again on every later allocation and free, so the count
> exceeds the 64 leaked pages.  The control case that unmaps first stays
> silent.
> 
> The generic_pt format code can be built as a module (e.g.
> CONFIG_IOMMU_PT_AMDV1=m), so the new helper and the
> iommu_debug_initialized static key are exported GPL.
> 
> Yuanhe Shu (2):
>   iommu: Add __iommu_debug_unmap_phys() to drop refs by physical address
>   iommupt: Drop pagealloc references during domain deinit
> 
>  drivers/iommu/generic_pt/iommu_pt.h   | 22 +++++++++++++++++---
>  drivers/iommu/iommu-debug-pagealloc.c | 30 ++++++++++++++++++++++++---
>  drivers/iommu/iommu-priv.h            | 24 +++++++++++++++++++++
>  3 files changed, 70 insertions(+), 6 deletions(-)
> 
> base-commit: 45c13f3f9e3bb15fd89ff2864c6f627a3b4b4229
> -- 
> 2.43.5
Re: [PATCH 0/2] iommu: Drop IOMMU_DEBUG_PAGEALLOC refs on iommupt domain teardown
Posted by Yuanhe Shu 3 weeks, 5 days ago
On Thu, Aug 27, 2026 at 04:05:14PM +0000, Mostafa Saleh wrote:
> Which drivers cause this?

None that I can point to.  The trigger was the generic_pt kunit suite,
which frees domains with live mappings, plus a module I wrote to confirm
the mechanism.  You and Jason are both right that an iommu API user has to
unmap, so the premise of the series was wrong; I am dropping it and fixing
the kunit cases to unmap instead.

And from your reply on 1/2:

> But the IOVA can be calculated when walking the table from freeing
> context or am I missing something?

You weren't missing anything - Jason pointed out the same thing, and no
iova_to_phys() is involved: pts->range->va already holds the IOVA of the
current iteration position.

Thanks,
Yuanhe