[PATCH v2 0/3] drm/vmwgfx: Fix dma-buf import leaks and a crash on imported BOs

[PATCH v2 0/3] drm/vmwgfx: Fix dma-buf import leaks and a crash on imported BOs

[Michal TOMA]

From: Michal TOMA <mt@sicoop.com>

v1 was a single patch, "drm/vmwgfx: Release PRIME import in the BO
destroy path":

  https://lore.kernel.org/r/20260910142359.13716-1-michaltoma@sicoop.com

Please do not apply that one on its own. An automated review flagged an
interaction with the existing TTM_TT_FLAG_EXTERNAL handling in
vmw_ttm_map_dma(), and it turned out to be real: with v1 applied and
nothing else, an unprivileged render node client can oops the kernel.
That is what patch 1 of this series fixes, and it has to come first.

  1/3 drm/vmwgfx: Don't map or free the exporter's sg_table for
      imported BOs
      Fixes: b32233acceff, Cc: stable # v6.9+
  2/3 drm/vmwgfx: Release PRIME import in the BO destroy path
      (v1, unchanged)
      Fixes: b32233acceff, Cc: stable # v6.6+
  3/3 drm/vmwgfx: Don't leak a GEM handle when referencing a surface
      by fd
      Fixes: d6667f0ddf46, Cc: stable # v6.11+

Changes since v1:
 - new 1/3. Without it, 2/3 turns a silent leak into a NULL pointer
   dereference.
 - new 3/3. It was written before v1 was posted but held back, since
   the two fixes are independent.
 - 2/3 is byte for byte what was posted as v1.

Why the order matters
---------------------
For a TTM tt with TTM_TT_FLAG_EXTERNAL, vmw_ttm_map_dma() points
vsgt->sgt at the exporter's sg_table and then calls
vmw_ttm_map_for_dma(), which maps &vmw_tt->sgt: the inline table, which
for an imported buffer is never populated. dma_map_sgtable() is called
with orig_nents == 0, warns, and returns -EIO, and out_map_fail then
calls sg_free_table() on the exporter's table, leaving sgl == NULL and
orig_nents unchanged.

Today that only corrupts and leaks, because vmwgfx never releases a
PRIME import, so nothing ever unmaps the attachment. 2/3 fixes exactly
that leak, and in doing so makes the exporter unmap the table that
vmwgfx already freed:

  dma_unmap_sgtable(dev, sg with sgl == NULL, orig_nents == N)
    dma_direct_unmap_sg() -> for_each_sg(NULL, sg, N, i)

which dereferences NULL, in the TTM delayed-delete worker.

1/3 is worth having on its own: binding an imported dma-buf has never
worked, it always failed with -EIO and damaged the exporter's table on
the way out. With 1/3 it works.

Reachability, and how this was reproduced
-----------------------------------------
Three ioctls, all DRM_RENDER_ALLOW, as an ordinary user: import a
dma-buf with DRM_IOCTL_PRIME_FD_TO_HANDLE (a udmabuf here), create a
guest-backed surface on it with DRM_VMW_GB_SURFACE_CREATE_EXT
(base.buffer_handle = the imported handle), then submit
SVGA_3D_CMD_UPDATE_GB_SURFACE for that surface with DRM_VMW_EXECBUF.
Validation moves the buffer to VMW_BO_DOMAIN_MOB, which binds the tt.

Without 1/3, with 2/3 applied:

  WARNING: kernel/dma/mapping.c:266 at __dma_map_sg_attrs+0xdd/0x1d0
   dma_map_sgtable+0x1d/0x30
   vmw_ttm_map_dma+0xf6/0x140 [vmwgfx]
   vmw_move+0x1cd/0x2c0 [vmwgfx]
   ttm_bo_handle_move_mem+0xc0/0x180 [ttm]
   ttm_bo_validate+0xd2/0x1d0 [ttm]
   vmw_validation_bo_validate+0xb5/0x180 [vmwgfx]
   vmw_execbuf_process+0x852/0x1330 [vmwgfx]
   vmw_execbuf_ioctl+0x10d/0x1d0 [vmwgfx]

  vmwgfx 0000:00:02.0: [drm] VSG table map failed!

and then, when the buffer is released:

  BUG: kernel NULL pointer dereference, address: 000000000000001c
  Workqueue: ttm ttm_bo_delayed_delete [ttm]
  RIP: 0010:dma_direct_unmap_sg+0x62/0x200
   unmap_udmabuf+0x24/0x40
   dma_buf_unmap_attachment_unlocked+0x46/0x70
   drm_prime_gem_destroy+0x28/0x50
   vmw_bo_free+0x15b/0x1f0 [vmwgfx]

The closing process exits normally, since this runs in the delete
worker; the worker dies holding TTM and dma-resv locks and the machine
wedges shortly afterwards.

With the full series, the same program logs nothing, the execbuf is
accepted, and the buffer is released with no entry left in
/sys/kernel/debug/dma_buf/bufinfo.

Test results
------------
Kernel 7.2.3 on the guest below, comparing modules built from its own
vmwgfx sources, which are identical to drm-misc-fixes for the files
involved.

  import + surface + execbuf + release, as above:
    2/3 only:      WARN, "VSG table map failed!", execbuf -EIO,
                   then an oops in ttm_bo_delayed_delete and a hang
    1/3 + 2/3 + 3/3: no WARN, execbuf accepted, buffer released, no
                   oops, kernel taint unchanged

  PRIME_FD_TO_HANDLE reproducer, 8 x 4 MiB udmabufs (2/3):
    stock:  all 8 remain, count 1, still attached
    series: none remain, with 3D acceleration on and off

  DRM_VMW_GB_SURFACE_REF_EXT(PRIME) from a second render file (3/3),
  each checked while that file was still open:
    udmabuf fd, 8 calls:
      without 3/3: -EINVAL, 8 imports held (count 3), 8 WARNs
      with 3/3:    -EINVAL, nothing imported, no WARN
    dumb buffer fd, 4 calls:
      without 3/3: -EINVAL, 4 stray GEM handles, 4 WARNs
      with 3/3:    -EINVAL, no stray handle, no WARN
    buffer exported before a surface was created on it, 4 calls
    (reaches the changed function and succeeds):
      without 3/3: surface returned, 4 stray GEM handles
      with 3/3:    surface returned, no stray handle
    buffer created together with its surface (a TTM prime export that
    never reaches the changed function), 4 calls:
      unchanged either way

  KWin 6.7.4, 5-minute replay of a terminal workload, 3D on:
    2/3 only:  udmabufs 4 -> 20 (140 MiB), never released, WARNs
    2/3 + 3/3: udmabufs 4 -> 6 -> 4, no pinned pages, no WARN
  (not re-run with 1/3 in place; 1/3 does not touch that path)

