[PATCH 0/6] drm/vmwgfx: fix cursor validation and buffer lifetimes

[PATCH 0/6] drm/vmwgfx: fix cursor validation and buffer lifetimes

[Zack Rusin]

This series fixes cursor validation and buffer lifetime handling in
vmwgfx, including guest-backed devices without cursor MOB support.

Each prepared plane state keeps its own mapping, reference and pin, so
cleanup of an older commit cannot release resources needed by a newer
one. Cursor MOB teardown and dumb-buffer surface detachment also hold the
buffer reservations required by TTM.

Validation uses the newly selected framebuffer and rejects zero-sized
images and surfaces without backing storage. GB-only cursors use the
define-cursor command limits, including the 32 KiB whole-command guarantee
when using command buffers. Legacy cursor-bypass hotspots are included
consistently in the image and position, fixing displaced Xorg cursors.

Thanks to Yunfeng Li and Tomas M for reporting and diagnosing the GB-only
cursor issues, and to Tomas for the hotspot fix.

LLM assistance was used for the fixes, tests and commit descriptions;
the patches carry Assisted-by: LLM.

Tomas M (1):
  drm/vmwgfx: include legacy hotspots in GB-only cursor images

Zack Rusin (5):
  drm/vmwgfx: keep cursor mappings and pins with each plane state
  drm/vmwgfx: validate GB-only cursor command limits
  drm/vmwgfx: reserve dumb buffers while detaching their surfaces
  drm/vmwgfx: validate the framebuffer selected for cursor updates
  drm/vmwgfx: reserve cursor MOBs before unpinning them

 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c           |   3 +
 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 166 +++++++++++++------
 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.h |   4 +
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c          |   1 +
 4 files changed, 125 insertions(+), 49 deletions(-)


base-commit: 8ef66514d7b993c9fb96a5886f001e3556610c1c
-- 
2.53.0

[PATCH 1/6] drm/vmwgfx: keep cursor mappings and pins with each plane state

[Zack Rusin]

Cursor plane states duplicate the user object reference without acquiring
its mapping or pin. Preparing the duplicated state nevertheless unmaps
that object, and only pins the new buffer if it is not already pinned.
Cleanup of an older state can therefore remove a mapping or pin still
needed by another cursor commit. Repeated cursor updates from KMS clients
can then access an unmapped or movable buffer.

Give each prepared state its own mapping, buffer reference and pin, and
reset the mapping when duplicating a state. Release only the resources
owned by that state in cleanup_fb(). Map the required image extent for
every state so resizing a cursor on the same buffer cannot reuse a
smaller cached mapping. Unwind the pin and reservation if preparation
fails.

Fixes: 965544150d1c ("drm/vmwgfx: Refactor cursor handling")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 69 ++++++++++++--------
 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.h |  4 ++
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c          |  1 +
 3 files changed, 48 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
