On Mon, Apr 14, 2025 at 03:06:09PM +0300, Alexander Kuznetsov wrote:
> path is allocated by asprintf() and must be freed later if realloc() fails or
> at
> the end of each while() iteration
>
> Move the free() call out of LIBVIRT_NSS_GUEST macro and add another one if
> realloc() fails
>
> Found by Linux Verification Center (linuxtesting.org) with Svace.
>
> Reported-by: Dmitry Fedin <d.fe...@fobos-nt.ru>
> Signed-off-by: Alexander Kuznetsov <kuznetso...@altlinux.org>
> ---
> tools/nss/libvirt_nss.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c
> index d79a00a1b0..190cc7a3dd 100644
> --- a/tools/nss/libvirt_nss.c
> +++ b/tools/nss/libvirt_nss.c
> @@ -141,8 +141,11 @@ findLease(const char *name,
> goto cleanup;
>
> tmpLease = realloc(leaseFiles, sizeof(char *) * (nleaseFiles +
> 1));
> - if (!tmpLease)
> + if (!tmpLease) {
> + free(path);
> goto cleanup;
> + }
> +
> leaseFiles = tmpLease;
> leaseFiles[nleaseFiles++] = path;
> #if defined(LIBVIRT_NSS_GUEST)
> @@ -155,8 +158,8 @@ findLease(const char *name,
> free(path);
> goto cleanup;
> }
> - free(path);
> #endif /* LIBVIRT_NSS_GUEST */
> + free(path);
AFAICT in this 'else if' branch, 'path' is only allocated inside the
'LIBVIRT_NSS_GUEST' condition, so movnig the free outside is pointless.
Rather than adding more "free" calls, I think this code would be
improved by declaring 'path' with "g_autofree" and using
g_steal_pointer when assigning it to 'leaseFiles[nleaseFiles++]',
then existing 'free' calls can be removed.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
