Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys Make sure that ifaddr is published after its initializat...



details:   https://anonhg.NetBSD.org/src/rev/3c3ec3aa92f9
branches:  trunk
changeset: 346168:3c3ec3aa92f9
user:      ozaki-r <ozaki-r%NetBSD.org@localhost>
date:      Thu Jun 30 01:34:53 2016 +0000

description:
Make sure that ifaddr is published after its initialization finished

Basically we should insert an item to a collection (say a list) after
item's initialization has been completed to avoid accessing an item
that is initialized halfway. ifaddr (in{,6}_ifaddr) isn't processed
like so and needs to be fixed.

In order to do so, we need to tweak {arp,nd6}_rtrequest that depend
on that an ifaddr is inserted during its initialization; they explore
interface's address list to determine that rt_getkey(rt) of a given
rtentry is in the list to know whether the route's interface should
be a loopback, which doesn't work after the change. To make it work,
first check RTF_LOCAL flag that is set in rt_ifa_addlocal that calls
{arp,nd6}_rtrequest eventually. Note that we still need the original
code for the case to remove and re-add a local interface route.

diffstat:

 sys/net/if_spppsubr.c |  10 ++++++-
 sys/netinet/if_arp.c  |  18 ++++++++++++-
 sys/netinet/in.c      |  63 +++++++++++++++++++++++++++++++++-----------------
 sys/netinet6/in6.c    |  43 +++++++++++++++++++++-------------
 sys/netinet6/nd6.c    |  15 ++++++++++-
 5 files changed, 104 insertions(+), 45 deletions(-)

diffs (truncated from 376 to 300 lines):

diff -r 7893be1a2681 -r 3c3ec3aa92f9 sys/net/if_spppsubr.c
--- a/sys/net/if_spppsubr.c     Thu Jun 30 00:23:36 2016 +0000
+++ b/sys/net/if_spppsubr.c     Thu Jun 30 01:34:53 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_spppsubr.c,v 1.143 2016/06/20 08:30:59 knakahara Exp $       */
+/*     $NetBSD: if_spppsubr.c,v 1.144 2016/06/30 01:34:53 ozaki-r Exp $         */
 
 /*
  * Synchronous PPP/Cisco link level subroutines.
@@ -41,7 +41,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_spppsubr.c,v 1.143 2016/06/20 08:30:59 knakahara Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_spppsubr.c,v 1.144 2016/06/30 01:34:53 ozaki-r Exp $");
 
 #if defined(_KERNEL_OPT)
 #include "opt_inet.h"
@@ -4897,7 +4897,10 @@
                                *dest = new_dst; /* fix dstaddr in place */
                        }
                }
+               LIST_REMOVE(ifatoia(ifa), ia_hash);
                error = in_ifinit(ifp, ifatoia(ifa), &new_sin, 0, hostIsNew);
