[PATCH v2 1/9] virConnectOpenInternal: Avoid double free() when alias is an invalid URI

Ján Tomko jtomko at redhat.com
Fri Sep 9 16:21:04 UTC 2022


On a Friday in 2022, Peter Krempa wrote:
>Configuring an URI alias such as
>
>  uri_aliases = [
>      "blah=qemu://invaliduri@@@",
>  ]
>
>Results in a double free when the alias is used:
>
>  $ virsh -c blah
>  free(): double free detected in tcache 2
>  Aborted (core dumped)
>
>This happens as the 'alias' variable is first assigned to 'uristr' which
>is cleared in the 'failed' label and then is explicitly freed again.
>
>Fix this by switching to 'g_autofree' for alias and stealing it into
>'uristr'.
>
>Signed-off-by: Peter Krempa <pkrempa at redhat.com>
>---
> src/libvirt.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
>diff --git a/src/libvirt.c b/src/libvirt.c
>index b78b49a632..7e7c0fc8e3 100644
>--- a/src/libvirt.c
>+++ b/src/libvirt.c
>@@ -933,21 +933,19 @@ virConnectOpenInternal(const char *name,
>     }
>
>     if (uristr) {
>-        char *alias = NULL;
>+        g_autofree char *alias = NULL;
>

Is this necessary? In the case 'alias' will be assigned a value,
we will steal the value right away.

>         if (!(flags & VIR_CONNECT_NO_ALIASES) &&
>             virURIResolveAlias(conf, uristr, &alias) < 0)
>             goto failed;
>
>         if (alias) {
>-            VIR_FREE(uristr);
>-            uristr = alias;
>+            g_free(uristr);
>+            uristr = g_steal_pointer(&alias);
>         }
>
>-        if (!(ret->uri = virURIParse(uristr))) {
>-            VIR_FREE(alias);
>+        if (!(ret->uri = virURIParse(uristr)))
>             goto failed;
>-        }

Reviewed-by: Ján Tomko <jtomko at redhat.com>

Jano


More information about the libvir-list mailing list