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

On 03.04.2013 04:24, Li Zhang wrote: > On 2013年04月02日 19:29, Michal Privoznik wrote: >> On 02.04.2013 07: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); >>> VIR_FREE(def->label); >>> VIR_FREE(def->imagelabel); >>> >> NACK >> >> As you already found out, we are freeing invalid pointers. We need to >> find out root cause. I wonder where those pointers come from, as >> VIR_ALLOC(), which is used to alloc a virSecurityLabelDefPtr, fill >> allocated memory with zeros, so calling VIR_FREE() even for struct >> members is just fine. Are you able to reproduce this crash? What are the >> steps? > I think it is freed twice. > After the pointer is freed, it will be a wild pointer. > When freeing it the second time, this error occurs. > > I am trying to reproduce this crash. > This steps are: > 1. start libvirtd > 2. create VM > #virsh create /etc/libvirt/qemu/virt-tests-vm1.xml > > 3. Run autotest tests > #cd /DIR/autotest-power > #./client/autotest-local client/tests/virt/libvirt/control > Did you try the latest release? I've fixed a seclabel double free just before a while ago: commit a1c68a1fcbc27fff19e11d0b2a801b416e94366d Author: Michal Privoznik <mprivozn(a)redhat.com&gt; AuthorDate: Thu Mar 28 16:13:01 2013 +0100 Commit: Michal Privoznik <mprivozn(a)redhat.com&gt; CommitDate: Thu Mar 28 16:13:01 2013 +0100 security_manager.c: Append seclabel iff generated With my previous patches, we unconditionally appended a seclabel, even if it wasn't generated but found in array of defined seclabels. This resulted in double free later when doing virDomainDefFree and iterating over the array of defined seclabels. Moreover, there was another possibility of double free, if the seclabel was generated in the last iteration of the process of walking trough security managers array.

Michal