[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@xxxxxxxxxxxxxxx # v6.6+
Assisted-by: LLM
Signed-off-by: Michal TOMA <michaltoma@xxxxxxxxxx>
---
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