Re: [PATCH v2 23/30] vfio/iommufd: use iova_to_phys_length for efficient unmap

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Out-of-bounds memory unpinning and unmapping in vfio_unmap_unpin() due to missing length cap.
--

commit cb17e198ab7d19ccbb139738e16d729d0fafa5dc
Author: Guanghui Feng <guanghuifeng@linux.alibaba.com>

vfio/iommufd: use iova_to_phys_length for efficient unmap

This commit modifies the IOMMU unmap paths to use
iommu_iova_to_phys_length(), allowing the code to skip ahead by the
mapping granularity rather than walking the page tables in PAGE_SIZE
steps.

> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index c8151ba54de33..c86315b1fcdad 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -1177,25 +1177,41 @@ static long vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma,
[ ... ]
>  		/*
>  		 * To optimize for fewer iommu_unmap() calls, each of which
>  		 * may require hardware cache flushing, try to find the
>  		 * largest contiguous physical memory chunk to unmap.
> +		 *
> +		 * Calculate remaining contiguous bytes within this PTE from
> +		 * our position, then try to join following physically
> +		 * contiguous PTEs.
>  		 */
> -		for (len = PAGE_SIZE; pos + len < dma->size; len += PAGE_SIZE) {
> -			next = iommu_iova_to_phys(domain->domain, iova + len);
> +		len = pgsize - (iova & (pgsize - 1));
> +		for (; pos + len < dma->size; ) {
> +			size_t next_pgsize;
> +
> +			next = iommu_iova_to_phys_length(domain->domain,
> +							  iova + len,
> +							  &next_pgsize);
>  			if (next != phys + len)
>  				break;
> +			if (WARN_ON(!next_pgsize || next_pgsize < PAGE_SIZE))
> +				next_pgsize = PAGE_SIZE;
> +			len += next_pgsize;
>  		}

[Severity: Critical]
Does this code introduce an out-of-bounds regression when an IOMMU PTE covers
space beyond the current vfio_dma region?

If the IOMMU driver dynamically coalesces adjacent mappings into a larger PTE
(such as a 2M hugepage covering two contiguous vfio_dma regions), pgsize
can exceed the remaining dma->size.

Because len is initialized to pgsize - (iova & (pgsize - 1)) and is not
clamped to dma->size - pos, it appears this larger length will be passed down
to unmap_unpin_fast().

Could this result in unpinning physical pages that belong to adjacent DMA
regions, which might later be unpinned a second time when those adjacent
regions are processed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602104637.1219810-1-guanghuifeng@linux.alibaba.com?part=23