[PATCH 4/5] hw/xen: Fix double-free in xen_console store_con_info()

David Woodhouse posted 5 patches 2 years, 10 months ago
There is a newer version of this series
[PATCH 4/5] hw/xen: Fix double-free in xen_console store_con_info()
Posted by David Woodhouse 2 years, 10 months ago
From: David Woodhouse <dwmw@amazon.co.uk>

Coverity spotted a double-free (CID 1508254); we g_string_free(path) and
then for some reason immediately call free(path) too.

We should just use g_autoptr() for it anyway, which simplifies the code
a bit.

Fixes: 7a8a749da7d3 ("hw/xen: Move xenstore_store_pv_console_info to xen_console.c")
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 hw/char/xen_console.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index c7a19c0e7c..810dae3f44 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -178,8 +178,7 @@ 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;
+    g_autoptr(GString) path = NULL;
 
     /* Only continue if we're talking to a pty. */
     if (!CHARDEV_IS_PTY(cs)) {
@@ -204,15 +203,9 @@ static int store_con_info(struct XenConsole *con)
 
     if (xenstore_write_str(con->console, path->str, pts)) {
         fprintf(stderr, "xenstore_write_str for '%s' fail", path->str);
-        goto out;
+        return -1;
     }
-    ret = 0;
-
-out:
-    g_string_free(path, true);
-    free(path);
-
-    return ret;
+    return 0;
 }
 
 static int con_init(struct XenLegacyDevice *xendev)
-- 
2.39.2
Re: [PATCH 4/5] hw/xen: Fix double-free in xen_console store_con_info()
Posted by Paul Durrant 2 years, 9 months ago
On 12/04/2023 19:51, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
> 
> Coverity spotted a double-free (CID 1508254); we g_string_free(path) and
> then for some reason immediately call free(path) too.
> 
> We should just use g_autoptr() for it anyway, which simplifies the code
> a bit.
> 
> Fixes: 7a8a749da7d3 ("hw/xen: Move xenstore_store_pv_console_info to xen_console.c")
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>   hw/char/xen_console.c | 13 +++----------
>   1 file changed, 3 insertions(+), 10 deletions(-)
> 

Reviewed-by: Paul Durrant <paul@xen.org>
Re: [PATCH 4/5] hw/xen: Fix double-free in xen_console store_con_info()
Posted by Peter Maydell 2 years, 10 months ago
On Wed, 12 Apr 2023 at 19:52, David Woodhouse <dwmw2@infradead.org> wrote:
>
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> Coverity spotted a double-free (CID 1508254); we g_string_free(path) and
> then for some reason immediately call free(path) too.
>
> We should just use g_autoptr() for it anyway, which simplifies the code
> a bit.
>
> Fixes: 7a8a749da7d3 ("hw/xen: Move xenstore_store_pv_console_info to xen_console.c")
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM