[PATCH] ui/vnc: Fix crash when specifying [vnc] without id in the config file

Thomas Huth posted 1 patch 2 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250821145130.845104-1-thuth@redhat.com
Maintainers: "Marc-André Lureau" <marcandre.lureau@redhat.com>
ui/vnc.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
[PATCH] ui/vnc: Fix crash when specifying [vnc] without id in the config file
Posted by Thomas Huth 2 months, 3 weeks ago
From: Thomas Huth <thuth@redhat.com>

QEMU currently crashes when there is a [vnc] section in the config
file that does not have an "id = ..." line:

 $ echo "[vnc]" > /tmp/qemu.conf
 $ ./qemu-system-x86_64 -readconfig /tmp/qemu.conf
 qemu-system-x86_64: ../../devel/qemu/ui/vnc.c:4347: vnc_init_func:
  Assertion `id' failed.
 Aborted (core dumped)

The required "id" is only set up automatically while parsing the command
line, but not when reading the options from the config file.
Thus let's move code that automatically adds the id (if it does not
exist yet) to the init function that needs the id for the first time,
replacing the assert() statement there.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2836
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 ui/vnc.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 68ca4a68e7a..9054fc81253 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -4309,8 +4309,9 @@ void vnc_display_add_client(const char *id, int csock, bool skipauth)
     }
 }
 
-static void vnc_auto_assign_id(QemuOptsList *olist, QemuOpts *opts)
+static char *vnc_auto_assign_id(QemuOpts *opts)
 {
+    QemuOptsList *olist = qemu_find_opts("vnc");
     int i = 2;
     char *id;
 
@@ -4320,23 +4321,18 @@ static void vnc_auto_assign_id(QemuOptsList *olist, QemuOpts *opts)
         id = g_strdup_printf("vnc%d", i++);
     }
     qemu_opts_set_id(opts, id);
+
+    return id;
 }
 
 void vnc_parse(const char *str)
 {
     QemuOptsList *olist = qemu_find_opts("vnc");
     QemuOpts *opts = qemu_opts_parse_noisily(olist, str, !is_help_option(str));
-    const char *id;
 
     if (!opts) {
         exit(1);
     }
-
-    id = qemu_opts_id(opts);
-    if (!id) {
-        /* auto-assign id if not present */
-        vnc_auto_assign_id(olist, opts);
-    }
 }
 
 int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp)
@@ -4344,7 +4340,11 @@ int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp)
     Error *local_err = NULL;
     char *id = (char *)qemu_opts_id(opts);
 
-    assert(id);
+    if (!id) {
+        /* auto-assign id if not present */
+        id = vnc_auto_assign_id(opts);
+    }
+
     vnc_display_init(id, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
-- 
2.50.1
Re: [PATCH] ui/vnc: Fix crash when specifying [vnc] without id in the config file
Posted by Marc-André Lureau 2 months, 3 weeks ago
Hi

On Thu, Aug 21, 2025 at 6:51 PM Thomas Huth <thuth@redhat.com> wrote:

> From: Thomas Huth <thuth@redhat.com>
>
> QEMU currently crashes when there is a [vnc] section in the config
> file that does not have an "id = ..." line:
>
>  $ echo "[vnc]" > /tmp/qemu.conf
>  $ ./qemu-system-x86_64 -readconfig /tmp/qemu.conf
>  qemu-system-x86_64: ../../devel/qemu/ui/vnc.c:4347: vnc_init_func:
>   Assertion `id' failed.
>  Aborted (core dumped)
>
> The required "id" is only set up automatically while parsing the command
> line, but not when reading the options from the config file.
> Thus let's move code that automatically adds the id (if it does not
> exist yet) to the init function that needs the id for the first time,
> replacing the assert() statement there.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2836
> Signed-off-by: Thomas Huth <thuth@redhat.com>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  ui/vnc.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 68ca4a68e7a..9054fc81253 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -4309,8 +4309,9 @@ void vnc_display_add_client(const char *id, int
> csock, bool skipauth)
>      }
>  }
>
> -static void vnc_auto_assign_id(QemuOptsList *olist, QemuOpts *opts)
> +static char *vnc_auto_assign_id(QemuOpts *opts)
>  {
> +    QemuOptsList *olist = qemu_find_opts("vnc");
>      int i = 2;
>      char *id;
>
> @@ -4320,23 +4321,18 @@ static void vnc_auto_assign_id(QemuOptsList
> *olist, QemuOpts *opts)
>          id = g_strdup_printf("vnc%d", i++);
>      }
>      qemu_opts_set_id(opts, id);
> +
> +    return id;
>  }
>
>  void vnc_parse(const char *str)
>  {
>      QemuOptsList *olist = qemu_find_opts("vnc");
>      QemuOpts *opts = qemu_opts_parse_noisily(olist, str,
> !is_help_option(str));
> -    const char *id;
>
>      if (!opts) {
>          exit(1);
>      }
> -
> -    id = qemu_opts_id(opts);
> -    if (!id) {
> -        /* auto-assign id if not present */
> -        vnc_auto_assign_id(olist, opts);
> -    }
>  }
>
>  int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp)
> @@ -4344,7 +4340,11 @@ int vnc_init_func(void *opaque, QemuOpts *opts,
> Error **errp)
>      Error *local_err = NULL;
>      char *id = (char *)qemu_opts_id(opts);
>
> -    assert(id);
> +    if (!id) {
> +        /* auto-assign id if not present */
> +        id = vnc_auto_assign_id(opts);
> +    }
> +
>      vnc_display_init(id, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
> --
> 2.50.1
>
>