[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] esx_vi.c: avoid NULL deref for invalid inputs



2009/12/15 Jim Meyering <jim meyering net>:
> The first statement in this function tests for freeFunc == NULL,
> and if that's true, it does "goto failure" where it
> proceeds to call through that NULL function pointer:
>
>    if (list == NULL || *list != NULL ||
>        castFromAnyTypeFunc == NULL || freeFunc == NULL) {
>        ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, "Invalid argument");
>        return -1;
>    }
>
>    ...
>
>  failure:
>    freeFunc(&item);
>    freeFunc(list);
>
>
> >From 97c5e9c87eb99d881ee84c4f09bb943f3410d3b1 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering redhat com>
> Date: Tue, 15 Dec 2009 19:22:31 +0100
> Subject: [PATCH] esx_vi.c: avoid NULL deref for invalid inputs
>
> * src/esx/esx_vi.c (esxVI_List_CastFromAnyType): For invalid
> inputs, fail right away.  Do not "goto failure" where a NULL
> input pointer would be dereferenced.
> ---
>  src/esx/esx_vi.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
> index 5725b2f..1138e8c 100644
> --- a/src/esx/esx_vi.c
> +++ b/src/esx/esx_vi.c
> @@ -935,7 +935,7 @@ esxVI_List_CastFromAnyType(virConnectPtr conn, esxVI_AnyType *anyType,
>     if (list == NULL || *list != NULL ||
>         castFromAnyTypeFunc == NULL || freeFunc == NULL) {
>         ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, "Invalid argument");
> -        goto failure;
> +        return -1;
>     }

Okay, this fixes the NULL-deref.

>     if (anyType == NULL) {
> @@ -946,7 +946,7 @@ esxVI_List_CastFromAnyType(virConnectPtr conn, esxVI_AnyType *anyType,
>         ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR,
>                      "Expecting type to begin with 'ArrayOf' but found '%s'",
>                      anyType->other);
> -        goto failure;
> +        return -1;

This change isn't necessary to fix the NULL-deref bug, but it's okay
because there it nothing to cleanup at this point.

>     }
>
>     for (childNode = anyType->_node->xmlChildrenNode; childNode != NULL;
> --
> 1.6.6.rc2.275.g51e2d
>

ACK.

Matthias


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]