Re: [PATCH] iommu/dma: free the entire IOVA reservation in dma_iova_destroy()

[Leon Romanovsky <leonro@nvidia.com>]

On Wed, Jul 01, 2026 at 05:20:33PM +0800, Honglei Huang wrote:
> dma_iova_try_alloc() reserves IOVA for the whole requested size and
> records it in state->__size, but callers may subsequently link only a
> part of that reservation, for example the drm_gpusvm mixed range case,
> where a device page range is linked incrementally.
> 
> The doc for dma_iova_destroy() is:
> 
>   "Unlink the IOVA range up to @mapped_len and free the entire IOVA
>    space."
> 
> However __iommu_dma_iova_unlink() computed the amount of IOVA to free
> from @mapped_len rather than from the full reservation. When the
> reservation is larger than the linked length, the tail
> [mapped_len, reserved size] is never returned to the allocator and
> is leaked, contrary to the documented contract.
> 
> Free the whole reservation using dma_iova_size(), mirroring
> dma_iova_free(). The unmap step still operates on @mapped_len only, and
> the same iotlb_gather is reused so a single IOTLB flush is performed.
> 
> Fixes: 433a76207dcf ("dma-mapping: Implement link/unlink ranges API")
> Cc: stable@vger.kernel.org
> Signed-off-by: Honglei Huang <honghuan@amd.com>
> ---
>  drivers/iommu/dma-iommu.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 9abaec0703e..bb29c82d1c8 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -2096,8 +2096,11 @@ static void __iommu_dma_iova_unlink(struct device *dev,
>  
>  	if (!iotlb_gather.queued)
>  		iommu_iotlb_sync(domain, &iotlb_gather);
> -	if (free_iova)
> +	if (free_iova) {
> +		/* Free the whole reservation, not just the linked @size. */
> +		size = iova_align(iovad, dma_iova_size(state) + iova_start_pad);
>  		iommu_dma_free_iova(domain, addr, size, &iotlb_gather);
> +	}

Probably the best change will be something like this:

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 9abaec0703ef..56173e24c8cc 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -2068,10 +2068,20 @@ static void iommu_dma_iova_unlink_range_slow(struct device *dev,
 		arch_sync_dma_flush();
 }
 
-static void __iommu_dma_iova_unlink(struct device *dev,
-		struct dma_iova_state *state, size_t offset, size_t size,
-		enum dma_data_direction dir, unsigned long attrs,
-		bool free_iova)
+/**
+ * dma_iova_unlink - Unlink a range of IOVA space
+ * @dev: DMA device
+ * @state: IOVA state
+ * @offset: offset into the IOVA state to unlink
+ * @size: size of the buffer
+ * @dir: DMA direction
+ * @attrs: attributes of mapping properties
+ *
+ * Unlink a range of IOVA space for the given IOVA state.
+ */
+void dma_iova_unlink(struct device *dev, struct dma_iova_state *state,
+		size_t offset, size_t size, enum dma_data_direction dir,
+		unsigned long attrs)
 {
 	struct iommu_domain *domain = iommu_get_dma_domain(dev);
 	struct iommu_dma_cookie *cookie = domain->iova_cookie;
@@ -2096,26 +2106,6 @@ static void __iommu_dma_iova_unlink(struct device *dev,
 
 	if (!iotlb_gather.queued)
 		iommu_iotlb_sync(domain, &iotlb_gather);
-	if (free_iova)
-		iommu_dma_free_iova(domain, addr, size, &iotlb_gather);
-}
-
-/**
- * dma_iova_unlink - Unlink a range of IOVA space
- * @dev: DMA device
- * @state: IOVA state
- * @offset: offset into the IOVA state to unlink
- * @size: size of the buffer
- * @dir: DMA direction
- * @attrs: attributes of mapping properties
- *
- * Unlink a range of IOVA space for the given IOVA state.
- */
-void dma_iova_unlink(struct device *dev, struct dma_iova_state *state,
-		size_t offset, size_t size, enum dma_data_direction dir,
-		unsigned long attrs)
-{
-	 __iommu_dma_iova_unlink(dev, state, offset, size, dir, attrs, false);
 }
 EXPORT_SYMBOL_GPL(dma_iova_unlink);
 
@@ -2136,14 +2126,13 @@ void dma_iova_destroy(struct device *dev, struct dma_iova_state *state,
 		unsigned long attrs)
 {
 	if (mapped_len)
-		__iommu_dma_iova_unlink(dev, state, 0, mapped_len, dir, attrs,
-				true);
-	else
-		/*
-		 * We can be here if first call to dma_iova_link() failed and
-		 * there is nothing to unlink, so let's be more clear.
-		 */
-		dma_iova_free(dev, state);
+		dma_iova_unlink(dev, state, 0, mapped_len, dir, attrs);
+
+	/*
+	 * We can be here if first call to dma_iova_link() failed and
+	 * there is nothing to unlink, so let's be more clear.
+	 */
+	dma_iova_free(dev, state);
 }
 EXPORT_SYMBOL_GPL(dma_iova_destroy);
 

>  }
>  
>  /**
> 
> base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
> -- 
> 2.34.1
>