Re: [PATCH] getaddrinfo: Fix leak with AI_ALL [BZ #28852]

[Florian Weimer <fweimer@redhat.com>]

* Siddhesh Poyarekar via Libc-alpha:

> diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
> index 18dccd5924..652a1a43d4 100644
> --- a/sysdeps/posix/getaddrinfo.c
> +++ b/sysdeps/posix/getaddrinfo.c
> @@ -199,9 +199,6 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
>  				   struct hostent *h,
>  				   struct gaih_addrtuple **result)
>  {
> -  while (*result)
> -    result = &(*result)->next;
> -
>    /* Count the number of addresses in h->h_addr_list.  */
>    size_t count = 0;
>    for (char **p = h->h_addr_list; *p != NULL; ++p)
> @@ -212,10 +209,30 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
>    if (count == 0 || h->h_length > sizeof (((struct gaih_addrtuple) {}).addr))
>      return true;
>  
> -  struct gaih_addrtuple *array = calloc (count, sizeof (*array));
> +  struct gaih_addrtuple *array = *result;
> +  size_t old = 0;
> +
> +  while (array)
> +    {
> +      old++;
> +      array = array->next;
> +    }
> +
> +  array = realloc (*result, (old + count) * sizeof (*array));
> +
>    if (array == NULL)
>      return false;
>  
> +  *result = array;
> +
> +  /* Update the next pointers on reallocation.  */
> +  for (size_t i = 0; i < old; i++)
> +    array[i].next = array + i + 1;
> +
> +  array += old;
> +
> +  memset (array, 0, count * sizeof (*array));
> +
>    for (size_t i = 0; i < count; ++i)
>      {
>        if (family == AF_INET && req->ai_family == AF_INET6)
> @@ -235,7 +252,6 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
>    array[0].name = h->h_name;
>    array[count - 1].next = NULL;
>  
> -  *result = array;
>    return true;
>  }

I think this assumes that the addrmem block (now managed by realloc) is
always at the end of the “at” tuples list (which is not always backed by
addrmem memory).  If that is not the case, the tail after the addrmem
block is lost.

Reading the code, I can't really see if this assumption is true or
not. 8-(

Thanks,
Florian