3:59 a.m.
From: Alex Jia <ajia(a)redhat.com>
Detected by Coverity. Leaks introduced in commit 93ab585 and
commit e8d6b29.
Signed-off-by: Alex Jia <ajia(a)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
5:34 p.m.
On 12/31/2011 02:59 AM, ajia(a)redhat.com wrote:
From: Alex Jia <ajia(a)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(a)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(a)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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
5:42 p.m.
On 12/31/2011 02:59 AM, ajia(a)redhat.com wrote:
From: Alex Jia <ajia(a)redhat.com>
Detected by Coverity. Leaks introduced in commit 93ab585 and
commit e8d6b29.
Signed-off-by: Alex Jia <ajia(a)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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4710
days inactive
4710
days old
2 comments
2 participants
participants (2)
-
ajia＠redhat.com -
Eric Blake