Re: [RFC PATCH] Optimize VFIO and IOMMU mapping traversal

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The computed chunk size `len` in `vfio_unmap_unpin` is not clamped to the remaining size of the target DMA region (`dma->size - pos`). This allows `len` to overshoot the DMA region boundary, causing out-of-bounds unmapping and unpinning of adjacent memory.
--

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

Optimize VFIO and IOMMU mapping traversal

This commit adds an iommu_iova_to_pgsize() implementation to query the
hardware mapping page size. It uses this in VFIO to optimize
vfio_unmap_unpin() by traversing the mappings at the IOMMU map granularity
rather than PAGE_SIZE.

> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index c8151ba54de33..bf918a93a1591 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c

[ ... ]

> @@ -1191,11 +1191,24 @@ 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.
> +		 *
> +		 * Query the actual IOMMU PTE mapping granularity at this IOVA
> +		 * to determine the guaranteed contiguous range. Use only the
> +		 * remaining portion within the current PTE from our position,
> +		 * in case we start from the middle of a large page mapping.
>  		 */
> -		for (len = PAGE_SIZE; pos + len < dma->size; len += PAGE_SIZE) {
> +		pgsize = iommu_iova_to_pgsize(domain->domain, iova);
> +		if (!pgsize)
> +			pgsize = PAGE_SIZE;
> +		len = pgsize - (iova & (pgsize - 1));
> +		for (; pos + len < dma->size; len += pgsize) {

[Severity: Critical]
Could this len calculation exceed the boundaries of the DMA region?

If the hardware mapped a large page that extends beyond the target vfio_dma
region, or if a partial unmap split the vfio_dma region but left the hardware
mapping intact, the calculated len can be larger than dma->size - pos.

The loop condition pos + len < dma->size prevents further iterations but does
not clamp the oversized len. Does this result in unmap_unpin_fast() being
called with an out-of-bounds len, which would unmap and unpin pages belonging
to adjacent memory regions?

Would it be better to clamp len to the remaining size before the unmap?
len = min_t(size_t, len, dma->size - pos);

>  			next = iommu_iova_to_phys(domain->domain, iova + len);
>  			if (next != phys + len)
>  				break;
> +			pgsize = iommu_iova_to_pgsize(domain->domain,
> +						      iova + len);
> +			if (!pgsize)
> +				pgsize = PAGE_SIZE;
>  		}
>  
>  		/*

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