On Tue, 7 Mar 2023 at 18:28, David Woodhouse <dw...@infradead.org> wrote:
>
> From: David Woodhouse <d...@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 <d...@amazon.co.uk>
> Reviewed-by: Paul Durrant <p...@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
