2010/1/5 Jim Meyering <jim@xxxxxxxxxxxx>:
> Matthias Bolte wrote:
>> 2009/12/16 Jim Meyering <jim@xxxxxxxxxxxx>:
>>> Similar to others,
>>>
>>> >From 5dcb4d95a351cb6fa32ebaafb96756275e1079e2 Mon Sep 17 00:00:00 2001
>>> From: Jim Meyering <meyering@xxxxxxxxxx>
>>> 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.
>
> I reversed the condition, but left off the "=".
>
>> 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.
>
> So do I.
> Here's the adjusted patch.
>
> From 7e8355ea7c2e50995b139322de574ea4abf24fe3 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering@xxxxxxxxxx>
> 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 | 19 +++++++------------
> 1 files changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
> index 5a1d8dc..5889f32 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,19 +998,16 @@ 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;
> +
> + if (vboxDomainCreate(dom) < 0) {
> + vboxDomainUndefine(dom);
> + return NULL;
> }
>
> return dom;
> -
> -cleanup:
> - vboxDomainUndefine(dom);
> - return NULL;
> }
>
> static virDomainPtr vboxDomainLookupByID(virConnectPtr conn, int id) {
> --
> 1.6.6.387.g2649b1
>
ACK.
Matthias
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list