[src/netbsd-8]: src/sys Pull up following revision(s) (requested by ozaki-r i...

[martin <martin%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/9920624826a0
branches:  netbsd-8
changeset: 851421:9920624826a0
user:      martin <martin%NetBSD.org@localhost>
date:      Mon Feb 26 13:36:01 2018 +0000

description:
Pull up following revision(s) (requested by ozaki-r in ticket #589):
        sys/netinet/if_arp.c: revision 1.267
        sys/netinet6/nd6_nbr.c: revision 1.146-1.148

Use KASSERT for checking a programming error

Simplify; pass dp to nd6_dad_duplicated instead of looking it up again in it

Avoid a race condition of DAD timer destructions

When we see dp->dad_ifa == NULL, it means that the ifa is being deleted and also
the callout is scheduled again by someone.  We shouldn't rely on a result of
callout_pending to know if the callout is scheduled because it returns false if
the subsequent callout handler is already on the fly.
We have to always delegate the destruction of dp to the subsequent handler
unconditionally if dp->dad_ifa == NULL. Otherwise, the first handler destroys
the dp and the second handler tries to handle destroyed dp.

diffstat:

 sys/netinet/if_arp.c   |  17 ++++--------
 sys/netinet6/nd6_nbr.c |  64 +++++++++++++++++++------------------------------
 2 files changed, 31 insertions(+), 50 deletions(-)

diffs (206 lines):

diff -r 8f950de5b960 -r 9920624826a0 sys/netinet/if_arp.c
--- a/sys/netinet/if_arp.c      Mon Feb 26 13:32:01 2018 +0000
+++ b/sys/netinet/if_arp.c      Mon Feb 26 13:36:01 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_arp.c,v 1.250.2.5 2018/01/26 15:41:12 martin Exp $  */
+/*     $NetBSD: if_arp.c,v 1.250.2.6 2018/02/26 13:36:01 martin Exp $  */
 
 /*-
  * Copyright (c) 1998, 2000, 2008 The NetBSD Foundation, Inc.
@@ -68,7 +68,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_arp.c,v 1.250.2.5 2018/01/26 15:41:12 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_arp.c,v 1.250.2.6 2018/02/26 13:36:01 martin Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_ddb.h"
@@ -1686,15 +1686,10 @@
 
        ifa = dp->dad_ifa;
        if (ifa == NULL) {
-               /* ifa is being deleted. DAD should be freed too. */
-               if (callout_pending(&dp->dad_timer_ch)) {
-                       /*
-                        * The callout is scheduled again, so we cannot destroy
-                        * dp in this run.
-                        */
-                       goto done;
-               }
-               need_free = true;
+               /*
+                * ifa is being deleted and the callout is already scheduled
+                * again, so we cannot destroy dp in this run.
+                */
                goto done;
        }
 
diff -r 8f950de5b960 -r 9920624826a0 sys/netinet6/nd6_nbr.c
--- a/sys/netinet6/nd6_nbr.c    Mon Feb 26 13:32:01 2018 +0000
+++ b/sys/netinet6/nd6_nbr.c    Mon Feb 26 13:36:01 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: nd6_nbr.c,v 1.138.6.3 2018/02/02 12:55:08 martin Exp $ */
+/*     $NetBSD: nd6_nbr.c,v 1.138.6.4 2018/02/26 13:36:01 martin Exp $ */
 /*     $KAME: nd6_nbr.c,v 1.61 2001/02/10 16:06:14 jinmei Exp $        */
 
 /*
@@ -31,7 +31,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: nd6_nbr.c,v 1.138.6.3 2018/02/02 12:55:08 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: nd6_nbr.c,v 1.138.6.4 2018/02/26 13:36:01 martin Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -84,7 +84,7 @@
 static void nd6_dad_ns_output(struct dadq *, struct ifaddr *);
 static void nd6_dad_ns_input(struct ifaddr *);
 static void nd6_dad_na_input(struct ifaddr *);
-static void nd6_dad_duplicated(struct ifaddr *);
+static void nd6_dad_duplicated(struct dadq *);
 
 static int dad_ignore_ns = 0;  /* ignore NS in DAD - specwise incorrect*/
 static int dad_maxtry = 15;    /* max # of *tries* to transmit DAD packet */
@@ -1092,10 +1092,13 @@
 static void
 nd6_dad_destroytimer(struct dadq *dp)
 {
+       struct ifaddr *ifa;
 
        TAILQ_REMOVE(&dadq, dp, dad_list);
        /* Request the timer to destroy dp. */
+       ifa = dp->dad_ifa;
        dp->dad_ifa = NULL;
+       ifafree(ifa);
        callout_reset(&dp->dad_timer_ch, 0,
            (void (*)(void *))nd6_dad_timer, dp);
 }
@@ -1212,8 +1215,6 @@
        nd6_dad_destroytimer(dp);
 
        mutex_exit(&nd6_dad_lock);
-
-       ifafree(ifa);
 }
 
 static void
@@ -1230,15 +1231,10 @@
 
        ifa = dp->dad_ifa;
        if (ifa == NULL) {
-               /* ifa is being deleted. DAD should be freed too. */
-               if (callout_pending(&dp->dad_timer_ch)) {
-                       /*
-                        * The callout is scheduled again, so we cannot destroy
-                        * dp in this run.
-                        */
-                       goto done;
-               }
-               need_free = true;
+               /*
+                * ifa is being deleted and the callout is already scheduled
+                * again, so we cannot destroy dp in this run.
+                */
                goto done;
        }
 
@@ -1315,6 +1311,9 @@
                }
        }
 done:
