Re: [PATCH 1/3] Added net_bootp6 command
В Tue, 19 May 2015 16:42:00 +0800 Michael Chang mch...@suse.com пишет: +#define GRUB_DHCPv6_OPTION_CLIENTID 1 +#define GRUB_DHCPv6_OPTION_SERVERID 2 +#define GRUB_DHCPv6_OPTION_IA_NA 3 +#define GRUB_DHCPv6_OPTION_IAADDR 5 +#define GRUB_DHCPv6_OPTION_ORO 6 +#define GRUB_DHCPv6_OPTION_ELAPSED_TIME 8 +#define GRUB_DHCPv6_OPTION_DNS_SERVERS 23 +#define GRUB_DHCPv6_OPTION_BOOTFILE_URL 59 + enum please. These can also be anonymous, no need to invent fancy names. I don't understand. Do you want me to use enum type here rather than macro ? Where are the fancy names invented here ? Sorry I mean there is no need to name enume type. Just enum { GRUB_DHCPv6_OPTION_CLIENTID = 1 ... }; +static void +get_dhcpv6_dns_address (const struct grub_net_dhcpv6_packet *packet, grub_size_t size, + grub_net_network_level_address_t **addr, grub_uint16_t *naddr) +{ + const struct grub_dhcpv6_option* popt; + grub_uint16_t len, ln; + const grub_uint8_t *pa; + grub_net_network_level_address_t *la; + + if (!addr || !naddr) +{ + grub_error (GRUB_ERR_BAD_ARGUMENT, N_(bad argument for get_dhcpv6_dns_address)); + return; +} + it is called exactly once and grub_errno is immediately reset to GRUB_ERR_NONE. So what's the point of returning error at all? Because a function wants to report error for bad inputs, while it's caller wants to suppress the error message because dns information could be missing in packets and is allowed (not treated as error). Sure but it is called exactly once and result is not used. It is not like this is library function, it is more of a convenience macro. For the same reasons NULL check on input is probably redundant as well. + if (len == 0 || len 0xf) +{ + grub_error (GRUB_ERR_IO, N_(invalid dns address length)); + return; +} + same question about grub_error. May be grub_debug? No grub_debug, maybe grub_dprintf ? Maybe we should keep it and use specific error number for option not found ? See below for details. Sure, I mean dprintf, sorry. + + /* Follow elilo and edk2 that check for starting and ending delimiter '[..]' + in which IPv6 server address is enclosed. */ + if (*ip_start != '[') +ip_start = NULL; + else +ip_end = grub_strchr (++ip_start, ']'); + + if (!ip_start || !ip_end) +{ + grub_error (GRUB_ERR_IO, N_(IPv6-address not in square brackets)); + goto cleanup; +} + RFC5970 says Clients that have DNS implementations SHOULD support the use of domain names in the URL and in general string can also be valid IPv4 address, nothing restricts it to IPv6. But the descriptions preceding it did explicitly say IPv6: If the host in the URL is expressed using an IPv6 address rather than a domain name, the address in the URL then MUST be enclosed in [ and ] characters, conforming to [RFC3986]. it says *if* it is using IPv6. Or at least so I understand it. So I do not think you should return error here, just return full string. I wish grub supported IPv6 literals so we could just skip this check entirely. OK. I'll return the string by assuming it's domain name. ... + + get_dhcpv6_dns_address (v6, size, dns, num_dns); + + if (grub_errno) +grub_errno = GRUB_ERR_NONE; + This ignores legitimate errors like out of memory. As mentioned above, does it need to set any grub_errno on its own? If not, errors should not be ignored. Because missing dns option is totally fine so that I don't want to treat it as error. We can use a specific error number for missing options and report other errors, but I can't find a suitable one for it (in include/grub/err.h). If missing DNS option is totally fine it should not be returned as error. You already have indication (num_dns == 0) which is enough if you want to inform user. Errors should be returned in case of e-h-h errors :) Thanks! And sorry for delay. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 1/3] Added net_bootp6 command
Hi Andrey, Please see my inline comments. And if I have left anything un-answered or not clear please let me know. And thanks a lot to your time and effort for doing the review. Thanks, Michael On Fri, May 15, 2015 at 09:26:06AM +0300, Andrei Borzenkov wrote: В Tue, 12 May 2015 16:49:48 +0800 Michael Chang mch...@suse.com пишет: The net_bootp6 is used to configure the ipv6 network interface through the DHCPv6 protocol Solict/Advertise/Request/Reply. --- grub-core/net/bootp.c | 895 - grub-core/net/ip.c| 35 ++ include/grub/net.h| 19 + 3 files changed, 948 insertions(+), 1 deletions(-) diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c index 6136755..5c5eb6f 100644 --- a/grub-core/net/bootp.c +++ b/grub-core/net/bootp.c @@ -24,6 +24,8 @@ #include grub/net/netbuff.h #include grub/net/udp.h #include grub/datetime.h +#include grub/time.h +#include grub/list.h static void parse_dhcp_vendor (const char *name, const void *vend, int limit, int *mask) @@ -256,6 +258,646 @@ grub_net_configure_by_dhcp_ack (const char *name, return inter; } +struct grub_dhcpv6_option { + grub_uint16_t code; + grub_uint16_t len; Won't do; options in packet are unaligned (see below) so you have to walk byte buffer using get_unaligned and grub_uint8_t code[2] grub_uint8_t len[2] + 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; + Probably the same applies to all option definitions. +#define GRUB_DHCPv6_ADVERTISE 2 +#define GRUB_DHCPv6_REQUEST 3 +#define GRUB_DHCPv6_REPLY 7 + SOLICIT is missing; please enum as well. OK. +#define GRUB_DHCPv6_OPTION_CLIENTID 1 +#define GRUB_DHCPv6_OPTION_SERVERID 2 +#define GRUB_DHCPv6_OPTION_IA_NA 3 +#define GRUB_DHCPv6_OPTION_IAADDR 5 +#define GRUB_DHCPv6_OPTION_ORO 6 +#define GRUB_DHCPv6_OPTION_ELAPSED_TIME 8 +#define GRUB_DHCPv6_OPTION_DNS_SERVERS 23 +#define GRUB_DHCPv6_OPTION_BOOTFILE_URL 59 + enum please. These can also be anonymous, no need to invent fancy names. I don't understand. Do you want me to use enum type here rather than macro ? Where are the fancy names invented here ? (All of the listed maros have it's name taken from spec, and the prefix according to your previous reviews). +/* The default netbuff size for sending DHCPv6 packets which should be + large enough to hold the information */ +#define GRUB_DHCPv6_DEFAULT_NETBUFF_ALLOC_SIZE 512 + +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 *iface; +}; + +static struct grub_dhcpv6_session *grub_dhcpv6_sessions = NULL; +#define FOR_DHCPV6_SESSIONS(var) FOR_LIST_ELEMENTS (var, grub_dhcpv6_sessions) + +static void +grub_dhcpv6_session_add (struct grub_dhcpv6_session *session) +{ + struct grub_datetime date; + grub_err_t err; + grub_int32_t t = 0; + + err = grub_get_datetime (date); + if (err || !grub_datetime2unixtime (date, t)) +{ + grub_errno = GRUB_ERR_NONE; + t = 0; +} + + session-transaction_id = t; + session-start_time = grub_get_time_ms (); + grub_list_push (GRUB_AS_LIST_P (grub_dhcpv6_sessions), GRUB_AS_LIST (session)); + + return; return is redundant OK. +} + +static void +grub_dhcpv6_sessions_free (void) +{ + struct grub_dhcpv6_session *session; + + FOR_DHCPV6_SESSIONS (session) +{ + grub_list_remove (GRUB_AS_LIST (session)); + grub_free (session); + session = grub_dhcpv6_sessions; +} + + return; and here too OK. +} + +static const char * +get_dhcpv6_option_name (grub_uint16_t option) +{ + switch (option) +{ +case GRUB_DHCPv6_OPTION_BOOTFILE_URL: + return BOOTFILE URL; +case GRUB_DHCPv6_OPTION_DNS_SERVERS: + return DNS SERVERS; +case GRUB_DHCPv6_OPTION_IA_NA: + return IA NA; +case GRUB_DHCPv6_OPTION_IAADDR: + return IAADDR; +case GRUB_DHCPv6_OPTION_CLIENTID: + return CLIENTID; +case GRUB_DHCPv6_OPTION_SERVERID: + return SERVERID; +case GRUB_DHCPv6_OPTION_ORO: + return ORO; +case GRUB_DHCPv6_OPTION_ELAPSED_TIME: + return ELAPSED TIME; +default: + return UNKNOWN;
Re: [PATCH 1/3] Added net_bootp6 command
В Tue, 12 May 2015 16:49:48 +0800 Michael Chang mch...@suse.com пишет: The net_bootp6 is used to configure the ipv6 network interface through the DHCPv6 protocol Solict/Advertise/Request/Reply. --- grub-core/net/bootp.c | 895 - grub-core/net/ip.c| 35 ++ include/grub/net.h| 19 + 3 files changed, 948 insertions(+), 1 deletions(-) diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c index 6136755..5c5eb6f 100644 --- a/grub-core/net/bootp.c +++ b/grub-core/net/bootp.c @@ -24,6 +24,8 @@ #include grub/net/netbuff.h #include grub/net/udp.h #include grub/datetime.h +#include grub/time.h +#include grub/list.h static void parse_dhcp_vendor (const char *name, const void *vend, int limit, int *mask) @@ -256,6 +258,646 @@ grub_net_configure_by_dhcp_ack (const char *name, return inter; } +struct grub_dhcpv6_option { + grub_uint16_t code; + grub_uint16_t len; Won't do; options in packet are unaligned (see below) so you have to walk byte buffer using get_unaligned and grub_uint8_t code[2] grub_uint8_t len[2] + 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; + Probably the same applies to all option definitions. +#define GRUB_DHCPv6_ADVERTISE 2 +#define GRUB_DHCPv6_REQUEST 3 +#define GRUB_DHCPv6_REPLY 7 + SOLICIT is missing; please enum as well. +#define GRUB_DHCPv6_OPTION_CLIENTID 1 +#define GRUB_DHCPv6_OPTION_SERVERID 2 +#define GRUB_DHCPv6_OPTION_IA_NA 3 +#define GRUB_DHCPv6_OPTION_IAADDR 5 +#define GRUB_DHCPv6_OPTION_ORO 6 +#define GRUB_DHCPv6_OPTION_ELAPSED_TIME 8 +#define GRUB_DHCPv6_OPTION_DNS_SERVERS 23 +#define GRUB_DHCPv6_OPTION_BOOTFILE_URL 59 + enum please. These can also be anonymous, no need to invent fancy names. +/* The default netbuff size for sending DHCPv6 packets which should be + large enough to hold the information */ +#define GRUB_DHCPv6_DEFAULT_NETBUFF_ALLOC_SIZE 512 + +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 *iface; +}; + +static struct grub_dhcpv6_session *grub_dhcpv6_sessions = NULL; +#define FOR_DHCPV6_SESSIONS(var) FOR_LIST_ELEMENTS (var, grub_dhcpv6_sessions) + +static void +grub_dhcpv6_session_add (struct grub_dhcpv6_session *session) +{ + struct grub_datetime date; + grub_err_t err; + grub_int32_t t = 0; + + err = grub_get_datetime (date); + if (err || !grub_datetime2unixtime (date, t)) +{ + grub_errno = GRUB_ERR_NONE; + t = 0; +} + + session-transaction_id = t; + session-start_time = grub_get_time_ms (); + grub_list_push (GRUB_AS_LIST_P (grub_dhcpv6_sessions), GRUB_AS_LIST (session)); + + return; return is redundant +} + +static void +grub_dhcpv6_sessions_free (void) +{ + struct grub_dhcpv6_session *session; + + FOR_DHCPV6_SESSIONS (session) +{ + grub_list_remove (GRUB_AS_LIST (session)); + grub_free (session); + session = grub_dhcpv6_sessions; +} + + return; and here too +} + +static const char * +get_dhcpv6_option_name (grub_uint16_t option) +{ + switch (option) +{ +case GRUB_DHCPv6_OPTION_BOOTFILE_URL: + return BOOTFILE URL; +case GRUB_DHCPv6_OPTION_DNS_SERVERS: + return DNS SERVERS; +case GRUB_DHCPv6_OPTION_IA_NA: + return IA NA; +case GRUB_DHCPv6_OPTION_IAADDR: + return IAADDR; +case GRUB_DHCPv6_OPTION_CLIENTID: + return CLIENTID; +case GRUB_DHCPv6_OPTION_SERVERID: + return SERVERID; +case GRUB_DHCPv6_OPTION_ORO: + return ORO; +case GRUB_DHCPv6_OPTION_ELAPSED_TIME: + return ELAPSED TIME; +default: + return UNKNOWN; +} +} + +static const struct grub_dhcpv6_option* +find_dhcpv6_option (const struct grub_dhcpv6_option *popt, grub_size_t size, grub_uint16_t option) +{ + while (size 0) +{ + grub_uint16_t code, len; + if (size sizeof(*popt)) grub_error + code = grub_be_to_cpu16 (popt-code); + len = grub_be_to_cpu16 (popt-len) + sizeof (*popt); + According to rfc3315 - Options are byte-aligned but are not aligned in any other way such as on 2 or 4 byte boundaries; it should be grub_get_unaligned. To avoid warnings from static checkers (possible overflow) size -= sizeof(*popt) len = grub_be_to_cpu... + if (size len) + { +
Re: [PATCH 1/3] Added net_bootp6 command
On Fri, May 15, 2015 at 09:26:06AM +0300, Andrei Borzenkov wrote: В Tue, 12 May 2015 16:49:48 +0800 Michael Chang mch...@suse.com пишет: The net_bootp6 is used to configure the ipv6 network interface through the DHCPv6 protocol Solict/Advertise/Request/Reply. --- grub-core/net/bootp.c | 895 - grub-core/net/ip.c| 35 ++ include/grub/net.h| 19 + 3 files changed, 948 insertions(+), 1 deletions(-) diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c index 6136755..5c5eb6f 100644 --- a/grub-core/net/bootp.c +++ b/grub-core/net/bootp.c @@ -24,6 +24,8 @@ #include grub/net/netbuff.h #include grub/net/udp.h #include grub/datetime.h +#include grub/time.h +#include grub/list.h static void parse_dhcp_vendor (const char *name, const void *vend, int limit, int *mask) @@ -256,6 +258,646 @@ grub_net_configure_by_dhcp_ack (const char *name, return inter; } +struct grub_dhcpv6_option { + grub_uint16_t code; + grub_uint16_t len; Won't do; options in packet are unaligned (see below) so you have to walk byte buffer using get_unaligned and I don't get it. Wouldn't the structure attribute GRUB_PACKED do the trick for us? grub_uint8_t code[2] grub_uint8_t len[2] + 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; + Probably the same applies to all option definitions. That would require quite a lot of rewritting basically. (So I'd like it to be clear to me in the first). Thanks, Michael ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 1/3] Added net_bootp6 command
В Fri, 15 May 2015 21:57:28 +0800 Michael Chang mch...@suse.com пишет: On Fri, May 15, 2015 at 09:26:06AM +0300, Andrei Borzenkov wrote: В Tue, 12 May 2015 16:49:48 +0800 Michael Chang mch...@suse.com пишет: The net_bootp6 is used to configure the ipv6 network interface through the DHCPv6 protocol Solict/Advertise/Request/Reply. --- grub-core/net/bootp.c | 895 - grub-core/net/ip.c| 35 ++ include/grub/net.h| 19 + 3 files changed, 948 insertions(+), 1 deletions(-) diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c index 6136755..5c5eb6f 100644 --- a/grub-core/net/bootp.c +++ b/grub-core/net/bootp.c @@ -24,6 +24,8 @@ #include grub/net/netbuff.h #include grub/net/udp.h #include grub/datetime.h +#include grub/time.h +#include grub/list.h static void parse_dhcp_vendor (const char *name, const void *vend, int limit, int *mask) @@ -256,6 +258,646 @@ grub_net_configure_by_dhcp_ack (const char *name, return inter; } +struct grub_dhcpv6_option { + grub_uint16_t code; + grub_uint16_t len; Won't do; options in packet are unaligned (see below) so you have to walk byte buffer using get_unaligned and I don't get it. Wouldn't the structure attribute GRUB_PACKED do the trick for us? Yes, you are right. I apologize for confusion. Sorry :( grub_uint8_t code[2] grub_uint8_t len[2] + 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; + Probably the same applies to all option definitions. That would require quite a lot of rewritting basically. (So I'd like it to be clear to me in the first). ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 1/3] Added net_bootp6 command
The net_bootp6 is used to configure the ipv6 network interface through the DHCPv6 protocol Solict/Advertise/Request/Reply. --- grub-core/net/bootp.c | 895 - grub-core/net/ip.c| 35 ++ include/grub/net.h| 19 + 3 files changed, 948 insertions(+), 1 deletions(-) diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c index 6136755..5c5eb6f 100644 --- a/grub-core/net/bootp.c +++ b/grub-core/net/bootp.c @@ -24,6 +24,8 @@ #include grub/net/netbuff.h #include grub/net/udp.h #include grub/datetime.h +#include grub/time.h +#include grub/list.h static void parse_dhcp_vendor (const char *name, const void *vend, int limit, int *mask) @@ -256,6 +258,646 @@ grub_net_configure_by_dhcp_ack (const char *name, return inter; } +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; + +#define GRUB_DHCPv6_ADVERTISE 2 +#define GRUB_DHCPv6_REQUEST 3 +#define GRUB_DHCPv6_REPLY 7 + +#define GRUB_DHCPv6_OPTION_CLIENTID 1 +#define GRUB_DHCPv6_OPTION_SERVERID 2 +#define GRUB_DHCPv6_OPTION_IA_NA 3 +#define GRUB_DHCPv6_OPTION_IAADDR 5 +#define GRUB_DHCPv6_OPTION_ORO 6 +#define GRUB_DHCPv6_OPTION_ELAPSED_TIME 8 +#define GRUB_DHCPv6_OPTION_DNS_SERVERS 23 +#define GRUB_DHCPv6_OPTION_BOOTFILE_URL 59 + +/* The default netbuff size for sending DHCPv6 packets which should be + large enough to hold the information */ +#define GRUB_DHCPv6_DEFAULT_NETBUFF_ALLOC_SIZE 512 + +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 *iface; +}; + +static struct grub_dhcpv6_session *grub_dhcpv6_sessions = NULL; +#define FOR_DHCPV6_SESSIONS(var) FOR_LIST_ELEMENTS (var, grub_dhcpv6_sessions) + +static void +grub_dhcpv6_session_add (struct grub_dhcpv6_session *session) +{ + struct grub_datetime date; + grub_err_t err; + grub_int32_t t = 0; + + err = grub_get_datetime (date); + if (err || !grub_datetime2unixtime (date, t)) +{ + grub_errno = GRUB_ERR_NONE; + t = 0; +} + + session-transaction_id = t; + session-start_time = grub_get_time_ms (); + grub_list_push (GRUB_AS_LIST_P (grub_dhcpv6_sessions), GRUB_AS_LIST (session)); + + return; +} + +static void +grub_dhcpv6_sessions_free (void) +{ + struct grub_dhcpv6_session *session; + + FOR_DHCPV6_SESSIONS (session) +{ + grub_list_remove (GRUB_AS_LIST (session)); + grub_free (session); + session = grub_dhcpv6_sessions; +} + + return; +} + +static const char * +get_dhcpv6_option_name (grub_uint16_t option) +{ + switch (option) +{ +case GRUB_DHCPv6_OPTION_BOOTFILE_URL: + return BOOTFILE URL; +case GRUB_DHCPv6_OPTION_DNS_SERVERS: + return DNS SERVERS; +case GRUB_DHCPv6_OPTION_IA_NA: + return IA NA; +case GRUB_DHCPv6_OPTION_IAADDR: + return IAADDR; +case GRUB_DHCPv6_OPTION_CLIENTID: + return CLIENTID; +case GRUB_DHCPv6_OPTION_SERVERID: + return SERVERID; +case GRUB_DHCPv6_OPTION_ORO: + return ORO; +case GRUB_DHCPv6_OPTION_ELAPSED_TIME: + return ELAPSED TIME; +default: + return UNKNOWN; +} +} + +static const struct grub_dhcpv6_option* +find_dhcpv6_option (const struct grub_dhcpv6_option *popt, grub_size_t size, grub_uint16_t option) +{ + while (size 0) +{ + grub_uint16_t code, len; + + code = grub_be_to_cpu16 (popt-code); + len = grub_be_to_cpu16 (popt-len) + sizeof (*popt); + + if (size len) + { + grub_error (GRUB_ERR_OUT_OF_RANGE, N_(DHCPv6 option size overflow detected)); + return NULL; + } + + if (option == code) + return popt; + + if (code == 0) + break; + else + { + size -= len; + popt = (const struct grub_dhcpv6_option *)((grub_uint8_t *)popt + len); + } +} + + grub_error (GRUB_ERR_IO, N_(DHCPv6 Option (%u):%s not found), option, get_dhcpv6_option_name(option)); + return NULL; +} + +static const struct grub_dhcpv6_option* +find_dhcpv6_option_in_packet (const struct grub_net_dhcpv6_packet *packet, +grub_size_t size, grub_uint16_t option) +{ + if (!size || !packet) +{ + grub_error (GRUB_ERR_BAD_ARGUMENT, N_(null or zero sized DHCPv6 packet buffer)); + return NULL; +} + + if (size = sizeof (*packet)) +{ + grub_error (GRUB_ERR_BAD_ARGUMENT, N_(DHCPv6 packet size too small));
Re: [PATCH 1/3] Added net_bootp6 command
В Fri, 17 Apr 2015 13:04:27 +0800 Michael Chang mch...@suse.com пишет: +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 = (const struct grub_dhcpv6_option *)packet-dhcp_options; + code = grub_be_to_cpu16 (popt-code); + len = grub_be_to_cpu16 (popt-len); + + while (0 != code option != code) This probably needs upper boundary check in case we are dealing with corrupted packets. Do you mean checking the option length can't exceed netbuff length? Yes (and in all other cases below as well). The ( 0!= code ) did certain kind of upper boundary check for me, but I know you may disagree that. :) Btw, in grub-core/net/ip.c::handle_dgram() has checksum for the receiving packets, so the corrputed packets wouldn't sneak in easily. Correct checksum just means content was not altered; it does not mean content was correct to start with. + + ip_start = ip_end = NULL; + ip_start = bootfile_url + grub_strlen(pr); + + if (*ip_start != '[') +ip_start = NULL; + else +ip_end = 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? 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 = NULL; + + if (path) +*path = NULL; + + if (v6-message_type != DHCPv6_REPLY) Is it really possible? We come here if packet is DHCPv6_REPLY only. 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 == DHCPv6_REPLY. So the only reason it can be different here is physical memory corruption. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 1/3] Added net_bootp6 command
On Sun, Apr 19, 2015 at 11:15:21AM +0300, Andrei Borzenkov wrote: В Fri, 17 Apr 2015 13:04:27 +0800 Michael Chang mch...@suse.com пишет: +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 = (const struct grub_dhcpv6_option *)packet-dhcp_options; + code = grub_be_to_cpu16 (popt-code); + len = grub_be_to_cpu16 (popt-len); + + while (0 != code option != code) This probably needs upper boundary check in case we are dealing with corrupted packets. Do you mean checking the option length can't exceed netbuff length? Yes (and in all other cases below as well). OK. The ( 0!= code ) did certain kind of upper boundary check for me, but I know you may disagree that. :) Btw, in grub-core/net/ip.c::handle_dgram() has checksum for the receiving packets, so the corrputed packets wouldn't sneak in easily. Correct checksum just means content was not altered; it does not mean content was correct to start with. I agree with you that it doesn't mean the content was OK to start with, the sender might intentionally or unintentionally fill the payload length wrong that triggers buffer overflow on the client side. FWIW, the answer is subject to corrupted packet that usually means corruption during the transit and checksum is generally used for detecting such errors . + + ip_start = ip_end = NULL; + ip_start = bootfile_url + grub_strlen(pr); + + if (*ip_start != '[') +ip_start = NULL; + else +ip_end = 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? 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? The DNS name could be legit for URL, but current elilo and edk2 explicitly checks for the starting and ending delimiter '[..]' for using the server address and any name is treated as invalid parameters. The implementation just followed them. +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 = NULL; + + if (path) +*path = NULL; + + if (v6-message_type != DHCPv6_REPLY) Is it really possible? We come here if packet is DHCPv6_REPLY only. 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 == DHCPv6_REPLY. So the only reason it can be different here is physical memory corruption. It also gets called from grub_efi_net_config_real () which doesn't have any check for message type. Thanks, Michael ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 1/3] Added net_bootp6 command
В Mon, 20 Apr 2015 11:09:10 +0800 Michael Chang mch...@suse.com пишет: + + ip_start = ip_end = NULL; + ip_start = bootfile_url + grub_strlen(pr); + + if (*ip_start != '[') +ip_start = NULL; + else +ip_end = 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? 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? The DNS name could be legit for URL, but current elilo and edk2 explicitly checks for the starting and ending delimiter '[..]' for using the server address and any name is treated as invalid parameters. The implementation just followed them. OK, could you put in comments here which explain it, probably marked as TODO. Yes, but what I mean - this function is only called when we already checked for message_type == DHCPv6_REPLY. So the only reason it can be different here is physical memory corruption. It also gets called from grub_efi_net_config_real () which doesn't have any check for message type. So you do not trust firmware, do you? :) But OK, let's leave check in place. P.S. grub_efi_net_config_real should actually have checked DhcpAckReceived to start with, but I'm afraid to add it now, at least until we have real report of invalid packages. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 1/3] Added net_bootp6 command
В Wed, 15 Apr 2015 17:05:07 +0800 Michael Chang mch...@suse.com пишет: 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(-) 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 grub/net/netbuff.h #include grub/net/udp.h #include grub/datetime.h +#include grub/time.h static void parse_dhcp_vendor (const char *name, const void *vend, int limit, int *mask) @@ -256,6 +257,653 @@ grub_net_configure_by_dhcp_ack (const char *name, return inter; } +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 = NULL; +#define FOR_DHCPV6_SESSIONS(var) \ +for (var = grub_dhcpv6_sessions ; var; var = 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 = 0; + + err = grub_get_datetime (date); + if (err || !grub_datetime2unixtime (date, t)) +{ + grub_errno = GRUB_ERR_NONE; + t = 0; +} + + session-transaction_id = t; + session-start_time = grub_get_time_ms (); + + session-prev = grub_dhcpv6_sessions; + session-next = grub_dhcpv6_sessions; + + if (session-next) +session-next-prev = session-next; + grub_list_push + grub_dhcpv6_sessions = session; + return; +} + +static void +grub_dhcpv6_session_remove (struct grub_dhcpv6_session *session) +{ + *session-prev = session-next; + if (session-next) +session-next-prev = session-prev; + session-next = NULL; + session-prev = 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 = (const struct grub_dhcpv6_option *)packet-dhcp_options; + code = grub_be_to_cpu16 (popt-code); + len = grub_be_to_cpu16 (popt-len); + + while (0 != code option != code) This probably needs upper boundary check in case we are dealing with corrupted packets. +{ + popt = (const struct grub_dhcpv6_option *)((grub_uint8_t *)popt + + len + sizeof(*popt)); + code = grub_be_to_cpu16 (popt-code); + len = grub_be_to_cpu16 (popt-len); +} + + if (option == 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 = 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) != OPTION_IA_NA) if (popt == NULL) +{ + grub_error (GRUB_ERR_IO, N_(not an IA_NA DHCPv6 option)); + return NULL; +} + + ia_na = (const struct grub_dhcpv6_iana_option *)popt-data; +
Re: [PATCH 1/3] Added net_bootp6 command
Hi Andrei, Please see my comments and please let me know any problems remains. I'll submit next version soon. Thanks a lot for your detailed review. Regards, Michael On Thu, Apr 16, 2015 at 05:40:56PM +0300, Andrei Borzenkov wrote: В Wed, 15 Apr 2015 17:05:07 +0800 Michael Chang mch...@suse.com пишет: 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(-) 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 grub/net/netbuff.h #include grub/net/udp.h #include grub/datetime.h +#include grub/time.h static void parse_dhcp_vendor (const char *name, const void *vend, int limit, int *mask) @@ -256,6 +257,653 @@ grub_net_configure_by_dhcp_ack (const char *name, return inter; } +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? No, It doesn't really matter that much. I was just sticking to use structure to represent any option data and hopefully that helps in understanding what it is. But in this case it seems to be the oppsite by treating it as 16 bytes chunk of data generates more tidy code and easier to understand. I can change to your suggested code as commented in get_dns function and remove this structure. +#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. OK. +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? No, One session for configuring one interface, the plural just created confusing. I'll fix it. +}; + +static struct grub_dhcpv6_session *grub_dhcpv6_sessions = NULL; +#define FOR_DHCPV6_SESSIONS(var) \ +for (var = grub_dhcpv6_sessions ; var; var = var-next) + FOR_LIST_ELEMENTS OK. +static void +grub_dhcpv6_session_add (struct grub_dhcpv6_session *session) +{ + struct grub_datetime date; + grub_err_t err; + grub_int32_t t = 0; + + err = grub_get_datetime (date); + if (err || !grub_datetime2unixtime (date, t)) +{ + grub_errno = GRUB_ERR_NONE; + t = 0; +} + + session-transaction_id = t; + session-start_time = grub_get_time_ms (); + + session-prev = grub_dhcpv6_sessions; + session-next = grub_dhcpv6_sessions; + + if (session-next) +session-next-prev = session-next; + grub_list_push OK. + grub_dhcpv6_sessions = session; + return; +} + +static void +grub_dhcpv6_session_remove (struct grub_dhcpv6_session *session) +{ + *session-prev = session-next; + if (session-next) +session-next-prev = session-prev; + session-next = NULL; + session-prev = NULL; + return; +} + grub_list_remove OK. +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 = (const struct grub_dhcpv6_option *)packet-dhcp_options; + code = grub_be_to_cpu16 (popt-code); + len = grub_be_to_cpu16 (popt-len); + + while (0 != code option != code) This probably needs upper boundary check in case we are dealing with corrupted packets. Do you mean checking the option length can't exceed netbuff length? The
[PATCH 1/3] Added net_bootp6 command
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(-) 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 grub/net/netbuff.h #include grub/net/udp.h #include grub/datetime.h +#include grub/time.h static void parse_dhcp_vendor (const char *name, const void *vend, int limit, int *mask) @@ -256,6 +257,653 @@ grub_net_configure_by_dhcp_ack (const char *name, return inter; } +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; + +#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 + +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; +}; + +static struct grub_dhcpv6_session *grub_dhcpv6_sessions = NULL; +#define FOR_DHCPV6_SESSIONS(var) \ +for (var = grub_dhcpv6_sessions ; var; var = var-next) + +static void +grub_dhcpv6_session_add (struct grub_dhcpv6_session *session) +{ + struct grub_datetime date; + grub_err_t err; + grub_int32_t t = 0; + + err = grub_get_datetime (date); + if (err || !grub_datetime2unixtime (date, t)) +{ + grub_errno = GRUB_ERR_NONE; + t = 0; +} + + session-transaction_id = t; + session-start_time = grub_get_time_ms (); + + session-prev = grub_dhcpv6_sessions; + session-next = grub_dhcpv6_sessions; + + if (session-next) +session-next-prev = session-next; + + grub_dhcpv6_sessions = session; + return; +} + +static void +grub_dhcpv6_session_remove (struct grub_dhcpv6_session *session) +{ + *session-prev = session-next; + if (session-next) +session-next-prev = session-prev; + session-next = NULL; + session-prev = NULL; + return; +} + +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 = (const struct grub_dhcpv6_option *)packet-dhcp_options; + code = grub_be_to_cpu16 (popt-code); + len = grub_be_to_cpu16 (popt-len); + + while (0 != code option != code) +{ + popt = (const struct grub_dhcpv6_option *)((grub_uint8_t *)popt + + len + sizeof(*popt)); + code = grub_be_to_cpu16 (popt-code); + len = grub_be_to_cpu16 (popt-len); +} + + if (option == code) + return popt; + + return NULL; +} + +static const grub_uint8_t* +find_dhcpv6_address (const struct grub_net_dhcpv6_packet *packet) +{ + const struct grub_dhcpv6_option* popt = 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) != OPTION_IA_NA) +{ + grub_error (GRUB_ERR_IO, N_(not an IA_NA DHCPv6 option)); + return NULL; +} + + ia_na = (const struct grub_dhcpv6_iana_option *)popt-data; + + if (grub_be_to_cpu16(popt-len) = sizeof (*ia_na)) +{ + grub_error (GRUB_ERR_IO, N_(invalid size for IAADDR)); + return NULL; +} + + ia_na_data_len = grub_be_to_cpu16(popt-len) - sizeof (*ia_na); + ia_na_data_offset = 0; + + iaaddr_hdr = (const struct grub_dhcpv6_option *) ia_na-data; + len = grub_be_to_cpu16 (iaaddr_hdr-len); + + while (grub_be_to_cpu16(iaaddr_hdr-code) != OPTION_IAADDR) +{ + ia_na_data_offset += (len + sizeof (*iaaddr_hdr)); + + if (ia_na_data_offset ia_na_data_len) + { + iaaddr_hdr =(const struct grub_dhcpv6_option *)(ia_na-data + + ia_na_data_offset); + len = grub_be_to_cpu16 (iaaddr_hdr-len); +