Merge pull request #14883 from miri64/gnrc_ipv6_nib/fix/ipv6_addr_idx-check

gnrc_ipv6_nib: check return value of gnrc_netif_ipv6_addr_idx()

sys/net/gnrc/network_layer/ipv6/nib/_nib-6ln.c

@@ -96,6 +96,14 @@ uint8_t _handle_aro(gnrc_netif_t *netif, const ipv6_hdr_t *ipv6,
                     uint16_t ltime = byteorder_ntohs(aro->ltime);
                     uint32_t rereg_time;
                     int idx = gnrc_netif_ipv6_addr_idx(netif, &ipv6->dst);
+
+                    if (idx < 0) {
+                        DEBUG("nib: Address %s is not assigned to interface "
+                              "%d ignoring ARO\n",
+                              ipv6_addr_to_str(addr_str, &ipv6->dst,
+                                               sizeof(addr_str)), netif->pid);
+                        return _ADDR_REG_STATUS_IGNORE;
+                    }
                     /* if ltime 1min, reschedule NS in 30sec, otherwise 1min
                      * before timeout */
                     rereg_time = (ltime == 1U) ? (30 * MS_PER_SEC) :
@@ -176,6 +184,8 @@ static inline bool _is_valid(const gnrc_netif_t *netif, int idx)
 void _handle_rereg_address(const ipv6_addr_t *addr)
 {
     gnrc_netif_t *netif = gnrc_netif_get_by_ipv6_addr(addr);
+
+    gnrc_netif_acquire(netif);
     _nib_dr_entry_t *router = _nib_drl_get(NULL, netif->pid);
     const bool router_reachable = (router != NULL) &&
                                   _is_reachable(router->next_hop);
@@ -197,10 +207,15 @@ void _handle_rereg_address(const ipv6_addr_t *addr)
     if (netif != NULL) {
         int idx = gnrc_netif_ipv6_addr_idx(netif, addr);
 
-        if (router_reachable &&
+        if (idx < 0) {
-            (_is_valid(netif, idx) || (_is_tentative(netif, idx) &&
+            DEBUG("nib: %s is not assigned to interface %d anymore.\n",
-             (gnrc_netif_ipv6_addr_dad_trans(netif, idx) <
+                  ipv6_addr_to_str(addr_str, addr, sizeof(addr_str)),
-              SIXLOWPAN_ND_REG_TRANSMIT_NUMOF)))) {
+                  netif->pid);
+        }
+        else if (router_reachable &&
+                 (_is_valid(netif, idx) || (_is_tentative(netif, idx) &&
+                 (gnrc_netif_ipv6_addr_dad_trans(netif, idx) <
+                 SIXLOWPAN_ND_REG_TRANSMIT_NUMOF)))) {
             uint32_t retrans_time;
 
             if (_is_valid(netif, idx)) {
@@ -219,6 +234,7 @@ void _handle_rereg_address(const ipv6_addr_t *addr)
             _handle_search_rtr(netif);
         }
     }
+    gnrc_netif_release(netif);
 }
 
 #if IS_ACTIVE(CONFIG_GNRC_IPV6_NIB_MULTIHOP_P6C)

sys/net/gnrc/network_layer/ipv6/nib/_nib-6lr.c

@@ -69,6 +69,21 @@ uint8_t _reg_addr_upstream(gnrc_netif_t *netif, const ipv6_hdr_t *ipv6,
                 _handle_sl2ao(netif, ipv6, icmpv6, sl2ao);
                 /* re-get NCE in case it was updated */
                 nce = _nib_onl_get(&ipv6->src, netif->pid);
+                /* and re-check EUI-64 in case nce was not an NC before */
+                if ((memcmp(&nce->eui64, &aro->eui64,
+                            sizeof(aro->eui64)) != 0) &&
+                    (_get_ar_state(nce) != GNRC_IPV6_NIB_NC_INFO_AR_STATE_GC)) {
+                    /* ignore address registration requests from upstream */
+                    DEBUG("nib: Could not register %s, duplicate entry with "
+                          "EUI-64 %02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x\n",
+                          ipv6_addr_to_str(addr_str, &ipv6->src,
+                                           sizeof(addr_str)),
+                          nce->eui64.uint8[0], nce->eui64.uint8[1],
+                          nce->eui64.uint8[2], nce->eui64.uint8[3],
+                          nce->eui64.uint8[4], nce->eui64.uint8[5],
+                          nce->eui64.uint8[6], nce->eui64.uint8[7]);
+                    return SIXLOWPAN_ND_STATUS_DUP;
+                }
                 return _update_nce_ar_state(aro, nce);
             }
             else if (nce != NULL) {

sys/net/gnrc/network_layer/ipv6/nib/nib.c

@@ -916,21 +916,33 @@ static void _handle_nbr_sol(gnrc_netif_t *netif, const ipv6_hdr_t *ipv6,
     gnrc_netif_t *tgt_netif = gnrc_netif_get_by_ipv6_addr(&nbr_sol->tgt);
 
     if (tgt_netif != NULL) {
-        int idx = gnrc_netif_ipv6_addr_idx(tgt_netif, &nbr_sol->tgt);
+        int idx;
 
-        if (gnrc_netif_ipv6_addr_dad_trans(tgt_netif, idx)) {
+        gnrc_netif_acquire(tgt_netif);
+        idx = gnrc_netif_ipv6_addr_idx(tgt_netif, &nbr_sol->tgt);
+        /* if idx < 0:
+         * nbr_sol->tgt was removed between getting tgt_netif by nbr_sol->tgt
+         * and gnrc_netif_acquire(tgt_netif). This is like `tgt_netif` would
+         * have been NULL in the first place so just continue as if it would
+         * have. */
+        if ((idx >= 0) && gnrc_netif_ipv6_addr_dad_trans(tgt_netif, idx)) {
             if (!ipv6_addr_is_unspecified(&ipv6->src)) {
                 /* (see https://tools.ietf.org/html/rfc4862#section-5.4.3) */
                 DEBUG("nib: Neighbor is performing AR, but target address is "
                       "still TENTATIVE for us => Ignoring NS\n");
+                gnrc_netif_release(tgt_netif);
                 return;
             }
             /* cancel validation timer */
             evtimer_del(&_nib_evtimer,
                         &tgt_netif->ipv6.addrs_timers[idx].event);
+            /* _remove_tentative_addr() context switches to `tgt_netif->pid` so
+             * release `tgt_netif`. We are done here anyway. */
+            gnrc_netif_release(tgt_netif);
             _remove_tentative_addr(tgt_netif, &nbr_sol->tgt);
             return;
         }
+        gnrc_netif_release(tgt_netif);
     }
 #endif  /* CONFIG_GNRC_IPV6_NIB_SLAAC */
     if (ipv6_addr_is_unspecified(&ipv6->src)) {
@@ -1048,19 +1060,30 @@ static void _handle_nbr_adv(gnrc_netif_t *netif, const ipv6_hdr_t *ipv6,
     gnrc_netif_t *tgt_netif = gnrc_netif_get_by_ipv6_addr(&nbr_adv->tgt);
 
     if (tgt_netif != NULL) {
-        int idx = gnrc_netif_ipv6_addr_idx(tgt_netif, &nbr_adv->tgt);
+        int idx;
 
-        if (gnrc_netif_ipv6_addr_dad_trans(tgt_netif, idx)) {
+        gnrc_netif_acquire(tgt_netif);
+        idx = gnrc_netif_ipv6_addr_idx(tgt_netif, &nbr_adv->tgt);
+        /* if idx < 0:
+         * nbr_sol->tgt was removed between getting tgt_netif by nbr_sol->tgt
+         * and gnrc_netif_acquire(tgt_netif). This is like `tgt_netif` would
+         * have been NULL in the first place so just continue as if it would
+         * have. */
+        if ((idx >= 0) && gnrc_netif_ipv6_addr_dad_trans(tgt_netif, idx)) {
             DEBUG("nib: duplicate address detected, removing target address "
                   "from this interface\n");
             /* cancel validation timer */
             evtimer_del(&_nib_evtimer,
                         &tgt_netif->ipv6.addrs_timers[idx].event);
+            /* _remove_tentative_addr() context switches to `tgt_netif->pid` so
+             * release `tgt_netif`. We are done here anyway. */
+            gnrc_netif_release(tgt_netif);
             _remove_tentative_addr(tgt_netif, &nbr_adv->tgt);
             return;
         }
         /* else case beyond scope of RFC4862:
          * https://tools.ietf.org/html/rfc4862#section-5.4.4 */
+        gnrc_netif_release(tgt_netif);
     }
 #endif  /* CONFIG_GNRC_IPV6_NIB_SLAAC */
     if (((nce = _nib_onl_get(&nbr_adv->tgt, netif->pid)) != NULL) &&