[libvirt] [PATCH] virsh.c: avoid leak on OOM error path

[Jiri Denemark]

> > No one really cares if we leak memory while dying, but who knows...
> > freeing that buffer may let us go down more gracefully.
> >
> > FYI, the leak is triggered when virFileReadAll succeeds
> > (it allocates "buffer"), yet xmlNewDoc fails:
> >
> >     if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0)
> >         return FALSE;
> >
> >     doc = xmlNewDoc(NULL);
> >     if (doc == NULL)
> >         goto no_memory;
> 
> The above is correct, but there's another leak in the same function,
> so I've amended the patch to also free the "list" buffer.
> "list" is allocated in the for-loop.
> If on the 2nd or subsequent iteration of that loop we take
> the "goto no_memory", we'd leak that buffer.
> 
> diff --git a/tools/virsh.c b/tools/virsh.c
> index dd916f3..c8ae9f2 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -7139,6 +7139,8 @@ cleanup:
>      return ret;
> 
>  no_memory:
> +    VIR_FREE(list);
> +    VIR_FREE(buffer);
>      vshError(ctl, "%s", _("Out of memory"));
>      ret = FALSE;
>      return ret;

Actually that's not enough and it also duplicates code as all the cleanup code
is already there:

cleanup:
    xmlXPathFreeObject(obj);
    xmlXPathFreeContext(ctxt);
    xmlFreeDoc(doc);
    VIR_FREE(result);
    if ((list != NULL) && (count > 0)) {
        for (i = 0;i < count;i++)
            VIR_FREE(list[i]);
    }
    VIR_FREE(list);
    VIR_FREE(buffer);

    return ret;

The best solution is jumping to cleanup in OOM path. I should have spotted
missing goto cleanup at the end of no_memory when I was reviewing the code...

Patch attached...

Jirka
-------------- next part --------------
From fa16cdcbe2a9632edce3d9c227235d37518b7afa Mon Sep 17 00:00:00 2001
Message-Id: <fa16cdcbe2a9632edce3d9c227235d37518b7afa.1266922930.git.jdenemar at redhat.com>
From: Jiri Denemark <jdenemar at redhat.com>
Date: Tue, 23 Feb 2010 12:01:20 +0100
Subject: [PATCH] virsh.c: avoid all leaks in OOM path in cmdCPUBaseline
Mail-Followup-To: libvir-list at redhat.com

Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
---
 tools/virsh.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index dc9d44c..5fdbbe5 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -7140,11 +7140,9 @@ cleanup:
     return ret;
 
 no_memory:
-    VIR_FREE(list);
-    VIR_FREE(buffer);
     vshError(ctl, "%s", _("Out of memory"));
     ret = FALSE;
-    return ret;
+    goto cleanup;
 }
 
 /* Common code for the edit / net-edit / pool-edit functions which follow. */
-- 
1.7.0