[PATCH] drm/v3d: Fix double free of the sg_table when a PRIME import fails

[PATCH] drm/v3d: Fix double free of the sg_table when a PRIME import fails

[Aleksei Riapolov]

When v3d_bo_create_finish() fails in v3d_prime_import_sg_table(), the
error path calls drm_gem_shmem_free() on an object that the DRM core
does not consider imported yet: obj->import_attach is only assigned by
drm_gem_prime_import_dev() after the callback has returned. So
drm_gem_shmem_release() takes the branch for native objects and unmaps
and frees shmem->sgt, which is the sg_table of the dma-buf attachment:

  drm_gem_prime_import_dev()
    sgt = dma_buf_map_attachment_unlocked(attach)
    dev->driver->gem_prime_import_sg_table(dev, attach, sgt)
      v3d_prime_import_sg_table()
        drm_gem_shmem_prime_import_sg_table()   shmem->sgt = sgt
        v3d_bo_create_finish()                  -ENOSPC
        drm_gem_shmem_free()
          drm_gem_shmem_release()
            __drm_gem_shmem_free_sgt_locked()
              dma_unmap_sgtable(sgt)
              sg_free_table(sgt)
              kfree(sgt)
    dma_buf_unmap_attachment_unlocked(attach, sgt)   <- freed sgt

The caller then hands the freed table to the exporter, which unmaps and
frees it a second time.

v3d_bo_create_finish() fails here when the 4 GiB V3D address space is
exhausted and drm_mm_insert_node_generic() returns -ENOSPC, which a
client of the render node can arrange with imports alone. On a
Raspberry Pi 5 (BCM2712, v7.4-rc2) booted with slub_debug=FZPU the
import that hits -ENOSPC oopses:

  Unable to handle kernel paging request at virtual address 006b6b6b6b6b6b87
  pc : dma_direct_unmap_sg+0x90/0x268
  lr : dma_unmap_sg_attrs+0x60/0x1c8
  Call trace:
   dma_direct_unmap_sg+0x90/0x268 (P)
   dma_unmap_sg_attrs+0x60/0x1c8
   unmap_udmabuf+0x34/0x60
   dma_buf_unmap_attachment+0x3c/0xa0
   dma_buf_unmap_attachment_unlocked+0x58/0x98
   drm_gem_prime_import_dev+0x1a0/0x1e0
   drm_gem_prime_fd_to_handle+0x1a8/0x280
   drm_prime_fd_to_handle_ioctl+0x40/0x58
   drm_ioctl_kernel+0xc8/0x140
   drm_ioctl+0x368/0x4e8
   __arm64_sys_ioctl+0xb0/0x110

and the task is left unkillable, as it dies holding the reservation
lock of the dma-buf.

Hand the table back to its owner before freeing the object: with
shmem->sgt cleared, drm_gem_shmem_release() leaves it alone and it is
unmapped once, by the exporter, on drm_gem_prime_import_dev()'s error
path. With this change the same test gets -ENOSPC from the failing
import in each of three consecutive fills and the kernel log stays
clean.

A reproducer is available on request.

Fixes: 40609d4820b2 ("drm/v3d: Use the new shmem helpers to reduce driver boilerplate.")
Cc: stable@vger.kernel.org
Signed-off-by: Aleksei Riapolov <tsbol@icloud.com>
---
 drivers/gpu/drm/v3d/v3d_bo.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/v3d/v3d_bo.c b/drivers/gpu/drm/v3d/v3d_bo.c
index db95a2f..48c7807 100644
--- a/drivers/gpu/drm/v3d/v3d_bo.c
+++ b/drivers/gpu/drm/v3d/v3d_bo.c
@@ -185,6 +185,14 @@ v3d_prime_import_sg_table(struct drm_device *dev,
 
 	ret = v3d_bo_create_finish(obj);
 	if (ret) {
+		/*
+		 * obj->import_attach is only set by drm_gem_prime_import_dev()
+		 * after we return, so drm_gem_shmem_release() would treat this
+		 * object as native and unmap/free the attachment's sg_table,
+		 * which the caller unmaps again on its error path. Hand the
+		 * table back to its owner before freeing the object.
+		 */
+		to_v3d_bo(obj)->base.sgt = NULL;
 		drm_gem_shmem_free(&to_v3d_bo(obj)->base);
 		return ERR_PTR(ret);
 	}
-- 
2.54.0 (Apple Git-157)