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

[Huang, Honglei]

On 7/2/2026 6:24 PM, Leon Romanovsky wrote:
> 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@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);
> > +	}
>
> Probably the best change will be something like this:

Thanks for the review and for the shown rework, that's much cleaner
than my original approach.

Based on the your code, I have made some modifications to the 
compilation issues and will send V2.

And the v2 is essentially your diff, I wasn't sure which attribution 
you'd prefer:

  - Suggested-by, what I used in v2
	or
  - Co-developed-by: + your Signed-off-by.

Just let me know, will adjust in V3.

Regards,
Honglei


> 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