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

[Leon Romanovsky <leon@kernel.org>]

On Wed, Jul 01, 2026 at 01:36:00PM +0100, Robin Murphy wrote:
> 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 :/

I found only one sentence that was somewhat unclear in this
context. Are you referring to it?
""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
https://lore.kernel.org/all/?q=dma_iova_destroy+robin+romanovsky
https://lore.kernel.org/all/ad2312e0-10d5-467a-be5e-75e80805b311@arm.com/
> +	if (free_iova)
> +		iommu_dma_free_iova(cookie, addr, size, &iotlb_gather);

Case in point, can you spot the bug here if dma_iova_destroy() is used
as intended? At least it's the relatively benign direction of this bug,
not the really fun pagetable corruption one.
"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""

Regarding the fix, I think it is the correct one. I intended to free
all space previously allocated by iommu_dma_alloc_iova(), not just
"mapped_len", as I wrote at the end.

Thanks