Merge pull request #1394 from joakimeriksson/nbrtable-nd6-fix

Removed memcpy that cause inconsistency in nbr-table when adding nd6 neighbors.

core/net/ipv6/uip-nd6.c

@@ -135,11 +135,13 @@ static uip_ds6_prefix_t *prefix; /**  Pointer to a prefix list entry */
 #if UIP_ND6_SEND_NA || UIP_ND6_SEND_RA || !UIP_CONF_ROUTER
 /*------------------------------------------------------------------*/
 /* Copy link-layer address from LLAO option to a word-aligned uip_lladdr_t */
-static void
+static int
-extract_lladdr_aligned(uip_lladdr_t *dest) {
+extract_lladdr_from_llao_aligned(uip_lladdr_t *dest) {
   if(dest != NULL && nd6_opt_llao != NULL) {
     memcpy(dest, &nd6_opt_llao[UIP_ND6_OPT_DATA_OFFSET], UIP_LLADDR_LEN);
+    return 1;
   }
+  return 0;
 }
 #endif /* UIP_ND6_SEND_NA || UIP_ND6_SEND_RA || !UIP_CONF_ROUTER */
 /*------------------------------------------------------------------*/
@@ -199,17 +201,23 @@ ns_input(void)
         goto discard;
       } else {
 #endif /*UIP_CONF_IPV6_CHECKS */
+        uip_lladdr_t lladdr_aligned;
+        extract_lladdr_from_llao_aligned(&lladdr_aligned);
         nbr = uip_ds6_nbr_lookup(&UIP_IP_BUF->srcipaddr);
         if(nbr == NULL) {
-          uip_lladdr_t lladdr_aligned;
-          extract_lladdr_aligned(&lladdr_aligned);
           uip_ds6_nbr_add(&UIP_IP_BUF->srcipaddr, &lladdr_aligned,
 			  0, NBR_STALE, NBR_TABLE_REASON_IPV6_ND, NULL);
         } else {
-          uip_lladdr_t *lladdr = (uip_lladdr_t *)uip_ds6_nbr_get_ll(nbr);
+          const uip_lladdr_t *lladdr = uip_ds6_nbr_get_ll(nbr);
+          if(lladdr == NULL) {
+            goto discard;
+          }
           if(memcmp(&nd6_opt_llao[UIP_ND6_OPT_DATA_OFFSET],
               lladdr, UIP_LLADDR_LEN) != 0) {
-            memcpy(lladdr, &nd6_opt_llao[UIP_ND6_OPT_DATA_OFFSET], UIP_LLADDR_LEN);
+            if(nbr_table_update_lladdr((const linkaddr_t *)lladdr, (const linkaddr_t *)&lladdr_aligned, 1) == 0) {
+              /* failed to update the lladdr */
+              goto discard;
+            }
             nbr->state = NBR_STALE;
           } else {
             if(nbr->state == NBR_INCOMPLETE) {
@@ -428,6 +436,7 @@ na_input(void)
   uint8_t is_router;
   uint8_t is_solicited;
   uint8_t is_override;
+  uip_lladdr_t lladdr_aligned;
 
   PRINTF("Received NA from ");
   PRINT6ADDR(&UIP_IP_BUF->srcipaddr);
@@ -490,23 +499,29 @@ na_input(void)
     PRINTF("NA received is bad\n");
     goto discard;
   } else {
-    uip_lladdr_t *lladdr;
+    const uip_lladdr_t *lladdr;
     nbr = uip_ds6_nbr_lookup(&UIP_ND6_NA_BUF->tgtipaddr);
-    lladdr = (uip_lladdr_t *)uip_ds6_nbr_get_ll(nbr);
     if(nbr == NULL) {
       goto discard;
     }
-    if(nd6_opt_llao != 0) {
+    lladdr = uip_ds6_nbr_get_ll(nbr);
+    if(lladdr == NULL) {
+      goto discard;
+    }
+    if(nd6_opt_llao != NULL) {
       is_llchange =
-        memcmp(&nd6_opt_llao[UIP_ND6_OPT_DATA_OFFSET], (void *)lladdr,
+        memcmp(&nd6_opt_llao[UIP_ND6_OPT_DATA_OFFSET], lladdr,
                UIP_LLADDR_LEN);
     }
     if(nbr->state == NBR_INCOMPLETE) {
-      if(nd6_opt_llao == NULL) {
+      if(nd6_opt_llao == NULL || !extract_lladdr_from_llao_aligned(&lladdr_aligned)) {
         goto discard;
       }
-      memcpy(lladdr, &nd6_opt_llao[UIP_ND6_OPT_DATA_OFFSET],
+      if(nbr_table_update_lladdr((const linkaddr_t *)lladdr, (const linkaddr_t *)&lladdr_aligned, 1) == 0) {
-             UIP_LLADDR_LEN);
+        /* failed to update the lladdr */
+        goto discard;
+      }
+
       if(is_solicited) {
         nbr->state = NBR_REACHABLE;
         nbr->nscount = 0;
@@ -518,27 +533,29 @@ na_input(void)
         nbr->state = NBR_STALE;
       }
       nbr->isrouter = is_router;
-    } else {
+    } else { /* NBR is not INCOMPLETE */
       if(!is_override && is_llchange) {
         if(nbr->state == NBR_REACHABLE) {
           nbr->state = NBR_STALE;
         }
         goto discard;
       } else {
-        if(is_override || (!is_override && nd6_opt_llao != 0 && !is_llchange)
+        /**
-           || nd6_opt_llao == 0) {
+         *  If this is an cache override, or same lladdr, or no llao -
-          if(nd6_opt_llao != 0) {
+         *  do updates of nbr states.
-            memcpy(lladdr, &nd6_opt_llao[UIP_ND6_OPT_DATA_OFFSET],
+         */
-                   UIP_LLADDR_LEN);
+        if(is_override || !is_llchange || nd6_opt_llao == NULL) {
+          if(nd6_opt_llao != NULL && is_llchange) {
+            if(!extract_lladdr_from_llao_aligned(&lladdr_aligned) ||
+               nbr_table_update_lladdr((const linkaddr_t *) lladdr, (const linkaddr_t *) &lladdr_aligned, 1) == 0) {
+              /* failed to update the lladdr */
+              goto discard;
+            }
           }
           if(is_solicited) {
             nbr->state = NBR_REACHABLE;
             /* reachable time is stored in ms */
             stimer_set(&(nbr->reachable), uip_ds6_if.reachable_time / 1000);
-          } else {
-            if(nd6_opt_llao != 0 && is_llchange) {
-              nbr->state = NBR_STALE;
-            }
           }
         }
       }
@@ -632,15 +649,19 @@ rs_input(void)
     } else {
 #endif /*UIP_CONF_IPV6_CHECKS */
       uip_lladdr_t lladdr_aligned;
-      extract_lladdr_aligned(&lladdr_aligned);
+      extract_lladdr_from_llao_aligned(&lladdr_aligned);
       if((nbr = uip_ds6_nbr_lookup(&UIP_IP_BUF->srcipaddr)) == NULL) {
         /* we need to add the neighbor */
         uip_ds6_nbr_add(&UIP_IP_BUF->srcipaddr, &lladdr_aligned,
                         0, NBR_STALE, NBR_TABLE_REASON_IPV6_ND, NULL);
       } else {
         /* If LL address changed, set neighbor state to stale */
+        const uip_lladdr_t *lladdr = uip_ds6_nbr_get_ll(nbr);
+        if(lladdr == NULL) {
+          goto discard;
+        }
         if(memcmp(&nd6_opt_llao[UIP_ND6_OPT_DATA_OFFSET],
-                  uip_ds6_nbr_get_ll(nbr), UIP_LLADDR_LEN) != 0) {
+            lladdr, UIP_LLADDR_LEN) != 0) {
           uip_ds6_nbr_t nbr_data = *nbr;
           uip_ds6_nbr_rm(nbr);
           nbr = uip_ds6_nbr_add(&UIP_IP_BUF->srcipaddr, &lladdr_aligned,
@@ -826,6 +847,8 @@ uip_nd6_rs_output(void)
 void
 ra_input(void)
 {
+  uip_lladdr_t lladdr_aligned;
+
   PRINTF("Received RA from ");
   PRINT6ADDR(&UIP_IP_BUF->srcipaddr);
   PRINTF(" to ");
@@ -870,20 +893,28 @@ ra_input(void)
       PRINTF("Processing SLLAO option in RA\n");
       nd6_opt_llao = (uint8_t *) UIP_ND6_OPT_HDR_BUF;
       nbr = uip_ds6_nbr_lookup(&UIP_IP_BUF->srcipaddr);
+      if(!extract_lladdr_from_llao_aligned(&lladdr_aligned)) {
+        /* failed to extract llao - discard packet */
+        goto discard;
+      }
       if(nbr == NULL) {
-        uip_lladdr_t lladdr_aligned;
-        extract_lladdr_aligned(&lladdr_aligned);
         nbr = uip_ds6_nbr_add(&UIP_IP_BUF->srcipaddr, &lladdr_aligned,
                               1, NBR_STALE, NBR_TABLE_REASON_IPV6_ND, NULL);
       } else {
-        uip_lladdr_t *lladdr = (uip_lladdr_t *)uip_ds6_nbr_get_ll(nbr);
+        const uip_lladdr_t *lladdr = uip_ds6_nbr_get_ll(nbr);
+        if(lladdr == NULL) {
+          goto discard;
+        }
         if(nbr->state == NBR_INCOMPLETE) {
           nbr->state = NBR_STALE;
         }
         if(memcmp(&nd6_opt_llao[UIP_ND6_OPT_DATA_OFFSET],
                   lladdr, UIP_LLADDR_LEN) != 0) {
-          memcpy(lladdr, &nd6_opt_llao[UIP_ND6_OPT_DATA_OFFSET],
+          /* change of link layer address */
-                 UIP_LLADDR_LEN);
+          if(nbr_table_update_lladdr((const linkaddr_t *)lladdr, (const linkaddr_t *)&lladdr_aligned, 1) == 0) {
+            /* failed to update the lladdr */
+            goto discard;
+          }
           nbr->state = NBR_STALE;
         }
         nbr->isrouter = 1;

core/net/nbr-table.c

@@ -178,6 +178,25 @@ nbr_set_bit(uint8_t *bitmap, nbr_table_t *table, nbr_table_item_t *item, int val
   return 0;
 }
 /*---------------------------------------------------------------------------*/
+static void
+remove_key(nbr_table_key_t *least_used_key)
+{
+  int i;
+  for(i = 0; i < MAX_NUM_TABLES; i++) {
+    if(all_tables[i] != NULL && all_tables[i]->callback != NULL) {
+      /* Call table callback for each table that uses this item */
+      nbr_table_item_t *removed_item = item_from_key(all_tables[i], least_used_key);
+      if(nbr_get_bit(used_map, all_tables[i], removed_item) == 1) {
+        all_tables[i]->callback(removed_item);
+      }
+    }
+  }
+  /* Empty used map */
+  used_map[index_from_key(least_used_key)] = 0;
+  /* Remove neighbor from list */
+  list_remove(nbr_table_keys, least_used_key);
+}
+/*---------------------------------------------------------------------------*/
 static nbr_table_key_t *
 nbr_table_allocate(nbr_table_reason_t reason, void *data)
 {
@@ -253,21 +272,7 @@ nbr_table_allocate(nbr_table_reason_t reason, void *data)
       return NULL;
     } else {
       /* Reuse least used item */
-      int i;
+      remove_key(least_used_key);
-      for(i = 0; i<MAX_NUM_TABLES; i++) {
-        if(all_tables[i] != NULL && all_tables[i]->callback != NULL) {
-          /* Call table callback for each table that uses this item */
-          nbr_table_item_t *removed_item = item_from_key(all_tables[i], least_used_key);
-          if(nbr_get_bit(used_map, all_tables[i], removed_item) == 1) {
-            all_tables[i]->callback(removed_item);
-          }
-        }
-      }
-      /* Empty used map */
-      used_map[index_from_key(least_used_key)] = 0;
-      /* Remove neighbor from list */
-      list_remove(nbr_table_keys, least_used_key);
-      /* Return associated key */
       return least_used_key;
     }
   }
@@ -416,6 +421,39 @@ nbr_table_get_lladdr(nbr_table_t *table, const void *item)
   return key != NULL ? &key->lladdr : NULL;
 }
 /*---------------------------------------------------------------------------*/
+/* Update link-layer address of an item */
+int
+nbr_table_update_lladdr(const linkaddr_t *old_addr, const linkaddr_t *new_addr,
+                        int remove_if_duplicate)
+{
+  int index;
+  int new_index;
+  nbr_table_key_t *key;
+  index = index_from_lladdr(old_addr);
+  if(index == -1) {
+    /* Failure to change since there is nothing to change. */
+    return 0;
+  }
+  if((new_index = index_from_lladdr(new_addr)) != -1) {
+    /* check if it is a change or not - do not remove / fail if same */
+    if(new_index == index) {
+      return 1;
+    }
+    /* This new entry already exists - failure! - remove if requested. */
+    if(remove_if_duplicate) {
+      remove_key(key_from_index(index));
+    }
+    return 0;
+  }
+  key = key_from_index(index);
+  /**
+   * Copy the new lladdr into the key - since we know that there is no
+   * conflicting entry.
+   */
+  memcpy(&key->lladdr, new_addr, sizeof(linkaddr_t));
+  return 1;
+}
+/*---------------------------------------------------------------------------*/
 #if DEBUG
 static void
 print_table()

core/net/nbr-table.h

@@ -109,6 +109,7 @@ int nbr_table_unlock(nbr_table_t *table, nbr_table_item_t *item);
 /** \name Neighbor tables: address manipulation */
 /** @{ */
 linkaddr_t *nbr_table_get_lladdr(nbr_table_t *table, const nbr_table_item_t *item);
+int nbr_table_update_lladdr(const linkaddr_t *old_addr, const linkaddr_t *new_addr, int remove_if_duplicate);
 /** @} */
 
 #endif /* NBR_TABLE_H_ */