index 5a69606a0593..942d07079ef7 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
@@ -112,8 +112,7 @@ static void vmw_cursor_update_mob(struct vmw_private *vmw,
 {
 	SVGAGBCursorHeader *header;
 	SVGAGBAlphaCursorHeader *alpha_header;
-	struct vmw_bo *bo = vmw_user_object_buffer(&vps->uo);
-	u32 *image = vmw_bo_map_and_cache(bo);
+	u32 *image = vps->cursor.map.virtual;
 	const u32 image_size = vps->base.crtc_w * vps->base.crtc_h * sizeof(*image);
 
 	header = vmw_bo_map_and_cache(vps->cursor.mob);
@@ -132,7 +131,6 @@ static void vmw_cursor_update_mob(struct vmw_private *vmw,
 	memcpy(header + 1, image, image_size);
 	vmw_write(vmw, SVGA_REG_CURSOR_MOBID, vmw_bo_mobid(vps->cursor.mob));
 
-	vmw_bo_unmap(bo);
 	vmw_bo_unmap(vps->cursor.mob);
 }
 
@@ -490,9 +488,21 @@ vmw_cursor_plane_cleanup_fb(struct drm_plane *plane,
 {
 	struct vmw_cursor_plane *vcp = vmw_plane_to_vcp(plane);
 	struct vmw_plane_state *vps = vmw_plane_state_to_vps(old_state);
+	struct vmw_bo *bo;
+	int ret;
 
-	if (!vmw_user_object_is_null(&vps->uo))
-		vmw_user_object_unmap(&vps->uo);
+	if (vps->cursor.map.bo) {
+		bo = to_vmw_bo(&vps->cursor.map.bo->base);
+		ret = ttm_bo_reserve(&bo->tbo, false, false, NULL);
+		if (WARN_ON_ONCE(ret))
+			return;
+
+		ttm_bo_kunmap(&vps->cursor.map);
+		vps->cursor.map.bo = NULL;
+		ttm_bo_unpin(&bo->tbo);
+		ttm_bo_unreserve(&bo->tbo);
+		vmw_bo_unreference(&bo);
+	}
 
 	vmw_cursor_mob_unmap(vps);
 	vmw_cursor_mob_put(vcp, vps);
@@ -625,10 +635,12 @@ int vmw_cursor_plane_prepare_fb(struct drm_plane *plane,
 	struct vmw_surface *surface;
 	int ret = 0;
 
-	if (!vmw_user_object_is_null(&vps->uo)) {
-		vmw_user_object_unmap(&vps->uo);
+	/*
+	 * A duplicated state only owns the user object reference. Its mappings
+	 * and pins belong to the old state until cleanup_fb().
+	 */
+	if (!vmw_user_object_is_null(&vps->uo))
 		vmw_user_object_unref(&vps->uo);
-	}
 
 	if (fb) {
 		if (vmw_framebuffer_to_vfb(fb)->bo) {
@@ -652,31 +664,34 @@ int vmw_cursor_plane_prepare_fb(struct drm_plane *plane,
 		bo = vmw_user_object_buffer(&vps->uo);
 		if (bo) {
 			struct ttm_operation_ctx ctx = { false, false };
+			size_t size = (size_t)new_state->crtc_w *
+				      new_state->crtc_h * sizeof(u32);
 
 			ret = ttm_bo_reserve(&bo->tbo, true, false, NULL);
 			if (ret != 0)
-				return -ENOMEM;
+				return ret;
 
 			ret = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
 			if (ret != 0)
-				return -ENOMEM;
+				goto out_unreserve;
 
 			/*
-			 * vmw_bo_pin_reserved also validates, so to skip
-			 * the extra validation use ttm_bo_pin directly
+			 * vmw_bo_pin_reserved() also validates, so to skip
+			 * the extra validation use ttm_bo_pin directly.
+			 * Every state takes its own pin, the buffer can
+			 * still be in use by an older state whose commit
+			 * hasn't been cleaned up yet.
 			 */
-			if (!bo->tbo.pin_count)
-				ttm_bo_pin(&bo->tbo);
+			ttm_bo_pin(&bo->tbo);
 
-			if (vmw_framebuffer_to_vfb(fb)->bo) {
-				const u32 size = new_state->crtc_w *
-						 new_state->crtc_h *
-						 sizeof(u32);
-
-				(void)vmw_bo_map_and_cache_size(bo, size);
-			} else {
-				vmw_bo_map_and_cache(bo);
+			ret = ttm_bo_kmap(&bo->tbo, 0, PFN_UP(size),
+					  &vps->cursor.map);
+			if (ret) {
+				vps->cursor.map.bo = NULL;
+				ttm_bo_unpin(&bo->tbo);
+				goto out_unreserve;
 			}
+			vmw_bo_reference(bo);
 			ttm_bo_unreserve(&bo->tbo);
 		}
 		if (!vmw_user_object_is_null(&vps->uo)) {
@@ -703,6 +718,10 @@ int vmw_cursor_plane_prepare_fb(struct drm_plane *plane,
 	}
 
 	return 0;
+
+out_unreserve:
+	ttm_bo_unreserve(&bo->tbo);
+	return ret;
 }
 
 /**
@@ -796,7 +815,6 @@ void
 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 =
@@ -823,9 +841,8 @@ vmw_cursor_plane_atomic_update(struct drm_plane *plane,
 		vmw_cursor_update_mob(dev_priv, vps);
 		break;
 	case VMW_CURSOR_UPDATE_GB_ONLY:
-		bo = vmw_user_object_buffer(&vps->uo);
-		if (bo)
-			vmw_send_define_cursor_cmd(dev_priv, bo->map.virtual,
+		if (vps->cursor.map.virtual)
+			vmw_send_define_cursor_cmd(dev_priv, vps->cursor.map.virtual,
 						   vps->base.crtc_w,
 						   vps->base.crtc_h,
 						   vps->base.hotspot_x,
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.h b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.h
index d32cf17d013b..9b50f80f9635 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.h
@@ -14,6 +14,8 @@
 #include "drm/drm_fourcc.h"
 #include "drm/drm_plane.h"
 
+#include <drm/ttm/ttm_bo.h>
+
 #include <linux/types.h>
 
 struct SVGA3dCmdHeader;
@@ -41,6 +43,8 @@ struct vmw_cursor_plane_state {
 	enum vmw_cursor_update_type update_type;
 	bool changed;
 	bool surface_changed;
+	/* The mapped buffer is referenced and pinned by this state. */
+	struct ttm_bo_kmap_obj map;
 	struct vmw_bo *mob;
 	struct {
 		s32 hotspot_x;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 31d3ee0825ca..85228eb9c3e6 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -283,6 +283,7 @@ vmw_du_plane_duplicate_state(struct drm_plane *plane)
 	vps->cpp = 0;
 
 	vps->cursor.mob = NULL;
+	memset(&vps->cursor.map, 0, sizeof(vps->cursor.map));
 
 	/* Each ref counted resource needs to be acquired again */
 	vmw_user_object_ref(&vps->uo);
-- 
2.53.0

[PATCH 2/6] drm/vmwgfx: validate GB-only cursor command limits

[Zack Rusin]

Guest-backed devices without cursor MOB support define cursor images with
SVGA_CMD_DEFINE_ALPHA_CURSOR. SVGA_REG_CURSOR_MAX_DIMENSION belongs to the
cursor MOB register class and reads as zero on devices that do not expose
that class. Applying this register to GB-only cursors rejects every
nonempty image submitted by KMS clients, leaving Xorg without a hardware
cursor.

The submission transport also limits the command size. A 96x96 image fits
the 40 KiB FIFO image limit, but exceeds the 32 KiB command-buffer limit
and causes a device command error when submitted through that transport.

Validate GB-only images against SVGA_MAX_CURSOR_CMD_DIMENSION and
SVGA_MAX_CURSOR_CMD_BYTES. For command-buffer submission, also bound the
whole command by SVGA_CB_MAX_COMMAND_SIZE, including its header. Calculate
the image byte count in 64 bits and keep the register-based dimension and
MOB size checks for MOB cursors.

Fixes: d5ed8749168a ("drm/vmwgfx: enforce cursor size limits for MOB cursors")
Cc: stable@vger.kernel.org
Reported-by: Yunfeng Li <yunfeng.li.nb@gmail.com>
Closes: https://patch.msgid.link/20260830151619.2452307-1-yunfeng.li.nb@gmail.com
Assisted-by: LLM
Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 23 ++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
index 942d07079ef7..14490ead99cd 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
@@ -778,8 +778,27 @@ int vmw_cursor_plane_atomic_check(struct drm_plane *plane,
 				 "surface not suitable for cursor\n");
 			return -EINVAL;
 		}
-	} else if (update_type == VMW_CURSOR_UPDATE_GB_ONLY ||
-		   update_type == VMW_CURSOR_UPDATE_MOB) {
+	} else if (update_type == VMW_CURSOR_UPDATE_GB_ONLY) {
+		u64 image_size = (u64)new_state->crtc_w *
+				 new_state->crtc_h * sizeof(u32);
+		u32 max_size = SVGA_MAX_CURSOR_CMD_BYTES;
+
+		/* The command-buffer limit includes the define-cursor header. */
+		if (vmw->cman)
+			max_size = min_t(u32, max_size,
+					 SVGA_CB_MAX_COMMAND_SIZE -
+					 sizeof(struct vmw_svga_fifo_cmd_define_cursor));
+
+		if (!new_state->crtc_w || !new_state->crtc_h ||
+		    new_state->crtc_w > SVGA_MAX_CURSOR_CMD_DIMENSION ||
+		    new_state->crtc_h > SVGA_MAX_CURSOR_CMD_DIMENSION ||
+		    image_size > max_size) {
+			drm_dbg_kms(&vmw->drm,
+				    "Cursor dimensions (%u, %u) violate command limits\n",
+				    new_state->crtc_w, new_state->crtc_h);
+			return -EINVAL;
+		}
+	} else if (update_type == VMW_CURSOR_UPDATE_MOB) {
 		u32 cursor_max_dim =
 			vmw_read(vmw, SVGA_REG_CURSOR_MAX_DIMENSION);
 
-- 
2.53.0

[PATCH 3/6] drm/vmwgfx: include legacy hotspots in GB-only cursor images

[Zack Rusin]

From: Tomas M <tomas@slax.org>

The guest-backed cursor path without cursor MOB support defines the image
using only the hotspot from the plane state. Its position calculation
also includes the legacy hotspot supplied through DRM_VMW_CURSOR_BYPASS.

Xorg's xf86-video-vmware driver supplies its hotspot through that ioctl
and loads the image with drmModeSetCursor(), which does not set a plane
hotspot. The mismatched image and position hotspots displace the visible
cursor by the legacy offset.

Use the combined hotspot for both the GB-only image command and cursor
position, matching the legacy and MOB image paths.

Fixes: eef295a85082 ("drm/vmwgfx: Restore Guest-Backed only cursor plane support")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Tomas M <tomas@slax.org>
Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
index 14490ead99cd..f0661e5effeb 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
@@ -852,6 +852,10 @@ vmw_cursor_plane_atomic_update(struct drm_plane *plane,
 		return;
 	}
 
+	/* Include legacy Xorg cursor-bypass offsets in the image and position. */
+	hotspot_x = vps->cursor.legacy.hotspot_x + new_state->hotspot_x;
+	hotspot_y = vps->cursor.legacy.hotspot_y + new_state->hotspot_y;
+
 	switch (vps->cursor.update_type) {
 	case VMW_CURSOR_UPDATE_LEGACY:
 		vmw_cursor_plane_update_legacy(dev_priv, vps);
@@ -864,8 +868,7 @@ vmw_cursor_plane_atomic_update(struct drm_plane *plane,
 			vmw_send_define_cursor_cmd(dev_priv, vps->cursor.map.virtual,
 						   vps->base.crtc_w,
 						   vps->base.crtc_h,
-						   vps->base.hotspot_x,
-						   vps->base.hotspot_y);
+						   hotspot_x, hotspot_y);
 		break;
 	case VMW_CURSOR_UPDATE_NONE:
 		/* do nothing */
@@ -878,9 +881,6 @@ vmw_cursor_plane_atomic_update(struct drm_plane *plane,
 	cursor_x = new_state->crtc_x + du->set_gui_x;
 	cursor_y = new_state->crtc_y + du->set_gui_y;
 
-	hotspot_x = vps->cursor.legacy.hotspot_x + new_state->hotspot_x;
-	hotspot_y = vps->cursor.legacy.hotspot_y + new_state->hotspot_y;
-
 	vmw_cursor_update_position(dev_priv, true, cursor_x + hotspot_x,
 				   cursor_y + hotspot_y);
 }
-- 
2.53.0

[PATCH 4/6] drm/vmwgfx: reserve dumb buffers while detaching their surfaces

[Zack Rusin]

vmw_bo_free() detaches the surface associated with a dumb buffer and
clears its guest memory binding. It reserves the resource, but does not
hold the buffer reservation required by vmw_resource_mob_detach(). TTM
releases that reservation before invoking the buffer destructor.

KMS clients destroying dumb buffers can therefore trigger the reservation
assertion from vmw_resource_mob_detach(). GNOME reaches this path through
DRM_IOCTL_MODE_DESTROY_DUMB during cursor and display updates.

Reacquire the buffer reservation while detaching the surface and clearing
its dirty tracking and guest memory binding. Acquire it after
cmdbuf_mutex, following the command validation lock order, and release it
before releasing the resource and GEM object.

Fixes: d6667f0ddf46 ("drm/vmwgfx: Fix handling of dumb buffers")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
---
 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 9c7a73c0b0dc..a6d7156e4281 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -55,6 +55,8 @@ static void vmw_bo_free(struct ttm_buffer_object *bo)
 			/* Reserve and switch the backing mob. */
 			mutex_lock(&res->dev_priv->cmdbuf_mutex);
 			(void)vmw_resource_reserve(res, false, true);
+			/* TTM drops the reservation before calling the destructor. */
+			dma_resv_lock(bo->base.resv, NULL);
 			vmw_resource_mob_detach(res);
 			if (res->dirty)
 				res->func->dirty_free(res);
@@ -62,6 +64,7 @@ static void vmw_bo_free(struct ttm_buffer_object *bo)
 				vmw_bo_dirty_release(res->guest_memory_bo);
 			res->guest_memory_bo = NULL;
 			res->guest_memory_offset = 0;
+			dma_resv_unlock(bo->base.resv);
 			vmw_resource_unreserve(res, true, false, false, NULL,
 					       0);
 			mutex_unlock(&res->dev_priv->cmdbuf_mutex);
-- 
2.53.0

[PATCH 5/6] drm/vmwgfx: validate the framebuffer selected for cursor updates

[Zack Rusin]

Cursor atomic checks select an update path and look up surfaces through
vps->uo. That object is copied from the previous plane state and is only
replaced with the new framebuffer's object in prepare_fb(). Switching
between snooped surfaces and buffer objects therefore applies the old
framebuffer's cursor limits to the new image.

KMS clients can also submit a guest-backed surface without a backing BO.
The check accepts it, but the MOB update later copies from a NULL source.
Zero-sized MOB images pass the check as well, then fail preparation when
mapping zero pages, so TEST_ONLY accepts a commit that cannot succeed.

Resolve the user object directly from the selected framebuffer in both
validation and preparation. Apply the matching cursor limits, reject
zero dimensions and require a backing BO for GB-only and MOB images.
Recheck the backing BO in preparation before mapping it, while keeping
snooped surfaces independent of a BO mapping.

Fixes: 965544150d1c ("drm/vmwgfx: Refactor cursor handling")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 61 ++++++++++++++------
 1 file changed, 43 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
index f0661e5effeb..9e8ea9e07f40 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
@@ -89,10 +89,26 @@ vmw_cursor_plane_update_legacy(struct vmw_private *vmw,
 	}
 }
 
-static enum vmw_cursor_update_type
-vmw_cursor_update_type(struct vmw_private *vmw, struct vmw_plane_state *vps)
+static struct vmw_user_object
+vmw_cursor_plane_user_object(struct drm_framebuffer *fb)
 {
-	struct vmw_surface *surface = vmw_user_object_surface(&vps->uo);
+	struct vmw_user_object uo = { 0 };
+
+	if (!fb)
+		return uo;
+
+	if (vmw_framebuffer_to_vfb(fb)->bo)
+		uo.buffer = vmw_framebuffer_to_vfbd(fb)->buffer;
+	else
+		uo = vmw_framebuffer_to_vfbs(fb)->uo;
+
+	return uo;
+}
+
+static enum vmw_cursor_update_type
+vmw_cursor_update_type(struct vmw_private *vmw, struct vmw_user_object *uo)
+{
+	struct vmw_surface *surface = vmw_user_object_surface(uo);
 
 	if (surface && surface->snooper.image)
 		return VMW_CURSOR_UPDATE_LEGACY;
@@ -642,17 +658,10 @@ int vmw_cursor_plane_prepare_fb(struct drm_plane *plane,
 	if (!vmw_user_object_is_null(&vps->uo))
 		vmw_user_object_unref(&vps->uo);
 
-	if (fb) {
-		if (vmw_framebuffer_to_vfb(fb)->bo) {
-			vps->uo.buffer = vmw_framebuffer_to_vfbd(fb)->buffer;
-			vps->uo.surface = NULL;
-		} else {
-			memcpy(&vps->uo, &vmw_framebuffer_to_vfbs(fb)->uo, sizeof(vps->uo));
-		}
-		vmw_user_object_ref(&vps->uo);
-	}
+	vps->uo = vmw_cursor_plane_user_object(fb);
+	vmw_user_object_ref(&vps->uo);
 
-	vps->cursor.update_type = vmw_cursor_update_type(vmw, vps);
+	vps->cursor.update_type = vmw_cursor_update_type(vmw, &vps->uo);
 	switch (vps->cursor.update_type) {
 	case VMW_CURSOR_UPDATE_LEGACY:
 		surface = vmw_user_object_surface(&vps->uo);
@@ -662,6 +671,9 @@ int vmw_cursor_plane_prepare_fb(struct drm_plane *plane,
 	case VMW_CURSOR_UPDATE_GB_ONLY:
 	case VMW_CURSOR_UPDATE_MOB: {
 		bo = vmw_user_object_buffer(&vps->uo);
+		if (fb && !bo)
+			return -EINVAL;
+
 		if (bo) {
 			struct ttm_operation_ctx ctx = { false, false };
 			size_t size = (size_t)new_state->crtc_w *
@@ -744,7 +756,7 @@ int vmw_cursor_plane_atomic_check(struct drm_plane *plane,
 	int ret = 0;
 	struct drm_crtc_state *crtc_state = NULL;
 	struct vmw_surface *surface = NULL;
-	struct vmw_plane_state *vps = vmw_plane_state_to_vps(new_state);
+	struct vmw_user_object uo;
 	enum vmw_cursor_update_type update_type;
 	struct drm_framebuffer *fb = new_state->fb;
 
@@ -763,7 +775,21 @@ int vmw_cursor_plane_atomic_check(struct drm_plane *plane,
 	if (!fb)
 		return 0;
 
-	update_type = vmw_cursor_update_type(vmw, vps);
+	if (!new_state->crtc_w || !new_state->crtc_h) {
+		drm_dbg_kms(&vmw->drm,
+			    "Cursor dimensions (%u, %u) must be nonzero\n",
+			    new_state->crtc_w, new_state->crtc_h);
+		return -EINVAL;
+	}
+
+	/* The duplicated plane state's user object still belongs to the old FB. */
+	uo = vmw_cursor_plane_user_object(fb);
+	update_type = vmw_cursor_update_type(vmw, &uo);
+	if ((update_type == VMW_CURSOR_UPDATE_GB_ONLY ||
+	     update_type == VMW_CURSOR_UPDATE_MOB) &&
+	    !vmw_user_object_buffer(&uo))
+		return -EINVAL;
+
 	if (update_type == VMW_CURSOR_UPDATE_LEGACY) {
 		if (new_state->crtc_w != VMW_CURSOR_SNOOP_WIDTH ||
 		    new_state->crtc_h != VMW_CURSOR_SNOOP_HEIGHT) {
@@ -772,7 +798,7 @@ int vmw_cursor_plane_atomic_check(struct drm_plane *plane,
 				 new_state->crtc_w, new_state->crtc_h);
 			return -EINVAL;
 		}
-		surface = vmw_user_object_surface(&vps->uo);
+		surface = vmw_user_object_surface(&uo);
 		if (!surface || !surface->snooper.image) {
 			drm_warn(&vmw->drm,
 				 "surface not suitable for cursor\n");
@@ -789,8 +815,7 @@ int vmw_cursor_plane_atomic_check(struct drm_plane *plane,
 					 SVGA_CB_MAX_COMMAND_SIZE -
 					 sizeof(struct vmw_svga_fifo_cmd_define_cursor));
 
-		if (!new_state->crtc_w || !new_state->crtc_h ||
-		    new_state->crtc_w > SVGA_MAX_CURSOR_CMD_DIMENSION ||
+		if (new_state->crtc_w > SVGA_MAX_CURSOR_CMD_DIMENSION ||
 		    new_state->crtc_h > SVGA_MAX_CURSOR_CMD_DIMENSION ||
 		    image_size > max_size) {
 			drm_dbg_kms(&vmw->drm,
-- 
2.53.0

Re: [PATCH 5/6] drm/vmwgfx: validate the framebuffer selected for cursor updates

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/vmwgfx: cursor atomic_check accepts VMW_CURSOR_UPDATE_NONE framebuffers
--

commit 9cca2550f2f0603a47b5047da765ad6aeb14d8cc
Author: Zack Rusin <zack.rusin@broadcom.com>

drm/vmwgfx: validate the framebuffer selected for cursor updates

This commit resolves the user object directly from the selected framebuffer
during cursor plane validation and preparation rather than relying on the
duplicated plane state's previous user object. It also adds checks to reject
zero cursor dimensions and require a backing buffer object for GB-only and
MOB cursor updates.

> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
> index f0661e5effeb1..9e8ea9e07f40f 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c

[ ... ]

> @@ -763,7 +775,21 @@ int vmw_cursor_plane_atomic_check(struct drm_plane *plane,
>  	if (!fb)
>  		return 0;
>  
> -	update_type = vmw_cursor_update_type(vmw, vps);
> +	if (!new_state->crtc_w || !new_state->crtc_h) {
> +		drm_dbg_kms(&vmw->drm,
> +			    "Cursor dimensions (%u, %u) must be nonzero\n",
> +			    new_state->crtc_w, new_state->crtc_h);
> +		return -EINVAL;
> +	}
> +
> +	/* The duplicated plane state's user object still belongs to the old FB. */
> +	uo = vmw_cursor_plane_user_object(fb);
> +	update_type = vmw_cursor_update_type(vmw, &uo);
> +	if ((update_type == VMW_CURSOR_UPDATE_GB_ONLY ||
> +	     update_type == VMW_CURSOR_UPDATE_MOB) &&
> +	    !vmw_user_object_buffer(&uo))
> +		return -EINVAL;
> +
>  	if (update_type == VMW_CURSOR_UPDATE_LEGACY) {
>  		if (new_state->crtc_w != VMW_CURSOR_SNOOP_WIDTH ||
>  		    new_state->crtc_h != VMW_CURSOR_SNOOP_HEIGHT) {
> @@ -772,7 +798,7 @@ int vmw_cursor_plane_atomic_check(struct drm_plane *plane,
>  				 new_state->crtc_w, new_state->crtc_h);
>  			return -EINVAL;
>  		}
> -		surface = vmw_user_object_surface(&vps->uo);
> +		surface = vmw_user_object_surface(&uo);
>  		if (!surface || !surface->snooper.image) {
>  			drm_warn(&vmw->drm,
>  				 "surface not suitable for cursor\n");

[Severity: Medium]
Can this check for !surface || !surface->snooper.image ever be reached, and
what happens when update_type is VMW_CURSOR_UPDATE_NONE?

Looking at vmw_cursor_update_type(), it only returns VMW_CURSOR_UPDATE_LEGACY
when surface && surface->snooper.image is already true. If vmw->has_mob is
false and the new framebuffer does not have a snooper image (for example, a
BO-backed dumb framebuffer or an atomic client surface where
surface->snooper.image is NULL), vmw_cursor_update_type() returns
VMW_CURSOR_UPDATE_NONE:

drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c:vmw_cursor_update_type() {
	struct vmw_surface *surface = vmw_user_object_surface(uo);

	if (surface && surface->snooper.image)
		return VMW_CURSOR_UPDATE_LEGACY;

	if (vmw->has_mob) {
		if ((vmw->capabilities2 & SVGA_CAP2_CURSOR_MOB) != 0)
			return VMW_CURSOR_UPDATE_MOB;
		else
			return VMW_CURSOR_UPDATE_GB_ONLY;
	}
	drm_warn_once(&vmw->drm, "Unknown Cursor Type!\n");
	return VMW_CURSOR_UPDATE_NONE;
}

When vmw_cursor_update_type() returns VMW_CURSOR_UPDATE_NONE,
vmw_cursor_plane_atomic_check() skips the VMW_CURSOR_UPDATE_LEGACY,
VMW_CURSOR_UPDATE_GB_ONLY, and VMW_CURSOR_UPDATE_MOB branches and returns 0
at the end of the function.

Should vmw_cursor_plane_atomic_check() reject VMW_CURSOR_UPDATE_NONE with
-EINVAL so that non-snooped framebuffers on non-MOB devices are not accepted?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027211729.646988-1-zack.rusin@broadcom.com?part=5

[PATCH 6/6] drm/vmwgfx: reserve cursor MOBs before unpinning them

[Zack Rusin]

vmw_cursor_mob_destroy() unpins cursor MOBs when discarding an image from
the plane's cache or destroying the cursor plane. It calls ttm_bo_unpin()
without reserving the BO, although TTM requires its reservation to be held
when changing pin counts and LRU membership.

KMS clients issuing repeated nonblocking cursor updates can exhaust the
cache and reach this path. On a lockdep-enabled kernel this triggers
reservation assertions in ttm_bo_unpin() and ttm_resource_unevictable().

Reserve the MOB around ttm_bo_unpin(), then release the reservation before
dropping the BO reference.

Fixes: 485d98d472d5 ("drm/vmwgfx: Add support for CursorMob and CursorBypass 4")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
index 9e8ea9e07f40..b678e5b68db7 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
@@ -166,10 +166,17 @@ static u32 vmw_cursor_mob_size(enum vmw_cursor_update_type update_type,
 
 static void vmw_cursor_mob_destroy(struct vmw_bo **vbo)
 {
+	int ret;
+
 	if (!(*vbo))
 		return;
 
+	ret = ttm_bo_reserve(&(*vbo)->tbo, false, false, NULL);
+	if (WARN_ON_ONCE(ret))
+		return;
+
 	ttm_bo_unpin(&(*vbo)->tbo);
+	ttm_bo_unreserve(&(*vbo)->tbo);
 	vmw_bo_unreference(vbo);
 }
 
-- 
2.53.0