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: [email protected]
Assisted-by: LLM
Signed-off-by: Zack Rusin <[email protected]>
---
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
