Message ID | 20210728182331.1363922-1-rharwood@redhat.com |
---|---|
State | New |
Headers | show |
Series | [v2,1/1] nis: Fix leak on realloc failure in nis_getnames [BZ #28150] | expand |
On 7/28/21 2:23 PM, Robbie Harwood via Libc-alpha wrote: > If pos >= count but realloc fails, tmp will not have been placed in > getnames[pos] yet, and so will not be freed in free_null. Detected > by Coverity. > > Also remove misleading comment from nis_getnames(), since it actually > did properly release getnames when out of memory. The CI/CD patchwork trybot didn't trigger for this patch and I need to review that with DJ. I tested on x86_64 and i686 without regression. Florian reviewed. I'm pushing this fix. Tested-by: Carlos O'Donell <carlos@redhat.com> > --- > nis/nis_subr.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/nis/nis_subr.c b/nis/nis_subr.c > index dd0e30071d..6784fc353f 100644 > --- a/nis/nis_subr.c > +++ b/nis/nis_subr.c > @@ -103,9 +103,6 @@ count_dots (const_nis_name str) > return count; > } > > -/* If we run out of memory, we don't give already allocated memory > - free. The overhead for bringing getnames back in a safe state to > - free it is to big. */ > nis_name * > nis_getnames (const_nis_name name) > { > @@ -271,7 +268,10 @@ nis_getnames (const_nis_name name) > nis_name *newp = realloc (getnames, > (count + 1) * sizeof (char *)); > if (__glibc_unlikely (newp == NULL)) > - goto free_null; > + { > + free (tmp); > + goto free_null; > + } > getnames = newp; > } > getnames[pos] = tmp; >
diff --git a/nis/nis_subr.c b/nis/nis_subr.c index dd0e30071d..6784fc353f 100644 --- a/nis/nis_subr.c +++ b/nis/nis_subr.c @@ -103,9 +103,6 @@ count_dots (const_nis_name str) return count; } -/* If we run out of memory, we don't give already allocated memory - free. The overhead for bringing getnames back in a safe state to - free it is to big. */ nis_name * nis_getnames (const_nis_name name) { @@ -271,7 +268,10 @@ nis_getnames (const_nis_name name) nis_name *newp = realloc (getnames, (count + 1) * sizeof (char *)); if (__glibc_unlikely (newp == NULL)) - goto free_null; + { + free (tmp); + goto free_null; + } getnames = newp; } getnames[pos] = tmp;