Re: [PULL 14/27] hw/xen: Move xenstore_store_pv_console_info to xen_console.c

[Peter Maydell]

On Tue, 7 Mar 2023 at 18:28, David Woodhouse <dwmw2@infradead.org> wrote:
>
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> There's no need for this to be in the Xen accel code, and as we want to
> use the Xen console support with KVM-emulated Xen we'll want to have a
> platform-agnostic version of it. Make it use GString to build up the
> path while we're at it.
>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> Reviewed-by: Paul Durrant <paul@xen.org>

Hi; Coverity points out a double-free here (CID 1508254):

> +static int store_con_info(struct XenConsole *con)
> +{
> +    Chardev *cs = qemu_chr_fe_get_driver(&con->chr);
> +    char *pts = NULL;
> +    char *dom_path;
> +    GString *path;
> +    int ret = -1;
> +
> +    /* Only continue if we're talking to a pty. */
> +    if (!CHARDEV_IS_PTY(cs)) {
> +        return 0;
> +    }
> +    pts = cs->filename + 4;
> +
> +    dom_path = qemu_xen_xs_get_domain_path(xenstore, xen_domid);
> +    if (!dom_path) {
> +        return 0;
> +    }
> +
> +    path = g_string_new(dom_path);
> +    free(dom_path);
> +
> +    if (con->xendev.dev) {
> +        g_string_append_printf(path, "/device/console/%d", con->xendev.dev);
> +    } else {
> +        g_string_append(path, "/console");
> +    }
> +    g_string_append(path, "/tty");
> +
> +    if (xenstore_write_str(con->console, path->str, pts)) {
> +        fprintf(stderr, "xenstore_write_str for '%s' fail", path->str);
> +        goto out;
> +    }
> +    ret = 0;
> +
> +out:
> +    g_string_free(path, true);
> +    free(path);

g_string_free frees the GString, but then we call free() on it
as well. Presumably the free() should just be deleted ?

> +
> +    return ret;
> +}

thanks
-- PMM