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 eblake@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list