Message ID | xnpn0n4zrd.fsf@greed.delorie.com |
---|---|
State | New |
Headers | show |
Series | [v1] nscd: Fix double free in netgroupcache [BZ #27462] | expand |
On 2/26/21 2:43 AM, DJ Delorie via Libc-alpha wrote: > > In commit 745664bd798ec8fd50438605948eea594179fba1 a use-after-free > was fixed, but this led to an occasional double-free. This patch > tracks the "live" allocation better. > > Tested manually by a third party. > > Related: RHBZ 1927877 Looks fine to me. Now that we have container testing, we should add tests for nscd, especially since nscd doesn't seem to be going anywhere. Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org> Siddhesh
On 2/25/21 4:13 PM, DJ Delorie via Libc-alpha wrote: > > In commit 745664bd798ec8fd50438605948eea594179fba1 a use-after-free > was fixed, but this led to an occasional double-free. This patch > tracks the "live" allocation better. > > Tested manually by a third party. This looks like it should be logically the correct fix. There are only two xrealloc's that I see that could impact the buffer reuse here and we need to track the update to the pointer. LGTM. Reviewed-by: Carlos O'Donell <carlos@redhat.com> > Related: RHBZ 1927877 > --- > nscd/netgroupcache.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/nscd/netgroupcache.c b/nscd/netgroupcache.c > index dba6ceec1b..ad2daddafd 100644 > --- a/nscd/netgroupcache.c > +++ b/nscd/netgroupcache.c > @@ -248,7 +248,7 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req, > : NULL); > ndomain = (ndomain ? newbuf + ndomaindiff > : NULL); > - buffer = newbuf; > + *tofreep = buffer = newbuf; > } > > nhost = memcpy (buffer + bufused, > @@ -319,7 +319,7 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req, > else if (status == NSS_STATUS_TRYAGAIN && e == ERANGE) > { > buflen *= 2; > - buffer = xrealloc (buffer, buflen); > + *tofreep = buffer = xrealloc (buffer, buflen); > } > else if (status == NSS_STATUS_RETURN > || status == NSS_STATUS_NOTFOUND >
Please add the CVE reference. Andreas.
diff --git a/nscd/netgroupcache.c b/nscd/netgroupcache.c index dba6ceec1b..ad2daddafd 100644 --- a/nscd/netgroupcache.c +++ b/nscd/netgroupcache.c @@ -248,7 +248,7 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req, : NULL); ndomain = (ndomain ? newbuf + ndomaindiff : NULL); - buffer = newbuf; + *tofreep = buffer = newbuf; } nhost = memcpy (buffer + bufused, @@ -319,7 +319,7 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req, else if (status == NSS_STATUS_TRYAGAIN && e == ERANGE) { buflen *= 2; - buffer = xrealloc (buffer, buflen); + *tofreep = buffer = xrealloc (buffer, buflen); } else if (status == NSS_STATUS_RETURN || status == NSS_STATUS_NOTFOUND