07.07.2016 12:18, Michael Chang пишет:
> Since commit f9d1b4422efb2c06e5472fb2c304712e2029796b I occasionally bumped
> into heap corruption problem during dns lookup.
> 
> After tracing the issue, it looks the *data->addresses array is not correctly
> allocated. It need to hold accumulated dns look up result but not only the new
> result in new message. The heap corruption occured when appending new result to
> it.
> 
> This patch fixed the issue for me by reallocating the array if it found too
> small to hold all the result. 
> 

I'm not sure. I think we discussed this with Josef back then. The code
apparently was assuming single response; and if we are going to collect
multiple answers, we need to filter out duplicates at least and also not
depend on packet order to select between A and AAAA.

Does attached patch fix corruption for you? I think that is the least
intrusive as bug fix, and we need to revisit code to properly handle
multiple responses later.

> Thanks,
> 
> ---
>  grub-core/net/dns.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/grub-core/net/dns.c b/grub-core/net/dns.c
> index 89741dd..b8d8873 100644
> --- a/grub-core/net/dns.c
> +++ b/grub-core/net/dns.c
> @@ -276,14 +276,25 @@ recv_hook (grub_net_udp_socket_t sock __attribute__ ((unused)),
>        ptr++;
>        ptr += 4;
>      }
> -  *data->addresses = grub_malloc (sizeof ((*data->addresses)[0])
> -				 * grub_be_to_cpu16 (head->ancount));
> -  if (!*data->addresses)
> +
> +  if (ALIGN_UP (grub_be_to_cpu16 (head->ancount) + *data->naddresses, 4) > ALIGN_UP (*data->naddresses, 4))
>      {
> -      grub_errno = GRUB_ERR_NONE;
> -      grub_netbuff_free (nb);
> -      return GRUB_ERR_NONE;
> +      grub_net_network_level_address_t *old_addresses = *data->addresses;
> +      *data->addresses = grub_malloc (sizeof ((*data->addresses)[0])
> +				 * ALIGN_UP (grub_be_to_cpu16 (head->ancount) + *data->naddresses, 4));
> +      if (!*data->addresses)
> +	{
> +	  grub_errno = GRUB_ERR_NONE;
> +	  grub_netbuff_free (nb);
> +	  return GRUB_ERR_NONE;
> +	}
> +      if (*data->naddresses)
> +	{
> +	  grub_memcpy (*data->addresses, old_addresses, sizeof ((*data->addresses)[0]) * (*data->naddresses));
> +	  grub_free (old_addresses);
> +	}
>      }
> +
>    reparse_ptr = ptr;
>   reparse:
>    for (i = 0, ptr = reparse_ptr; i < grub_be_to_cpu16 (head->ancount); i++)
>