[libvirt] [PATCH] vbox_tmpl.c: avoid NULL deref upon vboxDomainCreateXML failure
Matthias Bolte
matthias.bolte at googlemail.com
Wed Dec 16 21:08:17 UTC 2009
2009/12/16 Jim Meyering <jim at meyering.net>:
> Similar to others,
>
> >From 5dcb4d95a351cb6fa32ebaafb96756275e1079e2 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering at 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
More information about the libvir-list
mailing list