Re: [RFT 3/5] drm/msm: Extract out map/unmap helpers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/msm: missing IOTLB flush on pagetable map error path
--

commit 7d5e2545b852417f316f5873d935ec2b95998bbf
Author: Rob Clark <robin.clark@oss.qualcomm.com>

drm/msm: Extract out map/unmap helpers

This commit extracts `__do_map()` and `__do_unmap()` helpers from
`msm_iommu_pagetable_map()` and `msm_iommu_pagetable_unmap()` respectively.
It aims to reuse these helpers without any functional changes.

> diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
> index da6782fca6bd2..a658493f80072 100644
> --- a/drivers/gpu/drm/msm/msm_iommu.c
> +++ b/drivers/gpu/drm/msm/msm_iommu.c
[ ... ]
> @@ -204,10 +169,9 @@ static int msm_iommu_pagetable_map(struct msm_mmu *mmu, u64 iova,
>  			size_t pgsize, count, mapped = 0;
>  			int ret;
>  
> -			pgsize = calc_pgsize(pagetable, addr, phys, size, &count);
> +			pgsize = calc_pgsize(pgsize_bitmap, addr, phys, size, &count);
>  
> -			ret = ops->map_pages(ops, addr, phys, pgsize, count,
> -					     prot, GFP_KERNEL, &mapped);
> +			ret = map(arg, phys, addr, pgsize, count, prot, &mapped);
>  
>  			/* map_pages could fail after mapping some of the pages,
>  			 * so update the counters before error handling.
> @@ -218,7 +182,7 @@ static int msm_iommu_pagetable_map(struct msm_mmu *mmu, u64 iova,
>  			len  -= mapped;
>  
>  			if (ret) {
> -				msm_iommu_pagetable_unmap(mmu, iova, addr - iova);
> +				__do_unmap(pgsize_bitmap, iova, addr - iova, arg, unmap);
>  				return -EINVAL;
>  			}

[Severity: High]
Does this error path miss an IOTLB flush?

Before this commit, a failure in the mapping loop would call
msm_iommu_pagetable_unmap(), which flushes the IOTLB after unmapping.

Now, the refactored __do_map() calls the new __do_unmap() helper directly.
Because __do_unmap() skips the IOTLB flush, the error rollback leaves stale
virtual-to-physical address translations for partially mapped pages in the
TLB, which can lead to rogue DMA and memory corruption (use-after-free).

[ ... ]
> @@ -227,6 +191,78 @@ static int msm_iommu_pagetable_map(struct msm_mmu *mmu, u64 iova,
>  	return 0;
>  }
>  
> +static size_t
> +__unmap_pgtable(void *arg, u64 iova, size_t pgsize, size_t pgcount)
[ ... ]
> +static int msm_iommu_pagetable_map(struct msm_mmu *mmu, u64 iova,
> +				   struct sg_table *sgt, size_t off, size_t len,
> +				   int prot)
> +{
> +	struct msm_iommu_pagetable *pagetable = to_pagetable(mmu);
> +	struct io_pgtable_ops *ops = pagetable->pgtbl_ops;
> +
> +	if (!sgt)
> +		return msm_iommu_pagetable_map_prr(mmu, iova, len, prot);
> +
> +	return __do_map(pagetable->pgsize_bitmap, iova, sgt, off, len, prot,
> +			ops, __map_pgtable, __unmap_pgtable);
> +}

[Severity: High]
Can this propagate the error from __do_map() without flushing the IOTLB?

When __do_map() hits an error and returns, msm_iommu_pagetable_map() passes
the error up the stack without initiating a TLB flush, cementing the
missing flush bug from the __do_map() failure path.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006131000.81501-4-robin.clark@oss.qualcomm.com?part=1