[v1] nscd: Fix double free in netgroupcache [BZ #27462]
Commit Message
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
---
nscd/netgroupcache.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Comments
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.
@@ -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