grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] dns: fix heap corruption


From: Andrei Borzenkov
Subject: Re: [PATCH] dns: fix heap corruption
Date: Fri, 8 Jul 2016 22:54:37 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0

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++)
> 

Attachment: dns-heap-corruption.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]