2009/12/16 Jim Meyering <jim@meyering.net>:
Similar to others,
From 5dcb4d95a351cb6fa32ebaafb96756275e1079e2 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Wed, 16 Dec 2009 13:56:57 +0100 Subject: [PATCH] vbox_tmpl.c: avoid NULL deref upon vboxDomainCreateXML failure
* src/vbox/vbox_tmpl.c (vboxDomainCreateXML): Don't call vboxDomainUndefine on a NULL "dom". --- src/vbox/vbox_tmpl.c | 16 +++++----------- 1 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index d6b681c..ba70053 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -990,8 +990,6 @@ cleanup:
static virDomainPtr vboxDomainCreateXML(virConnectPtr conn, const char *xml, unsigned int flags ATTRIBUTE_UNUSED) { - virDomainPtr dom = NULL; - /* VirtualBox currently doesn't have support for running * virtual machines without actually defining them and thus * for time being just define new machine and start it. @@ -1000,17 +998,13 @@ static virDomainPtr vboxDomainCreateXML(virConnectPtr conn, const char *xml, * change this behaviour to the expected one. */
- dom = vboxDomainDefineXML(conn, xml); - if (dom) { - if (vboxDomainCreate(dom) < 0) - goto cleanup; - } else { - goto cleanup; - } + virDomainPtr dom = vboxDomainDefineXML(conn, xml); + if (dom == NULL) + return NULL;
- return dom; + if (0 < vboxDomainCreate(dom)) + return dom;
This check is wrong. You meant to check for !(vboxDomainCreate(dom) < 0) but resolved the ! incorrectly. Either change it to if (vboxDomainCreate(dom) >= 0) return dom; or keep the negative check if (vboxDomainCreate(dom) < 0) { vboxDomainUndefine(dom); return NULL; } return dom; I prefer the second version. The original code also contains a domain pointer reference leak. vboxDomainUndefine does not unref the domain pointer, so there is a virUnrefDomain call missing in the error case. So this could be changed to if (vboxDomainCreate(dom) < 0) { vboxDomainUndefine(dom); virUnrefDomain(dom); return NULL; } return dom; But I think this is worth a separate commit.
-cleanup: vboxDomainUndefine(dom); return NULL; } -- 1.6.6.rc2.275.g51e2d
Matthias