Re: [PATCH] nscd: Drop local address tuple variable [BZ #29607]

[Carlos O'Donell <carlos@redhat.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
>