[libvirt] [PATCH] qemu: Fix memory leaks

From: Alex Jia <ajia@redhat.com> Detected by Coverity. Leaks introduced in commit 93ab585 and commit e8d6b29. Signed-off-by: Alex Jia <ajia@redhat.com> --- src/qemu/qemu_driver.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fbaa824..6a1e7cc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6014,6 +6014,7 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, } if (j != ndevices) { ret = -1; + VIR_FREE(devices); continue; } if (qemuDomainMergeDeviceWeights(&vm->def->blkio.devices, @@ -7867,7 +7868,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, virDomainDefPtr persistentDef = NULL; int ret = -1; virDomainNetDefPtr net = NULL, persistentNet = NULL; - virNetDevBandwidthPtr bandwidth = NULL; + virNetDevBandwidthPtr bandwidth = NULL, newBandwidth = NULL; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -7989,8 +7990,6 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - virNetDevBandwidthPtr newBandwidth; - if (VIR_ALLOC(newBandwidth) < 0) { virReportOOMError(); goto cleanup; @@ -8056,6 +8055,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, ret = 0; cleanup: virNetDevBandwidthFree(bandwidth); + virNetDevBandwidthFree(newBandwidth); virCgroupFree(&group); if (vm) virDomainObjUnlock(vm); -- 1.7.1

On 12/31/2011 02:59 AM, ajia@redhat.com wrote:
From: Alex Jia <ajia@redhat.com>
Detected by Coverity. Leaks introduced in commit 93ab585 and commit e8d6b29.
Let's fix this in two parts, to ease backporting (since one was pre-0.9.8 and the other only in 0.9.9-rc1). Part 1:
Signed-off-by: Alex Jia <ajia@redhat.com> --- src/qemu/qemu_driver.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fbaa824..6a1e7cc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6014,6 +6014,7 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, } if (j != ndevices) { ret = -1; + VIR_FREE(devices); continue;
Good catch, but still a leak; this needs to call virBlkioDeviceWeightArrayClear to avoid a leak. I'm pushing this instead: From 8267aea5a6149c9fad399530fc0e8d7f406d22fd Mon Sep 17 00:00:00 2001 From: Eric Blake <eblake@redhat.com> Date: Sat, 31 Dec 2011 16:32:35 -0700 Subject: [PATCH] qemu: fix blkio memory leak on failure Leak detected by Coverity, and introduced in commit 93ab585. Reported by Alex Jia. * src/qemu/qemu_driver.c (qemuDomainSetBlkioParameters): Free devices array on error. --- src/qemu/qemu_driver.c | 7 ++----- 1 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fbaa824..d89303e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6012,11 +6012,8 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, break; } } - if (j != ndevices) { - ret = -1; - continue; - } - if (qemuDomainMergeDeviceWeights(&vm->def->blkio.devices, + if (j != ndevices || + qemuDomainMergeDeviceWeights(&vm->def->blkio.devices, &vm->def->blkio.ndevices, devices, ndevices) < 0) ret = -1; -- 1.7.7.4 -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 12/31/2011 02:59 AM, ajia@redhat.com wrote:
From: Alex Jia <ajia@redhat.com>
Detected by Coverity. Leaks introduced in commit 93ab585 and commit e8d6b29.
Signed-off-by: Alex Jia <ajia@redhat.com> --- src/qemu/qemu_driver.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
Part 2:
@@ -7867,7 +7868,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, virDomainDefPtr persistentDef = NULL; int ret = -1; virDomainNetDefPtr net = NULL, persistentNet = NULL; - virNetDevBandwidthPtr bandwidth = NULL; + virNetDevBandwidthPtr bandwidth = NULL, newBandwidth = NULL;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -7989,8 +7990,6 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, }
if (flags & VIR_DOMAIN_AFFECT_LIVE) { - virNetDevBandwidthPtr newBandwidth; - if (VIR_ALLOC(newBandwidth) < 0) { virReportOOMError(); goto cleanup; @@ -8056,6 +8055,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, ret = 0; cleanup: virNetDevBandwidthFree(bandwidth); + virNetDevBandwidthFree(newBandwidth); virCgroupFree(&group); if (vm) virDomainObjUnlock(vm);
ACK, but I'm also squashing this in before pushing (VIR_ALLOC guarantees 0 initialization so memset is not needed; fix some grammar in a comment, and avoid long lines): diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c index 0885a5e..e93fe87 100644 --- i/src/qemu/qemu_driver.c +++ w/src/qemu/qemu_driver.c @@ -7991,20 +7991,20 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, goto cleanup; } - memset(newBandwidth, 0, sizeof(newBandwidth)); - /* virNetDevBandwidthSet() will clear any previous value of * bandwidth parameters, so merge with old bandwidth parameters - * here to prevent them from losing. */ + * here to prevent them from being lost. */ if (bandwidth->in || net->bandwidth->in) { if (VIR_ALLOC(newBandwidth->in) < 0) { virReportOOMError(); goto cleanup; } if (bandwidth->in) - memcpy(newBandwidth->in, bandwidth->in, sizeof(*newBandwidth->in)); + memcpy(newBandwidth->in, bandwidth->in, + sizeof(*newBandwidth->in)); else if (net->bandwidth->in) - memcpy(newBandwidth->in, net->bandwidth->in, sizeof(*newBandwidth->in)); + memcpy(newBandwidth->in, net->bandwidth->in, + sizeof(*newBandwidth->in)); } if (bandwidth->out || net->bandwidth->out) { if (VIR_ALLOC(newBandwidth->out) < 0) { @@ -8012,9 +8012,11 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, goto cleanup; } if (bandwidth->out) - memcpy(newBandwidth->out, bandwidth->out, sizeof(*newBandwidth->out)); + memcpy(newBandwidth->out, bandwidth->out, + sizeof(*newBandwidth->out)); else if (net->bandwidth->out) - memcpy(newBandwidth->out, net->bandwidth->out, sizeof(*newBandwidth->out)); + memcpy(newBandwidth->out, net->bandwidth->out, + sizeof(*newBandwidth->out)); } if (virNetDevBandwidthSet(net->ifname, newBandwidth) < 0) { -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
ajia@redhat.com
-
Eric Blake