qemu: don't leak in qemuGetDHCPInterfaces when failing to alloc

[libvirt] [PATCH] qemu: don't leak in qemuGetDHCPInterfaces when failing to alloc

Posted by Chen Hanxiao 6 years, 2 months ago

From: Chen Hanxiao <chenhanxiao@gmail.com>

We forgot to free alloced mem when failed to
dup ifname or macaddr.

Also use VIR_STEAL_PTR to simplify codes.

Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
---
 src/qemu/qemu_agent.c  | 3 +--
 src/qemu/qemu_driver.c | 7 +++----
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 5d125c413..0f36054a6 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -2190,8 +2190,7 @@ qemuAgentGetInterfaces(qemuAgentPtr mon,
         iface->naddrs = addrs_count;
     }
 
-    *ifaces = ifaces_ret;
-    ifaces_ret = NULL;
+    VIR_STEAL_PTR(*ifaces, ifaces_ret);
     ret = ifaces_count;
 
  cleanup:
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 978ecd4e0..60cdd237a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20569,10 +20569,10 @@ qemuGetDHCPInterfaces(virDomainPtr dom,
                 goto error;
 
             if (VIR_STRDUP(iface->name, vm->def->nets[i]->ifname) < 0)
-                goto cleanup;
+                goto error;
 
             if (VIR_STRDUP(iface->hwaddr, macaddr) < 0)
-                goto cleanup;
+                goto error;
         }
 
         for (j = 0; j < n_leases; j++) {
@@ -20592,8 +20592,7 @@ qemuGetDHCPInterfaces(virDomainPtr dom,
         VIR_FREE(leases);
     }
 
-    *ifaces = ifaces_ret;
-    ifaces_ret = NULL;
+    VIR_STEAL_PTR(*ifaces, ifaces_ret);
     rv = ifaces_count;
 
  cleanup:
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] qemu: don't leak in qemuGetDHCPInterfaces when failing to alloc

Posted by Michal Privoznik 6 years, 2 months ago

On 02/11/2018 04:07 AM, Chen Hanxiao wrote:
> From: Chen Hanxiao <chenhanxiao@gmail.com>
> 
> We forgot to free alloced mem when failed to
> dup ifname or macaddr.
> 
> Also use VIR_STEAL_PTR to simplify codes.
> 
> Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
> ---
>  src/qemu/qemu_agent.c  | 3 +--
>  src/qemu/qemu_driver.c | 7 +++----
>  2 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
> index 5d125c413..0f36054a6 100644
> --- a/src/qemu/qemu_agent.c
> +++ b/src/qemu/qemu_agent.c
> @@ -2190,8 +2190,7 @@ qemuAgentGetInterfaces(qemuAgentPtr mon,
>          iface->naddrs = addrs_count;
>      }
>  
> -    *ifaces = ifaces_ret;
> -    ifaces_ret = NULL;
> +    VIR_STEAL_PTR(*ifaces, ifaces_ret);
>      ret = ifaces_count;
>  
>   cleanup:
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 978ecd4e0..60cdd237a 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -20569,10 +20569,10 @@ qemuGetDHCPInterfaces(virDomainPtr dom,
>                  goto error;
>  
>              if (VIR_STRDUP(iface->name, vm->def->nets[i]->ifname) < 0)
> -                goto cleanup;
> +                goto error;
>  
>              if (VIR_STRDUP(iface->hwaddr, macaddr) < 0)
> -                goto cleanup;
> +                goto error;
>          }
>  
>          for (j = 0; j < n_leases; j++) {

Almost. There's another 'goto cleanup' in this for() loop. I'm fixing
that too. ACKing and pushing.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list