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

The trouble here is that "goto failure" would run this code:  failure:    VIR_FREE(*datastoreName);    VIR_FREE(*directoryName);    VIR_FREE(*fileName);    result = -1;    goto cleanup; And thus if any of those 3 input variables was NULL, that deref would probably provoke a segfault. By the way, that interface should be changed, because it encourages risky behavior:  int  esxUtil_ParseDatastoreRelatedPath(virConnectPtr conn,                                    const char *datastoreRelatedPath,                                    char **datastoreName,                                    char **directoryName, char **fileName) Note how it takes a buffer, datastoreRelatedPath, but with no length parameter.  Then this function writes into the buffer with this code:    if (sscanf(datastoreRelatedPath, "[%a[^]%]] %a[^\n]", datastoreName,               &directoryAndFileName) != 2) {

which cannot even check for buffer overrun because it doesn't know the length. You might want to add a new parameter, buf_len:  int  esxUtil_ParseDatastoreRelatedPath(virConnectPtr conn,                                    const char *datastoreRelatedPath,                                    size_t buf_len,                                    char **datastoreName,                                    char **directoryName, char **fileName) >From aba304e07835c123bd63f1d5d6777a898916349f Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com&gt; Date: Tue, 15 Dec 2009 19:08:49 +0100 Subject: [PATCH] esx_util.c: avoid NULL deref for invalid inputs * src/esx/esx_util.c (esxUtil_ParseDatastoreRelatedPath): Return right away, rather than trying to clean up and dereference NULL pointers, if any input is invalid. ---  src/esx/esx_util.c |    4 ++--  1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c index 3e53921..8703ac2 100644 --- a/src/esx/esx_util.c +++ b/src/esx/esx_util.c @@ -277,7 +277,7 @@ esxUtil_ParseDatastoreRelatedPath(virConnectPtr conn,         directoryName == NULL || *directoryName != NULL ||         fileName == NULL || *fileName != NULL) {         ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR, "Invalid argument"); -        goto failure; +        return -1;     }

    /* @@ -299,7 +299,7 @@ esxUtil_ParseDatastoreRelatedPath(virConnectPtr conn,         ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR,                   "Datastore related path '%s' doesn't have expected format "                   "'[<datastore>] <path>'", datastoreRelatedPath); -        goto failure; +        return -1;     }

    /* Split <path> into <directory>/<file>, where <directory> is optional */ -- 1.6.6.rc2.275.g51e2d