From: Denis V. Lunev <[email protected]>
qxl_spice_reset_cursor() unrefs qxl->ssd.cursor and installs the hidden
cursor without holding qxl->ssd.lock. Every other writer of that field
takes it: qxl_render_cursor(), display_mouse_define() and
qemu_spice_cursor_refresh_bh().
The unlocked path runs on a vCPU thread, reached from ioport_write() on
QXL_IO_DESTROY_PRIMARY and QXL_IO_DESTROY_PRIMARY_ASYNC, and holds only
the BQL, which the SPICE display worker never takes. Unlike
qxl_hard_reset(), it leaves that worker running.
spice_qxl_reset_cursor() does round trip through the dispatcher, but the
worker is free again as soon as it returns, so it can enter
qxl_render_cursor() and unref the same QEMUCursor a few instructions
later. Both threads then drop one reference for what is a single
reference, freeing a cursor that another user still holds. The store to
ssd.cursor races the same way, and a guest that keeps this up also ends
up waiting forever in qxl_fence_wait().
A guest reaches this by switching QXL mode while it also updates the
pointer shape.
Fixes: 958c2bceba06 ("qxl: fix cursor reset")
Cc: [email protected]
Cc: Marc-André Lureau <[email protected]>
Signed-off-by: Denis V. Lunev <[email protected]>
---
hw/display/qxl.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 384b8767b8..c4f547e88b 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -294,10 +294,12 @@ void qxl_spice_reset_cursor(PCIQXLDevice *qxl)
qemu_mutex_lock(&qxl->track_lock);
qxl->guest_cursor = 0;
qemu_mutex_unlock(&qxl->track_lock);
+ qemu_mutex_lock(&qxl->ssd.lock);
if (qxl->ssd.cursor) {
cursor_unref(qxl->ssd.cursor);
}
qxl->ssd.cursor = cursor_builtin_hidden();
+ qemu_mutex_unlock(&qxl->ssd.lock);
}
static uint32_t qxl_crc32(const uint8_t *p, unsigned len)
--
2.53.0
