Re: [libvirt] [PATCH] esx_util.c: avoid NULL deref for invalid inputs
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.
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(a)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