Re: [PATCH 7/9] 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] Missing bounds check in `vfio_unmap_unpin` allows out-of-bounds unmap and unpin
--

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

vfio/iommufd: use iova_to_phys_length for efficient unmap
    
Use iommu_iova_to_phys_length() to get PTE page size, allowing
traversal by actual mapping granularity instead of PAGE_SIZE steps.

> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index c8151ba54de33..393f9e8f1511e 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));

[Severity: Critical]
Is it possible for the hardware PTE size to extend beyond the remaining size
of the logical DMA region?

If the IOMMU driver returns a page size that extends beyond dma->size - pos,
len is never clamped to the remaining size. This would cause len to overshoot
the region boundary.

> +		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 (!next_pgsize || next_pgsize < PAGE_SIZE)
> +				next_pgsize = PAGE_SIZE;
> +			len += next_pgsize;

[Severity: Critical]
Could unconditionally adding next_pgsize to len cause it to overshoot the
region boundary before the loop condition is evaluated?

If the next page size extends beyond the end of the region, len will again
overshoot the target boundary.

When an oversized len is subsequently passed to unmap_unpin_fast(), will it
unmap and drop pin references for physical memory outside the target region?
Dropping pin counts for active adjacent memory regions could corrupt page
refcounts and lead to physical use-after-free conditions.

>  		}

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