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 363087af17cd51ea419c705b08215e2c785556f3 Author: Rob Clark <[email protected]> drm/msm: Treat !mapped as an error This commit treats zero mapped bytes after an ops->map() call in __do_map() as -ENOMEM so that mapping nothing is handled as an error rather than silently ignored. > 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; [Severity: Medium] Can __do_map() in drivers/gpu/drm/msm/msm_iommu.c still silently return 0 without mapping anything (or after mapping fewer than len bytes) when called via vm_map_op() -> msm_iommu_pagetable_map() or msm_iommu_map()? The new !mapped check is placed inside the inner while (size) loop, which only executes when size = sg_dma_len(sg) > 0 (and where ret == 0 with mapped == 0 would previously have caused an infinite loop rather than silently returning). However, if sg_dma_len(sg) == 0 (for example, when an sg_table is not DMA-mapped, or analogous to the sg->length == 0 case under CONFIG_DMABUF_DEBUG before commit b6b3547f9503 ("drm/msm: Convert map helper to use dma-address")), the outer loop in __do_map() skips the inner while (size) loop altogether because 0 <= off is always true for unsigned size_t: drivers/gpu/drm/msm/msm_iommu.c:__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; } ... while (size) { ... } } return 0; } Similarly, if sgt->nents == 0 or the total DMA length across sgt is smaller than off + len (which msm_iommu_map() checked via return (ret == len) ? 0 : -EINVAL before commit efc83727eef1 ("drm/msm: Convert iommu map/unmap to helpers")), for_each_sgtable_dma_sg() finishes with len > 0 and __do_map() still returns 0 without unmapping addr - iova via __do_unmap() or marking the VM unusable. Should __do_map() also check if len != 0 after the for_each_sgtable_dma_sg() loop and unmap any partial range before returning an error? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
