Re: [PATCH v2 21/30] iommu/tegra-smmu: implement iova_to_phys_length

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Lockless traversal in `tegra_smmu_iova_to_phys_length` can cause a use-after-free on page table memory and a TOCTOU race on the PTE value.
--

commit 681b9a6734c92cb4ecc9cde9011c68a58a0d83fd
Author: Guanghui Feng <guanghuifeng@linux.alibaba.com>

iommu/tegra-smmu: implement iova_to_phys_length

This commit implements iova_to_phys_length for the Tegra SMMU IOMMU driver,
returning the actual PTE mapping size.

> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> --- a/drivers/iommu/tegra-smmu.c
> +++ b/drivers/iommu/tegra-smmu.c

[ ... ]

> @@ -803,20 +803,26 @@ static size_t tegra_smmu_unmap(struct iommu_domain *domain, unsigned long iova,
> -static phys_addr_t tegra_smmu_iova_to_phys(struct iommu_domain *domain,
> -					   dma_addr_t iova)
> +static phys_addr_t tegra_smmu_iova_to_phys_length(struct iommu_domain *domain,
> +					   dma_addr_t iova, size_t *mapped_length)
>  {
>  	struct tegra_smmu_as *as = to_smmu_as(domain);
>  	unsigned long pfn;
>  	dma_addr_t pte_dma;
>  	u32 *pte;
>  
> +	if (mapped_length)
> +		*mapped_length = 0;
> +
>  	pte = tegra_smmu_pte_lookup(as, iova, &pte_dma);
>  	if (!pte || !*pte)
> -		return 0;
> +		return PHYS_ADDR_MAX;

[Severity: High]
This is a pre-existing issue, but does this lockless traversal risk a
use-after-free on the page table memory?

Unlike tegra_smmu_map() and tegra_smmu_unmap(), this function looks up the
page table and reads the PTE locklessly. If a concurrent unmap operation
removes the last IOVA in a page directory, tegra_smmu_pte_put_use() will
synchronously free the page table:

tegra_smmu_unmap()
  __tegra_smmu_unmap()
    tegra_smmu_set_pte()
      tegra_smmu_pte_put_use() {
        ...
        iommu_free_pages(pt);
        as->pts[pde] = NULL;
      }

If this runs concurrently on an unmapped IOVA within the same directory,
could tegra_smmu_pte_lookup() return a pointer to freed memory, leading
to a crash when evaluating !*pte?

Additionally, is there a time-of-check to time-of-use race here? The pointer
*pte is evaluated twice without READ_ONCE(). If an unmap happens between
the check (!*pte) and the assignment:

>  	pfn = *pte & as->smmu->pfn_mask;

Could *pte become 0 during the second read, causing the function to silently
return a physical address of 0 instead of PHYS_ADDR_MAX?

> +	if (mapped_length)
> +		*mapped_length = SZ_4K;
> +
>  	return SMMU_PFN_PHYS(pfn) + SMMU_OFFSET_IN_PAGE(iova);
>  }

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