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

["Huang, Honglei" <honghuan@amd.com>]

On 7/1/2026 8:36 PM, 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 :/
> 

Got it, I checked the code, the current code behavior does not cause a 
leak. Leon proposed a new modification, will send V2.
And I believe is a cleanup rather than a fix.

Regards,
Honglei


> 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@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);
>> +    }
>>   }
>>   /**
>>
>> base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
>