Re: [PATCH v3 01/32] iommu: introduce iova_to_phys_length in iommu_domain_ops

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The iova_to_phys_length callback API lacks error-value documentation, leading to implementations returning 0 on error instead of PHYS_ADDR_MAX, causing critical security issues when physical page 0 is wrongly unpinned.
--

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

iommu: introduce iova_to_phys_length in iommu_domain_ops

This commit introduces a new iova_to_phys_length callback in
iommu_domain_ops and a corresponding iommu_iova_to_phys_length() core
function. The callback allows returning both the physical address and the
PTE mapping page size in a single page table walk. Callers like VFIO can
use this to traverse IOVA space more efficiently.

> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index d1a9e713d3a05..320ea13488e7b 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c

[ ... ]

> @@ -2545,15 +2545,55 @@ 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 PHYS_ADDR_MAX if no
> + *         translation exists.
> + */
> +phys_addr_t iommu_iova_to_phys_length(struct iommu_domain *domain,
> +				       dma_addr_t iova,
> +				       size_t *mapped_length)
>  {
> -	if (domain->type == IOMMU_DOMAIN_IDENTITY)
> +	phys_addr_t phys;
> +
> +	if (domain->type == IOMMU_DOMAIN_IDENTITY) {
> +		if (mapped_length)
> +			*mapped_length = PAGE_SIZE;
>  		return iova;
> +	}
>  
> -	if (domain->type == IOMMU_DOMAIN_BLOCKED)
> -		return 0;
> +	if (mapped_length)
> +		*mapped_length = 0;
> +
> +	if (domain->ops->iova_to_phys_length)
> +		return domain->ops->iova_to_phys_length(domain, iova, mapped_length);

[Severity: Critical]
Does this code handle failures correctly if the driver returns 0 on error?

The documentation for iommu_iova_to_phys_length() states that it returns
PHYS_ADDR_MAX if no translation exists. However, if the underlying driver
incorrectly returns 0 on failure, this wrapper directly propagates that 0.

If callers like VFIO check for PHYS_ADDR_MAX to detect errors, they will
assume 0 is a valid physical address. This can lead to unmapping and
unpinning physical PFN 0, which corrupts page refcounts and can result in
use-after-free vulnerabilities.

Could we sanitize the return value here?

> 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

[ ... ]

> @@ -804,6 +807,9 @@ struct iommu_domain_ops {
>  
>  	phys_addr_t (*iova_to_phys)(struct iommu_domain *domain,
>  				    dma_addr_t iova);
> +	phys_addr_t (*iova_to_phys_length)(struct iommu_domain *domain,
> +					    dma_addr_t iova,
> +					    size_t *mapped_length);

[Severity: Critical]
Could we add documentation specifying the error return value for this new
callback?

Since legacy iova_to_phys implementations often returned 0 on error, drivers
implementing this new callback might also return 0 instead of PHYS_ADDR_MAX
on failure.

For example, arm_smmu_iova_to_phys_hard() defaults its address variable to 0
on translation faults.

Additionally, drivers might fail to set mapped_length on success, which
can cause kernel warnings in callers like VFIO and iommufd.

Explicitly documenting the expectations for mapped_length and the return
value on failure (PHYS_ADDR_MAX) might prevent these driver implementation
errors.

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