From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1aUch7-0004xI-0Q for mharc-grub-devel@gnu.org; Sat, 13 Feb 2016 11:05:13 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55223) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aUch5-0004wp-4R for grub-devel@gnu.org; Sat, 13 Feb 2016 11:05:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aUch1-0000eu-3A for grub-devel@gnu.org; Sat, 13 Feb 2016 11:05:11 -0500 Received: from mail-lf0-x244.google.com ([2a00:1450:4010:c07::244]:35146) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aUch0-0000eP-S5 for grub-devel@gnu.org; Sat, 13 Feb 2016 11:05:07 -0500 Received: by mail-lf0-x244.google.com with SMTP id j99so5521700lfi.2 for ; Sat, 13 Feb 2016 08:05:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to:content-type:content-transfer-encoding; bh=+AHBLO2y9CctVzfG0LW1e4zH+1jO2XW0guATxOOgmpM=; b=Sp9OXp0hH6FWJsgULo8yr2wfnHiy3Idsi0tE9LqlZtqE5MYSDwR5xaRzH+ls8QUUBc 0c3Wrx09q+zxEQLfOIagkdARtu2i3yDymZHWEmmpZm0Fb/S52UCnsze9mqsrmiWthmr9 2fpKkF61kAdGUuT/ugC/YsPxoc0Zu44jD+wAKY2Sx2fT4gyeeWnMIZ3FEXNgq6ymkAe7 pOVNK9uG7lF/cFFLDRLVnxv9FWgcZZ2bafFTeKN/mm4PPdMI1fr+MKYtTARmFH2W7XKg BFVzEOdFGjApPJvLlIkiyBmSy9aP25DVIpKDw62ntEHTNiDjCYADo5KSWdMPO0sD07W2 yvAA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-type :content-transfer-encoding; bh=+AHBLO2y9CctVzfG0LW1e4zH+1jO2XW0guATxOOgmpM=; b=cox7G3UA5uixRF+Am/Lj3V8bPJwMNljrdhkdJxEVl9eK/ZwyFH1UePudUwL45YoQEL Fl6ZvRkpA5yEjRpaHIUHZztjaGLt99JYqgO5l2XloPXszBflQeLi4HTccbR+OdZ/An7R cEiEt0EZb2Mqoez68EkcwwlBdlT3Cr8eIHtmQbtNxj4OhRKKOP0RbuFHHA3o2q6vIunS +2uxn5O1KJzjfNZeJs8bv6rVHWY1bfmvfgjjC1gK+RVHIQ4biyEP+Eip12I/o/Q02uKs 7/xX+IHlNXfFfMM918ss3yY3oZ7QtSKOHHTbuKUh18YuoLCk5Aj5ML1uvUOXkAeKMD4E Xmhw== X-Gm-Message-State: AG10YOQJ75XpIcOhBtMvKXmiFJYSxwKx7RrcZAWmIttiIeYny6oyHJE1BXTcS/3ytN4m9w== X-Received: by 10.25.19.156 with SMTP id 28mr3193915lft.92.1455379505879; Sat, 13 Feb 2016 08:05:05 -0800 (PST) Received: from [192.168.1.41] (ppp109-252-76-159.pppoe.spdop.ru. [109.252.76.159]) by smtp.gmail.com with ESMTPSA id v140sm2580795lfd.24.2016.02.13.08.05.04 (version=TLSv1/SSLv3 cipher=OTHER); Sat, 13 Feb 2016 08:05:04 -0800 (PST) Subject: Re: [PATCH 11/14] dns: reset data->naddresses for every packet we receive To: The development of GNU GRUB , kernel-team@fb.com References: <1455139268-3241273-1-git-send-email-jbacik@fb.com> <1455139268-3241273-12-git-send-email-jbacik@fb.com> From: Andrei Borzenkov Message-ID: <56BF5430.5000508@gmail.com> Date: Sat, 13 Feb 2016 19:05:04 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <1455139268-3241273-12-git-send-email-jbacik@fb.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2a00:1450:4010:c07::244 Cc: Josef Bacik X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 13 Feb 2016 16:05:12 -0000 11.02.2016 00:21, Josef Bacik пишет: > I noticed when debugging a problem that we'd corrupt memory if our dns server > didn't respond fast enough and we ended up asking for both an AAAA and A record > for a server. The problem is we alloc data->addresses based on the number of > addresses in the packet, but we populate it based on data->naddresses. So we > get the AAAA record with one address, and we add that, then we get the A record > with one address and now data->naddresses == 1 but the ancount is 1, so we > allocate data->addresses to hold one address but write the new address outside > the array. We also leak the old addresses memory. So fix this by noticing if > we already have an address and free the old memory and reset naddresses so we > don't overflow our new array. > > Signed-off-by: Josef Bacik > --- > grub-core/net/dns.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/grub-core/net/dns.c b/grub-core/net/dns.c > index 86e609b..7a6c4b4 100644 > --- a/grub-core/net/dns.c > +++ b/grub-core/net/dns.c > @@ -276,6 +276,9 @@ recv_hook (grub_net_udp_socket_t sock __attribute__ ((unused)), > ptr++; > ptr += 4; > } > + if (*data->naddresses) > + grub_free (*data->addresses); > + *data->naddresses = 0; > *data->addresses = grub_malloc (sizeof ((*data->addresses)[0]) > * grub_be_to_cpu16 (head->ancount)); Hmm ... cannot we resize it? *data->addresses = grub_realloc (*data->addresses, sizeof ((*data->addresses)[0]) * (*data->naddresses += grub_be_to_cpu16 (head->ancount))) as adjusted to not leak old pointer. This way answers we got before would not be lost. > if (!*data->addresses) >