Test environment
----------------
- VirtualBox 7.2 (Guest Additions 7.2.16), VMSVGA adapter, 4 vCPUs,
  3.8 GiB RAM. Tested with 3D acceleration on (vmwgfx shader model
  SM_5) and off (Legacy). DMA map mode is vmw_dma_map_populate.
- openSUSE Tumbleweed kernel 7.2.3-1-default. The vmwgfx files
  involved are byte-identical to drm-misc-fixes 4600b4d1a9ee. The test
  modules were built out of tree from those sources, which adds the E
  taint.
- The kernel was already tainted W+O before any test: two boot-time
  warnings unrelated to vmwgfx (arch/x86/mm/pat/set_memory.c:727 and
  kernel/rcu/tree_plugin.h:823), plus VirtualBox's out-of-tree
  vboxguest/vboxsf.
- KDE Plasma / KWin 6.7.4, Mesa 26.2.1.
- W=1 build of drivers/gpu/drm/vmwgfx/ on drm-misc-fixes: no warnings
  before or after. checkpatch.pl --strict is clean on all three.

Not tested
----------
- VMware Workstation or ESXi hosts; only VirtualBox VMSVGA.
- A full kernel built from drm-misc-fixes; only the 7.2.3 kernel with
  modules built from identical vmwgfx sources.
- Kernels with KASAN, lockdep or kmemleak enabled. IGT was not run.
- Stable backports were not built. 3/3 uses
  drm_gem_is_prime_exported_dma_buf(), added in v6.17 (660cd44659a0);
  older trees carrying d6667f0ddf46 would need that helper as well.

Not fixed here
--------------
The review of v1 also pointed out that vmw_prime_import_sg_table()
returns NULL rather than an ERR_PTR when vmw_bo_create() fails, and
that drm_gem_prime_import_dev() only checks IS_ERR() before
dereferencing the result. That is real, but reaching it needs a buffer
object allocation failure; I have not managed to trigger it, so I am
not posting an untested patch for it here.

Classification
--------------
Per Documentation/process/threat-model.rst I am treating these as
regular bugs. The leaks are bounded by the importing process's memory
cgroup, and the crash needs 2/3 applied without 1/3, which exists only
in this posting and in the v1 mail.

Workaround for the leaks
------------------------
KWIN_DISABLE_UDMABUF_IMPORT=1 in the session environment makes KWin 6.7
use its copy path, which avoids the imports entirely.

Tool use
--------
I found the problem because my VM repeatedly ran out of memory and had
to be rebooted. An LLM coding assistant (Claude, Anthropic) did the
diagnostics that traced it back to this code, wrote these patches and
their changelogs, and built the reproducers and the test tooling used
to confirm the results above, including the crash that prompted 1/3.

Michal TOMA (3):
  drm/vmwgfx: Don't map or free the exporter's sg_table for imported BOs
  drm/vmwgfx: Release PRIME import in the BO destroy path
  drm/vmwgfx: Don't leak a GEM handle when referencing a surface by fd

 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c         |  3 ++
 drivers/gpu/drm/vmwgfx/vmwgfx_surface.c    | 48 +++++++++++++---------
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 20 +++++++--
 3 files changed, 47 insertions(+), 24 deletions(-)


base-commit: 4600b4d1a9ee730d03ddac5ce409cd2730ce8c0c
-- 
2.55.0

[PATCH v2 1/3] drm/vmwgfx: Don't map or free the exporter's sg_table for imported BOs

[Michal TOMA]

For a TTM tt with TTM_TT_FLAG_EXTERNAL, vmw_ttm_map_dma() points
vsgt->sgt at the exporter's sg_table and then calls
vmw_ttm_map_for_dma(), which maps &vmw_tt->sgt: the inline table, not
the one vsgt->sgt points to. For an imported buffer object that inline
table is never populated, since vmw_ttm_tt_create() allocates the
vmw_ttm_tt with kzalloc() and only the non-external branch fills it in.
dma_map_sgtable() is therefore called with orig_nents == 0, which trips
the WARN_ON_ONCE() in __dma_map_sg_attrs() and returns -EIO.

The error path then calls sg_free_table() on vsgt->sgt, which for an
imported object is the exporter's table. Its scatterlist is freed while
the attachment is still live, leaving sgl == NULL and orig_nents
unchanged. vmw_ttm_unmap_dma() would free that table too, if a mapping
ever succeeded.

Any process that can open the render node reaches this with three
DRM_RENDER_ALLOW ioctls: import a dma-buf with
DRM_IOCTL_PRIME_FD_TO_HANDLE, create a guest-backed surface on it with
DRM_VMW_GB_SURFACE_CREATE_EXT, and submit SVGA_3D_CMD_UPDATE_GB_SURFACE
for that surface with DRM_VMW_EXECBUF. Validation moves the buffer to
VMW_BO_DOMAIN_MOB, and binding the tt takes the path above:

  WARNING: kernel/dma/mapping.c:266 at __dma_map_sg_attrs+0xdd/0x1d0
   dma_map_sgtable+0x1d/0x30
   vmw_ttm_map_dma+0xf6/0x140 [vmwgfx]
   vmw_move+0x1cd/0x2c0 [vmwgfx]
   ttm_bo_handle_move_mem+0xc0/0x180 [ttm]
   ttm_bo_validate+0xd2/0x1d0 [ttm]
   vmw_validation_bo_validate+0xb5/0x180 [vmwgfx]
   vmw_execbuf_process+0x852/0x1330 [vmwgfx]
   vmw_execbuf_ioctl+0x10d/0x1d0 [vmwgfx]
   drm_ioctl_kernel+0xa6/0x100
   drm_ioctl+0x2ad/0x590
   __x64_sys_ioctl+0xb9/0x100

  vmwgfx 0000:00:02.0: [drm] VSG table map failed!

