[libvirt] [PATCH 1/1] Fix the crash when seclable is freed

[Li Zhang]

On 2013年04月02日 16:45, Osier Yang wrote:
> On 02/04/13 16:04, Li Zhang wrote:
>> On 2013年04月02日 15:03, Li Zhang wrote:
>>> On 2013年04月02日 14:47, Osier Yang wrote:
>>>> On 02/04/13 13:58, Li Zhang wrote:
>>>>> From: Li Zhang <zhlcindy at linux.vnet.ibm.com>
>>>>>
>>>>> When seclabel's type is VIR_DOMAIN_SECLABEL_NONE,
>>>>> virSecurityLabelDefPtr's members are not allocated.
>>>>> So it will cause crash when calling VIR_FREE.
>>>>>
>>>>> This problem is found when running autotest on PPC.
>>>>>
>>>>> Failed to remove cgroup for virt-tests-vm1
>>>>> *** glibc detected *** /usr/sbin/libvirtd: free(): invalid 
>>>>> pointer: 0x00003fff9c187510 ***
>>>>> ======= Backtrace: =========
>>>>> /lib64/libc.so.6(+0xb89c4)[0x3fffa9bc89c4]
>>>>> /lib64/libvirt.so.0(virFree-0x3e2320)[0x3fffaa82e9c0]
>>>>> /lib64/libvirt.so.0(virSecurityLabelDefFree-0x378984)[0x3fffaa89d69c]
>>>>> /lib64/libvirt.so.0(virDomainDefFree-0x367c98)[0x3fffaa8ae968]
>>>>> /usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so(qemuProcessStop-0xc85f8)[0x3fffa2899d58] 
>>>>>
>>>>> /usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so(+0xc3668)[0x3fffa28e3668] 
>>>>>
>>>>> /lib64/libvirt.so.0(virDomainDestroy-0x309bd0)[0x3fffaa90f6f0]
>>>>> /usr/sbin/libvirtd[0x10035230]
>>>>> /lib64/libvirt.so.0(virNetServerProgramDispatch-0x289b50)[0x3fffaa995930] 
>>>>>
>>>>> /lib64/libvirt.so.0(+0x20db18)[0x3fffaa98db18]
>>>>> /lib64/libvirt.so.0(+0xfbd24)[0x3fffaa87bd24]
>>>>> /lib64/libvirt.so.0(+0xfaec8)[0x3fffaa87aec8]
>>>>> /lib64/libpthread.so.0(+0xc604)[0x3fffa9d7c604]
>>>>> /lib64/libc.so.6(clone-0xb8fe4)[0x3fffa9c3f094]
>>>>>
>>>>> Signed-off-by: Li Zhang <zhlcindy at linux.vnet.ibm.com>
>>>>> ---
>>>>> src/conf/domain_conf.c | 2 ++
>>>>> 1 files changed, 2 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>>>>> index f3fca7f..2856660 100644
>>>>> --- a/src/conf/domain_conf.c
>>>>> +++ b/src/conf/domain_conf.c
>>>>> @@ -1006,6 +1006,8 @@ 
>>>>> virSecurityLabelDefFree(virSecurityLabelDefPtr def)
>>>>> {
>>>>> if (!def)
>>>>> return;
>>>>> + if (def->type == VIR_DOMAIN_SECLABEL_NONE)
>>>>> + return;
>>>>> VIR_FREE(def->model);
>>>>
>>>> model is always parsed. So it will be leaked if someone specifies
>>>> "model" even the type is "none".
>>> Okay, it seems that is not always non-NULL although it is always 
>>> parsed.
>>>
>>> My XML file is as:
>>> <seclabel type='none'/>
>>>
>>> It should be better to add if clause before VIR_FREE.
>>>
>> Sorry, it seems that this is not right solution to resolve the problem.
>> free(ptr): ptr can be NULL.
>> This root cause is because of invalid pointer which is not NULL.
>> I need to find out the invalid pointer.
>
> The solution is not to parse "model" when the seclabel type is "none".
> I might not be 100% correct. But I think the "model" is just useless
> for "none". Then your patch is right.
>

Originally, I was thinking about when model is not specified, then model 
is NULL.
And free(NULL) is not right. I look into the lib, free(NULL) can work.

I think the invalid pointer which is not NULL should be one wild pointer 
here.
I need to verify it on my machine with autotest.

>>
>>>>
>>>>> VIR_FREE(def->label);
>>>>> VIR_FREE(def->imagelabel);
>>>>
>>>
>>
>>
>