2009/12/15 Daniel P. Berrange <berrange@xxxxxxxxxx>:
> On Tue, Dec 15, 2009 at 04:43:11PM +0100, Matthias Bolte wrote:
>> 2009/12/15 Jim Meyering <jim@xxxxxxxxxxxx>:
>> > The offending code starts here:
>> >
>> > int
>> > esxVMX_ParseSCSIController(virConnectPtr conn, virConfPtr conf, int controller,
>> > int *present, char **virtualDev)
>> > {
>> > char present_name[32];
>> > char virtualDev_name[32];
>> >
>> > if (virtualDev == NULL || *virtualDev != NULL) {
>> > ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR, "Invalid argument");
>> > goto failure;
>> > }
>> >
>> > If the virtualDev parameter is NULL, then we'd issue the
>> > diagnostic and take the "goto", and (below), dereference NULL.
>> >
>> > >From 79283ba1d667534175d4c48079e6b500feba6480 Mon Sep 17 00:00:00 2001
>> > From: Jim Meyering <meyering@xxxxxxxxxx>
>> > Date: Tue, 15 Dec 2009 16:07:10 +0100
>> > Subject: [PATCH] esx_vmx.c: don't dereference NULL for a NULL virtualDev
>> >
>> > * src/esx/esx_vmx.c (esxVMX_ParseSCSIController): Don't deref
>> > "virtualDev" when it is NULL.
>> > ---
>> > src/esx/esx_vmx.c | 3 ++-
>> > 1 files changed, 2 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/src/esx/esx_vmx.c b/src/esx/esx_vmx.c
>> > index f5b4544..404617e 100644
>> > --- a/src/esx/esx_vmx.c
>> > +++ b/src/esx/esx_vmx.c
>> > @@ -1204,7 +1204,8 @@ esxVMX_ParseSCSIController(virConnectPtr conn, virConfPtr conf, int controller,
>> > return 0;
>> >
>> > failure:
>> > - VIR_FREE(*virtualDev);
>> > + if (virtualDev)
>> > + VIR_FREE(*virtualDev);
>> >
>> > return -1;
>> > }
>> > --
>> > 1.6.6.rc2.275.g51e2d
>> >
>>
>> This fixes the problem, but I would fix it differently, matching the
>> other functions. See attached patch.
>>
>> Matthias
>
>> commit 871cd31924308f963afd4df3834b3a1f354a5f8b
>> Author: Matthias Bolte <matthias.bolte@xxxxxxxxxxxxxx>
>> Date: Tue Dec 15 16:37:19 2009 +0100
>>
>> esx: Don't goto failure for invalid arguments
>>
>> This also fixes a NULL-deref of virtualDev in esxVMX_ParseSCSIController
>> found by Jim Meyering.
>>
>> diff --git a/src/esx/esx_vmx.c b/src/esx/esx_vmx.c
>> index f5b4544..7967718 100644
>> --- a/src/esx/esx_vmx.c
>> +++ b/src/esx/esx_vmx.c
>> @@ -1165,14 +1165,14 @@ esxVMX_ParseSCSIController(virConnectPtr conn, virConfPtr conf, int controller,
>>
>> if (virtualDev == NULL || *virtualDev != NULL) {
>> ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR, "Invalid argument");
>> - goto failure;
>> + return -1;
>> }
>>
>> if (controller < 0 || controller > 3) {
>> ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR,
>> "SCSI controller index %d out of [0..3] range",
>> controller);
>> - goto failure;
>> + return -1;
>> }
>>
>> snprintf(present_name, sizeof(present_name), "scsi%d.present", controller);
>> @@ -1642,7 +1642,7 @@ esxVMX_ParseEthernet(virConnectPtr conn, virConfPtr conf, int controller,
>> ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR,
>> "Ethernet controller index %d out of [0..3] range",
>> controller);
>> - goto failure;
>> + return -1;
>> }
>>
>> if (VIR_ALLOC(*def) < 0) {
>> @@ -1840,7 +1840,7 @@ esxVMX_ParseSerial(virConnectPtr conn, esxVI_Context *ctx, virConfPtr conf,
>> if (port < 0 || port > 3) {
>> ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR,
>> "Serial port index %d out of [0..3] range", port);
>> - goto failure;
>> + return -1;
>> }
>>
>> if (VIR_ALLOC(*def) < 0) {
>> @@ -1964,7 +1964,7 @@ esxVMX_ParseParallel(virConnectPtr conn, esxVI_Context *ctx, virConfPtr conf,
>> if (port < 0 || port > 2) {
>> ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR,
>> "Parallel port index %d out of [0..2] range", port);
>> - goto failure;
>> + return -1;
>> }
>>
>> if (VIR_ALLOC(*def) < 0) {
>
> ACK
>
>
> Daniel
Okay, pushed.
Matthias
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list