Today the damage stops there, because vmwgfx never releases a PRIME
import: nothing unmaps the attachment afterwards, so the freed table is
only leaked. Adding the missing drm_prime_gem_destroy() call to
vmw_bo_free(), which is the next patch in this series, makes the
exporter unmap that table on release, and dma_unmap_sgtable() then
walks a NULL scatterlist:

  BUG: kernel NULL pointer dereference, address: 000000000000001c
  Workqueue: ttm ttm_bo_delayed_delete [ttm]
  RIP: 0010:dma_direct_unmap_sg+0x62/0x200
   unmap_udmabuf+0x24/0x40
   dma_buf_unmap_attachment_unlocked+0x46/0x70
   drm_prime_gem_destroy+0x28/0x50
   vmw_bo_free+0x15b/0x1f0 [vmwgfx]

The exporter has already mapped the table for this device in
dma_buf_map_attachment(), so there is nothing for vmwgfx to do here:
use the table as it is, and leave mapping, unmapping and freeing to its
owner.

Build tested on drm-misc-fixes (4600b4d1a9ee) with W=1 (no warnings in
drivers/gpu/drm/vmwgfx/) and checkpatch.pl --strict. Runtime tested on
a VirtualBox 7.2 VMSVGA guest running kernel 7.2.3, whose vmwgfx
sources for the files involved are identical to drm-misc-fixes, using a
module built from them.

Without this patch the reproducer above logs the WARN_ON_ONCE() and
"VSG table map failed!", and DRM_VMW_EXECBUF returns -EIO; releasing the
buffer afterwards, with the next patch of this series applied, oopsed in
the TTM delete worker and hung the machine.

With this patch the same reproducer logs nothing: the execbuf is
accepted, so an imported buffer object can now be bound at all, and
releasing it afterwards leaves no entry behind in
/sys/kernel/debug/dma_buf/bufinfo.

Fixes: b32233acceff ("drm/vmwgfx: Fix prime import/export")
Cc: stable@vger.kernel.org # v6.9+
Assisted-by: LLM
Signed-off-by: Michal TOMA <michaltoma@sicoop.com>
---
Reproducer (results and environment are in the cover letter). Run as an
ordinary user with access to the render node and /dev/udmabuf; it needs
no root. Without this patch it logs the WARN and "VSG table map
failed!"; with it, the execbuf is accepted and nothing is logged.

// Can userspace get an imported (external) dma-buf BO bound for the device?
//
// Chain under test:
//   udmabuf -> PRIME_FD_TO_HANDLE            (external BO, sits in SYSTEM)
//   GB_SURFACE_CREATE_EXT buffer_handle=...  (surface backed by the import)
//   execbuf SVGA_3D_CMD_UPDATE_GB_SURFACE    (validation forces the BO to MOB)
//     -> vmw_ttm_bind() -> vmw_ttm_map_dma()
//        external branch maps the wrong (empty, inline) sg_table, fails, and
//        the out_map_fail path calls sg_free_table() on the EXPORTER's table.
//
// Proof of reachability is the kernel message "VSG table map failed!".
//
//   vmw-import-execbuf-bind-test            hold the buffer, never release it
//   vmw-import-execbuf-bind-test --release  also close everything at the end
//
// WARNING: --release is the dangerous half. With "drm/vmwgfx: Release PRIME
// import in the BO destroy path" applied, releasing a buffer whose exporter
// sg_table was freed in step 3 makes udmabuf run dma_unmap_sgtable() on a
// table with sgl == NULL and orig_nents != 0 -> NULL pointer dereference.
// Without that patch the release path never touches the table (it leaks).
#define _GNU_SOURCE
#include <errno.h>
#include <fcntl.h>
#include <stdint.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <sys/ioctl.h>
#include <sys/mman.h>
#include <linux/udmabuf.h>
#include <drm/drm.h>
#include <drm/vmwgfx_drm.h>

#define DRM_IOCTL_VMW_GB_SURFACE_CREATE_EXT \
	DRM_IOWR(DRM_COMMAND_BASE + DRM_VMW_GB_SURFACE_CREATE_EXT, \
		 union drm_vmw_gb_surface_create_ext_arg)
#define DRM_IOCTL_VMW_UNREF_SURFACE \
	DRM_IOW(DRM_COMMAND_BASE + DRM_VMW_UNREF_SURFACE, struct drm_vmw_surface_arg)
#define DRM_IOCTL_VMW_EXECBUF \
	DRM_IOW(DRM_COMMAND_BASE + DRM_VMW_EXECBUF, struct drm_vmw_execbuf_arg)

#define SVGA_3D_CMD_UPDATE_GB_SURFACE 1102
#define SVGA3D_INVALID_ID 0xffffffff
#define SIZE (6UL << 20)

struct update_gb_surface_cmd {
	uint32_t id;		/* SVGA3dCmdHeader.id */
	uint32_t size;		/* SVGA3dCmdHeader.size: bytes that follow */
	uint32_t sid;		/* SVGA3dCmdUpdateGBSurface.sid */
};

