Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] dma_buf_vmap() is called without holding the required dma_resv lock. -- --- Patch [1]: [PATCH v3 1/3] accel/amdxdna: refuse an I/O memory mapping of an imported BO --- commit a77f8bd3c9728d98291116a86706c7b30d341618 Author: Taimuraz Kaitmazov <[email protected]> accel/amdxdna: refuse an I/O memory mapping of an imported BO This patch modifies amdxdna_gem_obj_vmap() to refuse I/O memory mappings from imported buffer objects. If an exporter returns an IOMEM mapping, the mapping is rejected with -EOPNOTSUPP. This prevents invalid memory accesses when later code assumes a regular kernel virtual address. [ ... ] > diff --git a/drivers/accel/amdxdna/amdxdna_gem.c > b/drivers/accel/amdxdna/amdxdna_gem.c > index b01f5d95fac83..d7d36c9523253 100644 > --- a/drivers/accel/amdxdna/amdxdna_gem.c > +++ b/drivers/accel/amdxdna/amdxdna_gem.c > @@ -710,10 +710,16 @@ static int amdxdna_gem_obj_vmap(struct drm_gem_object > *obj, struct iosys_map *ma > > dma_resv_assert_held(obj->resv); > > - if (is_import_bo(abo)) > + if (is_import_bo(abo)) { > ret = dma_buf_vmap(abo->dma_buf, map); [Severity: High] This is a pre-existing issue, but while looking at dma_buf_vmap() usage, I noticed a missing lock in drivers/accel/amdxdna/amdxdna_cbuf.c. Is dma_buf_vmap() being called without holding the required dma_resv lock during carveout initialization? When amdxdna_get_cbuf() exports a new dma_buf, it returns an unlocked buffer. It then immediately calls amdxdna_cbuf_clear(), which invokes dma_buf_vmap() on the dbuf without taking its reservation lock: drivers/accel/amdxdna/amdxdna_cbuf.c:amdxdna_cbuf_clear() { ... struct iosys_map vmap = IOSYS_MAP_INIT_VADDR(NULL); dma_buf_vmap(dbuf, &vmap); if (!vmap.vaddr) return -EFAULT; ... } Because dma_buf_vmap() unconditionally asserts that this lock is held (dma_resv_assert_held(dmabuf->resv)), does this unlocked call violate the dma-buf API contract and produce kernel warnings? > - else > + /* Callers use mem.kva as an ordinary kernel address. */ > + if (!ret && map->is_iomem) { > + dma_buf_vunmap(abo->dma_buf, map); > + return -EOPNOTSUPP; > + } > + } else { > ret = drm_gem_shmem_object_vmap(obj, map); > + } > if (ret) > return ret; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
