From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1Yikyf-0006gv-Qm for mharc-grub-devel@gnu.org; Thu, 16 Apr 2015 10:41:13 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55736) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YikyZ-0006f5-Ng for grub-devel@gnu.org; Thu, 16 Apr 2015 10:41:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YikyT-0002x1-9t for grub-devel@gnu.org; Thu, 16 Apr 2015 10:41:07 -0400 Received: from mail-la0-x232.google.com ([2a00:1450:4010:c03::232]:35910) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YikyS-0002wX-In for grub-devel@gnu.org; Thu, 16 Apr 2015 10:41:01 -0400 Received: by lagv1 with SMTP id v1so58419277lag.3 for ; Thu, 16 Apr 2015 07:40:59 -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=6aMn59elAPwpCRWXw3SyvyEmTgMp4q40MkcFvQO/NhA=; b=TkDmk6S7r7eAscJj7sIJnFnZPW4Hj3tr+2kXmd183DQlm6UqXn4oAKsXmBvuBxwdh+ e4l6RYX4NzjW7kTV0I33tg7lHGg6H0YhEYpnnXlUNOMPT3g/esaIZjQwDtD92dRMz1tN 4Yt2q/GG9zn1okiZyPu25nOYwWfZm/aWOx3e9Im+fVLJbyI+MsLwL3tnZWh3Zth+zKdB +l9IkbdBw4342fh7Zq9ZdkfAg9atfsK6v0gEi4k5ssfiBXRBKaOqEI2Q3o4QALzfKHjE XOT2HC4erbAzLny2gz5kJBs478/iMalFCeBRKhz0Uu2I/CPtY5QHrfCIqlAoXvUpue91 ONog== X-Received: by 10.112.163.168 with SMTP id yj8mr28349617lbb.36.1429195259613; Thu, 16 Apr 2015 07:40:59 -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 4sm1728956lal.36.2015.04.16.07.40.57 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 16 Apr 2015 07:40:58 -0700 (PDT) Date: Thu, 16 Apr 2015 17:40:56 +0300 From: Andrei Borzenkov To: Michael Chang Subject: Re: [PATCH 1/3] Added net_bootp6 command Message-ID: <20150416174056.020fd96d@opensuse.site> In-Reply-To: <1429088709-924-2-git-send-email-mchang@suse.com> References: <1429088709-924-1-git-send-email-mchang@suse.com> <1429088709-924-2-git-send-email-mchang@suse.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:c03::232 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: Thu, 16 Apr 2015 14:41:11 -0000 =D0=92 Wed, 15 Apr 2015 17:05:07 +0800 Michael Chang =D0=BF=D0=B8=D1=88=D0=B5=D1=82: > The net_bootp6 is used to configure the ipv6 network interface through the > DHCPv6 protocol Solict/Advertise/Request/Reply. > --- > grub-core/net/bootp.c | 885 ++++++++++++++++++++++++++++++++++++++++++= +++++- > grub-core/net/ip.c | 35 ++ > include/grub/efi/api.h | 56 +++- > include/grub/net.h | 19 + > 4 files changed, 993 insertions(+), 2 deletions(-) >=20 > diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c > index 6136755..477f205 100644 > --- a/grub-core/net/bootp.c > +++ b/grub-core/net/bootp.c > @@ -24,6 +24,7 @@ > #include > #include > #include > +#include > =20 > static void > parse_dhcp_vendor (const char *name, const void *vend, int limit, int *m= ask) > @@ -256,6 +257,653 @@ grub_net_configure_by_dhcp_ack (const char *name, > return inter; > } > =20 > +struct grub_dhcpv6_option { > + grub_uint16_t code; > + grub_uint16_t len; > + grub_uint8_t data[0]; > +} GRUB_PACKED; > + > + > +struct grub_dhcpv6_iana_option { > + grub_uint32_t iaid; > + grub_uint32_t t1; > + grub_uint32_t t2; > + grub_uint8_t data[0]; > +} GRUB_PACKED; > + > +struct grub_dhcpv6_iaaddr_option { > + grub_uint8_t addr[16]; > + grub_uint32_t preferred_lifetime; > + grub_uint32_t valid_lifetime; > + grub_uint8_t data[0]; > +} GRUB_PACKED; > + > +struct grub_DUID_LL > +{ > + grub_uint16_t type; > + grub_uint16_t hw_type; > + grub_uint8_t hwaddr[6]; > +} GRUB_PACKED; > + > +struct grub_dhcpv6_dns_servers { > + grub_uint8_t addr[16]; > + grub_uint8_t next_addr[0]; > +} GRUB_PACKED; > + Do we really need it? Code just fetches 16 bytes blocks, it does not really need any structure definition? > +#define DHCPv6_REPLY 7 > +#define DHCPv6_ADVERTISE 2 > +#define DHCPv6_REQUEST 3 > +#define OPTION_BOOTFILE_URL 59 > +#define OPTION_DNS_SERVERS 23 > +#define OPTION_IA_NA 3 > +#define OPTION_IAADDR 5 > +#define OPTION_CLIENTID 1 > +#define OPTION_SERVERID 2 > +#define OPTION_ORO 6 > +#define OPTION_ELAPSED_TIME 8 > + This is better as enum and GRUB_ prefix. Also options need GRUB_DHCPv6_ prefix. > +struct grub_dhcpv6_session > +{ > + struct grub_dhcpv6_session *next; > + struct grub_dhcpv6_session **prev; > + grub_uint32_t iaid; > + grub_uint32_t transaction_id:24; > + grub_uint64_t start_time; > + struct grub_net_network_level_interface *ifaces; Can there be multiple interfaces as implied by plural? > +}; > + > +static struct grub_dhcpv6_session *grub_dhcpv6_sessions =3D NULL; > +#define FOR_DHCPV6_SESSIONS(var) \ > + for (var =3D grub_dhcpv6_sessions ; var; var =3D var->next) > + FOR_LIST_ELEMENTS > +static void > +grub_dhcpv6_session_add (struct grub_dhcpv6_session *session) > +{ > + struct grub_datetime date; > + grub_err_t err; > + grub_int32_t t =3D 0; > + > + err =3D grub_get_datetime (&date); > + if (err || !grub_datetime2unixtime (&date, &t)) > + { > + grub_errno =3D GRUB_ERR_NONE; > + t =3D 0; > + } > + > + session->transaction_id =3D t; > + session->start_time =3D grub_get_time_ms (); > + > + session->prev =3D &grub_dhcpv6_sessions; > + session->next =3D grub_dhcpv6_sessions; > + > + if (session->next) > + session->next->prev =3D &session->next; > + grub_list_push > + grub_dhcpv6_sessions =3D session; > + return; > +} > + > +static void > +grub_dhcpv6_session_remove (struct grub_dhcpv6_session *session) > +{ > + *session->prev =3D session->next; > + if (session->next) > + session->next->prev =3D session->prev; > + session->next =3D NULL; > + session->prev =3D NULL; > + return; > +} > + grub_list_remove > +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) This probably needs upper boundary check in case we are dealing with corrupted packets. > + { > + popt =3D (const struct grub_dhcpv6_option *)((grub_uint8_t *)popt + > + len + sizeof(*popt)); > + code =3D grub_be_to_cpu16 (popt->code); > + len =3D grub_be_to_cpu16 (popt->len); > + } > + > + if (option =3D=3D code) > + return popt; > + This can just be moved inside a loop, right? > + return NULL; > +} > + > +static const grub_uint8_t* > +find_dhcpv6_address (const struct grub_net_dhcpv6_packet *packet) > +{ > + const struct grub_dhcpv6_option* popt =3D find_dhcpv6_option (packet, = OPTION_IA_NA); > + const struct grub_dhcpv6_iana_option *ia_na; > + const struct grub_dhcpv6_option *iaaddr_hdr; > + const struct grub_dhcpv6_iaaddr_option *iaaddr; > + grub_uint16_t ia_na_data_offset, ia_na_data_len, len; > + > + if (grub_be_to_cpu16 (popt->code) !=3D OPTION_IA_NA) if (popt =3D=3D NULL)=20 > + { > + grub_error (GRUB_ERR_IO, N_("not an IA_NA DHCPv6 option")); > + return NULL; > + } > + > + ia_na =3D (const struct grub_dhcpv6_iana_option *)popt->data; > + > + if (grub_be_to_cpu16(popt->len) <=3D sizeof (*ia_na)) > + { > + grub_error (GRUB_ERR_IO, N_("invalid size for IAADDR")); > + return NULL; > + } > + Does it need upper boundary check? > + ia_na_data_len =3D grub_be_to_cpu16(popt->len) - sizeof (*ia_na); > + ia_na_data_offset =3D 0; > + > + iaaddr_hdr =3D (const struct grub_dhcpv6_option *) ia_na->data; > + len =3D grub_be_to_cpu16 (iaaddr_hdr->len); > + > + while (grub_be_to_cpu16(iaaddr_hdr->code) !=3D OPTION_IAADDR) grub_cpu_to_be16_compile_time(OPTION_IAADDR) > + { > + ia_na_data_offset +=3D (len + sizeof (*iaaddr_hdr)); > + > + if (ia_na_data_offset < ia_na_data_len) > + { > + iaaddr_hdr =3D(const struct grub_dhcpv6_option *)(ia_na->data + > + ia_na_data_offset); > + len =3D grub_be_to_cpu16 (iaaddr_hdr->len); > + } > + else > + { > + iaaddr_hdr =3D NULL; > + break; > + } > + } > + > + if (!iaaddr_hdr) > + { > + grub_error (GRUB_ERR_IO, N_("IAADDR not found")); > + return NULL; > + } > + > + if ((ia_na_data_offset + sizeof (*iaaddr_hdr) + len) > ia_na_data_len) > + { > + grub_error (GRUB_ERR_IO, N_("IAADDR size check failed")); > + return NULL; > + } > + > + iaaddr =3D (const struct grub_dhcpv6_iaaddr_option *) iaaddr_hdr->data; > + > + return iaaddr->addr; It sounds again like most code could be folded inside a loop while (remaining_length > 0) if (option_length > remaining_length) error if (option_code =3D=3D IAADDR) return hdr->addr hdr =3D (char *)hdr + option_length remaining_length -=3D option_length return not found > +} > + > +static void > +get_dhcpv6_dns_address (const struct grub_net_dhcpv6_packet *packet, > + grub_net_network_level_address_t **addr, grub_uint16_t *naddr) > +{ > + const struct grub_dhcpv6_option* popt; > + const struct grub_dhcpv6_dns_servers *dns; > + grub_uint16_t len; > + const grub_uint8_t *pa; > + int i, ln; > + grub_net_network_level_address_t *la; > + > + if (addr) > + *addr =3D NULL; > + > + if (naddr) > + *naddr =3D 0; > + > + popt =3D find_dhcpv6_option (packet, OPTION_DNS_SERVERS); > + if (!popt) > + return; > + > + len =3D grub_be_to_cpu16 (popt->len); > + if ((len % 16) !=3D 0) len & 0xf; 0 comparison probably redundant as well. Again, what about upper boundary check? > + { > + grub_error (GRUB_ERR_IO, N_("invalid dns address length")); > + return; > + } > + > + dns =3D (const struct grub_dhcpv6_dns_servers *)popt->data; > + > + ln =3D len / 16; len >> 4 > + la =3D grub_zalloc (sizeof (grub_net_network_level_address_t) * ln); > + *addr =3D la =3D grub_zalloc (...) NULL check > + for (i =3D 0, pa =3D dns->addr; i < ln; i++, pa =3D dns->next_addr) ^^^ not needed pa +=3D 16, la++ > + { > + (la + i)->type =3D GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6; > + (la + i)->ipv6[0] =3D grub_get_unaligned64 (pa); > + (la + i)->ipv6[1] =3D grub_get_unaligned64 (pa + 8); > + (la + i)->option =3D DNS_OPTION_PREFER_IPV6; la->... > + } > + > + *addr =3D la; not needed > + *naddr =3D ln; > + > + return; > +} > + > +static void > +find_dhcpv6_bootfile_url (const struct grub_net_dhcpv6_packet *packet, > + char **proto, char **server_ip, char **boot_file) > +{ > + char *bootfile_url; > + const struct grub_dhcpv6_option* opt_url; > + char *ip_start, *ip_end; > + char *path; > + grub_size_t ip_len; > + grub_uint16_t len; > + const char *protos[] =3D {"tftp://", "http://", NULL}; > + const char *pr; > + int i; > + > + if (proto) > + *proto =3D NULL; > + > + if (server_ip) > + *server_ip =3D NULL; > + > + if (boot_file) > + *boot_file =3D NULL; > + > + opt_url =3D find_dhcpv6_option (packet, OPTION_BOOTFILE_URL); > + > + if (!opt_url) > + { > + grub_error (GRUB_ERR_IO, N_("no bootfile-url in DHCPv6 option")); > + return; > + } > + > + len =3D grub_be_to_cpu16 (opt_url->len); > + Obligatory question about upper boundary check :) > + bootfile_url =3D grub_malloc (len + 1); > + > + if (!bootfile_url) > + return; > + > + grub_memcpy (bootfile_url, opt_url->data, len); > + bootfile_url[len] =3D '\0'; > + > + for (i =3D 0; (pr =3D *(protos + i)); ++i) > + if (grub_strncmp (bootfile_url, pr, grub_strlen(pr)) =3D=3D 0) > + break; > + > + if (!pr) > + { > + grub_error (GRUB_ERR_IO, > + N_("unsupported protocol, only tftp and http are supported")); > + goto cleanup; > + } > + > + 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 brackets")= ); > + goto cleanup; > + } > + Can bootfile_url contain a name and not IPv6 address? Or is address mandatory? > + ip_len =3D ip_end - ip_start; > + > + if (proto) > + { > + grub_size_t proto_len =3D grub_strlen (pr) - 3; > + > + *proto =3D grub_malloc (proto_len + 1); > + if (!*proto) > + goto cleanup; > + > + grub_memcpy (*proto, pr, proto_len); > + *(*proto + proto_len) =3D '\0'; Umm ... we really need something like grub_mem2str. Actually grub_xasprintf("%.*s") would be just fine if grub supported it. > + } > + > + if (server_ip) > + { > + *server_ip =3D grub_malloc (ip_len + 1); > + > + if (!*server_ip) > + goto cleanup; > + > + grub_memcpy (*server_ip, ip_start, ip_len); > + *(*server_ip + ip_len) =3D '\0'; > + } > + > + path =3D ip_end + 1; > + > + if (boot_file) > + { > + *boot_file =3D grub_strdup (path); > + > + if (!*boot_file) > + goto cleanup; > + } > + > +cleanup: > + > + if (bootfile_url) > + grub_free (bootfile_url); > + grub_free checks for NULL > + if (grub_errno) > + { > + if (proto && *proto) > + { > + grub_free (proto); grub_free (*proto) > + *proto =3D NULL; > + } > + > + if (server_ip && *server_ip) > + { > + grub_free (server_ip); grub_free (*server_ip) > + *server_ip =3D NULL; > + } > + > + if (boot_file && *boot_file) > + { > + grub_free (boot_file); grub_free (*boot_file) > + *boot_file =3D NULL; > + } > + } > + > + return; > +} > + > + > +static grub_err_t > +grub_net_configure_by_dhcpv6_adv (const struct grub_net_dhcpv6_packet *v= 6_adv, > + struct grub_dhcpv6_session *session) > +{ > + struct grub_net_buff *nb; > + const struct grub_dhcpv6_option *opt_client, *opt_server, *opt_iana; > + struct grub_dhcpv6_option *popt; > + struct grub_net_dhcpv6_packet *v6; > + struct udphdr *udph; > + grub_net_network_level_address_t multicast; > + grub_net_link_level_address_t ll_multicast; > + struct grub_net_network_level_interface *inf; > + grub_err_t err; > + grub_uint16_t len; > + grub_uint64_t elapsed; > + char err_msg[64]; > + > + if (v6_adv->message_type !=3D DHCPv6_ADVERTISE) We come here after checking message type, or am I wrong? > + { > + grub_error (GRUB_ERR_IO, N_("DHCPv6 info not found")); > + return grub_errno; > + } > + > + opt_client =3D find_dhcpv6_option (v6_adv, OPTION_CLIENTID); > + opt_server =3D find_dhcpv6_option (v6_adv, OPTION_SERVERID); > + opt_iana =3D find_dhcpv6_option (v6_adv, OPTION_IA_NA); > + > + err_msg[0] =3D '\0'; > + if (!opt_client) > + grub_strcpy (err_msg, "client id"); > + > + if (!opt_server) > + { > + if (grub_strlen (err_msg)) > + grub_strcpy (err_msg + grub_strlen (err_msg), ", server id"); > + else > + grub_strcpy (err_msg, "server id"); > + } > + > + if (!opt_iana) > + { > + if (grub_strlen (err_msg)) > + grub_strcpy (err_msg + grub_strlen (err_msg), ", iana"); > + else > + grub_strcpy (err_msg, "iana"); > + } > + > + if (grub_strlen (err_msg)) > + { > + grub_strcpy (err_msg + grub_strlen (err_msg), " missing"); > + grub_error (GRUB_ERR_IO, N_(err_msg)); This means it will not be extracted by xgettext and we need 7 different strings added manually. This needs some change if you really want it to be translated. May be use grub_error_push with "Mandatory DHCPv6 option %s missing" or similar? > + return grub_errno; > + } > + > + inf =3D session->ifaces; > + > + multicast.type =3D GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6; > + multicast.ipv6[0] =3D grub_cpu_to_be64_compile_time (0xff02ULL << 48); > + multicast.ipv6[1] =3D grub_cpu_to_be64_compile_time (0x10002ULL); > + > + err =3D grub_net_link_layer_resolve (inf, &multicast, &ll_multicast); > + if (err) > + return err; > + > + nb =3D grub_netbuff_alloc (512); Symbolic constant would be nice. Where this size comes from? > + > + if (!nb) > + { > + grub_netbuff_free (nb); nb is NULL at this point, not? > + return grub_errno; > + } > + > + err =3D grub_netbuff_reserve (nb, 512); > + if (err) > + { > + grub_netbuff_free (nb); > + return err; > + } > + > + len =3D grub_cpu_to_be16(opt_client->len); > + err =3D grub_netbuff_push (nb, len + 4); len + sizeof (*opt_client) > + if (err) > + { > + grub_netbuff_free (nb); > + return err; > + } > + grub_memcpy (nb->data, opt_client, len + 4); ditto > + > + len =3D grub_cpu_to_be16(opt_server->len); > + err =3D grub_netbuff_push (nb, len + 4); ditto > + if (err) > + { > + grub_netbuff_free (nb); > + return err; > + } > + grub_memcpy (nb->data, opt_server, len + 4); ditto > + > + len =3D grub_cpu_to_be16(opt_iana->len); > + err =3D grub_netbuff_push (nb, len + 4); ditto > + if (err) > + { > + grub_netbuff_free (nb); > + return err; > + } > + grub_memcpy (nb->data, opt_iana, len + 4); ditto > + > + err =3D grub_netbuff_push (nb, 8); sizeof (struct grub_dhcpv6_option) + 2 * sizeof (grub_uint16_t) probably makes it more clear. > + if (err) > + { > + grub_netbuff_free (nb); > + return err; > + } > + > + popt =3D (struct grub_dhcpv6_option*) nb->data; > + popt->code =3D grub_cpu_to_be16_compile_time (OPTION_ORO); > + popt->len =3D grub_cpu_to_be16_compile_time (4); and here 2 * sizeof (grub_uint16_t) as well. > + grub_set_unaligned16 (popt->data, grub_cpu_to_be16_compile_time (OPTIO= N_BOOTFILE_URL)); > + grub_set_unaligned16 (popt->data + 2, grub_cpu_to_be16_compile_time (O= PTION_DNS_SERVERS)); > + > + err =3D grub_netbuff_push (nb, 6); > + if (err) > + { > + grub_netbuff_free (nb); > + return err; > + } > + popt =3D (struct grub_dhcpv6_option*) nb->data; > + popt->code =3D grub_cpu_to_be16_compile_time (OPTION_ELAPSED_TIME); > + popt->len =3D grub_cpu_to_be16_compile_time (2); > + similar > + // the time is expressed in hundredths of a second Please, let's stick to C comments > + elapsed =3D grub_divmod64 (grub_get_time_ms () - session->start_time, = 10, 0); > + > + if (elapsed > 0xffff) > + elapsed =3D 0xffff; > + > + grub_set_unaligned16 (popt->data, grub_cpu_to_be16 ((grub_uint16_t)el= apsed)); > + > + err =3D grub_netbuff_push (nb, 4); also better sizeof to avoid magic constants > + if (err) > + { > + grub_netbuff_free (nb); > + return err; > + } > + > + v6 =3D (struct grub_net_dhcpv6_packet *) nb->data; > + v6->message_type =3D DHCPv6_REQUEST; > + v6->transaction_id =3D v6_adv->transaction_id; > + > + err =3D grub_netbuff_push (nb, sizeof (*udph)); > + if (err) > + { > + grub_netbuff_free (nb); > + return err; > + } > + > + udph =3D (struct udphdr *) nb->data; > + udph->src =3D grub_cpu_to_be16_compile_time (546); > + udph->dst =3D grub_cpu_to_be16_compile_time (547); > + udph->chksum =3D 0; > + udph->len =3D grub_cpu_to_be16 (nb->tail - nb->data); > + > + udph->chksum =3D grub_net_ip_transport_checksum (nb, GRUB_NET_IP_UDP, > + &inf->address, > + &multicast); > + err =3D grub_net_send_ip_packet (inf, &multicast, &ll_multicast, nb, > + GRUB_NET_IP_UDP); > + > + grub_netbuff_free (nb); > + > + if (err) Just initialize it from start to GRUB_ERR_NONE? > + return err; > + > + return GRUB_ERR_NONE; > +} > + > + > +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) Is it really possible? We come here if packet is DHCPv6_REPLY only. > + { > + grub_error (GRUB_ERR_IO, N_("DHCPv6 info not found")); > + return NULL; > + } > + > + your_ip =3D find_dhcpv6_address(v6); > + > + if (!your_ip) > + { > + grub_error (GRUB_ERR_IO, N_("DHCPv6 address not found")); > + return NULL; > + } > + > + get_dhcpv6_dns_address (v6, &dns, &num_dns); > + > + if (dns && num_dns) > + { > + int i; > + > + for (i =3D 0; i < num_dns; ++i) > + grub_net_add_dns_server (dns + i); > + > + grub_free (dns); > + } > + else > + { > + if (grub_errno) > + grub_print_error (); > + } > + braces not needed. > + find_dhcpv6_bootfile_url (v6, &proto, &server_ip, &boot_file); > + > + if (grub_errno) > + grub_print_error (); > + > + addr.type =3D GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6; > + addr.ipv6[0] =3D grub_get_unaligned64 (your_ip); > + addr.ipv6[1] =3D grub_get_unaligned64 (your_ip + 8); > + inf =3D grub_net_add_addr (name, card, &addr, &card->default_address, = flags); > + > + netaddr.type =3D GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6; > + netaddr.ipv6.base[0] =3D grub_get_unaligned64 (your_ip); > + netaddr.ipv6.base[1] =3D 0; > + netaddr.ipv6.masksize =3D 64; > + grub_net_add_route (name, netaddr, inf); > + > + grub_env_set_net_property (name, "boot_file", boot_file, > + grub_strlen (boot_file)); > + boot_file can be NULL, right? > + if (is_def && server_ip) > + { > + grub_net_default_server =3D grub_strdup (server_ip); > + grub_env_set ("net_default_interface", name); > + grub_env_export ("net_default_interface"); > + } > + > + if (device && server_ip && proto) > + { > + *device =3D grub_xasprintf ("%s,%s", proto, server_ip); > + if (!*device) > + return NULL; > + } > + > + if (path && boot_file) > + { > + *path =3D grub_strdup (boot_file); > + if (*path) > + { > + char *slash; > + slash =3D grub_strrchr (*path, '/'); > + if (slash) > + *slash =3D 0; > + else > + **path =3D 0; > + } > + else > + return NULL; > + } > + > + return inf; This smells like memory leak in multiple places above. proto, server_ip, etc > +} > + > void > grub_net_process_dhcp (struct grub_net_buff *nb, > struct grub_net_card *card) > @@ -288,6 +936,67 @@ grub_net_process_dhcp (struct grub_net_buff *nb, > } > } > =20 > +void > +grub_net_process_dhcp6 (struct grub_net_buff *nb, > + struct grub_net_card *card __attribute__ ((unused))) > +{ > + const struct grub_net_dhcpv6_packet *v6; > + struct grub_dhcpv6_session *session; > + const struct grub_dhcpv6_option *opt_iana; > + const struct grub_dhcpv6_iana_option *ia_na; > + > + v6 =3D (const struct grub_net_dhcpv6_packet *) nb->data; > + > + opt_iana =3D find_dhcpv6_option (v6, OPTION_IA_NA); > + if (!opt_iana) > + return; > + > + ia_na =3D (const struct grub_dhcpv6_iana_option *)opt_iana->data; > + FOR_DHCPV6_SESSIONS (session) > + { > + if (session->transaction_id =3D=3D v6->transaction_id > + && session->iaid =3D=3D grub_cpu_to_be32 (ia_na->iaid)) > + break; > + } > + > + if (!session) > + return; > + > + > + if (v6->message_type =3D=3D DHCPv6_ADVERTISE) > + { > + grub_net_configure_by_dhcpv6_adv ( > + (const struct grub_net_dhcpv6_packet*) nb->data, session); > + } > + else if (v6->message_type =3D=3D DHCPv6_REPLY) > + { > + char *name; > + struct grub_net_network_level_interface *inf; > + > + inf =3D session->ifaces; > + name =3D grub_xasprintf ("%s:dhcp", inf->card->name); > + if (!name) > + return; > + > + grub_net_configure_by_dhcpv6_reply (name, inf->card, > + 0, (const struct grub_net_dhcpv6_packet *) nb->data, > + (nb->tail - nb->data), 0, 0, 0); > + > + if (!grub_errno) > + { > + grub_dhcpv6_session_remove (session); > + grub_free (session); > + } > + > + grub_free (name); > + } > + > + if (grub_errno) > + grub_print_error (); > + > + return; > +} > + > static char > hexdigit (grub_uint8_t val) > { > @@ -564,7 +1273,177 @@ grub_cmd_bootp (struct grub_command *cmd __attribu= te__ ((unused)), > return err; > } > =20 > -static grub_command_t cmd_getdhcp, cmd_bootp; > +static grub_err_t > +grub_cmd_bootp6 (struct grub_command *cmd __attribute__ ((unused)), > + int argc, char **args) > +{ > + struct grub_net_card *card; > + grub_size_t ncards =3D 0; > + unsigned j =3D 0; > + int interval; > + grub_err_t err; > + struct grub_dhcpv6_session *session; > + > + err =3D GRUB_ERR_NONE; > + > + FOR_NET_CARDS (card) > + { > + if (argc > 0 && grub_strcmp (card->name, args[0]) !=3D 0) > + continue; > + ncards++; > + } > + where ncards is used? > + FOR_NET_CARDS (card) > + { > + struct grub_net_network_level_interface *ifaces; > + Why plural? It was array in original code, but not here, right? > + if (argc > 0 && grub_strcmp (card->name, args[0]) !=3D 0) > + continue; > + > + ifaces =3D grub_net_ipv6_get_link_local (card, &card->default_addres= s); > + if (!ifaces) > + { > + grub_free (ifaces); You still insist on freeing NULL pointers :) > + return grub_errno; > + } > + Allocated sessions are leaked. > + session =3D grub_zalloc (sizeof (*session)); > + session->ifaces =3D ifaces; > + session->iaid =3D j; > + grub_dhcpv6_session_add (session); > + j++; > + } > + > + for (interval =3D 200; interval < 10000; interval *=3D 2) > + { > + int done =3D 1; > + > + FOR_DHCPV6_SESSIONS (session) > + { Something strange with indentation. > + struct grub_net_buff *nb; > + struct grub_dhcpv6_option *opt; > + struct grub_net_dhcpv6_packet *v6; > + struct grub_DUID_LL *duid; > + struct grub_dhcpv6_iana_option *ia_na; > + grub_net_network_level_address_t multicast; > + grub_net_link_level_address_t ll_multicast; > + struct udphdr *udph; > + > + multicast.type =3D GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6; > + multicast.ipv6[0] =3D grub_cpu_to_be64_compile_time (0xff02ULL << 48); > + multicast.ipv6[1] =3D grub_cpu_to_be64_compile_time (0x10002ULL); > + > + err =3D grub_net_link_layer_resolve (session->ifaces, > + &multicast, &ll_multicast); > + if (err) > + return grub_errno; Allocated sessions? > + nb =3D grub_netbuff_alloc (512); > + if (!nb) > + { > + grub_netbuff_free (nb); free(NULL) > + return grub_errno; > + } > + memory leak > + err =3D grub_netbuff_reserve (nb, 512); > + if (err) > + { > + grub_netbuff_free (nb); > + return err; > + } > + memory leak > + err =3D grub_netbuff_push (nb, 6); I wonder if some good macro around sizeof could be used here to avoid hardcoding magic numbers. > + if (err) > + { > + grub_netbuff_free (nb); > + return err; > + } > + memory leak > + opt =3D (struct grub_dhcpv6_option *)nb->data; > + opt->code =3D grub_cpu_to_be16_compile_time (OPTION_ELAPSED_TIME); > + opt->len =3D grub_cpu_to_be16_compile_time (2); > + grub_set_unaligned16 (opt->data, 0); > + > + err =3D grub_netbuff_push (nb, sizeof(*duid) + 4); > + if (err) > + { > + grub_netbuff_free (nb); > + return err; > + } > + memory leak > + opt =3D (struct grub_dhcpv6_option *)nb->data; > + opt->code =3D grub_cpu_to_be16_compile_time (OPTION_CLIENTID); //opti= on_client_id C++ comments > + opt->len =3D grub_cpu_to_be16 (sizeof(*duid)); > + > + duid =3D (struct grub_DUID_LL *) opt->data; > + > + duid->type =3D grub_cpu_to_be16_compile_time (3) ; > + duid->hw_type =3D grub_cpu_to_be16_compile_time (1); > + grub_memcpy (&duid->hwaddr, &session->ifaces->hwaddress.mac, > + sizeof (session->ifaces->hwaddress.mac)); > + > + err =3D grub_netbuff_push (nb, sizeof (*ia_na) + 4); > + if (err) > + { > + grub_netbuff_free (nb); > + return err; > + } > + memory leak > + opt =3D (struct grub_dhcpv6_option *)nb->data; > + opt->code =3D grub_cpu_to_be16_compile_time (OPTION_IA_NA); > + opt->len =3D grub_cpu_to_be16 (sizeof (*ia_na)); > + ia_na =3D (struct grub_dhcpv6_iana_option *)opt->data; > + ia_na->iaid =3D grub_cpu_to_be32 (session->iaid); > + ia_na->t1 =3D 0; > + ia_na->t2 =3D 0; > + > + err =3D grub_netbuff_push (nb, 4); > + if (err) > + { > + grub_netbuff_free (nb); > + return err; > + } > + memory leak > + v6 =3D (struct grub_net_dhcpv6_packet *)nb->data; > + v6->message_type =3D 1; > + v6->transaction_id =3D session->transaction_id; > + > + grub_netbuff_push (nb, sizeof (*udph)); > + > + udph =3D (struct udphdr *) nb->data; > + udph->src =3D grub_cpu_to_be16_compile_time (546); > + udph->dst =3D grub_cpu_to_be16_compile_time (547); > + udph->chksum =3D 0; > + udph->len =3D grub_cpu_to_be16 (nb->tail - nb->data); > + > + udph->chksum =3D grub_net_ip_transport_checksum (nb, GRUB_NET_IP_UDP, > + &session->ifaces->address, &multicast); > + > + err =3D grub_net_send_ip_packet (session->ifaces, &multicast, > + &ll_multicast, nb, GRUB_NET_IP_UDP); > + done =3D 0; > + grub_netbuff_free (nb); > + > + if (err) > + return err; memory leak > + } > + if (!done) > + grub_net_poll_cards (interval, 0); > + } > + > + FOR_DHCPV6_SESSIONS (session) > + { > + err =3D grub_error (GRUB_ERR_FILE_NOT_FOUND, > + N_("couldn't autoconfigure %s"), > + session->ifaces->card->name); Unless I misunderstand something only last error is printed anyway. May be it should push and then print them outside of loop? > + grub_dhcpv6_session_remove (session); > + grub_free (session); > + } > + > + > + return err; > +} > + > +static grub_command_t cmd_getdhcp, cmd_bootp, cmd_bootp6; > =20 > void > grub_bootp_init (void) > @@ -575,6 +1454,9 @@ grub_bootp_init (void) > cmd_getdhcp =3D grub_register_command ("net_get_dhcp_option", grub_cmd= _dhcpopt, > N_("VAR INTERFACE NUMBER DESCRIPTION"), > N_("retrieve DHCP option and save it into VAR. If VAR is - th= en print the value.")); > + cmd_bootp6 =3D grub_register_command ("net_bootp6", grub_cmd_bootp6, > + N_("[CARD]"), > + N_("perform a dhcpv6 autoconfiguration")); > } > =20 > void > @@ -582,4 +1464,5 @@ grub_bootp_fini (void) > { > grub_unregister_command (cmd_getdhcp); > grub_unregister_command (cmd_bootp); > + grub_unregister_command (cmd_bootp6); > } > diff --git a/grub-core/net/ip.c b/grub-core/net/ip.c > index 8c56baa..8bb56be 100644 > --- a/grub-core/net/ip.c > +++ b/grub-core/net/ip.c > @@ -238,6 +238,41 @@ handle_dgram (struct grub_net_buff *nb, > { > struct udphdr *udph; > udph =3D (struct udphdr *) nb->data; > + > + if (proto =3D=3D GRUB_NET_IP_UDP && grub_be_to_cpu16 (udph->dst) =3D= =3D 546) udph->dst =3D=3D grub_cpu_to_be16_compile_time > + { > + if (udph->chksum) > + { > + grub_uint16_t chk, expected; > + chk =3D udph->chksum; > + udph->chksum =3D 0; > + expected =3D grub_net_ip_transport_checksum (nb, > + GRUB_NET_IP_UDP, > + source, > + dest); > + if (expected !=3D chk) > + { > + grub_dprintf ("net", "Invalid UDP checksum. " > + "Expected %x, got %x\n", > + grub_be_to_cpu16 (expected), > + grub_be_to_cpu16 (chk)); > + grub_netbuff_free (nb); > + return GRUB_ERR_NONE; > + } > + udph->chksum =3D chk; > + } > + > + err =3D grub_netbuff_pull (nb, sizeof (*udph)); > + if (err) > + { > + grub_netbuff_free (nb); > + return err; > + } > + grub_net_process_dhcp6 (nb, card); > + grub_netbuff_free (nb); > + return GRUB_ERR_NONE; > + } > + > if (proto =3D=3D GRUB_NET_IP_UDP && grub_be_to_cpu16 (udph->dst) =3D= =3D 68) > { > const struct grub_net_bootp_packet *bootp; > diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h > index e5dd543..ff77750 100644 > --- a/include/grub/efi/api.h > +++ b/include/grub/efi/api.h This does not really belong to this patch. > @@ -1340,14 +1340,68 @@ typedef struct grub_efi_simple_text_output_interf= ace grub_efi_simple_text_output > =20 > typedef grub_uint8_t grub_efi_pxe_packet_t[1472]; > =20 > +typedef struct { > + grub_uint8_t addr[4]; > +} grub_efi_pxe_ipv4_address_t; > + > +typedef struct { > + grub_uint8_t addr[16]; > +} grub_efi_pxe_ipv6_address_t; > + > +typedef struct { > + grub_uint8_t addr[32]; > +} grub_efi_pxe_mac_address_t; > + > +typedef union { > + grub_uint32_t addr[4]; > + grub_efi_pxe_ipv4_address_t v4; > + grub_efi_pxe_ipv6_address_t v6; > +} grub_efi_pxe_ip_address_t; > + > +#define EFI_PXE_BASE_CODE_MAX_IPCNT 8 GRUB_EFI_* please > +typedef struct { > + grub_uint8_t filters; > + grub_uint8_t ip_cnt; > + grub_uint16_t reserved; > + grub_efi_pxe_ip_address_t ip_list[EFI_PXE_BASE_CODE_MAX_IPCNT]; > +} grub_efi_pxe_ip_filter_t; > + > +typedef struct { > + grub_efi_pxe_ip_address_t ip_addr; > + grub_efi_pxe_mac_address_t mac_addr; > +} grub_efi_pxe_arp_entry_t; > + > +typedef struct { > + grub_efi_pxe_ip_address_t ip_addr; > + grub_efi_pxe_ip_address_t subnet_mask; > + grub_efi_pxe_ip_address_t gw_addr; > +} grub_efi_pxe_route_entry_t; > + > + > +#define EFI_PXE_BASE_CODE_MAX_ARP_ENTRIES 8 > +#define EFI_PXE_BASE_CODE_MAX_ROUTE_ENTRIES 8 > + Same > typedef struct grub_efi_pxe_mode > { > - grub_uint8_t unused[52]; > + grub_uint8_t started; > + grub_uint8_t ipv6_available; > + grub_uint8_t ipv6_supported; > + grub_uint8_t using_ipv6; > + //grub_uint8_t unused[48]; > + grub_uint8_t unused[16]; > + grub_efi_pxe_ip_address_t station_ip; > + grub_efi_pxe_ip_address_t subnet_mask; > grub_efi_pxe_packet_t dhcp_discover; > grub_efi_pxe_packet_t dhcp_ack; > grub_efi_pxe_packet_t proxy_offer; > grub_efi_pxe_packet_t pxe_discover; > grub_efi_pxe_packet_t pxe_reply; > + grub_efi_pxe_packet_t pxe_bis_reply; > + grub_efi_pxe_ip_filter_t ip_filter; > + grub_uint32_t arp_cache_entries; > + grub_efi_pxe_arp_entry_t arp_cache[EFI_PXE_BASE_CODE_MAX_ARP_ENTRIES]; > + grub_uint32_t route_table_entries; > + grub_efi_pxe_route_entry_t route_table[EFI_PXE_BASE_CODE_MAX_ROUTE_ENT= RIES]; > } grub_efi_pxe_mode_t; > =20 > typedef struct grub_efi_pxe > diff --git a/include/grub/net.h b/include/grub/net.h > index 538baa3..71dc243 100644 > --- a/include/grub/net.h > +++ b/include/grub/net.h > @@ -418,6 +418,13 @@ struct grub_net_bootp_packet > grub_uint8_t vendor[0]; > } GRUB_PACKED; > =20 > +struct grub_net_dhcpv6_packet > +{ > + grub_uint32_t message_type:8; > + grub_uint32_t transaction_id:24; > + grub_uint8_t dhcp_options[0]; > +} GRUB_PACKED; > + > #define GRUB_NET_BOOTP_RFC1048_MAGIC_0 0x63 > #define GRUB_NET_BOOTP_RFC1048_MAGIC_1 0x82 > #define GRUB_NET_BOOTP_RFC1048_MAGIC_2 0x53 > @@ -444,6 +451,14 @@ grub_net_configure_by_dhcp_ack (const char *name, > grub_size_t size, > int is_def, char **device, char **path); > =20 > +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, > + int is_def, char **device, char **path); > + > grub_err_t > grub_net_add_ipv4_local (struct grub_net_network_level_interface *inf, > int mask); > @@ -452,6 +467,10 @@ void > grub_net_process_dhcp (struct grub_net_buff *nb, > struct grub_net_card *card); > =20 > +void > +grub_net_process_dhcp6 (struct grub_net_buff *nb, > + struct grub_net_card *card); > + > int > grub_net_hwaddr_cmp (const grub_net_link_level_address_t *a, > const grub_net_link_level_address_t *b);