int main(int argc, char **argv)
{
	int release = (argc > 1 && !strcmp(argv[1], "--release"));
	int render = open("/dev/dri/renderD128", O_RDWR | O_CLOEXEC);
	int udm = open("/dev/udmabuf", O_RDWR | O_CLOEXEC);
	int mfd, dfd;

	if (render < 0 || udm < 0) {
		perror("open");
		return 1;
	}
	mfd = memfd_create("import-execbuf", MFD_ALLOW_SEALING | MFD_CLOEXEC);
	if (mfd < 0 || ftruncate(mfd, SIZE) ||
	    fcntl(mfd, F_ADD_SEALS, F_SEAL_SHRINK)) {
		perror("memfd");
		return 1;
	}
	struct udmabuf_create c = {
		.memfd = mfd, .flags = UDMABUF_FLAGS_CLOEXEC, .size = SIZE,
	};
	dfd = ioctl(udm, UDMABUF_CREATE, &c);
	if (dfd < 0) {
		perror("UDMABUF_CREATE");
		return 1;
	}

	struct drm_prime_handle ph = { .fd = dfd };
	if (ioctl(render, DRM_IOCTL_PRIME_FD_TO_HANDLE, &ph)) {
		perror("PRIME_FD_TO_HANDLE");
		return 1;
	}
	printf("1. imported udmabuf as GEM handle %u\n", ph.handle);

	union drm_vmw_gb_surface_create_ext_arg a;
	memset(&a, 0, sizeof(a));
	a.req.version = drm_vmw_gb_surface_v1;
	/* HINT_TEXTURE | HINT_RENDERTARGET | BIND_SHADER_RESOURCE | BIND_RENDER_TARGET */
	a.req.base.svga3d_flags = (1U << 5) | (1U << 6) | (1U << 23) | (1U << 24);
	a.req.base.format = 142;			/* SVGA3D_B8G8R8X8_UNORM */
	a.req.base.mip_levels = 1;
	a.req.base.drm_surface_flags = drm_vmw_surface_flag_shareable;
	a.req.base.buffer_handle = ph.handle;
	a.req.base.base_size.width = 1536;
	a.req.base.base_size.height = 1024;
	a.req.base.base_size.depth = 1;
	if (ioctl(render, DRM_IOCTL_VMW_GB_SURFACE_CREATE_EXT, &a)) {
		printf("2. GB_SURFACE_CREATE_EXT on the imported buffer: FAILED, errno %d (%s)\n",
		       errno, strerror(errno));
		return 1;
	}
	printf("2. surface backed by the imported buffer: sid %u\n", a.rep.handle);

	struct update_gb_surface_cmd cmd = {
		.id = SVGA_3D_CMD_UPDATE_GB_SURFACE,
		.size = sizeof(uint32_t),
		.sid = a.rep.handle,
	};
	struct drm_vmw_execbuf_arg e;
	memset(&e, 0, sizeof(e));
	e.commands = (uint64_t)(uintptr_t)&cmd;
	e.command_size = sizeof(cmd);
	e.version = DRM_VMW_EXECBUF_VERSION;
	e.context_handle = SVGA3D_INVALID_ID;
	errno = 0;
	if (ioctl(render, DRM_IOCTL_VMW_EXECBUF, &e))
		printf("3. execbuf UPDATE_GB_SURFACE: returned errno %d (%s)\n",
		       errno, strerror(errno));
	else
		printf("3. execbuf UPDATE_GB_SURFACE: accepted\n");
	printf("   -> now check the kernel log for \"VSG table map failed!\"\n");

	if (!release) {
		printf("4. holding the buffer open (no release). Ctrl-C or kill to end;\n"
		       "   note that ending this process frees the BO, which is the\n"
		       "   step that can oops with the BO-destroy fix applied.\n");
		fflush(stdout);
		pause();
		return 0;
	}

	printf("4. --release: dropping the surface, the handle and every fd\n");
	fflush(stdout);
	struct drm_vmw_surface_arg u = { .sid = a.rep.handle, .handle_type = DRM_VMW_HANDLE_LEGACY };
	ioctl(render, DRM_IOCTL_VMW_UNREF_SURFACE, &u);
	struct drm_gem_close gc = { .handle = ph.handle };
	ioctl(render, DRM_IOCTL_GEM_CLOSE, &gc);
	close(dfd);
	close(mfd);
	close(udm);
	close(render);
	sleep(1);
	printf("5. released, still alive\n");
	return 0;
}

 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index dfd08ee19..3e8bdf246 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -189,6 +189,11 @@ static int vmw_ttm_map_dma(struct vmw_ttm_tt *vmw_tt)
 	case vmw_dma_map_bind:
 	case vmw_dma_map_populate:
 		if (vmw_tt->dma_ttm.page_flags  & TTM_TT_FLAG_EXTERNAL) {
+			/*
+			 * The exporter has already mapped its sg_table for
+			 * this device in dma_buf_map_attachment(). Use it as
+			 * it is: it is not ours to map, unmap or free.
+			 */
 			vsgt->sgt = vmw_tt->dma_ttm.sg;
 		} else {
 			vsgt->sgt = &vmw_tt->sgt;
@@ -199,11 +204,11 @@ static int vmw_ttm_map_dma(struct vmw_ttm_tt *vmw_tt)
 				GFP_KERNEL);
 			if (ret)
 				goto out_sg_alloc_fail;
-		}
 
-		ret = vmw_ttm_map_for_dma(vmw_tt);
-		if (unlikely(ret != 0))
-			goto out_map_fail;
+			ret = vmw_ttm_map_for_dma(vmw_tt);
+			if (unlikely(ret != 0))
+				goto out_map_fail;
+		}
 
 		break;
 	default:
@@ -237,6 +242,13 @@ static void vmw_ttm_unmap_dma(struct vmw_ttm_tt *vmw_tt)
 	if (!vmw_tt->vsgt.sgt)
 		return;
 
