[PATCH v6 0/4] Fix some issues from igt runs.

[PATCH v6 0/4] Fix some issues from igt runs.

[Maaz Mombasawala]

I ran igt-gpu-tools with vmwgfx and fixed some issues I found.

Changes in v6:
- Remove memleak fix in ttm_prime_fd_to_handle() since the fix
is already present.

Changes in v5:
- Get display unit from cursor plane, so there is no need for
old_crtc.
- Cast ALIGN to u64.
- Check return value of ttm_bo_reserve().

Changes in v4:
- For ttm ref change, changed to incrementing the refcount on
the ttm base object of the dumb surface instead of holding a
ttm ref object so that it is not exposed to tfile.
- Added Fixes messages.
- Removed BO cleanup patch.

Changes in v3:
- Minor changes so series can apply cleanly on drm-misc-next.

Changes in v2:
- Changes based on Zack's comments.
- For vrefresh changes, removed stdu specific checks so that test
kms_invalid_mode@overflow-vrefresh passes with sou and ldu displays.
For ttm ref changes, fixed a ref leak in ttm_prime_fd_to_handle()
which fixes a memleak in test vmw_prime@tri-map-dmabuf and removed
release callback for dumb buffer surface since the gem buffer will
handle release of the surface.
Added commit to remove duplicate functions in vmwgfx_bo.h.

Maaz Mombasawala (4):
  drm/vmwgfx: Add some checks to vmw_cursor_plane_atomic_update
  drm/vmwgfx: Check vrefresh in drm_mode_setcrtc.
  drm/vmwgfx: Reserve ttm object before resv usage
  drm/vmwgfx: Change ttm refs for dumb buffers.

 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c           |  8 +++++-
 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 16 +++++++----
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h          |  1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c          | 30 ++++++++++++++++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h          |  9 ++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c         | 12 ++------
 drivers/gpu/drm/vmwgfx/vmwgfx_surface.c      | 12 +++++++-
 drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c         |  8 ++++++
 8 files changed, 79 insertions(+), 17 deletions(-)

-- 
2.54.0

[PATCH v6 1/4] drm/vmwgfx: Add some checks to vmw_cursor_plane_atomic_update

[Maaz Mombasawala]

In cases where old_state may not be present we hit a null dereference.
Get display unit from cursor plane instead to fix this and also do an
early return if there is no new_state since there is nothing to do in
that case.
This fixes igt test kms_cursor_legacy@torture-bo.

Fixes: 965544150d1c ("drm/vmwgfx: Refactor cursor handling")
Cc: Zack Rusin <zack.rusin@broadcom.com>
Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Maaz Mombasawala <maaz.mombasawala@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
index d1e7df500190..563812f1e23e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
@@ -22,6 +22,9 @@
 #define VMW_CURSOR_SNOOP_WIDTH 64
 #define VMW_CURSOR_SNOOP_HEIGHT 64
 
