Re: [PATCH] ui/console: fix use-after-free in qemu_console_set_cursor

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Aug 27, 2026 at 11:06:47AM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> When the new cursor is the same object as the one already held by
> the console and the refcount is 1, cursor_unref frees it before
> cursor_ref can increment the count.
> 
> Ref the incoming cursor before unreffing the old one so the
> refcount goes 1-2-1 instead of 1-0 (freed) then use-after-free.

I don't see where/how this can happen.  The caller of
qemu_console_set_cursor should own its own reference
on 'cursor'. So on entry to this method, 'cursor'
should have a ref of 2 if it is the same as the
current con->cursor.

if anything the behaviour you describe sounds like a
bug in a caller not holding its own reference.

> 
> This is related to 385ac97f8fad ("ui: keep current cursor with
> QemuConsole"), but could have happened earlier with VNC too.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  ui/console.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/ui/console.c b/ui/console.c
> index a8a2a247d8f4..4db8f1cb431f 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -904,8 +904,9 @@ void qemu_console_set_cursor(QemuConsole *c, QEMUCursor *cursor)
>      DisplayState *s = c->ds;
>      DisplayChangeListener *dcl;
>  
> +    cursor_ref(cursor);
>      cursor_unref(con->cursor);
> -    con->cursor = cursor_ref(cursor);
> +    con->cursor = cursor;
>      QLIST_FOREACH(dcl, &s->listeners, next) {
>          if (c != dcl->con) {
>              continue;
> -- 
> 2.55.0.543.g5ebe2ebe4ea8
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|