[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] revert commit baade4cd2bf partly

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,

         net->bandwidth = newBandwidth;
+        newBandwidth = NULL;
     if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
         if (!persistentNet->bandwidth) {

Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]