[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++)
>
dns-heap-corruption.patch
Description: Text Data