2009/12/16 Jim Meyering <jim@meyering.net>:
Jim Meyering wrote:
if (sscanf(datastoreRelatedPath, "[%a[^]%]] %a[^\n]", datastoreName, &directoryAndFileName) != 2) { ... I suppose you know that %a[...] is non-standard (it's a glibc-specific addition) and that this esx code need only compile on systems with glibc.
No, I wasn't aware of that. The ESX driver is a libvirt client side driver and should be compilable on non-linux platforms too. I think I can replace the two sscanf calls with some strchr/strstr and strndup calls.
I confirm that with my change that would have leaked. Though note that it would do so only when parsing exactly one token.
This fix is not correct, it may leak memory allocated by sscanf. goto failure is correct and safe here, because the first if block in this function has check the char ** pointers to be not-NULL, so the VIR_FREEs after the failure label is safe.
Good catch. I'll adjust and resend.
I've folded in this correction:
diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c index 8703ac2..35a48e0 100644 --- a/src/esx/esx_util.c +++ b/src/esx/esx_util.c @@ -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); - return -1; + goto failure; }
/* Split <path> into <directory>/<file>, where <directory> is optional */
Here's the result:
From f34c4395b9b16965705f9158ac90e59c37b04507 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> 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 for invalid inputs, rather than using them (which would dereference NULL pointers) in clean-up code. --- src/esx/esx_util.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c index 3e53921..35a48e0 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; }
/* -- 1.6.6.rc2.275.g51e2d
ACK. Matthias