| Message ID | 20221004000657.1940145-1-siddhesh@sourceware.org |
|---|---|
| State | New |
| Headers | show |
| Series | nscd: Drop local address tuple variable [BZ #29607] | expand |
On 2022-10-04 02:06, Siddhesh Poyarekar wrote: > When a request needs to be resent (e.g. due to insufficient buffer > space), the references to subsequent tuples in the local variable are > stale and should not be used. This used to work by accident before, but > since 1d495912a it no longer does. Instead of trying to reset it, just > let gethostbyname4_r write into TUMPBUF6 for us, thus maintaining a > consistent state at all times. This is now consistent with what is done > in gaih_inet for getaddrinfo. > > Resolves: BZ #29607 > Reported-by: Holger Hoffstätte <holger@applied-asynchrony.com> > --- > > Tested on x86_64 with Fedora and nscd enabled. Testing with other > distributions would be really appreciated! > > Thanks, > Sid > > > nscd/aicache.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/nscd/aicache.c b/nscd/aicache.c > index 51e793199f..e0baed170b 100644 > --- a/nscd/aicache.c > +++ b/nscd/aicache.c > @@ -110,11 +110,10 @@ addhstaiX (struct database_dyn *db, int fd, request_header *req, > "gethostbyname4_r"); > if (fct4 != NULL) > { > - struct gaih_addrtuple atmem; > struct gaih_addrtuple *at; > while (1) > { > - at = &atmem; > + at = NULL; > rc6 = 0; > herrno = 0; > status[1] = DL_CALL_FCT (fct4, (key, &at, > @@ -137,7 +136,7 @@ addhstaiX (struct database_dyn *db, int fd, request_header *req, > goto next_nip; > > /* We found the data. Count the addresses and the size. */ > - for (const struct gaih_addrtuple *at2 = at = &atmem; at2 != NULL; > + for (const struct gaih_addrtuple *at2 = at; at2 != NULL; > at2 = at2->next) > { > ++naddrs; > Hello Siddesh, I am happy to report that this seems to be working fine and fixes the reported bug; there haven't been any nscd crashes for a few hours now on any of my machines. Thanks! Holger
On 2022-10-04 02:49, Holger Hoffstätte wrote: > I am happy to report that this seems to be working fine and fixes the > reported bug; > there haven't been any nscd crashes for a few hours now on any of my > machines. > Thanks! Thank you for confirming Holger! Tested-by: Holger Hoffstätte <holger@applied-asynchrony.com>
On Mon, Oct 03, 2022 at 08:06:57PM -0400, Siddhesh Poyarekar via Libc-alpha wrote: > When a request needs to be resent (e.g. due to insufficient buffer > space), the references to subsequent tuples in the local variable are > stale and should not be used. This used to work by accident before, but > since 1d495912a it no longer does. Instead of trying to reset it, just > let gethostbyname4_r write into TUMPBUF6 for us, thus maintaining a > consistent state at all times. This is now consistent with what is done > in gaih_inet for getaddrinfo. LGTM. Reviewed-by: Carlos O'Donell <carlos@redhat.com> > Resolves: BZ #29607 > Reported-by: Holger Hoffstätte <holger@applied-asynchrony.com> > --- > > Tested on x86_64 with Fedora and nscd enabled. Testing with other > distributions would be really appreciated! > > Thanks, > Sid > > > nscd/aicache.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/nscd/aicache.c b/nscd/aicache.c > index 51e793199f..e0baed170b 100644 > --- a/nscd/aicache.c > +++ b/nscd/aicache.c > @@ -110,11 +110,10 @@ addhstaiX (struct database_dyn *db, int fd, request_header *req, > "gethostbyname4_r"); > if (fct4 != NULL) > { > - struct gaih_addrtuple atmem; OK. Remove the single static struct (no longer used). > struct gaih_addrtuple *at; > while (1) > { > - at = &atmem; > + at = NULL; OK. Start with at NULL. > rc6 = 0; > herrno = 0; > status[1] = DL_CALL_FCT (fct4, (key, &at, > @@ -137,7 +136,7 @@ addhstaiX (struct database_dyn *db, int fd, request_header *req, > goto next_nip; > > /* We found the data. Count the addresses and the size. */ > - for (const struct gaih_addrtuple *at2 = at = &atmem; at2 != NULL; > + for (const struct gaih_addrtuple *at2 = at; at2 != NULL; OK. Remove &atmem. We set at2 to at. In all of these cases the memory is created by alloc_create_buffer() and the memory for the gaih_addrtuple comes from that buffer. We don't want to reset to &atmem sine this is no longer correct. The allocation is done at the lowest level in fct4. > at2 = at2->next) > { > ++naddrs; > -- > 2.37.2 >
diff --git a/nscd/aicache.c b/nscd/aicache.c index 51e793199f..e0baed170b 100644 --- a/nscd/aicache.c +++ b/nscd/aicache.c @@ -110,11 +110,10 @@ addhstaiX (struct database_dyn *db, int fd, request_header *req, "gethostbyname4_r"); if (fct4 != NULL) { - struct gaih_addrtuple atmem; struct gaih_addrtuple *at; while (1) { - at = &atmem; + at = NULL; rc6 = 0; herrno = 0; status[1] = DL_CALL_FCT (fct4, (key, &at, @@ -137,7 +136,7 @@ addhstaiX (struct database_dyn *db, int fd, request_header *req, goto next_nip; /* We found the data. Count the addresses and the size. */ - for (const struct gaih_addrtuple *at2 = at = &atmem; at2 != NULL; + for (const struct gaih_addrtuple *at2 = at; at2 != NULL; at2 = at2->next) { ++naddrs;