[Libvir] PATCH: Fix crash in cleanup when VM creation fails
If using the 'virDomainCreateLinux' call to create a VM, a so called 'transient' domain will be created - ie one without any config file. There is special code in the qemudShutdownVMDaemon method to cleanup the resources associated with such domains, in particuarly free'ing the struct qemud_vm object. Unfortunately in the virDomainCreateLinux codepath this is a problem, because we still need the 'struct qemud_vm' object in certain edge cases, and so the caller has to free it. We currently have a double free() in that scenario. This patch removes the call to qemudFreeVMDaemon from qemudShutdownVMDaemon. Instead it is now always the caller's responsibility to cleanup after transient domains. qemu_driver.c | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Mon, Jul 23, 2007 at 08:55:52PM +0100, Daniel P. Berrange wrote:
If using the 'virDomainCreateLinux' call to create a VM, a so called 'transient' domain will be created - ie one without any config file. There is special code in the qemudShutdownVMDaemon method to cleanup the resources associated with such domains, in particuarly free'ing the struct qemud_vm object. Unfortunately in the virDomainCreateLinux codepath this is a problem, because we still need the 'struct qemud_vm' object in certain edge cases, and so the caller has to free it. We currently have a double free() in that scenario. This patch removes the call to qemudFreeVMDaemon from qemudShutdownVMDaemon. Instead it is now always the caller's responsibility to cleanup after transient domains.
I think a bit weird that a routine which may still fail in some way can be switched back from returning an int to void and still generate an improvement. The way we used the return value of Shutdown is not needed anymore but I wonder about the possibility of other error case. +1 for the fix still, that's important Related bug is https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=249072 for those wondering. Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Tue, Jul 24, 2007 at 03:23:42AM -0400, Daniel Veillard wrote:
On Mon, Jul 23, 2007 at 08:55:52PM +0100, Daniel P. Berrange wrote:
If using the 'virDomainCreateLinux' call to create a VM, a so called 'transient' domain will be created - ie one without any config file. There is special code in the qemudShutdownVMDaemon method to cleanup the resources associated with such domains, in particuarly free'ing the struct qemud_vm object. Unfortunately in the virDomainCreateLinux codepath this is a problem, because we still need the 'struct qemud_vm' object in certain edge cases, and so the caller has to free it. We currently have a double free() in that scenario. This patch removes the call to qemudFreeVMDaemon from qemudShutdownVMDaemon. Instead it is now always the caller's responsibility to cleanup after transient domains.
I think a bit weird that a routine which may still fail in some way can be switched back from returning an int to void and still generate an improvement. The way we used the return value of Shutdown is not needed anymore but I wonder about the possibility of other error case.
The return value in the original code was more or less useless already as there was nothing that could be failing & the function was always returning success, so we were checking for a failure which would never occur. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
participants (2)
-
Daniel P. Berrange -
Daniel Veillard