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

[Robin Murphy]

On 01/07/2026 10:20 am, 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.

That's not what really happens in practice though - note that 
free_iova() doesn't even take a size, only a pfn with which to look up 
the corresponding rbtree entry. At worst, for sizes small enough for the 
rcaches, a larger IOVA may be put in a cache for a smaller size, which 
although wasteful, is otherwise pretty much benign.

This isn't to say that the allocator behaviour might not eventually 
change in future, but for now I can only assume that dma_iova_destroy() 
doing this is intentional, because I pointed it out at least 3 times 
over the course of the original review from RFC to eventual merge, and 
Leon made a point of refusing to do anything about it :/

Thanks,
Robin.

> 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@xxxxxxxxxxxxxxx
> Signed-off-by: Honglei Huang <honghuan@xxxxxxx>
> ---
>   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);
> +	}
>   }
>   
>   /**
>
> base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482