+	if (vmw_tt->dma_ttm.page_flags & TTM_TT_FLAG_EXTERNAL) {
+		/* The mapping and the table belong to the exporter. */
+		vmw_tt->vsgt.sgt = NULL;
+		vmw_tt->mapped = false;
+		return;
+	}
+
 	switch (dev_priv->map_mode) {
 	case vmw_dma_map_bind:
 	case vmw_dma_map_populate:
-- 
2.55.0

[PATCH v2 2/3] drm/vmwgfx: Release PRIME import in the BO destroy path

[Michal TOMA]

drm_gem_prime_import_dev() attaches to a foreign dma-buf, takes a
reference with get_dma_buf(), maps the attachment and, once
vmw_prime_import_sg_table() has created the TTM buffer object, stores
the attachment in obj->import_attach. Drivers that import this way have
to undo it by calling drm_prime_gem_destroy() when the object is freed.

vmwgfx never does. The TTM destroy callback, vmw_bo_free(), only calls
drm_gem_object_release() and kfree(). For every imported dma-buf, the
sg mapping, the attachment and the dma-buf reference are leaked when
the last GEM reference goes away, and the exporter's backing pages stay
pinned until reboot.

Any process that can open the vmwgfx render node can hit this by
importing a dma-buf from another exporter, for example with
DRM_IOCTL_PRIME_FD_TO_HANDLE. It showed up as a memory drain on a
VirtualBox VMSVGA guest running a Plasma 6.7 Wayland session with the
host's 3D acceleration off. KWin 6.7 wraps wl_shm client buffers in
udmabufs and imports them through EGL on llvmpipe, and it keeps many of
those imports referenced while it runs, which is a separate userspace
problem. Restarting the compositor did not give the memory back,
though: with about 1.5 GiB of client buffers imported, killing KWin
left 211 udmabufs in /sys/kernel/debug/dma_buf/bufinfo with a refcount
of 1, still attached to the vmwgfx device, after every userspace
reference was gone. Their pages are no longer mapped or in any page
cache, so reclaim and swap cannot free them.

Call drm_prime_gem_destroy() for imported objects before
drm_gem_object_release(), as amdgpu, radeon and nouveau do in their TTM
destroy callbacks, and include <drm/drm_prime.h> for it. The check is
safe on the import error path: drm_gem_prime_import_dev() only sets
obj->import_attach after gem_prime_import_sg_table() succeeded, so a
buffer object destroyed before that point is not unmapped, detached or
put a second time. ttm_bo_release() tears down the TTM backing and
drops the reservation lock before calling the destroy callback, which
fits the _unlocked unmap done by drm_prime_gem_destroy().

The leak was found by walking /proc/kpageflags, which attributed the
missing memory to orphaned shmem pages. Those matched the udmabuf
objects in dma_buf/bufinfo, and the remaining reference was traced to
the missing PRIME teardown. It was confirmed without a compositor using
a small reproducer: create a memfd, wrap it with UDMABUF_CREATE, import
it with DRM_IOCTL_PRIME_FD_TO_HANDLE on the vmwgfx render node, close
the GEM handle and every file descriptor, then check bufinfo.

Build tested on drm-misc-fixes (4600b4d1a9ee) with the openSUSE 7.2.3
config and W=1: drivers/gpu/drm/vmwgfx/ builds without warnings before
and after the change, and checkpatch.pl --strict is clean. Runtime
tested on a VirtualBox 7.2 VMSVGA guest with kernel 7.2.3, whose vmwgfx
sources for the files involved are identical to drm-misc-fixes, using a
vmwgfx.ko built from them with this change. With the reproducer, all 8
imported udmabufs stayed pinned without the change and none with it,
with 3D acceleration both on and off. With KWin on llvmpipe, killing
the compositor now released 84 of 92 udmabufs (610 MiB) within 10
seconds, where the same test on the unpatched driver released none.

Fixes: b32233acceff ("drm/vmwgfx: Fix prime import/export")
Cc: stable@vger.kernel.org # v6.6+
Assisted-by: LLM
Signed-off-by: Michal TOMA <michaltoma@sicoop.com>
---
Reproducer (results and environment are in the cover letter). Compare
the 4 MiB udmabuf entries in /sys/kernel/debug/dma_buf/bufinfo before
and after: without this patch all 8 remain, with count 1 and still
attached to the vmwgfx device.

#!/usr/bin/env python3
import fcntl, os, struct
N, SIZE = 8, 4 * 1024 * 1024
UDMABUF_CREATE     = 0x40187542  # _IOW('u', 0x42, struct udmabuf_create)
PRIME_FD_TO_HANDLE = 0xC00C642E  # DRM_IOWR(0x2e, struct drm_prime_handle)
GEM_CLOSE          = 0x40086409  # DRM_IOW(0x09, struct drm_gem_close)
render = os.open('/dev/dri/renderD128', os.O_RDWR | os.O_CLOEXEC)
udm = os.open('/dev/udmabuf', os.O_RDWR | os.O_CLOEXEC)
for i in range(N):
    mfd = os.memfd_create(f'prime-leak-{i}', os.MFD_ALLOW_SEALING)
    os.ftruncate(mfd, SIZE)
    fcntl.fcntl(mfd, fcntl.F_ADD_SEALS, fcntl.F_SEAL_SHRINK)
    create = bytearray(struct.pack('IIQQ', mfd, 1, 0, SIZE))
    dfd = fcntl.ioctl(udm, UDMABUF_CREATE, create)
    ph = bytearray(struct.pack('IIi', 0, 0, dfd))
    fcntl.ioctl(render, PRIME_FD_TO_HANDLE, ph)
    handle = struct.unpack('IIi', ph)[0]
    fcntl.ioctl(render, GEM_CLOSE, bytearray(struct.pack('II', handle, 0)))
    os.close(dfd)
    os.close(mfd)
os.close(udm)
os.close(render)

The same loop without the PRIME_FD_TO_HANDLE/GEM_CLOSE step frees
every udmabuf, so the leak is specific to the vmwgfx import.

 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 9c7a73c0b..56bc94edc 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -30,6 +30,7 @@
 #include "vmwgfx_drv.h"
 #include "vmwgfx_resource_priv.h"
 
+#include <drm/drm_prime.h>
 #include <drm/ttm/ttm_placement.h>
 
 /**
@@ -69,6 +70,8 @@ static void vmw_bo_free(struct ttm_buffer_object *bo)
 		vmw_surface_unreference(&vbo->dumb_surface);
 	}
 	WARN_ON(!RB_EMPTY_ROOT(&vbo->res_tree));
+	if (drm_gem_is_imported(&vbo->tbo.base))
+		drm_prime_gem_destroy(&vbo->tbo.base, vbo->tbo.sg);
 	drm_gem_object_release(&vbo->tbo.base);
 	WARN_ON(vbo->dirty);
 	kfree(vbo);
-- 
2.55.0

[PATCH v2 3/3] drm/vmwgfx: Don't leak a GEM handle when referencing a surface by fd

[Michal TOMA]

DRM_VMW_GB_SURFACE_REF_EXT with DRM_VMW_HANDLE_PRIME first tries
ttm_prime_fd_to_handle(). If the fd is not a vmwgfx surface export,
vmw_surface_handle_reference() falls back to
vmw_buffer_prime_to_surface_base(). That function calls
drm_gem_prime_fd_to_handle() to turn the fd into a GEM handle in the
caller's file, then looks for a surface attached to that buffer.

That handle is never returned to userspace, since the ioctl reports the
surface handle and a separate backup buffer handle, and it is never
deleted. Every call that gets this far therefore leaves a GEM handle
behind in the file, whether it fails or succeeds, and whatever the
handle references stays alive until the file is closed.

For a dma-buf from another exporter, such as udmabuf, this is an
actual import: drm_gem_prime_fd_to_handle() attaches to the dma-buf,
maps it and creates a new GEM object. The surface lookup then always
fails, because such a buffer never has a surface. The WARN_ON() fires
and the ioctl returns -EINVAL, but the import stays referenced by the
stray handle.

Mesa's svga winsys passes dma-buf fds to DRM_VMW_GB_SURFACE_REF_EXT as
DRM_VMW_HANDLE_PRIME, and KWin 6.7 imports every wl_shm client buffer
as a udmabuf through EGL. On a Plasma Wayland session this pins one
window-sized buffer inside the compositor's DRM file for every
attempted import, for as long as the compositor runs, and logs a full
WARN backtrace each time. On a VirtualBox VMSVGA guest with 3D
acceleration on and the previous patch applied, 103 udmabufs (750 MiB)
had accumulated this way 40 minutes after boot. 99 of them had a
dma-buf refcount of 3, were still attached to the vmwgfx device and
were held by no process fd or mapping.

Look the buffer up through the dma-buf instead of importing it. Only
GEM buffers exported by this device can have a surface, so reject
anything else with drm_gem_is_prime_exported_dma_buf() before touching
it. For our own buffers, use dma_buf->priv directly: the dma-buf holds
a reference to the GEM object for as long as we hold the dma-buf, so
neither a handle nor an extra object reference is needed. The surface
lookup and the ttm_ref_object_add() reference that the callers rely on
are unchanged.

While restructuring the function, also:
 - replace the WARN_ON() on a buffer without a surface with a debug
   message. Userspace can trigger that case at will.
 - drop the base object reference taken by
   vmw_lookup_user_surface_for_buffer() when ttm_ref_object_add() fails.
   It was leaked before.

Build tested on drm-misc-fixes (4600b4d1a9ee) with W=1 (no warnings in
drivers/gpu/drm/vmwgfx/) and checkpatch.pl --strict. Runtime tested on
the same guest and 7.2.3 kernel as the previous patch, comparing a
vmwgfx.ko with only the previous patch against one with both. Calls
from a second render file:
 - udmabuf fd: -EINVAL both times. Before, each call left the import
   held by the file (refcount 3, attached) and logged a WARN. Now
   nothing is imported and nothing is logged.
 - fd of a dumb buffer exported from the primary node: -EINVAL both
   times. Before, a stray GEM handle and a WARN per call; now neither.
 - fd of a buffer exported before a GB surface was created on it, which
   reaches this function and succeeds: the exporter's surface is
   returned both times. Before, each call left a GEM handle in the
   file; now it does not.
 - fd of a buffer created together with its surface, which is exported
   as a TTM prime surface and does not reach this function: succeeds
   both times.
During a 5-minute KWin 6.7 test replaying a terminal workload, the
udmabuf count went from 4 to 20 (140 MiB) with only the previous patch,
with WARNs logged. With both patches it stayed at 6 during the run,
returned to 4 afterwards, and no WARN was logged.

Fixes: d6667f0ddf46 ("drm/vmwgfx: Fix handling of dumb buffers")
Cc: stable@vger.kernel.org # v6.11+
Assisted-by: LLM
Signed-off-by: Michal TOMA <michaltoma@sicoop.com>
---
Reproducer (results and environment are in the cover letter). Run as
root, since it counts udmabufs in debugfs:

  cc -o vmwgfx-surfref-fd-leak vmwgfx-surfref-fd-leak.c
  ./vmwgfx-surfref-fd-leak

// SPDX-License-Identifier: MIT
/*
 * Reproducer for "drm/vmwgfx: Don't leak a GEM handle when referencing a
 * surface by fd": DRM_VMW_GB_SURFACE_REF_EXT(DRM_VMW_HANDLE_PRIME) with a
 * udmabuf fd. Run as root, since it counts udmabufs in debugfs.
 *
 *   cc -o vmwgfx-surfref-fd-leak vmwgfx-surfref-fd-leak.c && ./vmwgfx-surfref-fd-leak
 *
 * The render node stays open after the calls. Expected lines 2 and 3
 * (udmabufs left while the node is open / after it is closed):
 *   with this fix:       0 / 0
 *   without it:          8 / 0 if "drm/vmwgfx: Release PRIME import in
 *                        the BO destroy path" is applied, otherwise
 *                        8 / 8 (pinned until reboot)
 */
#define _GNU_SOURCE
#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <sys/ioctl.h>
#include <sys/mman.h>
#include <linux/udmabuf.h>
#include <drm/drm.h>
#include <drm/vmwgfx_drm.h>

#define DRM_IOCTL_VMW_GB_SURFACE_REF_EXT \
	DRM_IOWR(DRM_COMMAND_BASE + DRM_VMW_GB_SURFACE_REF_EXT, \
		 union drm_vmw_gb_surface_reference_ext_arg)
#define N 8
#define SIZE (6UL << 20)

/* udmabufs of @size listed in dma_buf/bufinfo */
static int count_udmabufs(unsigned long size)
{
	FILE *f = fopen("/sys/kernel/debug/dma_buf/bufinfo", "r");
	char line[512];
	int n = 0;

	if (!f) {
		perror("/sys/kernel/debug/dma_buf/bufinfo");
		return -1;
	}
	while (fgets(line, sizeof(line), f)) {
		unsigned long sz;

		if (sscanf(line, "%lu", &sz) == 1 && sz == size &&
		    strstr(line, "udmabuf"))
			n++;
	}
	fclose(f);
	return n;
}

int main(void)
{
	int render = open("/dev/dri/renderD128", O_RDWR | O_CLOEXEC);
	int udm = open("/dev/udmabuf", O_RDWR | O_CLOEXEC);
	int before, einval = 0;

	if (render < 0 || udm < 0) {
		perror("open");
		return 1;
	}
	before = count_udmabufs(SIZE);
	for (int i = 0; i < N; i++) {
		int memfd = memfd_create("surfref", MFD_ALLOW_SEALING | MFD_CLOEXEC);
		struct udmabuf_create create = {
			.memfd = memfd, .flags = UDMABUF_FLAGS_CLOEXEC, .size = SIZE,
		};
		union drm_vmw_gb_surface_reference_ext_arg arg;
		int dmabuf;

		if (memfd < 0 || ftruncate(memfd, SIZE) ||
		    fcntl(memfd, F_ADD_SEALS, F_SEAL_SHRINK)) {
			perror("memfd");
			return 1;
		}
		dmabuf = ioctl(udm, UDMABUF_CREATE, &create);
		if (dmabuf < 0) {
			perror("UDMABUF_CREATE");
			return 1;
		}
		memset(&arg, 0, sizeof(arg));
		arg.req.sid = dmabuf;
		arg.req.handle_type = DRM_VMW_HANDLE_PRIME;
		if (ioctl(render, DRM_IOCTL_VMW_GB_SURFACE_REF_EXT, &arg) &&
		    errno == EINVAL)
			einval++;
		close(dmabuf);
		close(memfd);
	}
	close(udm);
	usleep(500000);
	printf("GB_SURFACE_REF_EXT with a udmabuf fd: %d/%d calls failed with EINVAL\n",
	       einval, N);
	printf("6 MiB udmabufs left, render node open:   %d (before: %d)\n",
	       count_udmabufs(SIZE), before);
	close(render);
	usleep(500000);
	printf("6 MiB udmabufs left, render node closed: %d\n",
	       count_udmabufs(SIZE));
	return 0;
}

 drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 48 ++++++++++++++-----------
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index bd0563741..27f68fd9c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -16,8 +16,11 @@
 #include "device_include/svga3d_surfacedefs.h"
 
 #include <drm/drm_dumb_buffers.h>
+#include <drm/drm_prime.h>
 #include <drm/ttm/ttm_placement.h>
 
+#include <linux/dma-buf.h>
+
 #define SVGA3D_FLAGS_64(upper32, lower32) (((uint64_t)upper32 << 32) | lower32)
 
 /**
@@ -931,33 +934,37 @@ u32 vmw_lookup_surface_handle_for_buffer(struct vmw_private *vmw,
 
 static int vmw_buffer_prime_to_surface_base(struct vmw_private *dev_priv,
 					    struct drm_file *file_priv,
-					    u32 fd, u32 *handle,
+					    u32 fd,
 					    struct ttm_base_object **base_p)
 {
 	struct ttm_base_object *base;
-	struct vmw_bo *bo;
+	struct dma_buf *dma_buf;
 	struct ttm_object_file *tfile = vmw_fpriv(file_priv)->tfile;
 	struct vmw_user_surface *user_srf;
 	int ret;
 
-	ret = drm_gem_prime_fd_to_handle(&dev_priv->drm, file_priv, fd, handle);
-	if (ret) {
-		drm_warn(&dev_priv->drm,
-			 "Wasn't able to find user buffer for fd = %u.\n", fd);
-		return ret;
-	}
+	dma_buf = dma_buf_get(fd);
+	if (IS_ERR(dma_buf))
+		return PTR_ERR(dma_buf);
 
-	ret = vmw_user_bo_lookup(file_priv, *handle, &bo);
-	if (ret) {
-		drm_warn(&dev_priv->drm,
-			 "Wasn't able to lookup user buffer for handle = %u.\n", *handle);
-		return ret;
+	/*
+	 * Only buffers exported by this device can have a user surface.
+	 * Look the buffer up through the dma-buf, which holds a reference
+	 * to it, instead of importing the fd into file_priv: the GEM handle
+	 * that would create is never returned to userspace, so nothing
+	 * would release it (or a foreign dma-buf) until the file is closed.
+	 */
+	if (!drm_gem_is_prime_exported_dma_buf(&dev_priv->drm, dma_buf)) {
+		ret = -EINVAL;
+		goto out;
 	}
 
-	user_srf = vmw_lookup_user_surface_for_buffer(dev_priv, bo, *handle);
-	if (WARN_ON(!user_srf)) {
-		drm_warn(&dev_priv->drm,
-			 "User surface fd %d (handle %d) is null.\n", fd, *handle);
+	user_srf = vmw_lookup_user_surface_for_buffer(dev_priv,
+						      to_vmw_bo(dma_buf->priv),
+						      fd);
+	if (!user_srf) {
+		drm_dbg_driver(&dev_priv->drm,
+			       "No user surface for buffer fd %d.\n", fd);
 		ret = -EINVAL;
 		goto out;
 	}
@@ -966,13 +973,15 @@ static int vmw_buffer_prime_to_surface_base(struct vmw_private *dev_priv,
 	ret = ttm_ref_object_add(tfile, base, NULL, false);
 	if (ret) {
 		drm_warn(&dev_priv->drm,
-			 "Couldn't add an object ref for the buffer (%d).\n", *handle);
+			 "Couldn't add an object ref for buffer fd %d (%d).\n",
+			 fd, ret);
+		ttm_base_object_unref(&base);
 		goto out;
 	}
 
 	*base_p = base;
 out:
-	vmw_user_bo_unref(&bo);
+	dma_buf_put(dma_buf);
 
 	return ret;
 }
@@ -996,7 +1005,6 @@ vmw_surface_handle_reference(struct vmw_private *dev_priv,
 			return vmw_buffer_prime_to_surface_base(dev_priv,
 								file_priv,
 								u_handle,
-								&handle,
 								base_p);
 	} else {
 		handle = u_handle;
-- 
2.55.0

Re: [PATCH v2 2/3] drm/vmwgfx: Release PRIME import in the BO destroy path

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] NULL pointer dereference when dma_buf import fails.
--

commit 200f36633d8d14d0ae43912ea8f089cba49bace3
Author: Michal TOMA <michaltoma@sicoop.com>

drm/vmwgfx: Release PRIME import in the BO destroy path

This patch fixes a resource leak in the vmwgfx driver where PRIME imported
dma-bufs are not properly torn down when the buffer object is destroyed. It
adds a call to drm_prime_gem_destroy() in vmw_bo_free() to ensure the
attachment and sg mapping are released.

> drm_gem_prime_import_dev() attaches to a foreign dma-buf, takes a
> reference with get_dma_buf(), maps the attachment and, once
> vmw_prime_import_sg_table() has created the TTM buffer object, stores
> the attachment in obj->import_attach. Drivers that import this way have
> to undo it by calling drm_prime_gem_destroy() when the object is freed.

[Severity: High]
This is a pre-existing issue, but since we are looking at the dma_buf import
path, could a failed import trigger a NULL pointer dereference here?

When a process imports a dma_buf via DRM_IOCTL_PRIME_FD_TO_HANDLE on the
vmwgfx device, the DRM core calls vmw_prime_import_sg_table(). If BO creation
fails, the function returns NULL instead of an ERR_PTR:

drivers/gpu/drm/vmwgfx/vmwgfx_gem.c:vmw_prime_import_sg_table() {
    ...
out_no_bo:
	dma_resv_unlock(params.resv);
	return gem;
}

In the DRM core, drm_gem_prime_import_dev() only checks for IS_ERR(),
which is false for NULL:

drivers/gpu/drm/drm_prime.c:drm_gem_prime_import_dev() {
    ...
	obj = dev->driver->gem_prime_import_sg_table(dev, attach, sgt);
	if (IS_ERR(obj)) {
		ret = PTR_ERR(obj);
		goto fail_unmap;
	}

	obj->import_attach = attach;
    ...
}

Does this mean an unprivileged user could crash the kernel by forcing an
import failure (e.g. by exhausting memory or providing an excessive size)?

Should vmw_prime_import_sg_table() return ERR_PTR(-ENOMEM) instead of NULL
on failure?

[ ... ]

> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> index 9c7a73c0b0dc3..56bc94edc30b7 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c

[ ... ]

> @@ -69,6 +70,8 @@ static void vmw_bo_free(struct ttm_buffer_object *bo)
>  		vmw_surface_unreference(&vbo->dumb_surface);
>  	}
>  	WARN_ON(!RB_EMPTY_ROOT(&vbo->res_tree));
> +	if (drm_gem_is_imported(&vbo->tbo.base))
> +		drm_prime_gem_destroy(&vbo->tbo.base, vbo->tbo.sg);
>  	drm_gem_object_release(&vbo->tbo.base);
>  	WARN_ON(vbo->dirty);
>  	kfree(vbo);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910165221.7558-1-michaltoma@sicoop.com?part=2

