[libvirt] [PATCH]LXC: free dst before lxcDomainAttachDeviceDiskLive return
From: Chen Hanxiao <chenhanxiao@cn.fujitsu.com> free dst before lxcDomainAttachDeviceDiskLive return Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com> --- src/lxc/lxc_driver.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 9c58f67..7906ad1 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3053,6 +3053,7 @@ cleanup: virDomainAuditDisk(vm, NULL, def->src, "attach", ret == 0); if (dst && created && ret < 0) unlink(dst); + VIR_FREE(dst); return ret; } -- 1.8.2.1
On Thu, Sep 26, 2013 at 02:01:52PM +0800, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
free dst before lxcDomainAttachDeviceDiskLive return
Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com> --- src/lxc/lxc_driver.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 9c58f67..7906ad1 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3053,6 +3053,7 @@ cleanup: virDomainAuditDisk(vm, NULL, def->src, "attach", ret == 0); if (dst && created && ret < 0) unlink(dst); + VIR_FREE(dst); return ret; }
Hum, it is a bit complex as the allocated dst is saved in the definition a few lines earlier: /* Labelling normally operates on src, but we need * to actally label the dst here, so hack the config */ def->src = dst; and then in cleanup it's overriden again with the value saved at the start of the function: cleanup: def->src = tmpsrc; virDomainAuditDisk(vm, NULL, def->src, "attach", ret == 0); the leak is real and that free certainly need to be added but maybe there is a bit of cleanup needed rather than just adding the VIR_FREE() I will let someone with more knowledge of that code part decide :-) Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veillard@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/
-----Original Message----- From: Daniel Veillard [mailto:veillard@redhat.com] Sent: Thursday, September 26, 2013 4:42 PM To: Chen Hanxiao Cc: libvir-list@redhat.com Subject: Re: [libvirt] [PATCH]LXC: free dst before lxcDomainAttachDeviceDiskLive return
On Thu, Sep 26, 2013 at 02:01:52PM +0800, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
free dst before lxcDomainAttachDeviceDiskLive return
Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com> virDomainAuditDisk(vm, NULL, def->src, "attach", ret == 0); if (dst && created && ret < 0) unlink(dst); + VIR_FREE(dst); return ret; }
Hum, it is a bit complex as the allocated dst is saved in the definition a few lines earlier:
/* Labelling normally operates on src, but we need * to actally label the dst here, so hack the config */ def->src = dst;
and then in cleanup it's overriden again with the value saved at the start of the function:
cleanup: def->src = tmpsrc; virDomainAuditDisk(vm, NULL, def->src, "attach", ret == 0);
the leak is real and that free certainly need to be added but maybe there is a bit of cleanup needed rather than just adding the VIR_FREE()
This scenario is a little complex for me. As my investigation, VIR_FREE() can work in some cases.
I will let someone with more knowledge of that code part decide :-)
Thanks for any help :)
Daniel
-- Daniel Veillard | Open Source and Standards, Red Hat veillard@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/
On 09/26/2013 08:01 AM, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
free dst before lxcDomainAttachDeviceDiskLive return
Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com> --- src/lxc/lxc_driver.c | 1 + 1 file changed, 1 insertion(+)
ACK and pushed. Jan
On Thu, Sep 26, 2013 at 8:20 AM, Ján Tomko <jtomko@redhat.com> wrote:
On 09/26/2013 08:01 AM, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
free dst before lxcDomainAttachDeviceDiskLive return
Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com> --- src/lxc/lxc_driver.c | 1 + 1 file changed, 1 insertion(+)
ACK and pushed.
Jan
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Do we need to revert this before the release? It sounds like it can cause us to crash due to using freed memory. -- Doug Goldstein
On 09/26/2013 03:06 PM, Doug Goldstein wrote:
On Thu, Sep 26, 2013 at 8:20 AM, Ján Tomko <jtomko@redhat.com> wrote:
On 09/26/2013 08:01 AM, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
free dst before lxcDomainAttachDeviceDiskLive return
Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com> --- src/lxc/lxc_driver.c | 1 + 1 file changed, 1 insertion(+)
ACK and pushed.
Jan
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Do we need to revert this before the release? It sounds like it can cause us to crash due to using freed memory.
No, the patch looks correct to me. I did, however, spot a typo, so I'll be pushing the obvious fix s/actally/actually/. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (5)
-
Chen Hanxiao -
Daniel Veillard -
Doug Goldstein -
Eric Blake -
Ján Tomko