Re: [PATCH 3/9] iommu/generic_pt: implement iova_to_phys_length

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Direct calls to `ops->iova_to_phys` in the iommufd selftests will cause a NULL pointer dereference because `IOMMU_PT_DOMAIN_OPS` no longer initializes this function pointer.
--

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

iommu/generic_pt: implement iova_to_phys_length

Extend the Generic Page Table framework to implement iova_to_phys_length
by determining the PTE block size during the page table walk. This also
updates the IOMMU_PT_DOMAIN_OPS macro to configure the new function.

> diff --git a/include/linux/generic_pt/iommu.h b/include/linux/generic_pt/iommu.h
> index dd0edd02a48a2..859b853e9dc7e 100644
> --- a/include/linux/generic_pt/iommu.h
> +++ b/include/linux/generic_pt/iommu.h
[ ... ]
> @@ -267,11 +268,11 @@ struct pt_iommu_cfg {
>  	IOMMU_PROTOTYPES(fmt)
>  
>  /*
> - * A driver uses IOMMU_PT_DOMAIN_OPS to populate the iommu_domain_ops for the
> - * iommu_pt
> + * A driver uses IOMMU_PT_DOMAIN_OPS to populate the iommu_domain_ops for
> + * the iommu_pt
>   */
> -#define IOMMU_PT_DOMAIN_OPS(fmt)                        \
> -	.iova_to_phys = &pt_iommu_##fmt##_iova_to_phys
> +#define IOMMU_PT_DOMAIN_OPS(fmt)					\
> +	.iova_to_phys_length = &pt_iommu_##fmt##_iova_to_phys_length

[Severity: High]
Will removing .iova_to_phys here cause a NULL pointer dereference in the
iommufd selftests?

The amdv1_mock_ops in drivers/iommu/iommufd/selftest.c uses this macro,
so its .iova_to_phys field will now be implicitly NULL.

However, the selftest directly dereferences this function pointer without
checking:

drivers/iommu/iommufd/selftest.c:iommufd_test_md_check_pa() {
    ...
    io_phys = mock->domain.ops->iova_to_phys(&mock->domain, iova);
    ...
}

While I understand this is addressed later in the patch series when the
code properly switches to using the iommu_iova_to_phys() wrapper, will
this intermediate state break git bisect by crashing the kernel during
testing?

>  #define IOMMU_PT_DIRTY_OPS(fmt) \
>  	.read_and_clear_dirty = &pt_iommu_##fmt##_read_and_clear_dirty
>

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