A guest with a qxl display can make QEMU drop more references to a QEMUCursor than were taken. The cursor is freed while another owner still points at it, and that owner's later cursor_unref() decrements four bytes of a chunk the allocator has handed out again. Nothing aborts and nothing is logged; QEMU dies later in an unrelated allocation, in another thread.
Two defects get there, and neither fix is sufficient alone: - qxl_spice_reset_cursor() replaces qxl->ssd.cursor with no lock held, while every other writer of that field takes ssd.lock. It runs on a vCPU thread from QXL_IO_DESTROY_PRIMARY and, unlike qxl_hard_reset(), leaves the SPICE display worker running. - QEMUCursor.refcount is a plain int, taken and dropped from the main loop, the SPICE worker, ui/cocoa.m and ui/dbus-listener.c, with no lock common to all of them, so an increment can be lost. A qxl device starts a spice-server instance for local rendering even with no -spice, so this is not limited to SPICE console setups. Patch 2 also asserts that the refcount was positive. Only qxl was exercised here, so if another display backend drops a reference it never took, that assert turns a silent leak into an abort. Reproducer: a libdrm program in the guest queues cursor SET commands, then disables the CRTC so the driver issues QXL_IO_DESTROY_PRIMARY. Unpatched QEMU dies within seconds; with the series it does not. Happy to post it. This is independent of the pending "ui/console: fix use-after-free in qemu_console_set_cursor", which fixes a self-assignment ordering bug at the same line. With that applied the crash here is unchanged. Cc: Marc-André Lureau <[email protected]> Signed-off-by: Denis V. Lunev <[email protected]> Denis V. Lunev (2): hw/display/qxl: hold ssd.lock while replacing ssd.cursor ui/cursor: make the cursor refcount atomic hw/display/qxl.c | 2 ++ include/ui/console.h | 9 +++++++++ ui/cursor.c | 17 +++++++++++------ 3 files changed, 22 insertions(+), 6 deletions(-) -- 2.53.0
