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

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(a)linux.vnet.ibm.com&gt; >>>> >>>> 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(a)linux.vnet.ibm.com&gt; >>>> --- >>>> 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.

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