vmw_gem_object_get_sg_table() returns vmw_tt->vsgt.sgt when the buffer
object has already been DMA mapped. vmw_ttm_map_dma() sets that field
to &vmw_tt->sgt, which is a member embedded inside the struct
vmw_ttm_tt allocation and not a table of its own.
The core owns whatever .get_sg_table returns. drm_gem_map_dma_buf()
takes the table and drm_gem_unmap_dma_buf() destroys it in full:
dma_unmap_sgtable(attach->dev, sgt, dir, DMA_ATTR_SKIP_CPU_SYNC);
sg_free_table(sgt);
kfree(sgt);
Both are ops of drm_gem_prime_dmabuf_ops, so the core ends up calling
kfree() on &vmw_tt->sgt, which is not the start of an allocation. The
preceding sg_free_table() also destroys a scatterlist that vmwgfx still
considers its own and frees again from vmw_ttm_unmap_dma().
drm_gem_unmap_dma_buf() runs from dma_buf_detach(), including the
importer's error path, so a failed import is enough to reach it:
exporting a bound buffer with DRM_IOCTL_PRIME_HANDLE_TO_FD and
importing it on a second DRM device with DRM_IOCTL_PRIME_FD_TO_HANDLE
is sufficient. Both ioctls are DRM_RENDER_ALLOW.
Observed on 6.12.101 with KASAN, as uid 65534:
BUG: KASAN: invalid-free in drm_gem_unmap_dma_buf+0xb3/0xf0
Free of addr ffff888008290450 by task poc_mm04_sgtabl/321
CPU: 1 UID: 65534 PID: 321 Comm: poc_mm04_sgtabl Not tainted 6.12.101 #4
kasan_report_invalid_free+0x94/0xc0
check_slab_allocation+0x116/0x120
kfree+0x103/0x360
drm_gem_unmap_dma_buf+0xb3/0xf0
dma_buf_detach+0x165/0x510
drm_gem_prime_import_dev+0x33e/0x430
which belongs to the cache kmalloc-192 of size 192
The buggy address is located 80 bytes inside of
144-byte region [ffff888008290400, ffff888008290490)
80 is offsetof(struct vmw_ttm_tt, sgt) and 144 is sizeof(struct
vmw_ttm_tt), which identifies the freed pointer as the embedded member.
Always return a table the core can own, as the other drivers do.
Reusing the cached representation would require copying it into a
freshly allocated sg_table, never returning the alias.
Fixes: 8afa13a0583f ("drm/vmwgfx: Implement DRIVER_GEM")
Cc: [email protected]
Signed-off-by: Aldo Ariel Panzardo <[email protected]>
---
drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
index 39f8c4655..a0233729b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
@@ -73,10 +73,13 @@ static struct sg_table *vmw_gem_object_get_sg_table(struct
drm_gem_object *obj)
struct vmw_ttm_tt *vmw_tt =
container_of(bo->ttm, struct vmw_ttm_tt, dma_ttm);
- if (vmw_tt->vsgt.sgt)
- return vmw_tt->vsgt.sgt;
-
- return drm_prime_pages_to_sg(obj->dev, vmw_tt->dma_ttm.pages,
vmw_tt->dma_ttm.num_pages);
+ /*
+ * Do not return &vmw_tt->sgt: the core owns what this returns and
+ * drm_gem_unmap_dma_buf() sg_free_table()s and kfree()s it, but that
+ * sg_table is embedded in the vmw_ttm_tt allocation.
+ */
+ return drm_prime_pages_to_sg(obj->dev, vmw_tt->dma_ttm.pages,
+ vmw_tt->dma_ttm.num_pages);
}
static int vmw_gem_vmap(struct drm_gem_object *obj, struct iosys_map *map)
--
2.43.0