+               LIST_INSERT_HEAD(&IN_IFADDR_HASH(ifatoia(ifa)->ia_addr.sin_addr.s_addr),
+                   ifatoia(ifa), ia_hash);
                if (debug && error)
                {
                        log(LOG_DEBUG, "%s: sppp_set_ip_addrs: in_ifinit "
@@ -4950,7 +4953,10 @@
                if (sp->ipcp.flags & IPCP_HISADDR_DYN)
                        /* replace peer addr in place */
                        dest->sin_addr.s_addr = sp->ipcp.saved_hisaddr;
+               LIST_REMOVE(ifatoia(ifa), ia_hash);
                in_ifinit(ifp, ifatoia(ifa), &new_sin, 0, 0);
+               LIST_INSERT_HEAD(&IN_IFADDR_HASH(ifatoia(ifa)->ia_addr.sin_addr.s_addr),
+                   ifatoia(ifa), ia_hash);
                (void)pfil_run_hooks(if_pfil,
                    (struct mbuf **)SIOCDIFADDR, ifp, PFIL_IFADDR);
        }
diff -r 7893be1a2681 -r 3c3ec3aa92f9 sys/netinet/if_arp.c
--- a/sys/netinet/if_arp.c      Thu Jun 30 00:23:36 2016 +0000
+++ b/sys/netinet/if_arp.c      Thu Jun 30 01:34:53 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_arp.c,v 1.213 2016/06/28 02:02:56 ozaki-r Exp $     */
+/*     $NetBSD: if_arp.c,v 1.214 2016/06/30 01:34:53 ozaki-r 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.213 2016/06/28 02:02:56 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_arp.c,v 1.214 2016/06/30 01:34:53 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_ddb.h"
@@ -589,6 +589,20 @@
                if (rt->rt_flags & RTF_BROADCAST)
                        break;
 
+               /*
+                * When called from rt_ifa_addlocal, we cannot depend on that
+                * the address (rt_getkey(rt)) exits in the address list of the
+                * interface. So check RTF_LOCAL instead.
+                */
+               if (rt->rt_flags & RTF_LOCAL) {
+                       rt->rt_expire = 0;
+                       if (useloopback) {
+                               rt->rt_ifp = lo0ifp;
+                               rt->rt_rmx.rmx_mtu = 0;
+                       }
+                       break;
+               }
+
                INADDR_TO_IA(satocsin(rt_getkey(rt))->sin_addr, ia);
                while (ia && ia->ia_ifp != ifp)
                        NEXT_IA_WITH_SAME_ADDR(ia);
diff -r 7893be1a2681 -r 3c3ec3aa92f9 sys/netinet/in.c
--- a/sys/netinet/in.c  Thu Jun 30 00:23:36 2016 +0000
+++ b/sys/netinet/in.c  Thu Jun 30 01:34:53 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: in.c,v 1.168 2016/06/23 06:40:48 ozaki-r Exp $ */
+/*     $NetBSD: in.c,v 1.169 2016/06/30 01:34:53 ozaki-r Exp $ */
 
 /*
  * Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project.
@@ -91,7 +91,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: in.c,v 1.168 2016/06/23 06:40:48 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: in.c,v 1.169 2016/06/30 01:34:53 ozaki-r Exp $");
 
 #include "arp.h"
 
@@ -359,6 +359,8 @@
        struct sockaddr_in oldaddr;
        int error, hostIsNew, maskIsNew;
        int newifaddr = 0;
+       bool run_hook = false;
+       bool need_reinsert = false;
 
        switch (cmd) {
        case SIOCALIFADDR:
@@ -440,9 +442,6 @@
                        ia = malloc(sizeof(*ia), M_IFADDR, M_WAITOK|M_ZERO);
                        if (ia == NULL)
                                return (ENOBUFS);
-                       TAILQ_INSERT_TAIL(&in_ifaddrhead, ia, ia_list);
-                       ifaref(&ia->ia_ifa);
-                       ifa_insert(ifp, &ia->ia_ifa);
                        ia->ia_ifa.ifa_addr = sintosa(&ia->ia_addr);
                        ia->ia_ifa.ifa_dstaddr = sintosa(&ia->ia_dstaddr);
                        ia->ia_ifa.ifa_netmask = sintosa(&ia->ia_sockmask);
@@ -460,6 +459,7 @@
                        ia->ia_ifp = ifp;
                        ia->ia_idsalt = cprng_fast32() % 65535;
                        LIST_INIT(&ia->ia_multiaddrs);
+
                        newifaddr = 1;
                }
                break;
@@ -533,18 +533,24 @@
                break;
 
        case SIOCSIFADDR:
+               if (!newifaddr) {
+                       LIST_REMOVE(ia, ia_hash);
+                       need_reinsert = true;
+               }
                error = in_ifinit(ifp, ia, satocsin(ifreq_getaddr(cmd, ifr)),
                    1, hostIsNew);
-               if (error == 0) {
-                       (void)pfil_run_hooks(if_pfil,
-                           (struct mbuf **)SIOCSIFADDR, ifp, PFIL_IFADDR);
-               }
+
+               run_hook = true;
                break;
 
        case SIOCSIFNETMASK:
                in_ifscrub(ifp, ia);
                ia->ia_sockmask = *satocsin(ifreq_getaddr(cmd, ifr));
                ia->ia_subnetmask = ia->ia_sockmask.sin_addr.s_addr;
+               if (!newifaddr) {
+                       LIST_REMOVE(ia, ia_hash);
+                       need_reinsert = true;
+               }
                error = in_ifinit(ifp, ia, NULL, 0, 0);
                break;
 
@@ -570,15 +576,17 @@
                }
                if (ifra->ifra_addr.sin_family == AF_INET &&
                    (hostIsNew || maskIsNew)) {
+                       if (!newifaddr) {
+                               LIST_REMOVE(ia, ia_hash);
+                               need_reinsert = true;
+                       }
                        error = in_ifinit(ifp, ia, &ifra->ifra_addr, 0,
                            hostIsNew);
                }
                if ((ifp->if_flags & IFF_BROADCAST) &&
                    (ifra->ifra_broadaddr.sin_family == AF_INET))
                        ia->ia_broadaddr = ifra->ifra_broadaddr;
-               if (error == 0)
-                       (void)pfil_run_hooks(if_pfil,
-                           (struct mbuf **)SIOCAIFADDR, ifp, PFIL_IFADDR);
+               run_hook = true;
                break;
 
        case SIOCGIFALIAS:
@@ -600,8 +608,7 @@
 
        case SIOCDIFADDR:
                in_purgeaddr(&ia->ia_ifa);
-               (void)pfil_run_hooks(if_pfil, (struct mbuf **)SIOCDIFADDR,
-                   ifp, PFIL_IFADDR);
+               run_hook = true;
                break;
 
 #ifdef MROUTING
@@ -615,7 +622,26 @@
                return ENOTTY;
        }
 
-       if (error != 0 && newifaddr) {
+       /*
+        * XXX insert regardless of error to make in_purgeaddr below work.
+        * Need to improve.
+        */
+       if (newifaddr) {
+               TAILQ_INSERT_TAIL(&in_ifaddrhead, ia, ia_list);
+               ifaref(&ia->ia_ifa);
+               ifa_insert(ifp, &ia->ia_ifa);
+               LIST_INSERT_HEAD(&IN_IFADDR_HASH(ia->ia_addr.sin_addr.s_addr),
+                   ia, ia_hash);
+       } else if (need_reinsert) {
+               LIST_INSERT_HEAD(&IN_IFADDR_HASH(ia->ia_addr.sin_addr.s_addr),
+                   ia, ia_hash);
+       }
+
+       if (error == 0) {
+               if (run_hook)
+                       (void)pfil_run_hooks(if_pfil,
+                           (struct mbuf **)cmd, ifp, PFIL_IFADDR);
+       } else if (newifaddr) {
                KASSERT(ia != NULL);
                in_purgeaddr(&ia->ia_ifa);
        }
@@ -908,10 +934,7 @@
         * Set up new addresses.
         */
        oldaddr = ia->ia_addr;
-       if (ia->ia_addr.sin_family == AF_INET)
-               LIST_REMOVE(ia, ia_hash);
        ia->ia_addr = *sin;
-       LIST_INSERT_HEAD(&IN_IFADDR_HASH(ia->ia_addr.sin_addr.s_addr), ia, ia_hash);
 
        /* Set IN_IFF flags early for if_addr_init() */
        if (hostIsNew && if_do_dad(ifp) && !in_nullhost(ia->ia_addr.sin_addr)) {
@@ -1002,11 +1025,7 @@
        return (error);
 bad:
        splx(s);
-       LIST_REMOVE(ia, ia_hash);
        ia->ia_addr = oldaddr;
-       if (ia->ia_addr.sin_family == AF_INET)
-               LIST_INSERT_HEAD(&IN_IFADDR_HASH(ia->ia_addr.sin_addr.s_addr),
-                   ia, ia_hash);
        return (error);
 }
 
diff -r 7893be1a2681 -r 3c3ec3aa92f9 sys/netinet6/in6.c
--- a/sys/netinet6/in6.c        Thu Jun 30 00:23:36 2016 +0000
+++ b/sys/netinet6/in6.c        Thu Jun 30 01:34:53 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: in6.c,v 1.201 2016/06/28 02:36:54 ozaki-r Exp $        */
+/*     $NetBSD: in6.c,v 1.202 2016/06/30 01:34:53 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.201 2016/06/28 02:36:54 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: in6.c,v 1.202 2016/06/30 01:34:53 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -921,16 +921,6 @@
                    (struct sockaddr *)&ia->ia_prefixmask;
 
                ia->ia_ifp = ifp;
-               if ((oia = in6_ifaddr) != NULL) {
-                       for ( ; oia->ia_next; oia = oia->ia_next)
-                               continue;
-                       oia->ia_next = ia;
-               } else
-                       in6_ifaddr = ia;
-               /* gain a refcnt for the link from in6_ifaddr */
-               ifaref(&ia->ia_ifa);
-
-               ifa_insert(ifp, &ia->ia_ifa);
        }
 
        /* update timestamp */
@@ -950,7 +940,9 @@
                            " existing (%s) address should not be changed\n",
                            ip6_sprintf(&ia->ia_addr.sin6_addr));
                        error = EINVAL;
-                       goto unlink;
+                       if (hostIsNew)
+                               free(ia, M_IFADDR);
+                       goto exit;
                }
                ia->ia_prefixmask = ifra->ifra_prefixmask;
        }
@@ -1020,8 +1012,12 @@
        }
 
        /* reset the interface and routing table appropriately. */
-       if ((error = in6_ifinit(ifp, ia, &ifra->ifra_addr, hostIsNew)) != 0)
-               goto unlink;
+       if ((error = in6_ifinit(ifp, ia, &ifra->ifra_addr, hostIsNew)) != 0) {
+               if (hostIsNew)
+                       free(ia, M_IFADDR);
+               goto exit;
+       }
+
        /*
         * We are done if we have simply modified an existing address.
         */
@@ -1029,6 +1025,19 @@
                return error;
 
        /*



Home | Main Index | Thread Index | Old Index