On 9/3/26 21:26, Denis V. Lunev wrote: > This email originated from an IP that might not be authorized by the domain > it was sent from. > Do not click links or open attachments unless it is an email you expected to > receive. > 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(-) > Hi, Marc-Andre!
You have not posted the notification that patches have been queued. Are you going to accept them? Unfortunately I see your review without any further actions and patches were not landed yet. Sorry for bothering, Den
