On Mon, Apr 14, 2025 at 13:20:55 +0100, Daniel P. Berrangé via Devel wrote:
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.fedin@fobos-nt.ru> Signed-off-by: Alexander Kuznetsov <kuznetsovam@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.
IIUC the NSS module code is supposed to be glib-free to avoid pulling it into the code that uses lookups.