Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] drm/msm: __do_map() still returns 0 when sg_table maps nothing or short len --
commit 9f62bb971f89fec6afb029cd8bd570abb08a5696 Author: Rob Clark <[email protected]> drm/msm: Treat !mapped as an error This commit treats a zero-byte mapping result in __do_map() as an error so that the caller can mark the VM as unusable rather than silently mapping nothing. > diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c > index c2950d32c5075..255972323f9a9 100644 > --- a/drivers/gpu/drm/msm/msm_iommu.c > +++ b/drivers/gpu/drm/msm/msm_iommu.c > @@ -186,6 +186,10 @@ __do_map(unsigned long pgsize_bitmap, u64 iova, struct > sg_table *sgt, size_t off > size -= mapped; > len -= mapped; > > + /* If nothing was mapped, consider it an ENOMEM. */ > + if (!ret && !mapped) > + ret = -ENOMEM; > + > if (ret) { > __do_unmap(pgsize_bitmap, iova, addr - iova, > arg, ops); > return -EINVAL; [Severity: Medium] As reported by Sashiko on v3 [1], can __do_map() in drivers/gpu/drm/msm/msm_iommu.c still silently return 0 without mapping the requested range when sg_table entries have zero or short DMA lengths? The new check is inside the inner while (size) loop, where size is already non-zero (and where ops->map() returning ret == 0 with mapped == 0 would previously have spun in an infinite loop rather than returning early; note also that setting ret = -ENOMEM is immediately overwritten by returning -EINVAL). However, when an sg_table entry has size = sg_dma_len(sg) == 0 (for example, an unmapped sg_table when CONFIG_NEED_SG_DMA_LENGTH is enabled), the earlier check in __do_map(): for_each_sgtable_dma_sg (sgt, sg, i) { size_t size = sg_dma_len(sg); phys_addr_t phys = sg_dma_address(sg); if (!len) break; if (size <= off) { off -= size; continue; } evaluates 0 <= off as true for unsigned size_t off and continues past the while (size) loop without ever reaching the !mapped check. Similarly, if sgt->nents is 0 or the total sg_dma_len(sg) across sgt is less than off + len, the outer for_each_sgtable_dma_sg() loop finishes with len > 0 and __do_map() unconditionally returns 0 at the end of the function: if (ret) { __do_unmap(pgsize_bitmap, iova, addr - iova, arg, ops); return -EINVAL; } } } return 0; } without calling __do_unmap() on any partially mapped range or returning an error to mark the VM as unusable. Should __do_map() check whether len != 0 after the for_each_sgtable_dma_sg() loop (unmapping [iova, addr) and returning an error if so), and should the sibling while (len) loop in msm_iommu_pagetable_map_prr() also guard against !ret && !mapped? [1] https://lore.kernel.org/r/[email protected] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