Re: [PATCH v2 3/3] drm/vmwgfx: Don't leak a GEM handle when referencing a surface by fd

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Unsynchronized rbtree and xarray traversal in vmw_bo_surface() can lead to crashes or UAF.
- [High] Kernel stack memory leak in `vmw_gb_surface_reference_internal` error path.
--

commit 6ca6707ae183fa071e8ebb217050fe9e1947a0b2
Author: Michal TOMA <michaltoma@sicoop.com>

drm/vmwgfx: Don't leak a GEM handle when referencing a surface by fd

This commit prevents a GEM handle leak by looking up a surface buffer
through its dma-buf directly instead of importing the fd into the DRM file.
It rejects buffers not exported by the device and simplifies object
reference cleanup.

Cc: stable@vger.kernel.org # v6.11+

> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> index bd0563741e89d..27f68fd9c22bd 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c

[ ... ]

> @@ -931,33 +934,37 @@ u32 vmw_lookup_surface_handle_for_buffer(struct vmw_private *vmw,
>  
>  static int vmw_buffer_prime_to_surface_base(struct vmw_private *dev_priv,
>  					    struct drm_file *file_priv,
> -					    u32 fd, u32 *handle,
> +					    u32 fd,
>  					    struct ttm_base_object **base_p)
>  {

[ ... ]

> -	user_srf = vmw_lookup_user_surface_for_buffer(dev_priv, bo, *handle);
> -	if (WARN_ON(!user_srf)) {
> -		drm_warn(&dev_priv->drm,
> -			 "User surface fd %d (handle %d) is null.\n", fd, *handle);
> +	user_srf = vmw_lookup_user_surface_for_buffer(dev_priv,
> +						      to_vmw_bo(dma_buf->priv),
> +						      fd);

[Severity: Critical]
This is a pre-existing issue, but does this lockless lookup risk a
use-after-free or kernel crash?

When vmw_lookup_user_surface_for_buffer() is called here, it calls
vmw_bo_surface() which iterates over the detached_resources xarray and
res_tree rbtree without holding the dma_resv lock or an RCU read lock:

drivers/gpu/drm/vmwgfx/vmwgfx_bo.c:vmw_bo_surface() {
        ...
        xa_for_each(&vbo->detached_resources, index, res) {
                if (res->func->res_type == vmw_res_surface)
                        goto out;
        }
        ...
        for (rb_itr = rb_first(&vbo->res_tree); rb_itr;
             rb_itr = rb_next(rb_itr)) {
        ...
}

If another thread concurrently modifies these structures (for example, by
adding or removing resources), this lockless traversal could access freed
nodes. Should this lookup take the buffer object's dma_resv lock?

> +	if (!user_srf) {
> +		drm_dbg_driver(&dev_priv->drm,
> +			       "No user surface for buffer fd %d.\n", fd);
>  		ret = -EINVAL;
>  		goto out;
>  	}

[ ... ]

[Severity: High]
As an additional observation in this file, this is a pre-existing issue,
but does vmw_gb_surface_reference_internal() leak kernel stack memory to
userspace in its error path?

If srf->res.guest_memory_bo is NULL, the error path skips setting a
negative error code and jumps directly to out_bad_resource:

drivers/gpu/drm/vmwgfx/vmwgfx_surface.c:vmw_gb_surface_reference_internal() {
        ...
        if (!srf->res.guest_memory_bo) {
                DRM_ERROR("Shared GB surface is missing a backup buffer.\n");
                goto out_bad_resource;
        }
        ...
out_bad_resource:
        ...
        return ret;
}

Because ret is still 0 at this point, the caller
vmw_gb_surface_reference_ioctl() assumes success and copies the
uninitialized stack variable rep_ext into the user-visible rep pointer:

drivers/gpu/drm/vmwgfx/vmwgfx_surface.c:vmw_gb_surface_reference_ioctl() {
        struct drm_vmw_gb_surface_ref_ext_rep rep_ext;
        int ret;

        ret = vmw_gb_surface_reference_internal(dev, req, &rep_ext, file_priv);

        if (unlikely(ret != 0))
                return ret;

        rep->creq = rep_ext.creq.base;
        rep->crep = rep_ext.crep;
}

Could this be fixed by explicitly setting ret = -EINVAL before jumping to
out_bad_resource in vmw_gb_surface_reference_internal()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910165221.7558-1-michaltoma@sicoop.com?part=3