[libvirt] [PATCH 1/1] Fix the crash when seclable is freed
From: Li Zhang <zhlcindy@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@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); VIR_FREE(def->label); VIR_FREE(def->imagelabel); -- 1.7.5.4
On 02/04/13 13:58, Li Zhang wrote:
From: Li Zhang <zhlcindy@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@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".
VIR_FREE(def->label); VIR_FREE(def->imagelabel);
On 2013年04月02日 14:47, Osier Yang wrote:
On 02/04/13 13:58, Li Zhang wrote:
From: Li Zhang <zhlcindy@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@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.
VIR_FREE(def->label); VIR_FREE(def->imagelabel);
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@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@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.
VIR_FREE(def->label); VIR_FREE(def->imagelabel);
-- Li Zhang IBM China Linux Technology Centre
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@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@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.
VIR_FREE(def->label); VIR_FREE(def->imagelabel);
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 02/04/13 13:58, Li Zhang wrote:
From: Li Zhang <zhlcindy@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@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
On 2013年04月02日 14:47, Osier Yang wrote: 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);
On 02.04.2013 07:58, Li Zhang wrote:
From: Li Zhang <zhlcindy@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@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); 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? Michal
On 2013年04月02日 19:29, Michal Privoznik wrote:
On 02.04.2013 07:58, Li Zhang wrote:
From: Li Zhang <zhlcindy@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@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); 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
Michal
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@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@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); 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@redhat.com> AuthorDate: Thu Mar 28 16:13:01 2013 +0100 Commit: Michal Privoznik <mprivozn@redhat.com> 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
On 2013年04月03日 16:42, Michal Privoznik wrote:
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@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@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); 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@redhat.com> AuthorDate: Thu Mar 28 16:13:01 2013 +0100 Commit: Michal Privoznik <mprivozn@redhat.com> 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.
Let me have a try, it seems not to be the same problem.
Michal
On 2013年04月03日 16:42, Michal Privoznik wrote:
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@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@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); 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@redhat.com> AuthorDate: Thu Mar 28 16:13:01 2013 +0100 Commit: Michal Privoznik <mprivozn@redhat.com> 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. Just now, I did the test with this patch. The problem is fixed. :)
Michal
participants (4)
-
Li Zhang -
Li Zhang -
Michal Privoznik -
Osier Yang