2009/12/16 Jim Meyering <jim@xxxxxxxxxxxx>:
> 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@xxxxxxxxxx>
> 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
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list