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 <[email protected]> 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/[email protected]?part=1