+#define vmw_cursor_to_du(cursor_plane) \
+	container_of(cursor_plane, struct vmw_display_unit, cursor.base)
+
 struct vmw_svga_fifo_cmd_define_cursor {
 	u32 cmd;
 	SVGAFifoCmdDefineAlphaCursor cursor;
@@ -781,13 +784,14 @@ vmw_cursor_plane_atomic_update(struct drm_plane *plane,
 			       struct drm_atomic_commit *state)
 {
 	struct vmw_bo *bo;
-	struct drm_plane_state *new_state =
-		drm_atomic_get_new_plane_state(state, plane);
-	struct drm_plane_state *old_state =
-		drm_atomic_get_old_plane_state(state, plane);
-	struct drm_crtc *crtc = new_state->crtc ?: old_state->crtc;
+	struct drm_plane_state *new_state;
+
+	new_state = drm_atomic_get_new_plane_state(state, plane);
+	if (!new_state)
+		return;
+
 	struct vmw_private *dev_priv = vmw_priv(plane->dev);
-	struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
+	struct vmw_display_unit *du = vmw_cursor_to_du(plane);
 	struct vmw_plane_state *vps = vmw_plane_state_to_vps(new_state);
 	s32 hotspot_x, hotspot_y, cursor_x, cursor_y;
 
-- 
2.54.0

[PATCH v6 2/4] drm/vmwgfx: Check vrefresh in drm_mode_setcrtc.

[Maaz Mombasawala]

Add the mode_valid() callback in drm_mode_config_funcs to check for valid
mode during drm_mode_setcrtc.
Both the mode_valid() callbacks also check if the vrefresh value is
correct.
This fixes igt test kms_invalid_mode@overflow-vrefresh.

Signed-off-by: Maaz Mombasawala <maaz.mombasawala@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 30 ++++++++++++++++++++++++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  9 +++++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 12 +++--------
 3 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 1b407b61f683..c7d977aba3f9 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -1063,8 +1063,38 @@ vmw_kms_atomic_check_modeset(struct drm_device *dev,
 	return ret;
 }
 
+static enum drm_mode_status
+vmw_kms_config_mode_valid(struct drm_device *dev,
+			  const struct drm_display_mode *mode)
+{
+	enum drm_mode_status ret;
+	struct vmw_private *dev_priv = vmw_priv(dev);
+	u64 assumed_cpp = dev_priv->assume_16bpp ? 2 : 4;
+	/* Align width and height to account for GPU tile over-alignment */
+	u64 required_mem = 0;
+
+	ret = drm_mode_validate_size(mode, dev_priv->texture_max_width,
+				     dev_priv->texture_max_height);
+	if (ret != MODE_OK)
+		return ret;
+
+	required_mem = (u64)ALIGN(mode->hdisplay, GPU_TILE_SIZE) *
+		       ALIGN(mode->vdisplay, GPU_TILE_SIZE) *
+		       assumed_cpp;
+	required_mem = ALIGN(required_mem, PAGE_SIZE);
+
+	if (required_mem > dev_priv->max_primary_mem)
+		return MODE_MEM;
+
+	if (!drm_mode_vrefresh(mode))
+		return MODE_BAD;
+
+	return MODE_OK;
+}
+
 static const struct drm_mode_config_funcs vmw_kms_funcs = {
 	.fb_create = vmw_kms_fb_create,
+	.mode_valid = vmw_kms_config_mode_valid,
 	.atomic_check = vmw_kms_atomic_check_modeset,
 	.atomic_commit = drm_atomic_helper_commit,
 };
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
index 2224d7d91d1b..1b717caecc78 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
@@ -16,6 +16,15 @@
 #include <drm/drm_framebuffer.h>
 #include <drm/drm_probe_helper.h>
 
+/*
+ * Some renderers such as llvmpipe will align the width and height of their
+ * buffers to match their tile size. We need to keep this in mind when exposing
+ * modes to userspace so that this possible over-allocation will not exceed
+ * graphics memory. 64x64 pixels seems to be a reasonable upper bound for the
+ * tile size of current renderers.
+ */
+#define GPU_TILE_SIZE 64
+
 /**
  * struct vmw_du_update_plane - Closure structure for vmw_du_helper_plane_update
  * @plane: Plane which is being updated.
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index 4139837f4caf..04c5c20588d8 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -45,15 +45,6 @@
 #define vmw_connector_to_stdu(x) \
 	container_of(x, struct vmw_screen_target_display_unit, base.connector)
 
-/*
- * Some renderers such as llvmpipe will align the width and height of their
- * buffers to match their tile size. We need to keep this in mind when exposing
- * modes to userspace so that this possible over-allocation will not exceed
- * graphics memory. 64x64 pixels seems to be a reasonable upper bound for the
- * tile size of current renderers.
- */
-#define GPU_TILE_SIZE 64
-
 enum stdu_content_type {
 	SAME_AS_DISPLAY = 0,
 	SEPARATE_SURFACE,
@@ -870,6 +861,9 @@ vmw_stdu_connector_mode_valid(struct drm_connector *connector,
 	if (required_mem > dev_priv->max_mob_size)
 		return MODE_MEM;
 
+	if (!drm_mode_vrefresh(mode))
+		return MODE_BAD;
+
 	return MODE_OK;
 }
 
-- 
2.54.0

[PATCH v6 3/4] drm/vmwgfx: Reserve ttm object before resv usage

[Maaz Mombasawala]

We hit dma_resv_assert_held warnings in several places, reserve the ttm
base object when it's being used to avoid them.

Signed-off-by: Maaz Mombasawala <maaz.mombasawala@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c   | 6 ++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c | 8 ++++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 9c7a73c0b0dc..3a739a761f3a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -52,10 +52,16 @@ static void vmw_bo_free(struct ttm_buffer_object *bo)
 		WARN_ON(vbo != res->guest_memory_bo);
 		WARN_ON(!res->guest_memory_bo);
 		if (res->guest_memory_bo) {
+			int ret = 0;
 			/* Reserve and switch the backing mob. */
 			mutex_lock(&res->dev_priv->cmdbuf_mutex);
 			(void)vmw_resource_reserve(res, false, true);
+			ret = ttm_bo_reserve(bo, false, false, NULL);
+			if (ret != 0)
+				drm_warn(&res->dev_priv->drm,
+					 "%s: failed reserve\n", __func__);
 			vmw_resource_mob_detach(res);
+			ttm_bo_unreserve(bo);
 			if (res->dirty)
 				res->func->dirty_free(res);
 			if (res->coherent)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c
index 3d0d5dfa869f..ef163d4b64d1 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c
@@ -131,6 +131,7 @@ crc_generate_worker(struct work_struct *work)
 	spin_unlock_irq(&du->vkms.crc_state_lock);
 
 	if (surf) {
+		int ret;
 		if (vmw_surface_sync(vmw, surf)) {
 			drm_warn(
 				crtc->dev,
@@ -138,7 +139,14 @@ crc_generate_worker(struct work_struct *work)
 			return;
 		}
 
+		ret = ttm_bo_reserve(&surf->res.guest_memory_bo->tbo, false, false, NULL);
+		if (ret != 0) {
+			drm_warn(&vmw->drm, "%s: failed reserve\n", __func__);
+			goto done;
+		}
 		compute_crc(crtc, surf, &crc32);
+		ttm_bo_unreserve(&surf->res.guest_memory_bo->tbo);
+done:
 		vmw_surface_unreference(&surf);
 	}
 
-- 
2.54.0

Re: [PATCH v6 3/4] drm/vmwgfx: Reserve ttm object before resv usage

[sashiko-bot]

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

New issues:
- [High] Invalid error handling for ttm_bo_reserve failure causes unbalanced unlock.
- [High] Race condition leading to NULL pointer dereference or Use-After-Free of guest_memory_bo.

Pre-existing issues:
- [High] Early return in crc_generate_worker leaks surface reference and permanently blocks CRC generation.
--

--- Patch [3]: [PATCH v6 3/4] drm/vmwgfx: Reserve ttm object before resv usage ---
commit b992ba3fb6a3eb8690f4284b67dfcbe8813b4aea
Author: Maaz Mombasawala <maaz.mombasawala@broadcom.com>

drm/vmwgfx: Reserve ttm object before resv usage

This patch reserves the ttm base object when it is being used to avoid
dma_resv_assert_held warnings.

> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> index 9c7a73c0b0dc3..3a739a761f3a0 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> @@ -52,10 +52,16 @@ static void vmw_bo_free(struct ttm_buffer_object *bo)
>  		WARN_ON(vbo != res->guest_memory_bo);
>  		WARN_ON(!res->guest_memory_bo);
>  		if (res->guest_memory_bo) {
> +			int ret = 0;
>  			/* Reserve and switch the backing mob. */
>  			mutex_lock(&res->dev_priv->cmdbuf_mutex);
>  			(void)vmw_resource_reserve(res, false, true);
> +			ret = ttm_bo_reserve(bo, false, false, NULL);
> +			if (ret != 0)
> +				drm_warn(&res->dev_priv->drm,
> +					 "%s: failed reserve\n", __func__);
>  			vmw_resource_mob_detach(res);
> +			ttm_bo_unreserve(bo);

[Severity: High]
If ttm_bo_reserve() fails, the code logs a warning but continues execution. Does
this result in an unbalanced unlock when ttm_bo_unreserve(bo) is unconditionally
called later?

Unlocking a reservation lock that was never successfully acquired can corrupt
the lock state and trigger a kernel panic. Should the error path abort or skip
the unlock?

>  			if (res->dirty)
>  				res->func->dirty_free(res);
>  			if (res->coherent)

[ ... ]

> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c
> index 7b8163b5e5014..d0c371df7e4c6 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c
> @@ -131,6 +131,7 @@ crc_generate_worker(struct work_struct *work)
>  	spin_unlock_irq(&du->vkms.crc_state_lock);
>  
>  	if (surf) {
> +		int ret;
>  		if (vmw_surface_sync(vmw, surf)) {
>  			drm_warn(
>  				crtc->dev,
> @@ -138,7 +139,14 @@ crc_generate_worker(struct work_struct *work)
>  			return;
>  		}

[Severity: High]
This is a pre-existing issue, but does returning early here skip the
vmw_surface_unreference(&surf) call at the end of the block and leak the
surface memory?

Also, the early return skips resetting du->vkms.crc_pending to false. Will
this permanently block future CRC generation since subsequent worker executions
will think work is already pending?

>  
> +		ret = ttm_bo_reserve(&surf->res.guest_memory_bo->tbo, false, false, NULL);

[Severity: High]
Can this race with vmw_bo_free() and dereference a NULL pointer?

If userspace closes the GEM handle concurrently, vmw_bo_free() might execute,
take the cmdbuf_mutex, set res->guest_memory_bo = NULL, and free the BO.

Since guest_memory_bo is accessed here without being protected by
crc_state_lock or other locks held by the worker, could this lead to a NULL
pointer dereference or use-after-free?

> +		if (ret != 0) {
> +			drm_warn(&vmw->drm, "%s: failed reserve\n", __func__);
> +			goto done;
> +		}
>  		compute_crc(crtc, surf, &crc32);
> +		ttm_bo_unreserve(&surf->res.guest_memory_bo->tbo);
> +done:
>  		vmw_surface_unreference(&surf);
>  	}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727182622.236086-1-maaz.mombasawala@broadcom.com?part=3

[PATCH v6 4/4] drm/vmwgfx: Change ttm refs for dumb buffers.

[Maaz Mombasawala]

Preserve a ref to surface during dumb buffer creation. This keeps the dumb
buffer valid for framebuffer usage and fixes all igt tests that use dumb
buffers.

Fixes: f42c09e614f1 ("drm/vmwgfx: Fix dumb buffer leak")
Cc: Ian Forbes <ian.forbes@broadcom.com>
Cc: Zack Rusin <zack.rusin@broadcom.com>
Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Maaz Mombasawala <maaz.mombasawala@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c      |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h     |  1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 12 +++++++++++-
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 3a739a761f3a..215e208fd778 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -72,7 +72,7 @@ static void vmw_bo_free(struct ttm_buffer_object *bo)
 					       0);
 			mutex_unlock(&res->dev_priv->cmdbuf_mutex);
 		}
-		vmw_surface_unreference(&vbo->dumb_surface);
+		vmw_dumb_surface_unref(&vbo->dumb_surface);
 	}
 	WARN_ON(!RB_EMPTY_ROOT(&vbo->res_tree));
 	drm_gem_object_release(&vbo->tbo.base);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 38bea8abab84..cda55ee009d2 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -1186,6 +1186,7 @@ u32 vmw_lookup_surface_handle_for_buffer(struct vmw_private *vmw,
 int vmw_dumb_create(struct drm_file *file_priv,
 		    struct drm_device *dev,
 		    struct drm_mode_create_dumb *args);
+void vmw_dumb_surface_unref(struct vmw_surface **dumb_surface);
 
 /*
  * Shader management - vmwgfx_shader.c
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index bd0563741e89..74e11baa7095 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -2340,11 +2340,21 @@ int vmw_dumb_create(struct drm_file *file_priv,
 	struct vmw_user_surface *usurf = container_of(vbo->dumb_surface,
 						struct vmw_user_surface, srf);
 	usurf->prime.base.refcount_release = NULL;
+	ttm_base_object_lookup_for_ref(dev_priv->tdev, arg.rep.handle);
 err:
 	if (res)
 		vmw_resource_unreference(&res);
-
 	ttm_ref_object_base_unref(tfile, arg.rep.handle);
 
 	return ret;
 }
+
+void vmw_dumb_surface_unref(struct vmw_surface **dumb_surface)
+{
+	struct vmw_user_surface *usurf = container_of(*dumb_surface,
+						struct vmw_user_surface, srf);
+	struct ttm_base_object *base = &usurf->prime.base;
+
+	ttm_base_object_unref(&base);
+	vmw_surface_unreference(dumb_surface);
+}
-- 
2.54.0

Re: [PATCH v6 4/4] drm/vmwgfx: Change ttm refs for dumb buffers.

[sashiko-bot]

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

New issues:
- [Critical] Artificial pinning of the base object causes a reliable Use-After-Free.
- [High] Refcount underflow and potential use-after-free because `ttm_base_object_lookup_for_ref` return value is ignored.
- [Medium] Leaked mutex state due to bypassing `ttm_prime_refcount_release`.

Pre-existing issues:
- [High] Race condition causing Use-After-Free due to premature `drm_gem_object_put`.
--

--- Patch [4]: [PATCH v6 4/4] drm/vmwgfx: Change ttm refs for dumb buffers. ---
commit 1eb9390d984c556a9fb59cfdc777988b3f118fac
Author: Maaz Mombasawala <maaz.mombasawala@broadcom.com>

drm/vmwgfx: Change ttm refs for dumb buffers.

This commit preserves a reference to the surface during dumb buffer creation
to keep the dumb buffer valid for framebuffer usage. This fixes IGT tests
that use dumb buffers.

> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> index bd0563741e89d..74e11baa7095f 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> @@ -2340,11 +2340,21 @@ int vmw_dumb_create(struct drm_file *file_priv,
>  	struct vmw_user_surface *usurf = container_of(vbo->dumb_surface,
>  						struct vmw_user_surface, srf);

[Severity: High]
This is a pre-existing issue, but can accessing vbo->dumb_surface here
lead to a use-after-free?

Looking at vmw_dumb_create(), the vbo reference is dropped slightly
earlier via drm_gem_object_put(). If a concurrent thread guesses the
gem handle and closes it, the final reference could be dropped. This
would free vbo before this container_of() calculation and assignment occur.

>  	usurf->prime.base.refcount_release = NULL;

[Severity: Medium]
Does setting the release callback to NULL bypass
ttm_prime_refcount_release() and leak the prime object's mutex?

This appears to prevent explicitly destroying the mutex, which can
trigger warnings when kernel mutex debugging is enabled.

> +	ttm_base_object_lookup_for_ref(dev_priv->tdev, arg.rep.handle);

[Severity: High]
Can this lookup fail if a concurrent thread guesses the sequential IDR
handle and calls the unref surface ioctl?

If the lookup fails and returns NULL, the internal reference is not
acquired. Because the return value is ignored here, the unconditional
unref later in vmw_dumb_surface_unref() might cause a kref underflow.

>  err:
>  	if (res)
>  		vmw_resource_unreference(&res);
> -
>  	ttm_ref_object_base_unref(tfile, arg.rep.handle);
>  
>  	return ret;
>  }
> +
> +void vmw_dumb_surface_unref(struct vmw_surface **dumb_surface)
> +{
> +	struct vmw_user_surface *usurf = container_of(*dumb_surface,
> +						struct vmw_user_surface, srf);
> +	struct ttm_base_object *base = &usurf->prime.base;
> +
> +	ttm_base_object_unref(&base);
> +	vmw_surface_unreference(dumb_surface);

[Severity: Critical]
Does setting refcount_release to NULL earlier and decoupling the base
object lifecycle from the underlying resource create a use-after-free
regression here?

If userspace concurrently grabs an extra reference to the surface via
the ref surface ioctl, the base object's refcount is incremented.
When the gem handle is later closed, ttm_base_object_unref() drops one
reference, leaving the refcount > 0.

However, vmw_surface_unreference() unconditionally drops the resource
refcount to 0, which frees the usurf object containing the base object.
The base object would remain in the IDR tree, and later accesses to it
would use freed memory.

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727182622.236086-1-maaz.mombasawala@broadcom.com?part=4