Module Name: src Committed By: ozaki-r Date: Thu Jun 30 01:34:53 UTC 2016
Modified Files: src/sys/net: if_spppsubr.c src/sys/netinet: if_arp.c in.c src/sys/netinet6: in6.c nd6.c Log Message: 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. To generate a diff of this commit: cvs rdiff -u -r1.143 -r1.144 src/sys/net/if_spppsubr.c cvs rdiff -u -r1.213 -r1.214 src/sys/netinet/if_arp.c cvs rdiff -u -r1.168 -r1.169 src/sys/netinet/in.c cvs rdiff -u -r1.201 -r1.202 src/sys/netinet6/in6.c cvs rdiff -u -r1.197 -r1.198 src/sys/netinet6/nd6.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/sys/net/if_spppsubr.c diff -u src/sys/net/if_spppsubr.c:1.143 src/sys/net/if_spppsubr.c:1.144 --- src/sys/net/if_spppsubr.c:1.143 Mon Jun 20 08:30:59 2016 +++ src/sys/net/if_spppsubr.c Thu Jun 30 01:34:53 2016 @@ -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 @@ found: *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 @@ found: 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); } Index: src/sys/netinet/if_arp.c diff -u src/sys/netinet/if_arp.c:1.213 src/sys/netinet/if_arp.c:1.214 --- src/sys/netinet/if_arp.c:1.213 Tue Jun 28 02:02:56 2016 +++ src/sys/netinet/if_arp.c Thu Jun 30 01:34:53 2016 @@ -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 @@ arp_rtrequest(int req, struct rtentry *r 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); Index: src/sys/netinet/in.c diff -u src/sys/netinet/in.c:1.168 src/sys/netinet/in.c:1.169 --- src/sys/netinet/in.c:1.168 Thu Jun 23 06:40:48 2016 +++ src/sys/netinet/in.c Thu Jun 30 01:34:53 2016 @@ -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 @@ in_control(struct socket *so, u_long cmd 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 @@ in_control(struct socket *so, u_long cmd 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 @@ in_control(struct socket *so, u_long cmd ia->ia_ifp = ifp; ia->ia_idsalt = cprng_fast32() % 65535; LIST_INIT(&ia->ia_multiaddrs); + newifaddr = 1; } break; @@ -533,18 +533,24 @@ in_control(struct socket *so, u_long cmd 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 @@ in_control(struct socket *so, u_long cmd } 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 @@ in_control(struct socket *so, u_long cmd 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 @@ in_control(struct socket *so, u_long cmd 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 @@ in_ifinit(struct ifnet *ifp, struct in_i * 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 @@ in_ifinit(struct ifnet *ifp, struct in_i 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); } Index: src/sys/netinet6/in6.c diff -u src/sys/netinet6/in6.c:1.201 src/sys/netinet6/in6.c:1.202 --- src/sys/netinet6/in6.c:1.201 Tue Jun 28 02:36:54 2016 +++ src/sys/netinet6/in6.c Thu Jun 30 01:34:53 2016 @@ -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 @@ in6_update_ifa1(struct ifnet *ifp, struc (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 @@ in6_update_ifa1(struct ifnet *ifp, struc " 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 @@ in6_update_ifa1(struct ifnet *ifp, struc } /* 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 @@ in6_update_ifa1(struct ifnet *ifp, struc return error; /* + * Insert ia to the global list and ifa to the interface's list. + */ + 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); + + /* * Beyond this point, we should call in6_purgeaddr upon an error, * not just go to unlink. */ @@ -1271,13 +1280,13 @@ in6_update_ifa1(struct ifnet *ifp, struc return error; - unlink: /* * XXX: if a change of an existing address failed, keep the entry * anyway. */ if (hostIsNew) in6_unlink_ifa(ia, ifp); + exit: return error; cleanup: @@ -1695,7 +1704,7 @@ in6_ifinit(struct ifnet *ifp, struct in6 ia->ia_addr = *sin6; - if (ifacount <= 1 && + if (ifacount <= 0 && (error = if_addr_init(ifp, &ia->ia_ifa, true)) != 0) { splx(s); return error; Index: src/sys/netinet6/nd6.c diff -u src/sys/netinet6/nd6.c:1.197 src/sys/netinet6/nd6.c:1.198 --- src/sys/netinet6/nd6.c:1.197 Tue Jun 21 02:14:11 2016 +++ src/sys/netinet6/nd6.c Thu Jun 30 01:34:53 2016 @@ -1,4 +1,4 @@ -/* $NetBSD: nd6.c,v 1.197 2016/06/21 02:14:11 ozaki-r Exp $ */ +/* $NetBSD: nd6.c,v 1.198 2016/06/30 01:34:53 ozaki-r Exp $ */ /* $KAME: nd6.c,v 1.279 2002/06/08 11:16:51 itojun Exp $ */ /* @@ -31,7 +31,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: nd6.c,v 1.197 2016/06/21 02:14:11 ozaki-r Exp $"); +__KERNEL_RCSID(0, "$NetBSD: nd6.c,v 1.198 2016/06/30 01:34:53 ozaki-r Exp $"); #ifdef _KERNEL_OPT #include "opt_net_mpsafe.h" @@ -1450,6 +1450,17 @@ nd6_rtrequest(int req, struct rtentry *r RT_DPRINTF("rt_getkey(rt) = %p\n", rt_getkey(rt)); /* + * 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) { + if (nd6_useloopback) + rt->rt_ifp = lo0ifp; /* XXX */ + break; + } + + /* * check if rt_getkey(rt) is an address assigned * to the interface. */