Re: [libvirt] [PATCH] esx_vmx.c: don't dereference a NULL virtualDev

[Matthias Bolte <matthias.bolte@xxxxxxxxxxxxxx>]

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) {

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list