From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1YjkO3-0003YY-84 for mharc-grub-devel@gnu.org; Sun, 19 Apr 2015 04:15:31 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36052) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YjkO0-0003YN-Sq for grub-devel@gnu.org; Sun, 19 Apr 2015 04:15:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YjkNx-0005yT-9X for grub-devel@gnu.org; Sun, 19 Apr 2015 04:15:28 -0400 Received: from mail-lb0-x22f.google.com ([2a00:1450:4010:c04::22f]:34836) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YjkNw-0005yP-Sb for grub-devel@gnu.org; Sun, 19 Apr 2015 04:15:25 -0400 Received: by lbbuc2 with SMTP id uc2so110231912lbb.2 for ; Sun, 19 Apr 2015 01:15:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type:content-transfer-encoding; bh=mDiM2i7dyc5fTtKnb1Diu2IjMhf9iApgz6y/c7PLnxU=; b=pluJvxtDxhPUBVT9cMrqfPLUBL3KPsC/Ex0I00NYAA+URDUKQ0V1N1zWbEVLifRw9H IAO6b+f/mBB2IT9X5waXjYhqspI/pDtULgbf8jXsb9Q1Fc3r7MR18ERVShjoa7Y37+aT cQlbzyzpe3jw1gAvGQ4MZnemKcYuou7vBjJqOEkOfv4UehWIrD5i5x5cpth1MEtgki3N eAz2vD+GNOREU2Z0DYGRnR/zVfOkGofMGa63I7DUUouQujcAma51HHcLFYxl+juK0xWp Ee29vNTD/qvptc5PEiOxgDYn8uLHR6DTWsXCELB2s1uhvyLE8YmOmHxD7uSVMRtOe2Qn kWfQ== X-Received: by 10.112.16.97 with SMTP id f1mr11041424lbd.26.1429431324137; Sun, 19 Apr 2015 01:15:24 -0700 (PDT) Received: from opensuse.site (ppp91-76-14-38.pppoe.mtu-net.ru. [91.76.14.38]) by mx.google.com with ESMTPSA id t8sm3535988lby.24.2015.04.19.01.15.22 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 19 Apr 2015 01:15:23 -0700 (PDT) Date: Sun, 19 Apr 2015 11:15:21 +0300 From: Andrei Borzenkov To: Michael Chang Subject: Re: [PATCH 1/3] Added net_bootp6 command Message-ID: <20150419111521.039880c4@opensuse.site> In-Reply-To: <20150417050427.GA8017@linux-dsax.tai.apac.novell.com> References: <1429088709-924-1-git-send-email-mchang@suse.com> <1429088709-924-2-git-send-email-mchang@suse.com> <20150416174056.020fd96d@opensuse.site> <20150417050427.GA8017@linux-dsax.tai.apac.novell.com> X-Mailer: Claws Mail 3.11.0 (GTK+ 2.24.27; x86_64-suse-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2a00:1450:4010:c04::22f Cc: grub-devel@gnu.org 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: Sun, 19 Apr 2015 08:15:30 -0000 =D0=92 Fri, 17 Apr 2015 13:04:27 +0800 Michael Chang =D0=BF=D0=B8=D1=88=D0=B5=D1=82: > >=20 > > > +static const struct grub_dhcpv6_option* > > > +find_dhcpv6_option (const struct grub_net_dhcpv6_packet *packet, > > > + grub_uint16_t option) > > > +{ > > > + grub_uint16_t code, len; > > > + const struct grub_dhcpv6_option *popt; > > > + > > > + popt =3D (const struct grub_dhcpv6_option *)packet->dhcp_options; > > > + code =3D grub_be_to_cpu16 (popt->code); > > > + len =3D grub_be_to_cpu16 (popt->len); > > > + > > > + while (0 !=3D code && option !=3D code) > >=20 > > This probably needs upper boundary check in case we are dealing with > > corrupted packets. >=20 >=20 > Do you mean checking the option length can't exceed netbuff length? >=20 Yes (and in all other cases below as well). > The ( 0!=3D code ) did certain kind of upper boundary check for me, but > I know you may disagree that. :) >=20 > Btw, in grub-core/net/ip.c::handle_dgram() has checksum for the receiving > packets, so the corrputed packets wouldn't sneak in easily. >=20 Correct checksum just means content was not altered; it does not mean content was correct to start with. > > > + > > > + ip_start =3D ip_end =3D NULL; > > > + ip_start =3D bootfile_url + grub_strlen(pr); > > > + > > > + if (*ip_start !=3D '[') > > > + ip_start =3D NULL; > > > + else > > > + ip_end =3D grub_strchr (++ip_start, ']'); > > > + > > > + if (!ip_start || !ip_end) > > > + { > > > + grub_error (GRUB_ERR_IO, N_("IPv6-address not in square bracke= ts")); > > > + goto cleanup; > > > + } > > > + > >=20 > > Can bootfile_url contain a name and not IPv6 address? Or is address > > mandatory? >=20 > Yes. The string in URL form is mandatory. > I probably was not clear. Must it always be IPv6 address literal as implied by code or can it be e.g. DNS name? > > > +struct grub_net_network_level_interface * > > > +grub_net_configure_by_dhcpv6_reply (const char *name, > > > + struct grub_net_card *card, > > > + grub_net_interface_flags_t flags, > > > + const struct grub_net_dhcpv6_packet *v6, > > > + grub_size_t size __attribute__ ((unused)), > > > + int is_def, > > > + char **device, char **path) > > > +{ > > > + grub_net_network_level_address_t addr; > > > + grub_net_network_level_netaddress_t netaddr; > > > + struct grub_net_network_level_interface *inf; > > > + const grub_uint8_t *your_ip; > > > + char *proto; > > > + char *server_ip; > > > + char *boot_file; > > > + grub_net_network_level_address_t *dns; > > > + grub_uint16_t num_dns; > > > + > > > + if (device) > > > + *device =3D NULL; > > > + > > > + if (path) > > > + *path =3D NULL; > > > + > > > + if (v6->message_type !=3D DHCPv6_REPLY) > >=20 > > Is it really possible? We come here if packet is DHCPv6_REPLY only. >=20 > I'm in case some evil firmware to cache some other packets type rather > than reply (or even junks in it ..). > Yes, but what I mean - this function is only called when we already checked for message_type =3D=3D DHCPv6_REPLY. So the only reason it can be different here is physical memory corruption.