From e361c49f14fab76c9cc02116b10cecd5d2b3f789 Mon Sep 17 00:00:00 2001 From: Yasuyuki Tanaka Date: Mon, 27 Jun 2016 20:57:48 +0200 Subject: [PATCH 1/4] RPL non-storing: fix a bug causing an infinite loop It falls into an infinite loop if it goes to the default label in the switch statement of rpl_srh_get_next_hop() or rpl_process_srh_header(). --- core/net/rpl/rpl-ext-header.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/core/net/rpl/rpl-ext-header.c b/core/net/rpl/rpl-ext-header.c index b6d17d4a9..655df1674 100644 --- a/core/net/rpl/rpl-ext-header.c +++ b/core/net/rpl/rpl-ext-header.c @@ -195,12 +195,6 @@ rpl_srh_get_next_hop(uip_ipaddr_t *ipaddr) /* Look for routing header */ while(uip_next_hdr != NULL && *uip_next_hdr != UIP_PROTO_ROUTING) { switch(*uip_next_hdr) { - case UIP_PROTO_TCP: - case UIP_PROTO_UDP: - case UIP_PROTO_ICMP6: - case UIP_PROTO_NONE: - uip_next_hdr = NULL; - break; case UIP_PROTO_HBHO: case UIP_PROTO_DESTO: case UIP_PROTO_FRAG: @@ -211,6 +205,7 @@ rpl_srh_get_next_hop(uip_ipaddr_t *ipaddr) uip_next_hdr = &UIP_EXT_BUF->next; break; default: + uip_next_hdr = NULL; break; } } @@ -249,12 +244,6 @@ rpl_process_srh_header(void) /* Look for routing header */ while(uip_next_hdr != NULL && *uip_next_hdr != UIP_PROTO_ROUTING) { switch(*uip_next_hdr) { - case UIP_PROTO_TCP: - case UIP_PROTO_UDP: - case UIP_PROTO_ICMP6: - case UIP_PROTO_NONE: - uip_next_hdr = NULL; - break; case UIP_PROTO_HBHO: case UIP_PROTO_DESTO: case UIP_PROTO_FRAG: @@ -265,6 +254,7 @@ rpl_process_srh_header(void) uip_next_hdr = &UIP_EXT_BUF->next; break; default: + uip_next_hdr = NULL; break; } } From 5a79bad4b1da84d3e41985d113b800c30128dcab Mon Sep 17 00:00:00 2001 From: Yasuyuki Tanaka Date: Mon, 27 Jun 2016 20:57:48 +0200 Subject: [PATCH 2/4] RPL: prevent unintended memory access (rpl_remove_header) When it goes to the default label in the switch statement of rpl_remove_header(), UIP_EXT_BUF does not always point to an IPv6 extension header. "Move to next header" process should be done only in case of UIP_PROTO_DESTO. Otherwise, it returns with doing nothing. --- core/net/rpl/rpl-ext-header.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/core/net/rpl/rpl-ext-header.c b/core/net/rpl/rpl-ext-header.c index 655df1674..b13adb916 100644 --- a/core/net/rpl/rpl-ext-header.c +++ b/core/net/rpl/rpl-ext-header.c @@ -655,11 +655,6 @@ rpl_remove_header(void) /* Look for hop-by-hop and routing headers */ while(uip_next_hdr != NULL) { switch(*uip_next_hdr) { - case UIP_PROTO_TCP: - case UIP_PROTO_UDP: - case UIP_PROTO_ICMP6: - case UIP_PROTO_NONE: - return; case UIP_PROTO_HBHO: case UIP_PROTO_ROUTING: /* Remove hop-by-hop and routing headers */ @@ -674,13 +669,22 @@ rpl_remove_header(void) PRINTF("RPL: Removing RPL extension header (type %u, len %u)\n", *uip_next_hdr, rpl_ext_hdr_len); memmove(UIP_EXT_BUF, ((uint8_t *)UIP_EXT_BUF) + rpl_ext_hdr_len, uip_len - UIP_IPH_LEN); break; - default: + case UIP_PROTO_DESTO: + /* + * As per RFC 2460, any header other than the Destination + * Options header does not appear between the Hop-by-Hop + * Options header and the Routing header. + * + * We're moving to the next header only if uip_next_hdr has + * UIP_PROTO_DESTO. Otherwise, we'll return. + */ /* Move to next header */ if(uip_next_hdr != &UIP_IP_BUF->proto) { uip_ext_len += (UIP_EXT_BUF->len << 3) + 8; } uip_next_hdr = &UIP_EXT_BUF->next; - break; + default: + return; } } } From 13f18fd842bf93223fb54481a3e6d5bdef921f1c Mon Sep 17 00:00:00 2001 From: Yasuyuki Tanaka Date: Mon, 27 Jun 2016 20:57:48 +0200 Subject: [PATCH 3/4] RPL: update uip_ext_len whenever moving to next header (rpl-ext-header.c) --- core/net/rpl/rpl-ext-header.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/core/net/rpl/rpl-ext-header.c b/core/net/rpl/rpl-ext-header.c index b13adb916..e7f6428a3 100644 --- a/core/net/rpl/rpl-ext-header.c +++ b/core/net/rpl/rpl-ext-header.c @@ -199,10 +199,8 @@ rpl_srh_get_next_hop(uip_ipaddr_t *ipaddr) case UIP_PROTO_DESTO: case UIP_PROTO_FRAG: /* Move to next header */ - if(uip_next_hdr != &UIP_IP_BUF->proto) { - uip_ext_len += (UIP_EXT_BUF->len << 3) + 8; - } uip_next_hdr = &UIP_EXT_BUF->next; + uip_ext_len += (UIP_EXT_BUF->len << 3) + 8; break; default: uip_next_hdr = NULL; @@ -248,10 +246,8 @@ rpl_process_srh_header(void) case UIP_PROTO_DESTO: case UIP_PROTO_FRAG: /* Move to next header */ - if(uip_next_hdr != &UIP_IP_BUF->proto) { - uip_ext_len += (UIP_EXT_BUF->len << 3) + 8; - } uip_next_hdr = &UIP_EXT_BUF->next; + uip_ext_len += (UIP_EXT_BUF->len << 3) + 8; break; default: uip_next_hdr = NULL; @@ -679,10 +675,8 @@ rpl_remove_header(void) * UIP_PROTO_DESTO. Otherwise, we'll return. */ /* Move to next header */ - if(uip_next_hdr != &UIP_IP_BUF->proto) { - uip_ext_len += (UIP_EXT_BUF->len << 3) + 8; - } uip_next_hdr = &UIP_EXT_BUF->next; + uip_ext_len += (UIP_EXT_BUF->len << 3) + 8; default: return; } From ffdc53718ddef7175e911a3fc45b8eb1693c5800 Mon Sep 17 00:00:00 2001 From: Yasuyuki Tanaka Date: Mon, 27 Jun 2016 20:57:48 +0200 Subject: [PATCH 4/4] RPL: code cleanup (rpl-ext-header.c) --- core/net/rpl/rpl-ext-header.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/core/net/rpl/rpl-ext-header.c b/core/net/rpl/rpl-ext-header.c index e7f6428a3..5ba429a26 100644 --- a/core/net/rpl/rpl-ext-header.c +++ b/core/net/rpl/rpl-ext-header.c @@ -154,8 +154,8 @@ rpl_verify_hbh_header(int uip_ext_opt_offset) if((down && !sender_closer) || (!down && sender_closer)) { PRINTF("RPL: Loop detected - senderrank: %d my-rank: %d sender_closer: %d\n", - sender_rank, instance->current_dag->rank, - sender_closer); + sender_rank, instance->current_dag->rank, + sender_closer); /* Attempt to repair the loop by sending a unicast DIO back to the sender * so that it gets a fresh update of our rank. */ if(sender != NULL) { @@ -197,7 +197,11 @@ rpl_srh_get_next_hop(uip_ipaddr_t *ipaddr) switch(*uip_next_hdr) { case UIP_PROTO_HBHO: case UIP_PROTO_DESTO: - case UIP_PROTO_FRAG: + /* + * As per RFC 2460, only the Hop-by-Hop Options header and + * Destination Options header can appear before the Routing + * header. + */ /* Move to next header */ uip_next_hdr = &UIP_EXT_BUF->next; uip_ext_len += (UIP_EXT_BUF->len << 3) + 8; @@ -244,7 +248,11 @@ rpl_process_srh_header(void) switch(*uip_next_hdr) { case UIP_PROTO_HBHO: case UIP_PROTO_DESTO: - case UIP_PROTO_FRAG: + /* + * As per RFC 2460, only the Hop-by-Hop Options header and + * Destination Options header can appear before the Routing + * header. + */ /* Move to next header */ uip_next_hdr = &UIP_EXT_BUF->next; uip_ext_len += (UIP_EXT_BUF->len << 3) + 8;