On 01/04/2012 11:36 PM, Alex Jia wrote: > On 01/05/2012 09:13 AM, Hu Tao wrote: >> On Tue, Jan 03, 2012 at 11:14:15AM +0800, Hu Tao wrote: >>> This is not a memory leak. See line 8029 and 8030 of qemu_driver.c. >>> >>> To ensure this, I tested twice following these steps: >>> >>> 1. set bandwidth lively (--live) >>> 2. query bandwidth (--live) >>> 3. set bandwidth lively (--live) >>> >>> The first time libvirtd crashed at step 2. The second time >>> on step 2 I got strage data, and libvirtd crashed at step 3. > Yeah, I can reproduce this and libvirtd crashed at step 3 for me. > > In addition, valgrind can't find this memory leak, it's a negative > branch, coverity complains it, line 7994 called allocation function > "virAlloc" on "newBandwidth", and line 7999 variable "newBandwidth" > is not freed or pointed-to in function "memset", lines 8007 and 8017 > variable "newBandwidth" going out of scope leaks the storage it points to, > because 'cleanup' label hasn't freed allocated 'newBandwidth' variable > memory.
In fixing the memory leak on a negative path, we introduced a
use-after-free on the positive path. Reverting things would
re-introduce the memory leak on failure.
The _correct_ fix, which I'm pushing now, is this:
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 82bab67..110c31b 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -8034,6 +8034,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
virNetDevBandwidthFree(net->bandwidth);
net->bandwidth = newBandwidth;
+ newBandwidth = NULL;
}
if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
if (!persistentNet->bandwidth) {
--
Eric Blake ebl...@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