+       if (duplicate)
+               nd6_dad_duplicated(dp);
+
        mutex_exit(&nd6_dad_lock);
 
        if (need_free) {
@@ -1324,29 +1323,22 @@
                        ifafree(ifa);
        }
 
-       if (duplicate)
-               nd6_dad_duplicated(ifa);
-
        SOFTNET_KERNEL_UNLOCK_UNLESS_NET_MPSAFE();
 }
 
 static void
-nd6_dad_duplicated(struct ifaddr *ifa)
+nd6_dad_duplicated(struct dadq *dp)
 {
-       struct in6_ifaddr *ia = (struct in6_ifaddr *)ifa;
+       struct ifaddr *ifa = dp->dad_ifa;
+       struct in6_ifaddr *ia;
        struct ifnet *ifp;
-       struct dadq *dp;
        char ip6buf[INET6_ADDRSTRLEN];
 
-       mutex_enter(&nd6_dad_lock);
-       dp = nd6_dad_find(ifa);
-       if (dp == NULL) {
-               mutex_exit(&nd6_dad_lock);
-               /* DAD seems to be stopping, so do nothing. */
-               return;
-       }
+       KASSERT(mutex_owned(&nd6_dad_lock));
+       KASSERT(ifa != NULL);
 
        ifp = ifa->ifa_ifp;
+       ia = (struct in6_ifaddr *)ifa;
        log(LOG_ERR, "%s: DAD detected duplicate IPv6 address %s: "
            "NS in/out=%d/%d, NA in=%d\n",
            if_name(ifp), IN6_PRINT(ip6buf, &ia->ia_addr.sin6_addr),
@@ -1399,10 +1391,6 @@
 
        /* We are done with DAD, with duplicated address found. (failure) */
        nd6_dad_destroytimer(dp);
-
-       mutex_exit(&nd6_dad_lock);
-
-       ifafree(ifa);
 }
 
 static void
@@ -1467,9 +1455,7 @@
        /* XXX more checks for loopback situation - see nd6_dad_timer too */
 
        if (duplicate) {
-               dp = NULL;      /* will be freed in nd6_dad_duplicated() */
-               mutex_exit(&nd6_dad_lock);
-               nd6_dad_duplicated(ifa);
+               nd6_dad_duplicated(dp);
        } else {
                /*
                 * not sure if I got a duplicate.
@@ -1477,8 +1463,8 @@
                 */
                if (dp)
                        dp->dad_ns_icount++;
-               mutex_exit(&nd6_dad_lock);
        }
+       mutex_exit(&nd6_dad_lock);
 }
 
 static void
@@ -1486,15 +1472,15 @@
 {
        struct dadq *dp;
 
-       if (ifa == NULL)
-               panic("ifa == NULL in nd6_dad_na_input");
+       KASSERT(ifa != NULL);
 
        mutex_enter(&nd6_dad_lock);
        dp = nd6_dad_find(ifa);
        if (dp)
                dp->dad_na_icount++;
-       mutex_exit(&nd6_dad_lock);
 
        /* remove the address. */
-       nd6_dad_duplicated(ifa);
+       nd6_dad_duplicated(dp);
+
+       mutex_exit(&nd6_dad_lock);
 }