[libvirt] [PATCH] virBitmapFree: Change the function to a macro

[Eric Blake]

On 09/10/2013 02:29 AM, Liuji (Jeremy) wrote:
>>>>> Yes，it's an actual crash problem. I found the problem in the above
>> problem
>>>> scenario in my first mail.

>>
>> A problem scenario:
>> 1) The XML of VM contain the below segment:
>>      <numatune>
>>          <memory mode='preferred' placement='auto' nodeset='0'/>
>>      </numatune>
>> 2)virsh create the VM
>> 3)In the virDomainDefParseXML funtion:
>>                      /* Ignore 'nodeset' if 'placement' is 'auto' finally */
>>                      if (placement_mode ==
> VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_AUTO) {
>>
> virBitmapFree(def->numatune.memory.nodemask);
>>                          def->numatune.memory.nodemask = NULL;
>>                      }
>> 4)Then, virsh destroy the VM. In the virDomainDefFree funtion, it also call the
>> virBitmapFree function to free the nodemask:
>>      virBitmapFree(def->numatune.memory.nodemask);


>>>>
>>>> Then can you send a patch which explicitly fixes that problem on its
>>>> own, without doing this major refactoring, which obscures what is being
>>>> fixed.
>>>
>>> I had sent a patch in my first mail. You can find the mail at the following links:
>>>
>> https://www.redhat.com/archives/libvir-list/2013-September/msg00337.html
>>
>> No, I want the root cause fixed. This change to the bitmap APIs is just
>> papering over any root cause bug.
>>
> 
> This is a simple problem about an address is released twice in some scenario. 
> I think that released twice is the root cause. I'm not sure what you mean about root cause.
> Did you mean that why the address will be released two times?

What we want is a patch against virDomainDefParseXML - if it frees the
bitmap, it must set the bitmap member to NULL (ie. the root cause is
step 3 of your trace above).  That will then avoid the second free in
virDomainDefFree (step 4 of your trace).  The root cause is insidious
because leaving the bitmap pointer non-NULL after freeing it means that
ANY other use of the bitmap (not just the free in virDomainDefFree) is
accessing freed memory.  Creating a macro to avoid the double free is
overkill; compared to fixing the real bug of freeing but not marking the
memory freed.

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130910/d9a46112/attachment-0001.sig>