[libvirt] [PATCH 2/2] conf: Fix leaks in virNetworkObjUpdateParseFile

[Ján Tomko]

On 01/25/13 20:15, Laine Stump wrote:
> On 01/25/2013 10:36 AM, Ján Tomko wrote:
>> Free the bitmap before calling virBitmapParse, which will overwrite it.
>>
>> Also free xml.
>> ---
>>  src/conf/network_conf.c |   19 +++++++++++--------
>>  1 files changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
>> index c93916d..013333c 100644
>> --- a/src/conf/network_conf.c
>> +++ b/src/conf/network_conf.c
>> @@ -1855,14 +1855,16 @@ virNetworkObjUpdateParseFile(const char *filename,
>>  
>>          ctxt->node = node;
>>          class_id = virXPathString("string(./class_id[1]/@bitmap)", ctxt);
>> -        if (class_id &&
>> -            virBitmapParse(class_id, 0,
>> -                           &net->class_id, CLASS_ID_BITMAP_SIZE) < 0) {
>> -            virReportError(VIR_ERR_INTERNAL_ERROR,
>> -                           _("Malformed 'class_id' attribute: %s"),
>> -                           class_id);
>> -            VIR_FREE(class_id);
>> -            goto cleanup;
>> +        if (class_id) {
>> +            virBitmapFree(net->class_id);
> 
> If there is a situation where this function can be called and
> net->class_id is already filled in, then doesn't that also mean that
> net->floor_sum could have already been set?
> 
> If that's the case, then we need to also set net->floor_sum to 0, in
> case it was previously non-0 and the new status doesn't have anything
> set (implying 0).
> 
> If not, then this virBitmapFree() is a NOP.
> 

This function only gets called on the network driver initialization,
shortly after net->class_id gets allocated and filled with 111 in
virNetworkAssignDef (see valgrind log below). net->floor_sum is always
zero at this point.

It looks like networkFindActiveConfigs (which calls this function) is
missing from networkReload.

Also, net->class_id or net->floor_sum might get changed by this
function, even if the rest of the status file could not be parsed.

Jan

==22881== 8,216 (24 direct, 8,192 indirect) bytes in 1 blocks are
definitely lost in loss record 852 of 860
==22881==    at 0x4C2A462: calloc (vg_replace_malloc.c:593)
==22881==    by 0x528E0C1: virAlloc (viralloc.c:100)
==22881==    by 0x528FB9E: virBitmapNew (virbitmap.c:74)
==22881==    by 0x530F61F: virNetworkAssignDef (network_conf.c:347)
==22881==    by 0x530F821: virNetworkLoadConfig (network_conf.c:2392)
==22881==    by 0x530F9AA: virNetworkLoadAllConfigs (network_conf.c:2437)
==22881==    by 0x5104D9: networkStartup (bridge_driver.c:419)
==22881==    by 0x533EBF7: virStateInitialize (libvirt.c:822)
==22881==    by 0x428E8C: daemonRunStateInit (libvirtd.c:877)
==22881==    by 0x52CC815: virThreadHelper (virthreadpthread.c:161)
==22881==    by 0x805DEC5: start_thread (pthread_create.c:305)
==22881==    by 0x87636EC: clone (clone.S:115)