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(a)fobos-nt.ru&gt; Signed-off-by: Alexander Kuznetsov <kuznetsovam(a)altlinux.org&gt; --- 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);

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(a)fobos-nt.ru&gt; > > Signed-off-by: Alexander Kuznetsov <kuznetsovam(a)altlinux.org&gt; > > --- > > 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.

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(a)fobos-nt.ru&gt; Signed-off-by: Alexander Kuznetsov <kuznetsovam(a)altlinux.org&gt; --- 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);

} errno = 0; -- 2.42.4

path is allocated by asprintf() and must be freed later if realloc() fails Restructure the code to allocate path only after realloc() succeeds, avoiding the need for an extra free() Found by Linux Verification Center (linuxtesting.org) with Svace. Reported-by: Dmitry Fedin <d.fedin(a)fobos-nt.ru&gt; Signed-off-by: Alexander Kuznetsov <kuznetsovam(a)altlinux.org&gt; --- tools/nss/libvirt_nss.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c index d79a00a1b0..a1f33ee27b 100644 --- a/tools/nss/libvirt_nss.c +++ b/tools/nss/libvirt_nss.c @@ -137,12 +137,13 @@ findLease(const char *name, if (dlen >= 7 && !strcmp(entry->d_name + dlen - 7, ".status")) { char **tmpLease; - if (asprintf(&path, "%s/%s", leaseDir, entry->d_name) < 0) - goto cleanup; - tmpLease = realloc(leaseFiles, sizeof(char *) * (nleaseFiles + 1)); if (!tmpLease) goto cleanup; + + if (asprintf(&path, "%s/%s", leaseDir, entry->d_name) < 0) + goto cleanup; +

leaseFiles = tmpLease;

leaseFiles[nleaseFiles++] = path; #if defined(LIBVIRT_NSS_GUEST)