Re: [QEMU HCI-8.0 PATCH 0/7] qxl cursor use-after-free plus stability backports #VSTOR-144000

[Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>]

Ack
Applied to hci-8.0

On 9/3/26 11:25 PM, Denis V. Lunev wrote:
> 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.
> 
> Patches 1 and 2 were posted upstream and carry Marc-Andre's
> Reviewed-by, but are not merged yet, so they have no cherry-pick line:
> 
>   https://lore.kernel.org/qemu-devel/20260903192647.2677279-1-den@openvz.org/
> 
> Patches 3 to 7 are definitive stability fixes, cherry-picked from
> mainstream. All five are already reviewed and merged upstream, and none
> of them was present on our branch. They are unrelated to the crash
> above but sit in the same device, so they are worth taking in one go:
> 
>   3/7  mono cursor validation reading past a cursor chunk
>   4/7  TOCTOU in cursor chunk data_size handling
>   5/7  monitors_config heads[] validation in phys2virt
>   6/7  vm_change_state handler and BHs left registered on unrealize
>   7/7  primary surface stride not validated against width
> 
> Two of those carry CVE references in their upstream messages, 7/7
> CVE-2026-16271 and 6/7 CVE-2026-63322.
> 
> None of these seven touches qxl_post_load(), so none of them addresses
> the migration failure tracked separately in VSTOR-113533.
> 
> Denis V. Lunev (2):
>   hw/display/qxl: hold ssd.lock while replacing ssd.cursor #VSTOR-144000
>   ui/cursor: make the cursor refcount atomic #VSTOR-144000
> 
> Haotian Jiang (1):
>   hw/display/qxl: unregister vm_change_state handler and BHs
>     #VSTOR-144000
> 
> Marc-André Lureau (3):
>   hw/display/qxl: fix TOCTOU in cursor chunk data_size handling
>     #VSTOR-144000
>   hw/display/qxl: validate monitors_config heads[] in phys2virt
>     #VSTOR-144000
>   hw/display/qxl: validate primary surface stride against width
>     #VSTOR-144000
> 
> Thomas Huth (1):
>   hw/display/qxl: Fix mono cursor validation that can read past a cursor
>     chunk #VSTOR-144000
> 
>  hw/display/qxl-render.c | 98 +++++++++++++++++++++++++----------------
>  hw/display/qxl.c        | 79 ++++++++++++++++++++++++++++++++-
>  hw/display/qxl.h        |  3 ++
>  include/ui/console.h    |  9 ++++
>  ui/cursor.c             | 17 ++++---
>  5 files changed, 160 insertions(+), 46 deletions(-)
>