[libvirt] [PATCH] virsh: fix memory leak in virsh when starting a guest with invalid fd
When start a guest with --pass-fd, if the argument of --pass-fd is invalid, virsh will exit, but doesn't free the variable 'dom'. The valgrind said: ... ==24569== 63 (56 direct, 7 indirect) bytes in 1 blocks are definitely lost in loss record 130 of 234 ==24569== at 0x4C2A1D4: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==24569== by 0x4E879A4: virAllocVar (viralloc.c:544) ==24569== by 0x4EBD625: virObjectNew (virobject.c:190) ==24569== by 0x4F3A18A: virGetDomain (datatypes.c:226) ==24569== by 0x4F9311F: remoteDomainLookupByName (remote_driver.c:6636) ==24569== by 0x4F44F20: virDomainLookupByName (libvirt.c:2277) ==24569== by 0x12F616: vshCommandOptDomainBy (virsh-domain.c:105) ==24569== by 0x131C79: cmdStart (virsh-domain.c:3330) ==24569== by 0x12C4AB: vshCommandRun (virsh.c:1752) ==24569== by 0x127001: main (virsh.c:3218) https://bugzilla.redhat.com/show_bug.cgi?id=1067338 Signed-off-by: Jincheng Miao <jmiao@redhat.com> --- tools/virsh-domain.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index c3db94c..57653a2 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -3428,11 +3428,11 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) if (virDomainGetID(dom) != (unsigned int)-1) { vshError(ctl, "%s", _("Domain is already active")); virDomainFree(dom); - return false; + goto cleanup; } if (cmdStartGetFDs(ctl, cmd, &nfds, &fds) < 0) - return false; + goto cleanup; if (vshCommandOptBool(cmd, "paused")) flags |= VIR_DOMAIN_START_PAUSED; -- 1.8.5.3
On 02/20/2014 02:29 AM, Jincheng Miao wrote:
When start a guest with --pass-fd, if the argument of --pass-fd is invalid, virsh will exit, but doesn't free the variable 'dom'.
+++ b/tools/virsh-domain.c @@ -3428,11 +3428,11 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) if (virDomainGetID(dom) != (unsigned int)-1) { vshError(ctl, "%s", _("Domain is already active")); virDomainFree(dom); - return false; + goto cleanup; }
As is, this change ends up with a double-free calling virDomainFree(dom) twice. Delete the one here, and let the cleanup label handle it instead.
if (cmdStartGetFDs(ctl, cmd, &nfds, &fds) < 0) - return false; + goto cleanup;
at which point this is also correct. I'll push the amended patch shortly. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
----- Original Message -----
On 02/20/2014 02:29 AM, Jincheng Miao wrote:
When start a guest with --pass-fd, if the argument of --pass-fd is invalid, virsh will exit, but doesn't free the variable 'dom'.
+++ b/tools/virsh-domain.c @@ -3428,11 +3428,11 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) if (virDomainGetID(dom) != (unsigned int)-1) { vshError(ctl, "%s", _("Domain is already active")); virDomainFree(dom); - return false; + goto cleanup; }
As is, this change ends up with a double-free calling virDomainFree(dom) twice. Delete the one here, and let the cleanup label handle it instead.
Sorry, I've forget to delete this virDomainFree. Yes, my original meaning is let the cleanup label handle it. Thanks for your review.
if (cmdStartGetFDs(ctl, cmd, &nfds, &fds) < 0) - return false; + goto cleanup;
at which point this is also correct. I'll push the amended patch shortly.
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Jincheng Miao