[PATCH 6.18.y] iommu/vt-d: Gather the unmapped range before freeing its page tables

[Jose Fernandez (Anthropic)]

In the 6.12 and 6.18 stable trees, when an unmapped range covers a
whole page table, intel_iommu_unmap() can free that table before the
range has been invalidated. The freed table goes on gather->freelist
before the range is added to the gather. If
iommu_iotlb_gather_add_page() syncs before adding it, that sync
flushes only the earlier ranges but frees the whole freelist, that
table included. The range itself is flushed later with an empty
freelist, which means the flush is sent with the invalidation hint set
and the IOMMU may keep its paging-structure cache entry for the freed
table. DMA to the next mapping at that IOVA is then translated through
whatever the freed page holds by then, which is usually a silent wrong
translation and sometimes a DMAR fault.

Under a userspace driver that maps and unmaps DMA buffers through VFIO
type1 continuously, this shows up as wrong data in device reads and
writes. An occasional DMAR fault on a mapped IOVA is the only thing in
the logs. With an Intel DSA engine assigned through vfio-pci, remapping
a 16 MiB buffer at a fixed IOVA and reading it through the device
returned data from the wrong pages in 280 of 400 iterations. With a
fresh IOVA per iteration it never did.

Add the range to the gather first and splice the freed tables into
gather->freelist afterwards, so that they are only freed by a sync that
also invalidates their range.

Mainline removed this code in v6.19 with commit d373449d8e97
("iommu/vt-d: Use the generic iommu page table") and is not affected.

Fixes: 2a2b8eaa5b25 ("iommu: Handle freelists when using deferred flushing in iommu drivers")
Cc: stable@xxxxxxxxxxxxxxx # 6.12.y, 6.18.y
Reported-by: Mohammed Almaroof <moh@xxxxxxxxxxxxx>
Reviewed-by: Ben Cressey <ben@xxxxxxxxxxx>
Assisted-by: Claude:unspecified
Signed-off-by: Jose Fernandez (Anthropic) <jose.fernandez@xxxxxxxxx>
---
 drivers/iommu/intel/iommu.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index cee1851b69245..8b38c65f403b2 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -3620,6 +3620,7 @@ static size_t intel_iommu_unmap(struct iommu_domain *domain,
 				unsigned long iova, size_t size,
 				struct iommu_iotlb_gather *gather)
 {
+	struct iommu_pages_list freelist = IOMMU_PAGES_LIST_INIT(freelist);
 	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
 	unsigned long start_pfn, last_pfn;
 	int level = 0;
@@ -3636,7 +3637,7 @@ static size_t intel_iommu_unmap(struct iommu_domain *domain,
 	start_pfn = iova >> VTD_PAGE_SHIFT;
 	last_pfn = (iova + size - 1) >> VTD_PAGE_SHIFT;
 
-	domain_unmap(dmar_domain, start_pfn, last_pfn, &gather->freelist);
+	domain_unmap(dmar_domain, start_pfn, last_pfn, &freelist);
 
 	if (dmar_domain->max_addr == iova + size)
 		dmar_domain->max_addr = iova;
@@ -3648,6 +3649,14 @@ static size_t intel_iommu_unmap(struct iommu_domain *domain,
 	if (!iommu_iotlb_gather_queued(gather))
 		iommu_iotlb_gather_add_page(domain, gather, iova, size);
 
+	/*
+	 * iommu_iotlb_gather_add_page() may have synced, which frees
+	 * gather->freelist. Hand this range's page tables over only after
+	 * that call. A queued gather frees them from the flush queue
+	 * instead.
+	 */
+	iommu_pages_list_splice(&freelist, &gather->freelist);
+
 	return size;
 }
 

---
base-commit: 1efe5d048a391de3ead2804b2e7f86376c356cc5
change-id: 20260812-b4-vtd-unmap-gather-972bdd990123

Best regards,
--  
Jose Fernandez (Anthropic) <jose.fernandez@xxxxxxxxx>