Re: [PATCH 1/9] iommu: introduce iova_to_phys_length in iommu_domain_ops

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Out-of-bounds physical memory access in IOMMUFD due to non-power-of-2 mapping size returned by virtio-iommu.
- [Medium] Missing static inline stub for iommu_iova_to_phys_length when CONFIG_IOMMU_API is disabled.
--

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

iommu: introduce iova_to_phys_length in iommu_domain_ops

Add iova_to_phys_length callback to struct iommu_domain_ops alongside
the existing iova_to_phys. The new callback returns both the physical
address and the PTE mapping page size in a single page table walk.

> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index d1a9e713d3a05..43323229a1df3 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
[ ... ]
> @@ -2545,15 +2545,45 @@ void iommu_detach_group(struct iommu_domain *domain, struct iommu_group *group)
>  }
>  EXPORT_SYMBOL_GPL(iommu_detach_group);
>  
> -phys_addr_t iommu_iova_to_phys(struct iommu_domain *domain, dma_addr_t iova)
> +/**
> + * iommu_iova_to_phys_length - Translate IOVA and return mapping page size
> + * @domain: IOMMU domain to query
> + * @iova: IO virtual address to translate
> + * @mapped_length: Output parameter for the PTE page size (e.g. 4KB/2MB/1GB)
> + *
> + * Like iommu_iova_to_phys() but additionally returns the page size of the
> + * PTE mapping at @iova through @mapped_length.
> + *
> + * Return: The physical address for the given IOVA, or 0 if no translation.
> + */
> +phys_addr_t iommu_iova_to_phys_length(struct iommu_domain *domain,
> +				       dma_addr_t iova,
> +				       size_t *mapped_length)

[Severity: Critical]
Is it possible for the underlying domain implementation to return a
non-power-of-2 length here?

If a driver like virtio-iommu implements iova_to_phys_length by returning
the size of the user mapping region stored in its interval tree
(e.g., mapping->iova.last - mapping->iova.start + 1), it can return an
unaligned or non-power-of-2 value (like 12KB).

Callers like IOMMUFD use bitwise math assuming the length is a power-of-2:

drivers/iommu/iommufd/pages.c:batch_from_domain() {
    ...
    npages = (pgsize - (iova & (pgsize - 1)) + page_offset) / PAGE_SIZE;
    for (i = 0; i < npages; i++) {
        if (!batch_add_pfn(batch, PHYS_PFN(phys) + i))
            return;
    }
    ...
}

VFIO does the same:

drivers/vfio/vfio_iommu_type1.c:vfio_unmap_unpin() {
    ...
    len = pgsize - (iova & (pgsize - 1));
    ...
}

When an unaligned or non-power-of-2 length is returned, pgsize - 1 is not
a valid bitmask, which can cause the formula to overestimate the remaining
contiguous length. This exposes unmapped physical memory, and could allow
an out-of-bounds physical memory access or unmapping lengths that exceed
the actual mapping.

Could this be documented to strictly require power-of-2 PTE page sizes,
or could the callers be updated to handle non-power-of-2 region lengths?

[ ... ]
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index bf8a77a164e4d..29c7c042d400d 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
[ ... ]
> @@ -958,6 +964,9 @@ extern ssize_t iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
>  			    struct scatterlist *sg, unsigned int nents,
>  			    int prot, gfp_t gfp);
>  extern phys_addr_t iommu_iova_to_phys(struct iommu_domain *domain, dma_addr_t iova);
> +extern phys_addr_t iommu_iova_to_phys_length(struct iommu_domain *domain,
> +					      dma_addr_t iova,
> +					      size_t *mapped_length);

[Severity: Medium]
Will this cause build failures for configurations without CONFIG_IOMMU_API?

It appears that the static inline stub for iommu_iova_to_phys_length() is
missing in the #else block for CONFIG_IOMMU_API in this header. Generic
drivers that use this API might encounter compilation and linkage failures
when IOMMU_API is disabled.

>  extern void iommu_set_fault_handler(struct iommu_domain *domain,
>  			iommu_fault_handler_t handler, void *token);
>

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