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(a)redhat.com&gt; 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; }

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(a)redhat.com&gt; 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

On Tue, Dec 15, 2009 at 04:43:11PM +0100, Matthias Bolte wrote: > 2009/12/15 Jim Meyering <jim(a)meyering.net&gt;: > > 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(a)redhat.com&gt; > > 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(a)googlemail.com&gt; > 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