[src/trunk]: src/sys/netinet6 Fix a race condition of in6_ifinit

[ozaki-r <ozaki-r%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/07bcb6b50c2f
branches:  trunk
changeset: 357726:07bcb6b50c2f
user:      ozaki-r <ozaki-r%NetBSD.org@localhost>
date:      Thu Nov 23 07:09:20 2017 +0000

description:
Fix a race condition of in6_ifinit

in6_ifinit checks the number of IPv6 addresses on a given interface and
if it's zero (i.e., an IPv6 address being assigned to the interface
is the first one), call if_addr_init. However, the actual assignment of
the address (ifa_insert) is out of in6_ifinit. The check and the
assignment must be done atomically.

Fix it by holding in6_ifaddr_lock during in6_ifinit and ifa_insert.
And also add missing pserialize to IFADDR_READER_FOREACH.

diffstat:

 sys/netinet6/in6.c |  26 +++++++++++++++++---------
 1 files changed, 17 insertions(+), 9 deletions(-)

diffs (93 lines):

diff -r 8c57d9057e82 -r 07bcb6b50c2f sys/netinet6/in6.c
--- a/sys/netinet6/in6.c        Thu Nov 23 07:06:14 2017 +0000
+++ b/sys/netinet6/in6.c        Thu Nov 23 07:09:20 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: in6.c,v 1.253 2017/11/23 07:06:14 ozaki-r Exp $        */
+/*     $NetBSD: in6.c,v 1.254 2017/11/23 07:09:20 ozaki-r Exp $        */
 /*     $KAME: in6.c,v 1.198 2001/07/18 09:12:38 itojun Exp $   */
 
 /*
@@ -62,7 +62,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: in6.c,v 1.253 2017/11/23 07:06:14 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: in6.c,v 1.254 2017/11/23 07:09:20 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -1258,30 +1258,36 @@
                 */
                ifaref(&ia->ia_ifa);
        }
+
+       /* Must execute in6_ifinit and ifa_insert atomically */
+       mutex_enter(&in6_ifaddr_lock);
+
        /* reset the interface and routing table appropriately. */
        error = in6_ifinit(ifp, ia, &ifra->ifra_addr, hostIsNew);
        if (error != 0) {
                if (hostIsNew)
                        free(ia, M_IFADDR);
+               mutex_exit(&in6_ifaddr_lock);
                return error;
        }
 
        /*
         * We are done if we have simply modified an existing address.
         */
-       if (!hostIsNew)
+       if (!hostIsNew) {
+               mutex_exit(&in6_ifaddr_lock);
                return error;
+       }
 
        /*
         * Insert ia to the global list and ifa to the interface's list.
         * A reference to it is already gained above.
         */
-       mutex_enter(&in6_ifaddr_lock);
        IN6_ADDRLIST_WRITER_INSERT_TAIL(ia);
+       ifa_insert(ifp, &ia->ia_ifa);
+
        mutex_exit(&in6_ifaddr_lock);
 
-       ifa_insert(ifp, &ia->ia_ifa);
-
        /*
         * Beyond this point, we should call in6_purgeaddr upon an error,
         * not just go to unlink.
@@ -1755,28 +1761,30 @@
        const struct sockaddr_in6 *sin6, int newhost)
 {
        int     error = 0, ifacount = 0;
-       int     s = splsoftnet();
+       int s;
        struct ifaddr *ifa;
 
+       KASSERT(mutex_owned(&in6_ifaddr_lock));
+
        /*
         * Give the interface a chance to initialize
         * if this is its first address,
         * and to validate the address if necessary.
         */
+       s = pserialize_read_enter();
        IFADDR_READER_FOREACH(ifa, ifp) {
                if (ifa->ifa_addr->sa_family != AF_INET6)
                        continue;
                ifacount++;
        }
+       pserialize_read_exit(s);
 
        ia->ia_addr = *sin6;
 
        if (ifacount == 0 &&
            (error = if_addr_init(ifp, &ia->ia_ifa, true)) != 0) {
-               splx(s);
                return error;
        }
-       splx(s);
 
        ia->ia_ifa.ifa_metric = ifp->if_metric;