This patch takes a first cut at locking three types of global variables: The IPv4 global hash table, accessed using INADDR_HASH(), ia_hash, INADDR_TO_IFADDR(), and INADDR_TO_IFP(). These are protected by a new global mutex, in_ifaddrhashtbl_lock. The global IPv4 address list, accessed using in_ifaddrhead, ia_link, and IFP_TO_IA. These are protected by a new global mutex, in_ifaddrhead_lock. Per-interface address lists, accessed using ifp->if_addrhead, ifp->if_addr, ifa_link. These are protected by the existing per-ifnet mutex, ifp->if_addr_mtx. Note that in many cases, the lock is released without ensuring the stability of a looked up ifaddr, which will need to be provied using the existing ifaddr reference count, but isn't currently. Also, the foo_control protocol-specific address manipulation using ioctl is not well-thought out and may require extensive rewriting. Finally, only IPv4 global address lists are protected, similar work will be required for other protocols. Index: nfsclient/nfs_vnops.c =================================================================== --- nfsclient/nfs_vnops.c (revision 188604) +++ nfsclient/nfs_vnops.c (working copy) @@ -1417,11 +1417,13 @@ tl = nfsm_build(u_int32_t *, NFSX_V3CREATEVERF); #ifdef INET INIT_VNET_INET(curvnet); + IN_IFADDRHEAD_RLOCK(); if (!TAILQ_EMPTY(&V_in_ifaddrhead)) *tl++ = IA_SIN(TAILQ_FIRST(&V_in_ifaddrhead))->sin_addr.s_addr; else #endif *tl++ = create_verf; + IN_IFADDRHEAD_RUNLOCK(); *tl = ++create_verf; CURVNET_RESTORE(); } else { Index: nfsclient/bootp_subr.c =================================================================== --- nfsclient/bootp_subr.c (revision 188604) +++ nfsclient/bootp_subr.c (working copy) @@ -406,11 +406,13 @@ for (ifp = TAILQ_FIRST(&V_ifnet); ifp != NULL; ifp = TAILQ_NEXT(ifp, if_link)) { + IF_ADDR_LOCK(ifp); for (ifa = TAILQ_FIRST(&ifp->if_addrhead); ifa != NULL; ifa = TAILQ_NEXT(ifa, ifa_link)) if (ifa->ifa_addr->sa_family == AF_INET) bootpboot_p_if(ifp, ifa); + IF_ADDR_UNLOCK(ifp); } IFNET_RUNLOCK(); } @@ -1049,12 +1051,14 @@ /* Get HW address */ sdl = NULL; + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifctx->ifp->if_addrhead, ifa_link) if (ifa->ifa_addr->sa_family == AF_LINK) { sdl = (struct sockaddr_dl *)ifa->ifa_addr; if (sdl->sdl_type == IFT_ETHER) break; } + IF_ADDR_UNLOCK(ifp); if (sdl == NULL) panic("bootpc: Unable to find HW address for %s", Index: nfsclient/nfs_diskless.c =================================================================== --- nfsclient/nfs_diskless.c (revision 188604) +++ nfsclient/nfs_diskless.c (working copy) @@ -181,6 +181,7 @@ ifa = NULL; IFNET_RLOCK(); TAILQ_FOREACH(ifp, &V_ifnet, if_link) { + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { if (ifa->ifa_addr->sa_family == AF_LINK) { sdl = (struct sockaddr_dl *)ifa->ifa_addr; @@ -189,11 +190,13 @@ !bcmp(LLADDR(sdl), LLADDR(&ourdl), sdl->sdl_alen)) { + IF_ADDR_UNLOCK(ifp); IFNET_RUNLOCK(); goto match_done; } } } + IF_ADDR_UNLOCK(ifp); } IFNET_RUNLOCK(); printf("nfs_diskless: no interface\n"); Index: kern/kern_uuid.c =================================================================== --- kern/kern_uuid.c (revision 188604) +++ kern/kern_uuid.c (working copy) @@ -98,16 +98,19 @@ IFNET_RLOCK(); TAILQ_FOREACH(ifp, &V_ifnet, if_link) { /* Walk the address list */ + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { sdl = (struct sockaddr_dl*)ifa->ifa_addr; if (sdl != NULL && sdl->sdl_family == AF_LINK && sdl->sdl_type == IFT_ETHER) { /* Got a MAC address. */ bcopy(LLADDR(sdl), node, UUID_NODE_LEN); + IF_ADDR_UNLOCK(ifp); IFNET_RUNLOCK(); return; } } + IF_ADDR_UNLOCK(ifp); } IFNET_RUNLOCK(); Index: netatalk/at_control.c =================================================================== --- netatalk/at_control.c (revision 188604) +++ netatalk/at_control.c (working copy) @@ -160,7 +160,7 @@ */ if (aa == NULL) { aa0 = malloc(sizeof(struct at_ifaddr), M_IFADDR, - M_WAITOK | M_ZERO); + M_NOWAIT | M_ZERO); callout_init(&aa0->aa_callout, CALLOUT_MPSAFE); if ((aa = at_ifaddr_list) != NULL) { /* @@ -184,14 +184,9 @@ at_ifaddr_list = aa0; aa = aa0; - /* - * Find the end of the interface's addresses - * and link our new one on the end - */ ifa = (struct ifaddr *)aa; IFA_LOCK_INIT(ifa); ifa->ifa_refcnt = 1; - TAILQ_INSERT_TAIL(&ifp->if_addrhead, ifa, ifa_link); /* * As the at_ifaddr contains the actual sockaddrs, @@ -214,10 +209,20 @@ * and link it all together */ aa->aa_ifp = ifp; + + /* + * Find the end of the interface's addresses + * and link our new one on the end + */ + IF_ADDR_LOCK(ifp); + TAILQ_INSERT_TAIL(&ifp->if_addrhead, ifa, ifa_link); + IF_ADDR_UNLOCK(ifp); } else { /* * If we DID find one then we clobber any routes * dependent on it.. + * + * XXXRW: Lock order reversal? */ at_scrub(ifp, aa); } @@ -247,8 +252,10 @@ } } - if (aa == NULL) + if (aa == NULL) { + AT_IFADDRLIST_WUNLOCK(); return (EADDRNOTAVAIL); + } break; } @@ -296,7 +303,9 @@ * remove the ifaddr from the interface */ ifa0 = (struct ifaddr *)aa; + IF_ADDR_LOCK(ifp); TAILQ_REMOVE(&ifp->if_addrhead, ifa0, ifa_link); + IF_ADDR_UNLOCK(ifp); /* * Now remove the at_ifaddr from the parallel structure @@ -817,6 +826,7 @@ ifp = aa->aa_ifp; at_scrub(ifp, aa); at_ifaddr_list = aa->aa_next; + IF_ADDR_LOCK(ifp); if ((ifa = ifp->if_addrlist) == (struct ifaddr *)aa) ifp->if_addrlist = ifa->ifa_next; else { @@ -829,6 +839,7 @@ else panic("at_entry"); } + IF_ADDR_UNLOCK(ifp); } } Index: netatalk/at_var.h =================================================================== --- netatalk/at_var.h (revision 188604) +++ netatalk/at_var.h (working copy) @@ -62,6 +62,15 @@ #ifdef _KERNEL extern struct at_ifaddr *at_ifaddr_list; +extern struct mtx at_ifaddr_list_lock; + +#define AT_IFADDRLIST_LOCK_ASSERT() mtx_assert(&at_ifaddr_list_lock, \ + MA_OWNED) +#define AT_IFADDRLIST_RLOCK() mtx_lock(&at_ifaddr_list_lock) +#define AT_IFADDRLIST_RUNLOCK() mtx_unlock(&at_ifaddr_list_lock) +#define AT_IFADDRLIST_WLOCK() mtx_lock(&at_ifaddr_list_lock) +#define AT_IFADDRLIST_WUNLOCK() mtx_lock(&at_ifaddr_list_lock) + #endif #endif /* _NETATALK_AT_VAR_H_ */ Index: netatalk/aarp.c =================================================================== --- netatalk/aarp.c (revision 188604) +++ netatalk/aarp.c (working copy) @@ -1,5 +1,5 @@ /*- - * Copyright (c) 2004-2005 Robert N. M. Watson + * Copyright (c) 2004-2009 Robert N. M. Watson * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -380,6 +380,7 @@ * Since we don't know the net, we just look for the first * phase 1 address on the interface. */ + IF_ADDR_LOCK(ifp); for (aa = (struct at_ifaddr *)TAILQ_FIRST(&ifp->if_addrhead); aa; aa = (struct at_ifaddr *)aa->aa_ifa.ifa_link.tqe_next) { @@ -389,10 +390,12 @@ } } if (aa == NULL) { + IF_ADDR_UNLOCK(ifp); m_freem(m); return; } tpa.s_net = spa.s_net = AA_SAT(aa)->sat_addr.s_net; + IF_ADDR_UNLOCK(ifp); } spa.s_node = ea->aarp_spnode; @@ -583,12 +586,14 @@ * generate an 802.2 and SNAP header. */ AARPTAB_LOCK(); + IF_ADDR_LOCK(ifp); for (aa = (struct at_ifaddr *)TAILQ_FIRST(&ifp->if_addrhead); aa; aa = (struct at_ifaddr *)aa->aa_ifa.ifa_link.tqe_next) { if (AA_SAT(aa)->sat_family == AF_APPLETALK && (aa->aa_flags & AFA_PROBING)) break; } + IF_ADDR_UNLOCK(ifp); if (aa == NULL) { /* Serious error XXX. */ AARPTAB_UNLOCK(); Index: netinet/in.c =================================================================== --- netinet/in.c (revision 188604) +++ netinet/in.c (working copy) @@ -96,15 +96,21 @@ register u_long i = ntohl(in.s_addr); register struct in_ifaddr *ia; + IN_IFADDRHEAD_RLOCK(); if (V_subnetsarelocal) { TAILQ_FOREACH(ia, &V_in_ifaddrhead, ia_link) - if ((i & ia->ia_netmask) == ia->ia_net) + if ((i & ia->ia_netmask) == ia->ia_net) { + IN_IFADDRHEAD_RUNLOCK(); return (1); + } } else { TAILQ_FOREACH(ia, &V_in_ifaddrhead, ia_link) - if ((i & ia->ia_subnetmask) == ia->ia_subnet) + if ((i & ia->ia_subnetmask) == ia->ia_subnet) { + IN_IFADDRHEAD_RUNLOCK(); return (1); + } } + IN_IFADDRHEAD_RUNLOCK(); return (0); } @@ -118,10 +124,14 @@ INIT_VNET_INET(curvnet); struct in_ifaddr *ia; + IN_IFADDRHASHTBL_RLOCK(); LIST_FOREACH(ia, INADDR_HASH(in.s_addr), ia_hash) { - if (IA_SIN(ia)->sin_addr.s_addr == in.s_addr) + if (IA_SIN(ia)->sin_addr.s_addr == in.s_addr) { + IN_IFADDRHASHTBL_RUNLOCK(); return (1); + } } + IN_IFADDRHASHTBL_RUNLOCK(); return (0); } @@ -217,7 +227,7 @@ struct in_ifaddr *oia; struct in_aliasreq *ifra = (struct in_aliasreq *)data; struct sockaddr_in oldaddr; - int error, hostIsNew, iaIsNew, maskIsNew, s; + int error, hostIsNew, iaIsNew, maskIsNew; int iaIsFirst; ia = NULL; @@ -260,6 +270,7 @@ */ if (ifp != NULL) { dst = ((struct sockaddr_in *)&ifr->ifr_addr)->sin_addr; + IN_IFADDRHASHTBL_RLOCK(); LIST_FOREACH(iap, INADDR_HASH(dst.s_addr), ia_hash) if (iap->ia_ifp == ifp && iap->ia_addr.sin_addr.s_addr == dst.s_addr) { @@ -268,7 +279,9 @@ ia = iap; break; } - if (ia == NULL) + IN_IFADDRHASHTBL_RUNLOCK(); + if (ia == NULL) { + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { iap = ifatoia(ifa); if (iap->ia_addr.sin_family == AF_INET) { @@ -280,6 +293,8 @@ break; } } + IF_ADDR_UNLOCK(ifp); + } if (ia == NULL) iaIsFirst = 1; } @@ -291,12 +306,14 @@ if (ifp == NULL) return (EADDRNOTAVAIL); if (ifra->ifra_addr.sin_family == AF_INET) { + IN_IFADDRHEAD_RLOCK(); for (oia = ia; ia; ia = TAILQ_NEXT(ia, ia_link)) { if (ia->ia_ifp == ifp && ia->ia_addr.sin_addr.s_addr == ifra->ifra_addr.sin_addr.s_addr) break; } + IN_IFADDRHEAD_RUNLOCK(); if ((ifp->if_flags & IFF_POINTOPOINT) && (cmd == SIOCAIFADDR) && (ifra->ifra_dstaddr.sin_addr.s_addr @@ -324,18 +341,15 @@ malloc(sizeof *ia, M_IFADDR, M_WAITOK | M_ZERO); if (ia == NULL) return (ENOBUFS); - /* - * Protect from ipintr() traversing address list - * while we're modifying it. - */ - s = splnet(); ifa = &ia->ia_ifa; IFA_LOCK_INIT(ifa); ifa->ifa_addr = (struct sockaddr *)&ia->ia_addr; ifa->ifa_dstaddr = (struct sockaddr *)&ia->ia_dstaddr; ifa->ifa_netmask = (struct sockaddr *)&ia->ia_sockmask; ifa->ifa_refcnt = 1; + IF_ADDR_LOCK(ifp); TAILQ_INSERT_TAIL(&ifp->if_addrhead, ifa, ifa_link); + IF_ADDR_UNLOCK(ifp); ia->ia_sockmask.sin_len = 8; ia->ia_sockmask.sin_family = AF_INET; @@ -345,8 +359,9 @@ } ia->ia_ifp = ifp; + IN_IFADDRHEAD_WLOCK(); TAILQ_INSERT_TAIL(&V_in_ifaddrhead, ia, ia_link); - splx(s); + IN_IFADDRHEAD_WUNLOCK(); iaIsNew = 1; } break; @@ -503,22 +518,25 @@ return (error); } - /* - * Protect from ipintr() traversing address list while we're modifying - * it. - */ - s = splnet(); + IF_ADDR_LOCK(ifp); TAILQ_REMOVE(&ifp->if_addrhead, &ia->ia_ifa, ifa_link); + IF_ADDR_UNLOCK(ifp); + IN_IFADDRHEAD_WLOCK(); TAILQ_REMOVE(&V_in_ifaddrhead, ia, ia_link); + IN_IFADDRHEAD_WUNLOCK(); if (ia->ia_addr.sin_family == AF_INET) { + IN_IFADDRHASHTBL_WLOCK(); LIST_REMOVE(ia, ia_hash); + IN_IFADDRHASHTBL_WUNLOCK(); /* * If this is the last IPv4 address configured on this * interface, leave the all-hosts group. * XXX: This is quite ugly because of locking and structure. */ oia = NULL; + IN_IFADDRHEAD_RLOCK(); IFP_TO_IA(ifp, oia); + IN_IFADDRHEAD_RUNLOCK(); if (oia == NULL) { struct in_multi *inm; @@ -532,8 +550,6 @@ } } IFAFREE(&ia->ia_ifa); - splx(s); - return (error); } @@ -650,6 +666,7 @@ } } + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { if (ifa->ifa_addr->sa_family != AF_INET6) continue; @@ -660,6 +677,7 @@ if (candidate.s_addr == match.s_addr) break; } + IF_ADDR_UNLOCK(ifp); if (ifa == NULL) return (EADDRNOTAVAIL); ia = (struct in_ifaddr *)ifa; @@ -730,12 +748,15 @@ int s = splimp(), flags = RTF_UP, error = 0; oldaddr = ia->ia_addr; + IN_IFADDRHASHTBL_WLOCK(); if (oldaddr.sin_family == AF_INET) LIST_REMOVE(ia, ia_hash); ia->ia_addr = *sin; if (ia->ia_addr.sin_family == AF_INET) LIST_INSERT_HEAD(INADDR_HASH(ia->ia_addr.sin_addr.s_addr), ia, ia_hash); + IN_IFADDRHASHTBL_WUNLOCK(); + /* * Give the interface a chance to initialize * if this is its first address, @@ -749,6 +770,7 @@ splx(s); /* LIST_REMOVE(ia, ia_hash) is done in in_control */ ia->ia_addr = oldaddr; + IN_IFADDRHASHTBL_WLOCK(); if (ia->ia_addr.sin_family == AF_INET) LIST_INSERT_HEAD(INADDR_HASH( ia->ia_addr.sin_addr.s_addr), ia, ia_hash); @@ -760,6 +782,7 @@ * with bogus ia entries in hash */ LIST_REMOVE(ia, ia_hash); + IN_IFADDRHASHTBL_WUNLOCK(); return (error); } } @@ -841,6 +864,7 @@ prefix.s_addr &= mask.s_addr; } + IN_IFADDRHEAD_RLOCK(); TAILQ_FOREACH(ia, &V_in_ifaddrhead, ia_link) { if (rtinitflags(ia)) { p = ia->ia_addr.sin_addr; @@ -864,12 +888,16 @@ if (ia->ia_flags & IFA_ROUTE) { if (V_sameprefixcarponly && target->ia_ifp->if_type != IFT_CARP && - ia->ia_ifp->if_type != IFT_CARP) + ia->ia_ifp->if_type != IFT_CARP) { + IN_IFADDRHEAD_RUNLOCK(); return (EEXIST); - else + } else { + IN_IFADDRHEAD_RUNLOCK(); return (0); + } } } + IN_IFADDRHEAD_RUNLOCK(); /* * No-one seem to have this prefix route, so we try to insert it. @@ -908,6 +936,7 @@ arp_ifscrub(target->ia_ifp, IA_SIN(target)->sin_addr.s_addr); } + IN_IFADDRHEAD_RLOCK(); TAILQ_FOREACH(ia, &V_in_ifaddrhead, ia_link) { if (rtinitflags(ia)) p = ia->ia_dstaddr.sin_addr; @@ -931,6 +960,7 @@ && (ia->ia_ifp->if_type != IFT_CARP) #endif ) { + IN_IFADDRHEAD_RUNLOCK(); rtinit(&(target->ia_ifa), (int)RTM_DELETE, rtinitflags(target)); target->ia_flags &= ~IFA_ROUTE; @@ -942,6 +972,7 @@ return (error); } } + IN_IFADDRHEAD_RUNLOCK(); /* * As no-one seem to have this prefix, we can remove the route. @@ -973,6 +1004,7 @@ * with a broadcast address. */ #define ia ((struct in_ifaddr *)ifa) + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) if (ifa->ifa_addr->sa_family == AF_INET && (in.s_addr == ia->ia_broadaddr.sin_addr.s_addr || @@ -986,8 +1018,11 @@ * only exist when an interface gets a secondary * address. */ - ia->ia_subnetmask != (u_long)0xffffffff) + ia->ia_subnetmask != (u_long)0xffffffff) { + IF_ADDR_UNLOCK(ifp); return (1); + } + IF_ADDR_UNLOCK(ifp); return (0); #undef ia } Index: netinet/ip_carp.c =================================================================== --- netinet/ip_carp.c (revision 188604) +++ netinet/ip_carp.c (working copy) @@ -1471,6 +1471,7 @@ /* we have to do it by hands to check we won't match on us */ ia_if = NULL; own = 0; + IN_IFADDRHEAD_RLOCK(); TAILQ_FOREACH(ia, &V_in_ifaddrhead, ia_link) { /* and, yeah, we need a multicast-capable iface too */ if (ia->ia_ifp != SC2IFP(sc) && @@ -1483,6 +1484,7 @@ own++; } } + IN_IFADDRHEAD_RUNLOCK(); if (!ia_if) return (EADDRNOTAVAIL); Index: netinet/raw_ip.c =================================================================== --- netinet/raw_ip.c (revision 188604) +++ netinet/raw_ip.c (working copy) @@ -617,9 +617,11 @@ switch (cmd) { case PRC_IFDOWN: + IN_IFADDRHEAD_RLOCK(); TAILQ_FOREACH(ia, &V_in_ifaddrhead, ia_link) { if (ia->ia_ifa.ifa_addr == sa && (ia->ia_flags & IFA_ROUTE)) { + IN_IFADDRHEAD_RUNLOCK(); /* * in_ifscrub kills the interface route. */ @@ -634,13 +636,16 @@ break; } } + IN_IFADDRHEAD_RUNLOCK(); break; case PRC_IFUP: + IN_IFADDRHEAD_RLOCK(); TAILQ_FOREACH(ia, &V_in_ifaddrhead, ia_link) { if (ia->ia_ifa.ifa_addr == sa) break; } + IN_IFADDRHEAD_RUNLOCK(); if (ia == 0 || (ia->ia_flags & IFA_ROUTE)) return; flags = RTF_UP; Index: netinet/ip_divert.c =================================================================== --- netinet/ip_divert.c (revision 188604) +++ netinet/ip_divert.c (working copy) @@ -244,18 +244,22 @@ divsrc.sin_port = divert_cookie(mtag); /* record matching rule */ if (incoming) { struct ifaddr *ifa; + struct ifnet *ifp; /* Sanity check */ M_ASSERTPKTHDR(m); /* Find IP address for receive interface */ - TAILQ_FOREACH(ifa, &m->m_pkthdr.rcvif->if_addrhead, ifa_link) { + ifp = m->m_pkthdr.rcvif; + IF_ADDR_LOCK(ifp); + TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { if (ifa->ifa_addr->sa_family != AF_INET) continue; divsrc.sin_addr = ((struct sockaddr_in *) ifa->ifa_addr)->sin_addr; break; } + IF_ADDR_UNLOCK(ifp); } /* * Record the incoming interface name whenever we have one. Index: netinet/in_pcb.c =================================================================== --- netinet/in_pcb.c (revision 188604) +++ netinet/in_pcb.c (working copy) @@ -605,8 +605,8 @@ ifp = ia->ia_ifp; ia = NULL; + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { - sa = ifa->ifa_addr; if (sa->sa_family != AF_INET) continue; @@ -618,8 +618,10 @@ } if (ia != NULL) { laddr->s_addr = ia->ia_addr.sin_addr.s_addr; + IF_ADDR_UNLOCK(ifp); goto done; } + IF_ADDR_UNLOCK(ifp); /* 3. As a last resort return the 'default' jail address. */ error = prison_get_ip4(cred, laddr); @@ -636,7 +638,10 @@ * 3. as a last resort return the 'default' jail address. */ if ((sro.ro_rt->rt_ifp->if_flags & IFF_LOOPBACK) == 0) { + struct ifnet *ifp; + ifp = sro.ro_rt->rt_ifp; + /* If not jailed, use the default returned. */ if (cred == NULL || !jailed(cred)) { ia = (struct in_ifaddr *)sro.ro_rt->rt_ifa; @@ -657,7 +662,8 @@ * 2. Check if we have any address on the outgoing interface * belonging to this jail. */ - TAILQ_FOREACH(ifa, &sro.ro_rt->rt_ifp->if_addrhead, ifa_link) { + IF_ADDR_LOCK(ifp); + TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { sa = ifa->ifa_addr; if (sa->sa_family != AF_INET) @@ -670,8 +676,10 @@ } if (ia != NULL) { laddr->s_addr = ia->ia_addr.sin_addr.s_addr; + IF_ADDR_UNLOCK(ifp); goto done; } + IF_ADDR_UNLOCK(ifp); /* 3. As a last resort return the 'default' jail address. */ error = prison_get_ip4(cred, laddr); @@ -718,6 +726,7 @@ ifp = ia->ia_ifp; ia = NULL; + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { sa = ifa->ifa_addr; @@ -732,8 +741,10 @@ } if (ia != NULL) { laddr->s_addr = ia->ia_addr.sin_addr.s_addr; + IF_ADDR_UNLOCK(ifp); goto done; } + IF_ADDR_UNLOCK(ifp); } /* 3. As a last resort return the 'default' jail address. */ @@ -795,6 +806,7 @@ faddr = sin->sin_addr; fport = sin->sin_port; + IN_IFADDRHEAD_RLOCK(); if (!TAILQ_EMPTY(&V_in_ifaddrhead)) { /* * If the destination address is INADDR_ANY, @@ -807,14 +819,17 @@ faddr = IA_SIN(TAILQ_FIRST(&V_in_ifaddrhead))->sin_addr; if (cred != NULL && - (error = prison_get_ip4(cred, &faddr)) != 0) + (error = prison_get_ip4(cred, &faddr)) != 0) { + IN_IFADDRHEAD_RUNLOCK(); return (error); + } } else if (faddr.s_addr == (u_long)INADDR_BROADCAST && (TAILQ_FIRST(&V_in_ifaddrhead)->ia_ifp->if_flags & IFF_BROADCAST)) faddr = satosin(&TAILQ_FIRST( &V_in_ifaddrhead)->ia_broadaddr)->sin_addr; } + IN_IFADDRHEAD_RUNLOCK(); if (laddr.s_addr == INADDR_ANY) { error = in_pcbladdr(inp, &faddr, &laddr, cred); if (error) @@ -833,9 +848,11 @@ imo = inp->inp_moptions; if (imo->imo_multicast_ifp != NULL) { ifp = imo->imo_multicast_ifp; + IN_IFADDRHEAD_RLOCK(); TAILQ_FOREACH(ia, &V_in_ifaddrhead, ia_link) if (ia->ia_ifp == ifp) break; + IN_IFADDRHEAD_RUNLOCK(); if (ia == NULL) return (EADDRNOTAVAIL); laddr = ia->ia_addr.sin_addr; Index: netinet/sctp_bsd_addr.c =================================================================== --- netinet/sctp_bsd_addr.c (revision 188604) +++ netinet/sctp_bsd_addr.c (working copy) @@ -205,7 +205,9 @@ struct sctp_ifa *sctp_ifa; uint32_t ifa_flags; + IFNET_RLOCK(); TAILQ_FOREACH(ifn, &MODULE_GLOBAL(MOD_NET, ifnet), if_list) { + IF_ADDR_LOCK(ifn); TAILQ_FOREACH(ifa, &ifn->if_addrlist, ifa_list) { if (ifa->ifa_addr == NULL) { @@ -248,7 +250,9 @@ sctp_ifa->localifa_flags &= ~SCTP_ADDR_DEFER_USE; } } + IF_ADDR_UNLOCK(ifn); } + IFNET_RUNLOCK(); } @@ -335,14 +339,18 @@ struct ifnet *ifn; struct ifaddr *ifa; + IFNET_RLOCK(); TAILQ_FOREACH(ifn, &MODULE_GLOBAL(MOD_NET, ifnet), if_list) { if (!(*pred) (ifn)) { continue; } + IF_ADDR_LOCK(ifn); TAILQ_FOREACH(ifa, &ifn->if_addrlist, ifa_list) { sctp_addr_change(ifa, add ? RTM_ADD : RTM_DELETE); } + IF_ADDR_UNLOCK(ifn); } + IFNET_RUNLOCK(); } struct mbuf * Index: netinet/in_mcast.c =================================================================== --- netinet/in_mcast.c (revision 188604) +++ netinet/in_mcast.c (working copy) @@ -507,8 +507,11 @@ ssa->sin.sin_len = sizeof(struct sockaddr_in); ssa->sin.sin_addr = mreqs.imr_sourceaddr; - if (mreqs.imr_interface.s_addr != INADDR_ANY) + if (mreqs.imr_interface.s_addr != INADDR_ANY) { + IN_IFADDRHASHTBL_RLOCK(); INADDR_TO_IFP(mreqs.imr_interface, ifp); + IN_IFADDRHASHTBL_RUNLOCK(); + } if (sopt->sopt_name == IP_BLOCK_SOURCE) block = 1; @@ -896,7 +899,9 @@ mreqn.imr_address = imo->imo_multicast_addr; } else if (ifp != NULL) { mreqn.imr_ifindex = ifp->if_index; + IN_IFADDRHEAD_RLOCK(); IFP_TO_IA(ifp, ia); + IN_IFADDRHEAD_RUNLOCK(); if (ia != NULL) { mreqn.imr_address = IA_SIN(ia)->sin_addr; @@ -1030,7 +1035,9 @@ * reject the IPv4 multicast join. */ if (mreqs.imr_interface.s_addr != INADDR_ANY) { + IN_IFADDRHASHTBL_RLOCK(); INADDR_TO_IFP(mreqs.imr_interface, ifp); + IN_IFADDRHASHTBL_RUNLOCK(); } else { struct route ro; @@ -1046,6 +1053,7 @@ } else { struct in_ifaddr *ia; struct ifnet *mfp = NULL; + IN_IFADDRHEAD_RLOCK(); TAILQ_FOREACH(ia, &V_in_ifaddrhead, ia_link) { mfp = ia->ia_ifp; if (!(mfp->if_flags & IFF_LOOPBACK) && @@ -1054,6 +1062,7 @@ break; } } + IN_IFADDRHEAD_RUNLOCK(); } } #ifdef DIAGNOSTIC @@ -1275,8 +1284,11 @@ ssa->sin.sin_addr = mreqs.imr_sourceaddr; } - if (gsa->sin.sin_addr.s_addr != INADDR_ANY) + if (gsa->sin.sin_addr.s_addr != INADDR_ANY) { + IN_IFADDRHASHTBL_RLOCK(); INADDR_TO_IFP(mreqs.imr_interface, ifp); + IN_IFADDRHASHTBL_RUNLOCK(); + } #ifdef DIAGNOSTIC if (bootverbose) { @@ -1450,7 +1462,9 @@ if (addr.s_addr == INADDR_ANY) { ifp = NULL; } else { + IN_IFADDRHASHTBL_RLOCK(); INADDR_TO_IFP(addr, ifp); + IN_IFADDRHASHTBL_RUNLOCK(); if (ifp == NULL) return (EADDRNOTAVAIL); } Index: netinet/ip_output.c =================================================================== --- netinet/ip_output.c (revision 188604) +++ netinet/ip_output.c (working copy) @@ -241,7 +241,9 @@ * packets if the interface is specified. */ ifp = imo->imo_multicast_ifp; + IN_IFADDRHEAD_RLOCK(); IFP_TO_IA(ifp, ia); + IN_IFADDRHEAD_RUNLOCK(); isbroadcast = 0; /* fool gcc */ } else { /* Index: netinet/ip_input.c =================================================================== --- netinet/ip_input.c (revision 188604) +++ netinet/ip_input.c (working copy) @@ -121,6 +121,17 @@ static int nipq; /* Total # of reass queues */ #endif +/* + * For now, locks protecting per-vimage address lists are global. + */ +struct mtx in_ifaddrhead_lock; +struct mtx in_ifaddrhashtbl_lock; + +MTX_SYSINIT(in_ifaddrhead_lock, &in_ifaddrhead_lock, "in_ifaddrhead_lock", + MTX_DEF); +MTX_SYSINIT(in_ifaddrhashtbl_lock, &in_ifaddrhashtbl_lock, + "in_ifaddrhashtbl_lock", MTX_DEF); + SYSCTL_V_INT(V_NET, vnet_inet, _net_inet_ip, IPCTL_FORWARDING, forwarding, CTLFLAG_RW, ipforwarding, 0, "Enable IP forwarding between interfaces"); @@ -339,6 +350,7 @@ struct ip *ip = NULL; struct in_ifaddr *ia = NULL; struct ifaddr *ifa; + struct ifnet *ifp; int checkif, hlen = 0; u_short sum; int dchg = 0; /* dest changed after fw */ @@ -389,9 +401,10 @@ } /* 127/8 must not appear on wire - RFC1122 */ + ifp = m->m_pkthdr.rcvif; if ((ntohl(ip->ip_dst.s_addr) >> IN_CLASSA_NSHIFT) == IN_LOOPBACKNET || (ntohl(ip->ip_src.s_addr) >> IN_CLASSA_NSHIFT) == IN_LOOPBACKNET) { - if ((m->m_pkthdr.rcvif->if_flags & IFF_LOOPBACK) == 0) { + if ((ifp->if_flags & IFF_LOOPBACK) == 0) { V_ipstat.ips_badaddr++; goto bad; } @@ -466,8 +479,7 @@ goto passin; odst = ip->ip_dst; - if (pfil_run_hooks(&inet_pfil_hook, &m, m->m_pkthdr.rcvif, - PFIL_IN, NULL) != 0) + if (pfil_run_hooks(&inet_pfil_hook, &m, ifp, PFIL_IN, NULL) != 0) return; if (m == NULL) /* consumed by filter */ return; @@ -539,16 +551,16 @@ * checked with carp_iamatch() and carp_forus(). */ checkif = V_ip_checkinterface && (V_ipforwarding == 0) && - m->m_pkthdr.rcvif != NULL && - ((m->m_pkthdr.rcvif->if_flags & IFF_LOOPBACK) == 0) && + ifp != NULL && ((ifp->if_flags & IFF_LOOPBACK) == 0) && #ifdef DEV_CARP - !m->m_pkthdr.rcvif->if_carp && + !ifp->if_carp && #endif (dchg == 0); /* * Check for exact addresses in the hash bucket. */ + IN_IFADDRHASHTBL_RLOCK(); LIST_FOREACH(ia, INADDR_HASH(ip->ip_dst.s_addr), ia_hash) { /* * If the address matches, verify that the packet @@ -556,9 +568,13 @@ * enabled. */ if (IA_SIN(ia)->sin_addr.s_addr == ip->ip_dst.s_addr && - (!checkif || ia->ia_ifp == m->m_pkthdr.rcvif)) + (!checkif || ia->ia_ifp == ifp)) { + IN_IFADDRHASHTBL_RUNLOCK(); goto ours; + } } + IN_IFADDRHASHTBL_RUNLOCK(); + /* * Check for broadcast addresses. * @@ -567,22 +583,29 @@ * be handled via ip_forward() and ether_output() with the loopback * into the stack for SIMPLEX interfaces handled by ether_output(). */ - if (m->m_pkthdr.rcvif != NULL && - m->m_pkthdr.rcvif->if_flags & IFF_BROADCAST) { - TAILQ_FOREACH(ifa, &m->m_pkthdr.rcvif->if_addrhead, ifa_link) { + if (ifp != NULL && ifp->if_flags & IFF_BROADCAST) { + IF_ADDR_LOCK(ifp); + TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { if (ifa->ifa_addr->sa_family != AF_INET) continue; ia = ifatoia(ifa); if (satosin(&ia->ia_broadaddr)->sin_addr.s_addr == - ip->ip_dst.s_addr) + ip->ip_dst.s_addr) { + IF_ADDR_UNLOCK(ifp); goto ours; - if (ia->ia_netbroadcast.s_addr == ip->ip_dst.s_addr) + } + if (ia->ia_netbroadcast.s_addr == ip->ip_dst.s_addr) { + IF_ADDR_UNLOCK(ifp); goto ours; + } #ifdef BOOTP_COMPAT - if (IA_SIN(ia)->sin_addr.s_addr == INADDR_ANY) + if (IA_SIN(ia)->sin_addr.s_addr == INADDR_ANY) { + IF_ADDR_UNLOCK(ifp); goto ours; + } #endif } + IF_ADDR_UNLOCK(ifp); } /* RFC 3927 2.7: Do not forward datagrams for 169.254.0.0/16. */ if (IN_LINKLOCAL(ntohl(ip->ip_dst.s_addr))) { @@ -602,7 +625,7 @@ * must be discarded, else it may be accepted below. */ if (ip_mforward && - ip_mforward(ip, m->m_pkthdr.rcvif, m, 0) != 0) { + ip_mforward(ip, ifp, m, 0) != 0) { V_ipstat.ips_cantforward++; m_freem(m); return; @@ -622,7 +645,7 @@ * arrival interface. */ IN_MULTI_LOCK(); - IN_LOOKUP_MULTI(ip->ip_dst, m->m_pkthdr.rcvif, inm); + IN_LOOKUP_MULTI(ip->ip_dst, ifp, inm); IN_MULTI_UNLOCK(); if (inm == NULL) { V_ipstat.ips_notmember++; @@ -639,7 +662,7 @@ /* * FAITH(Firewall Aided Internet Translator) */ - if (m->m_pkthdr.rcvif && m->m_pkthdr.rcvif->if_type == IFT_FAITH) { + if (ifp != NULL && ifp->if_type == IFT_FAITH) { if (V_ip_keepfaith) { if (ip->ip_p == IPPROTO_TCP || ip->ip_p == IPPROTO_ICMP) goto ours; @@ -1603,15 +1626,18 @@ if (((ifp = m->m_pkthdr.rcvif)) && ( ifp->if_index && (ifp->if_index <= V_if_index))) { + IF_ADDR_LOCK(ifp); sdp = (struct sockaddr_dl *)ifp->if_addr->ifa_addr; /* * Change our mind and don't try copy. */ if ((sdp->sdl_family != AF_LINK) || (sdp->sdl_len > sizeof(sdlbuf))) { + IF_ADDR_UNLOCK(ifp); goto makedummy; } bcopy(sdp, sdl2, sdp->sdl_len); + IF_ADDR_UNLOCK(ifp); } else { makedummy: sdl2->sdl_len Index: netinet/in_gif.c =================================================================== --- netinet/in_gif.c (revision 188604) +++ netinet/in_gif.c (working copy) @@ -360,12 +360,16 @@ return 0; } /* reject packets with broadcast on source */ + IN_IFADDRHEAD_RLOCK(); TAILQ_FOREACH(ia4, &V_in_ifaddrhead, ia_link) { if ((ia4->ia_ifa.ifa_ifp->if_flags & IFF_BROADCAST) == 0) continue; - if (ip->ip_src.s_addr == ia4->ia_broadaddr.sin_addr.s_addr) + if (ip->ip_src.s_addr == ia4->ia_broadaddr.sin_addr.s_addr) { + IN_IFADDRHEAD_RUNLOCK(); return 0; + } } + IN_IFADDRHEAD_RUNLOCK(); /* ingress filters on outer source */ if ((GIF2IFP(sc)->if_flags & IFF_LINK2) == 0 && ifp) { Index: netinet/ip_icmp.c =================================================================== --- netinet/ip_icmp.c (revision 188604) +++ netinet/ip_icmp.c (working copy) @@ -650,7 +650,7 @@ INIT_VNET_INET(curvnet); struct ip *ip = mtod(m, struct ip *); struct ifaddr *ifa; - struct ifnet *ifn; + struct ifnet *ifn, *ifp; struct in_ifaddr *ia; struct in_addr t; struct mbuf *opts = 0; @@ -673,39 +673,56 @@ * If the incoming packet was addressed directly to one of our * own addresses, use dst as the src for the reply. */ + IN_IFADDRHASHTBL_RLOCK(); LIST_FOREACH(ia, INADDR_HASH(t.s_addr), ia_hash) - if (t.s_addr == IA_SIN(ia)->sin_addr.s_addr) + if (t.s_addr == IA_SIN(ia)->sin_addr.s_addr) { + IN_IFADDRHASHTBL_RUNLOCK(); + t = IA_SIN(ia)->sin_addr; goto match; + } + IN_IFADDRHASHTBL_RUNLOCK(); + /* * If the incoming packet was addressed to one of our broadcast * addresses, use the first non-broadcast address which corresponds * to the incoming interface. */ - if (m->m_pkthdr.rcvif != NULL && - m->m_pkthdr.rcvif->if_flags & IFF_BROADCAST) { - TAILQ_FOREACH(ifa, &m->m_pkthdr.rcvif->if_addrhead, ifa_link) { + ifp = m->m_pkthdr.rcvif; + if (ifp != NULL && ifp->if_flags & IFF_BROADCAST) { + IF_ADDR_LOCK(ifp); + TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { if (ifa->ifa_addr->sa_family != AF_INET) continue; ia = ifatoia(ifa); if (satosin(&ia->ia_broadaddr)->sin_addr.s_addr == - t.s_addr) + t.s_addr) { + IF_ADDR_UNLOCK(ifp); + t = IA_SIN(ia)->sin_addr; goto match; + } } + IF_ADDR_UNLOCK(ifp); } + /* * If the packet was transiting through us, use the address of * the interface the packet came through in. If that interface * doesn't have a suitable IP address, the normal selection * criteria apply. */ - if (V_icmp_rfi && m->m_pkthdr.rcvif != NULL) { - TAILQ_FOREACH(ifa, &m->m_pkthdr.rcvif->if_addrhead, ifa_link) { + if (V_icmp_rfi && ifp != NULL) { + IF_ADDR_LOCK(ifp); + TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { if (ifa->ifa_addr->sa_family != AF_INET) continue; ia = ifatoia(ifa); + t = IA_SIN(ia)->sin_addr; + IF_ADDR_UNLOCK(ifp); goto match; } + IF_ADDR_UNLOCK(ifp); } + /* * If the incoming packet was not addressed directly to us, use * designated interface for icmp replies specified by sysctl @@ -713,13 +730,18 @@ * with normal source selection. */ if (V_reply_src[0] != '\0' && (ifn = ifunit(V_reply_src))) { + IF_ADDR_LOCK(ifn); TAILQ_FOREACH(ifa, &ifn->if_addrhead, ifa_link) { if (ifa->ifa_addr->sa_family != AF_INET) continue; ia = ifatoia(ifa); + t = IA_SIN(ia)->sin_addr; + IF_ADDR_UNLOCK(ifn); goto match; } + IF_ADDR_UNLOCK(ifn); } + /* * If the packet was transiting through us, use the address of * the interface that is the closest to the packet source. @@ -732,11 +754,12 @@ V_icmpstat.icps_noroute++; goto done; } + t = IA_SIN(ia)->sin_addr; + match: #ifdef MAC mac_netinet_icmp_replyinplace(m); #endif - t = IA_SIN(ia)->sin_addr; ip->ip_src = t; ip->ip_ttl = V_ip_defttl; Index: netinet/in_var.h =================================================================== --- netinet/in_var.h (revision 188604) +++ netinet/in_var.h (working copy) @@ -92,6 +92,23 @@ extern u_long in_ifaddrhmask; /* mask for hash table */ #endif +extern struct mtx in_ifaddrhead_lock; +extern struct mtx in_ifaddrhashtbl_lock; + +#define IN_IFADDRHEAD_LOCK_ASSERT() mtx_assert(&in_ifaddrhead_lock, \ + MA_OWNED) +#define IN_IFADDRHEAD_RLOCK() mtx_lock(&in_ifaddrhead_lock) +#define IN_IFADDRHEAD_RUNLOCK() mtx_unlock(&in_ifaddrhead_lock) +#define IN_IFADDRHEAD_WLOCK() mtx_lock(&in_ifaddrhead_lock) +#define IN_IFADDRHEAD_WUNLOCK() mtx_unlock(&in_ifaddrhead_lock) + +#define IN_IFADDRHASHTBL_LOCK_ASSERT() mtx_assert(&in_ifaddrhashtbl_lock, \ + MA_OWNED) +#define IN_IFADDRHASHTBL_RLOCK() mtx_lock(&in_ifaddrhashtbl_lock) +#define IN_IFADDRHASHTBL_RUNLOCK() mtx_unlock(&in_ifaddrhashtbl_lock) +#define IN_IFADDRHASHTBL_WLOCK() mtx_lock(&in_ifaddrhashtbl_lock) +#define IN_IFADDRHASHTBL_WUNLOCK() mtx_unlock(&in_ifaddrhashtbl_lock) + #define INADDR_NHASH_LOG2 9 #define INADDR_NHASH (1 << INADDR_NHASH_LOG2) #define INADDR_HASHVAL(x) fnv_32_buf((&(x)), sizeof(x), FNV1_32_INIT) @@ -107,6 +124,7 @@ /* struct in_ifaddr *ia; */ \ do { \ \ + IN_IFADDRHASHTBL_LOCK_ASSERT(); \ LIST_FOREACH(ia, INADDR_HASH((addr).s_addr), ia_hash) \ if (IA_SIN(ia)->sin_addr.s_addr == (addr).s_addr) \ break; \ @@ -122,6 +140,7 @@ { \ struct in_ifaddr *ia; \ \ + IN_IFADDRHASHTBL_LOCK_ASSERT(); \ INADDR_TO_IFADDR(addr, ia); \ (ifp) = (ia == NULL) ? NULL : ia->ia_ifp; \ } @@ -134,6 +153,7 @@ /* struct ifnet *ifp; */ \ /* struct in_ifaddr *ia; */ \ { \ + IN_IFADDRHEAD_LOCK_ASSERT(); \ for ((ia) = TAILQ_FIRST(&V_in_ifaddrhead); \ (ia) != NULL && (ia)->ia_ifp != (ifp); \ (ia) = TAILQ_NEXT((ia), ia_link)) \ Index: netinet/if_ether.c =================================================================== --- netinet/if_ether.c (revision 188604) +++ netinet/if_ether.c (working copy) @@ -184,6 +184,10 @@ struct arphdr *ah; struct sockaddr sa; + if ((m = m_gethdr(M_DONTWAIT, MT_DATA)) == NULL) + return; + + IF_ADDR_LOCK(ifp); if (sip == NULL) { /* XXX don't believe this can happen (or explain why) */ /* @@ -207,8 +211,6 @@ } } - if ((m = m_gethdr(M_DONTWAIT, MT_DATA)) == NULL) - return; m->m_len = sizeof(*ah) + 2*sizeof(struct in_addr) + 2*ifp->if_data.ifi_addrlen; m->m_pkthdr.len = m->m_len; @@ -224,6 +226,7 @@ ah->ar_op = htons(ARPOP_REQUEST); bcopy((caddr_t)enaddr, (caddr_t)ar_sha(ah), ah->ar_hln); bcopy((caddr_t)sip, (caddr_t)ar_spa(ah), ah->ar_pln); + IF_ADDR_UNLOCK(ifp); bcopy((caddr_t)tip, (caddr_t)ar_tpa(ah), ah->ar_pln); sa.sa_family = AF_ARP; sa.sa_len = 2; @@ -495,15 +498,19 @@ * request for the virtual host ip. * XXX: This is really ugly! */ + IN_IFADDRHASHTBL_RLOCK(); LIST_FOREACH(ia, INADDR_HASH(itaddr.s_addr), ia_hash) { if (((bridged && ia->ia_ifp->if_bridge != NULL) || ia->ia_ifp == ifp) && - itaddr.s_addr == ia->ia_addr.sin_addr.s_addr) + itaddr.s_addr == ia->ia_addr.sin_addr.s_addr) { + IN_IFADDRHASHTBL_RUNLOCK(); goto match; + } #ifdef DEV_CARP if (ifp->if_carp != NULL && carp_iamatch(ifp->if_carp, ia, &isaddr, &enaddr) && itaddr.s_addr == ia->ia_addr.sin_addr.s_addr) { + IN_IFADDRHASHTBL_RUNLOCK(); carp_match = 1; goto match; } @@ -512,8 +519,10 @@ LIST_FOREACH(ia, INADDR_HASH(isaddr.s_addr), ia_hash) if (((bridged && ia->ia_ifp->if_bridge != NULL) || ia->ia_ifp == ifp) && - isaddr.s_addr == ia->ia_addr.sin_addr.s_addr) + isaddr.s_addr == ia->ia_addr.sin_addr.s_addr) { + IN_IFADDRHASHTBL_RUNLOCK(); goto match; + } #define BDG_MEMBER_MATCHES_ARP(addr, ifp, ia) \ (ia->ia_ifp->if_bridge == ifp->if_softc && \ @@ -529,26 +538,37 @@ LIST_FOREACH(ia, INADDR_HASH(itaddr.s_addr), ia_hash) { if (BDG_MEMBER_MATCHES_ARP(itaddr.s_addr, ifp, ia)) { ifp = ia->ia_ifp; + IN_IFADDRHASHTBL_RUNLOCK(); goto match; } } } #undef BDG_MEMBER_MATCHES_ARP + IN_IFADDRHASHTBL_RUNLOCK(); /* * No match, use the first inet address on the receive interface * as a dummy address for the rest of the function. */ + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) if (ifa->ifa_addr->sa_family == AF_INET) { + IF_ADDR_UNLOCK(ifp); ia = ifatoia(ifa); goto match; } + IF_ADDR_UNLOCK(ifp); /* * If bridging, fall back to using any inet address. */ - if (!bridged || (ia = TAILQ_FIRST(&V_in_ifaddrhead)) == NULL) + if (!bridged) goto drop; + IN_IFADDRHEAD_RLOCK(); + if ((ia = TAILQ_FIRST(&V_in_ifaddrhead)) == NULL) { + IN_IFADDRHEAD_RUNLOCK(); + goto drop; + } + IN_IFADDRHEAD_RUNLOCK(); match: if (!enaddr) enaddr = (u_int8_t *)IF_LLADDR(ifp); Index: netinet/ip_fw2.c =================================================================== --- netinet/ip_fw2.c (revision 188604) +++ netinet/ip_fw2.c (working copy) @@ -483,14 +483,17 @@ } else { struct ifaddr *ia; - /* XXX lock? */ + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ia, &ifp->if_addrhead, ifa_link) { if (ia->ifa_addr->sa_family != AF_INET) continue; if (cmd->p.ip.s_addr == ((struct sockaddr_in *) - (ia->ifa_addr))->sin_addr.s_addr) + (ia->ifa_addr))->sin_addr.s_addr) { + IF_ADDR_UNLOCK(ifp); return(1); /* match */ + } } + IF_ADDR_UNLOCK(ifp); } return(0); /* no match, fail ... */ } @@ -592,17 +595,22 @@ struct in6_ifaddr *fdm; struct in6_addr copia; - TAILQ_FOREACH(mdc, &V_ifnet, if_link) + TAILQ_FOREACH(mdc, &V_ifnet, if_link) { + IF_ADDR_LOCK(mdc); TAILQ_FOREACH(mdc2, &mdc->if_addrlist, ifa_list) { if (mdc2->ifa_addr->sa_family == AF_INET6) { fdm = (struct in6_ifaddr *)mdc2; copia = fdm->ia_addr.sin6_addr; /* need for leaving scope_id in the sock_addr */ in6_clearscope(&copia); - if (IN6_ARE_ADDR_EQUAL(ip6_addr, &copia)) + if (IN6_ARE_ADDR_EQUAL(ip6_addr, &copia)) { + IF_ADDR_UNLOCK(mdc); return 1; + } } } + IF_ADDR_UNLOCK(mdc); + } return 0; } @@ -2740,7 +2748,9 @@ if (is_ipv4) { struct ifnet *tif; + IN_IFADDRHEAD_RLOCK(); INADDR_TO_IFP(src_ip, tif); + IN_IFADDRHEAD_RUNLOCK(); match = (tif != NULL); } break; @@ -2773,7 +2783,9 @@ if (is_ipv4) { struct ifnet *tif; + IN_IFADDRHEAD_RLOCK(); INADDR_TO_IFP(dst_ip, tif); + IN_IFADDRHEAD_RUNLOCK(); match = (tif != NULL); } break; Index: netinet/igmp.c =================================================================== --- netinet/igmp.c (revision 188604) +++ netinet/igmp.c (working copy) @@ -326,7 +326,9 @@ * potentially get looped back if we are a multicast router, * so discard reports sourced by me. */ + IN_IFADDRHEAD_RLOCK(); IFP_TO_IA(ifp, ia); + IN_IFADDRHEAD_RUNLOCK(); if (ia != NULL && ip->ip_src.s_addr == IA_SIN(ia)->sin_addr.s_addr) break; Index: netipx/ipx.c =================================================================== --- netipx/ipx.c (revision 188604) +++ netipx/ipx.c (working copy) @@ -173,7 +173,6 @@ ifa = (struct ifaddr *)ia; IFA_LOCK_INIT(ifa); ifa->ifa_refcnt = 1; - TAILQ_INSERT_TAIL(&ifp->if_addrhead, ifa, ifa_link); ia->ia_ifp = ifp; ifa->ifa_addr = (struct sockaddr *)&ia->ia_addr; @@ -185,6 +184,9 @@ ia->ia_broadaddr.sipx_len = sizeof(ia->ia_addr); ia->ia_broadaddr.sipx_addr.x_host = ipx_broadhost; } + IF_ADDR_LOCK(ifp); + TAILQ_INSERT_TAIL(&ifp->if_addrhead, ifa, ifa_link); + IF_ADDR_UNLOCK(ifp); } break; default: @@ -216,7 +218,9 @@ case SIOCDIFADDR: ipx_ifscrub(ifp, ia); ifa = (struct ifaddr *)ia; + IF_ADDR_LOCK(ifp); TAILQ_REMOVE(&ifp->if_addrhead, ifa, ifa_link); + IF_ADDR_UNLOCK(ifp); oia = ia; if (oia == (ia = ipx_ifaddr)) { ipx_ifaddr = ia->ia_next; Index: netipx/ipx_usrreq.c =================================================================== --- netipx/ipx_usrreq.c (revision 188604) +++ netipx/ipx_usrreq.c (working copy) @@ -170,6 +170,7 @@ if (ipx_neteqnn(ipx->ipx_sna.x_net, ipx_zeronet) && ifp != NULL) { struct ifaddr *ifa; + IF_ADDR_LOCK(ifp); for (ifa = TAILQ_FIRST(&ifp->if_addrhead); ifa != NULL; ifa = TAILQ_NEXT(ifa, ifa_link)) { if (ifa->ifa_addr->sa_family == AF_IPX) { @@ -178,6 +179,7 @@ break; } } + IF_ADDR_UNLOCK(ifp); } ipxp->ipxp_rpt = ipx->ipx_pt; if ((ipxp->ipxp_flags & IPXP_RAWIN) == 0) { Index: netipx/ipx_input.c =================================================================== --- netipx/ipx_input.c (revision 188604) +++ netipx/ipx_input.c (working copy) @@ -485,13 +485,16 @@ ipx->ipx_sna.x_host = ia->ia_addr.sipx_addr.x_host; - if (ifp != NULL && (ifp->if_flags & IFF_POINTOPOINT)) + if (ifp != NULL && (ifp->if_flags & IFF_POINTOPOINT)) { + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { if (ifa->ifa_addr->sa_family == AF_IPX) { ipx->ipx_sna = IA_SIPX(ifa)->sipx_addr; break; } } + IF_ADDR_UNLOCK(ifp); + } ipx->ipx_len = ntohl(m0->m_pkthdr.len); IPX_LOCK(ipxp); ipx_input(m0, ipxp); Index: netgraph/ng_eiface.c =================================================================== --- netgraph/ng_eiface.c (revision 188604) +++ netgraph/ng_eiface.c (working copy) @@ -452,10 +452,12 @@ /* Determine size of response and allocate it */ buflen = 0; + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) buflen += SA_SIZE(ifa->ifa_addr); NG_MKRESPONSE(resp, msg, buflen, M_NOWAIT); if (resp == NULL) { + IF_ADDR_UNLOCK(ifp); error = ENOMEM; break; } @@ -474,6 +476,7 @@ ptr += len; buflen -= len; } + IF_ADDR_UNLOCK(ifp); break; } Index: netgraph/ng_iface.c =================================================================== --- netgraph/ng_iface.c (revision 188604) +++ netgraph/ng_iface.c (working copy) @@ -667,6 +667,7 @@ struct ifaddr *ifa; /* Return the first configured IP address */ + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { struct ng_cisco_ipaddr *ips; @@ -684,6 +685,7 @@ ifa->ifa_netmask)->sin_addr; break; } + IF_ADDR_UNLOCK(ifp); /* No IP addresses on this interface? */ if (ifa == NULL) Index: net/if.c =================================================================== --- net/if.c (revision 188604) +++ net/if.c (working copy) @@ -228,10 +228,14 @@ struct ifaddr * ifaddr_byindex(u_short idx) { + struct ifnet *ifp; struct ifaddr *ifa; IFNET_RLOCK(); - ifa = ifnet_byindex_locked(idx)->if_addr; + ifp = ifnet_byindex_locked(idx); + IF_ADDR_LOCK(ifp); + ifa = ifp->if_addr; + IF_ADDR_UNLOCK(ifp); IFNET_RUNLOCK(); return (ifa); } @@ -738,6 +742,7 @@ { struct ifaddr *ifa, *next; + IF_ADDR_LOCK(ifp); TAILQ_FOREACH_SAFE(ifa, &ifp->if_addrhead, ifa_link, next) { if (ifa->ifa_addr->sa_family == AF_LINK) continue; @@ -750,6 +755,8 @@ ifr.ifra_addr = *ifa->ifa_addr; if (ifa->ifa_dstaddr) ifr.ifra_broadaddr = *ifa->ifa_dstaddr; + + /* XXXRW: Potential lock order issues here. */ if (in_control(NULL, SIOCDIFADDR, (caddr_t)&ifr, ifp, NULL) == 0) continue; @@ -765,6 +772,7 @@ TAILQ_REMOVE(&ifp->if_addrhead, ifa, ifa_link); IFAFREE(ifa); } + IF_ADDR_UNLOCK(ifp); } /* @@ -1181,19 +1189,26 @@ struct ifaddr *ifa; IFNET_RLOCK(); - TAILQ_FOREACH(ifp, &V_ifnet, if_link) + TAILQ_FOREACH(ifp, &V_ifnet, if_link) { + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { if (ifa->ifa_addr->sa_family != addr->sa_family) continue; - if (sa_equal(addr, ifa->ifa_addr)) + if (sa_equal(addr, ifa->ifa_addr)) { + IF_ADDR_UNLOCK(ifp); goto done; + } /* IP6 doesn't have broadcast */ if ((ifp->if_flags & IFF_BROADCAST) && ifa->ifa_broadaddr && ifa->ifa_broadaddr->sa_len != 0 && - sa_equal(ifa->ifa_broadaddr, addr)) + sa_equal(ifa->ifa_broadaddr, addr)) { + IF_ADDR_UNLOCK(ifp); goto done; + } } + IF_ADDR_UNLOCK(ifp); + } ifa = NULL; done: IFNET_RUNLOCK(); @@ -1212,16 +1227,21 @@ struct ifaddr *ifa; IFNET_RLOCK(); - TAILQ_FOREACH(ifp, &V_ifnet, if_link) + TAILQ_FOREACH(ifp, &V_ifnet, if_link) { + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { if (ifa->ifa_addr->sa_family != addr->sa_family) continue; if ((ifp->if_flags & IFF_BROADCAST) && ifa->ifa_broadaddr && ifa->ifa_broadaddr->sa_len != 0 && - sa_equal(ifa->ifa_broadaddr, addr)) + sa_equal(ifa->ifa_broadaddr, addr)) { + IF_ADDR_UNLOCK(ifp); goto done; + } } + IF_ADDR_UNLOCK(ifp); + } ifa = NULL; done: IFNET_RUNLOCK(); @@ -1243,13 +1263,17 @@ TAILQ_FOREACH(ifp, &V_ifnet, if_link) { if ((ifp->if_flags & IFF_POINTOPOINT) == 0) continue; + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { if (ifa->ifa_addr->sa_family != addr->sa_family) continue; if (ifa->ifa_dstaddr != NULL && - sa_equal(addr, ifa->ifa_dstaddr)) + sa_equal(addr, ifa->ifa_dstaddr)) { + IF_ADDR_UNLOCK(ifp); goto done; + } } + IF_ADDR_UNLOCK(ifp); } ifa = NULL; done: @@ -1287,6 +1311,7 @@ */ IFNET_RLOCK(); TAILQ_FOREACH(ifp, &V_ifnet, if_link) { + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { char *cp, *cp2, *cp3; @@ -1302,16 +1327,21 @@ * netmask for the remote end. */ if (ifa->ifa_dstaddr != NULL && - sa_equal(addr, ifa->ifa_dstaddr)) + sa_equal(addr, ifa->ifa_dstaddr)) { + IF_ADDR_UNLOCK(ifp); goto done; + } } else { /* * if we have a special address handler, * then use it instead of the generic one. */ if (ifa->ifa_claim_addr) { - if ((*ifa->ifa_claim_addr)(ifa, addr)) + if ((*ifa->ifa_claim_addr)(ifa, + addr)) { + IF_ADDR_UNLOCK(ifp); goto done; + } continue; } @@ -1345,6 +1375,7 @@ ifa_maybe = ifa; } } + IF_ADDR_UNLOCK(ifp); } ifa = ifa_maybe; done: @@ -1367,6 +1398,7 @@ if (af >= AF_MAX) return (0); + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { if (ifa->ifa_addr->sa_family != af) continue; @@ -1396,6 +1428,7 @@ } ifa = ifa_maybe; done: + IF_ADDR_UNLOCK(ifp); return (ifa); } @@ -1444,9 +1477,11 @@ ifp->if_flags &= ~flag; getmicrotime(&ifp->if_lastchange); + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) if (fam == PF_UNSPEC || (fam == ifa->ifa_addr->sa_family)) pfctlinput(PRC_IFDOWN, ifa->ifa_addr); + IF_ADDR_UNLOCK(ifp); ifp->if_qflush(ifp); #ifdef DEV_CARP @@ -1470,9 +1505,11 @@ ifp->if_flags |= flag; getmicrotime(&ifp->if_lastchange); + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) if (fam == PF_UNSPEC || (fam == ifa->ifa_addr->sa_family)) pfctlinput(PRC_IFUP, ifa->ifa_addr); + IF_ADDR_UNLOCK(ifp); #ifdef DEV_CARP if (ifp->if_carp) carp_carpdev_state(ifp->if_carp); @@ -1788,6 +1825,7 @@ ifp->if_xname, new_name); strlcpy(ifp->if_xname, new_name, sizeof(ifp->if_xname)); + IF_ADDR_LOCK(ifp); ifa = ifp->if_addr; IFA_LOCK(ifa); sdl = (struct sockaddr_dl *)ifa->ifa_addr; @@ -1809,6 +1847,7 @@ while (namelen != 0) sdl->sdl_data[--namelen] = 0xff; IFA_UNLOCK(ifa); + IF_ADDR_UNLOCK(ifp); EVENTHANDLER_INVOKE(ifnet_arrival_event, ifp); /* Announce the return of the interface. */ @@ -2268,6 +2307,7 @@ } addrs = 0; + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { struct sockaddr *sa = ifa->ifa_addr; @@ -2307,6 +2347,7 @@ if (!sbuf_overflowed(sb)) valid_len = sbuf_len(sb); } + IF_ADDR_UNLOCK(ifp); } IFNET_RUNLOCK(); @@ -2754,14 +2795,21 @@ struct ifaddr *ifa; struct ifreq ifr; + IF_ADDR_LOCK(ifp); ifa = ifp->if_addr; - if (ifa == NULL) + if (ifa == NULL) { + IF_ADDR_UNLOCK(ifp); return (EINVAL); + } sdl = (struct sockaddr_dl *)ifa->ifa_addr; - if (sdl == NULL) + if (sdl == NULL) { + IF_ADDR_UNLOCK(ifp); return (EINVAL); - if (len != sdl->sdl_alen) /* don't allow length to change */ + } + if (len != sdl->sdl_alen) { /* don't allow length to change */ + IF_ADDR_UNLOCK(ifp); return (EINVAL); + } switch (ifp->if_type) { case IFT_ETHER: case IFT_FDDI: @@ -2774,8 +2822,11 @@ bcopy(lladdr, LLADDR(sdl), len); break; default: + IF_ADDR_UNLOCK(ifp); return (ENODEV); } + IF_ADDR_UNLOCK(ifp); + /* * If the interface is already up, we need * to re-init it in order to reprogram its @@ -2799,10 +2850,12 @@ * Also send gratuitous ARPs to notify other nodes about * the address change. */ + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { if (ifa->ifa_addr->sa_family == AF_INET) arp_ifinit(ifp, ifa); } + IF_ADDR_UNLOCK(ifp); #endif } return (0); Index: net/if_stf.c =================================================================== --- net/if_stf.c (revision 188604) +++ net/if_stf.c (working copy) @@ -382,6 +382,7 @@ struct sockaddr_in6 *sin6; struct in_addr in; + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ia, &ifp->if_addrlist, ifa_list) { if (ia->ifa_addr->sa_family != AF_INET6) continue; @@ -390,15 +391,17 @@ continue; bcopy(GET_V4(&sin6->sin6_addr), &in, sizeof(in)); + IN_IFADDRHASHTBL_RLOCK(); LIST_FOREACH(ia4, INADDR_HASH(in.s_addr), ia_hash) if (ia4->ia_addr.sin_addr.s_addr == in.s_addr) break; + IN_IFADDRHASHTBL_RUNLOCK(); if (ia4 == NULL) continue; - + IF_ADDR_UNLOCK(ifp); return (struct in6_ifaddr *)ia; } - + IF_ADDR_UNLOCK(ifp); return NULL; } @@ -610,15 +613,19 @@ /* * reject packets with broadcast */ + IN_IFADDRHEAD_RLOCK(); for (ia4 = TAILQ_FIRST(&V_in_ifaddrhead); ia4; ia4 = TAILQ_NEXT(ia4, ia_link)) { if ((ia4->ia_ifa.ifa_ifp->if_flags & IFF_BROADCAST) == 0) continue; - if (in->s_addr == ia4->ia_broadaddr.sin_addr.s_addr) + if (in->s_addr == ia4->ia_broadaddr.sin_addr.s_addr) { + IN_IFADDRHEAD_RUNLOCK(); return -1; + } } + IN_IFADDRHEAD_RUNLOCK(); /* * perform ingress filter Index: net/if_tap.c =================================================================== --- net/if_tap.c (revision 188604) +++ net/if_tap.c (working copy) @@ -544,9 +544,15 @@ s = splimp(); if_down(ifp); if (ifp->if_drv_flags & IFF_DRV_RUNNING) { + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { + /* + * XXXRW: lock order reversal here? Why do + * we do this? + */ rtinit(ifa, (int)RTM_DELETE, 0); } + IF_ADDR_UNLOCK(ifp); if_purgeaddrs(ifp); ifp->if_drv_flags &= ~IFF_DRV_RUNNING; } Index: net/rtsock.c =================================================================== --- net/rtsock.c (revision 188604) +++ net/rtsock.c (working copy) @@ -356,6 +356,7 @@ * Try to find an address on the given outgoing interface * that belongs to the jail. */ + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { struct sockaddr *sa; sa = ifa->ifa_addr; @@ -371,14 +372,17 @@ /* * As a last resort return the 'default' jail address. */ - if (prison_get_ip4(cred, &ia) != 0) + if (prison_get_ip4(cred, &ia) != 0) { + IF_ADDR_UNLOCK(ifp); return (ESRCH); + } } bzero(&saun->sin, sizeof(struct sockaddr_in)); saun->sin.sin_len = sizeof(struct sockaddr_in); saun->sin.sin_family = AF_INET; saun->sin.sin_addr.s_addr = ia.s_addr; info->rti_info[RTAX_IFA] = (struct sockaddr *)&saun->sin; + IF_ADDR_UNLOCK(ifp); break; } #endif @@ -394,6 +398,7 @@ * Try to find an address on the given outgoing interface * that belongs to the jail. */ + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { struct sockaddr *sa; sa = ifa->ifa_addr; @@ -410,16 +415,21 @@ /* * As a last resort return the 'default' jail address. */ - if (prison_get_ip6(cred, &ia6) != 0) + if (prison_get_ip6(cred, &ia6) != 0) { + IF_ADDR_UNLOCK(ifp); return (ESRCH); + } } bzero(&saun->sin6, sizeof(struct sockaddr_in6)); saun->sin6.sin6_len = sizeof(struct sockaddr_in6); saun->sin6.sin6_family = AF_INET6; bcopy(&ia6, &saun->sin6.sin6_addr, sizeof(struct in6_addr)); - if (sa6_recoverscope(&saun->sin6) != 0) + if (sa6_recoverscope(&saun->sin6) != 0) { + IF_ADDR_UNLOCK(ifp); return (ESRCH); + } info->rti_info[RTAX_IFA] = (struct sockaddr *)&saun->sin6; + IF_ADDR_UNLOCK(ifp); break; } #endif @@ -605,18 +615,21 @@ if (rtm->rtm_addrs & (RTA_IFP | RTA_IFA)) { ifp = rt->rt_ifp; if (ifp) { + IF_ADDR_LOCK(ifp); info.rti_info[RTAX_IFP] = ifp->if_addr->ifa_addr; error = rtm_get_jailed(&info, ifp, rt, &saun, curthread->td_ucred); if (error != 0) { RT_UNLOCK(rt); + IF_ADDR_UNLOCK(ifp); senderr(error); } if (ifp->if_flags & IFF_POINTOPOINT) info.rti_info[RTAX_BRD] = rt->rt_ifa->ifa_dstaddr; rtm->rtm_index = ifp->if_index; + IF_ADDR_UNLOCK(ifp); } else { info.rti_info[RTAX_IFP] = NULL; info.rti_info[RTAX_IFA] = NULL; @@ -1057,9 +1070,11 @@ int ncmd = cmd == RTM_ADD ? RTM_NEWADDR : RTM_DELADDR; info.rti_info[RTAX_IFA] = sa = ifa->ifa_addr; + IF_ADDR_LOCK(ifp); info.rti_info[RTAX_IFP] = ifp->if_addr->ifa_addr; info.rti_info[RTAX_NETMASK] = ifa->ifa_netmask; info.rti_info[RTAX_BRD] = ifa->ifa_dstaddr; + IF_ADDR_UNLOCK(ifp); if ((m = rt_msg1(ncmd, &info)) == NULL) continue; ifam = mtod(m, struct ifa_msghdr *); @@ -1102,6 +1117,8 @@ struct ifnet *ifp = ifma->ifma_ifp; struct ifma_msghdr *ifmam; + IF_ADDR_LOCK_ASSERT(ifp); + if (route_cb.any_count == 0) return; @@ -1247,10 +1264,12 @@ info.rti_info[RTAX_NETMASK] = rt_mask(rt); info.rti_info[RTAX_GENMASK] = 0; if (rt->rt_ifp) { + IF_ADDR_LOCK(rt->rt_ifp); info.rti_info[RTAX_IFP] = rt->rt_ifp->if_addr->ifa_addr; info.rti_info[RTAX_IFA] = rt->rt_ifa->ifa_addr; if (rt->rt_ifp->if_flags & IFF_POINTOPOINT) info.rti_info[RTAX_BRD] = rt->rt_ifa->ifa_dstaddr; + IF_ADDR_UNLOCK(rt->rt_ifp); } size = rt_msg2(RTM_GET, &info, NULL, w); if (w->w_req && w->w_tmem) { @@ -1282,8 +1301,10 @@ TAILQ_FOREACH(ifp, &V_ifnet, if_link) { if (w->w_arg && w->w_arg != ifp->if_index) continue; + IF_ADDR_LOCK(ifp); ifa = ifp->if_addr; info.rti_info[RTAX_IFP] = ifa->ifa_addr; + IF_ADDR_UNLOCK(ifp); len = rt_msg2(RTM_IFINFO, &info, NULL, w); info.rti_info[RTAX_IFP] = NULL; if (w->w_req && w->w_tmem) { @@ -1298,6 +1319,7 @@ if (error) goto done; } + IF_ADDR_LOCK(ifp); while ((ifa = TAILQ_NEXT(ifa, ifa_link)) != NULL) { if (af && af != ifa->ifa_addr->sa_family) continue; @@ -1316,11 +1338,16 @@ ifam->ifam_flags = ifa->ifa_flags; ifam->ifam_metric = ifa->ifa_metric; ifam->ifam_addrs = info.rti_addrs; + + /* XXXRW: Lock held over copyout(). */ error = SYSCTL_OUT(w->w_req, w->w_tmem, len); - if (error) + if (error) { + IF_ADDR_UNLOCK(ifp); goto done; + } } } + IF_ADDR_UNLOCK(ifp); info.rti_info[RTAX_IFA] = info.rti_info[RTAX_NETMASK] = info.rti_info[RTAX_BRD] = NULL; } @@ -1344,9 +1371,9 @@ TAILQ_FOREACH(ifp, &V_ifnet, if_link) { if (w->w_arg && w->w_arg != ifp->if_index) continue; + IF_ADDR_LOCK(ifp); ifa = ifp->if_addr; info.rti_info[RTAX_IFP] = ifa ? ifa->ifa_addr : NULL; - IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link) { if (af && af != ifma->ifma_addr->sa_family) continue; @@ -1365,6 +1392,8 @@ ifmam->ifmam_index = ifma->ifma_ifp->if_index; ifmam->ifmam_flags = 0; ifmam->ifmam_addrs = info.rti_addrs; + + /* XXXRW: Lock held over copyout(). */ error = SYSCTL_OUT(w->w_req, w->w_tmem, len); if (error) { IF_ADDR_UNLOCK(ifp); Index: net/if_tun.c =================================================================== --- net/if_tun.c (revision 188604) +++ net/if_tun.c (working copy) @@ -476,7 +476,10 @@ if (ifp->if_drv_flags & IFF_DRV_RUNNING) { struct ifaddr *ifa; - s = splimp(); + /* + * XXXRW: Potential lock order problem here with rtinit. + */ + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { /* deal w/IPv4 PtP destination; unlocked read */ if (ifa->ifa_addr->sa_family == AF_INET) { @@ -486,9 +489,9 @@ rtinit(ifa, (int)RTM_DELETE, 0); } } + IF_ADDR_UNLOCK(ifp); if_purgeaddrs(ifp); ifp->if_drv_flags &= ~IFF_DRV_RUNNING; - splx(s); } if_link_state_change(ifp, LINK_STATE_DOWN); CURVNET_RESTORE(); @@ -520,6 +523,7 @@ getmicrotime(&ifp->if_lastchange); #ifdef INET + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { if (ifa->ifa_addr->sa_family == AF_INET) { struct sockaddr_in *si; @@ -535,6 +539,7 @@ mtx_unlock(&tp->tun_mtx); } } + IF_ADDR_UNLOCK(ifp); #endif return (error); } Index: net/if_spppsubr.c =================================================================== --- net/if_spppsubr.c (revision 188604) +++ net/if_spppsubr.c (working copy) @@ -4928,6 +4928,7 @@ * aliases don't make any sense on a p2p link anyway. */ si = 0; + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) if (ifa->ifa_addr->sa_family == AF_INET) { si = (struct sockaddr_in *)ifa->ifa_addr; @@ -4946,6 +4947,7 @@ if (si && si->sin_addr.s_addr) ddst = si->sin_addr.s_addr; } + IF_ADDR_UNLOCK(ifp); if (dst) *dst = ntohl(ddst); if (src) *src = ntohl(ssrc); @@ -4969,6 +4971,7 @@ * aliases don't make any sense on a p2p link anyway. */ si = 0; + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { if (ifa->ifa_addr->sa_family == AF_INET) @@ -4978,6 +4981,7 @@ break; } } + IF_ADDR_UNLOCK(ifp); if (ifa && si) { @@ -4993,8 +4997,10 @@ /* set new address */ si->sin_addr.s_addr = htonl(src); ia = ifatoia(ifa); + IN_IFADDRHASHTBL_WLOCK(); LIST_REMOVE(ia, ia_hash); LIST_INSERT_HEAD(INADDR_HASH(si->sin_addr.s_addr), ia, ia_hash); + IN_IFADDRHASHTBL_WUNLOCK(); /* add new route */ error = rtinit(ifa, (int)RTM_ADD, RTF_HOST); @@ -5028,6 +5034,7 @@ * aliases don't make any sense on a p2p link anyway. */ si = 0; + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) if (ifa->ifa_addr->sa_family == AF_INET6) { si = (struct sockaddr_in6 *)ifa->ifa_addr; @@ -5048,6 +5055,7 @@ if (si && !IN6_IS_ADDR_UNSPECIFIED(&si->sin6_addr)) bcopy(&si->sin6_addr, &ddst, sizeof(ddst)); } + IF_ADDR_UNLOCK(ifp); if (dst) bcopy(&ddst, dst, sizeof(*dst)); @@ -5081,6 +5089,7 @@ */ sin6 = NULL; + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { if (ifa->ifa_addr->sa_family == AF_INET6) @@ -5090,6 +5099,7 @@ break; } } + IF_ADDR_UNLOCK(ifp); if (ifa && sin6) { Index: netinet6/in6_ifattach.c =================================================================== --- netinet6/in6_ifattach.c (revision 188604) +++ netinet6/in6_ifattach.c (working copy) @@ -231,6 +231,7 @@ static u_int8_t allone[8] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; + IF_ADDR_LOCK(ifp); for (ifa = ifp->if_addrlist.tqh_first; ifa; ifa = ifa->ifa_list.tqe_next) { @@ -245,6 +246,7 @@ goto found; } + IF_ADDR_UNLOCK(ifp); return -1; found: @@ -267,18 +269,24 @@ addrlen = 8; /* look at IEEE802/EUI64 only */ - if (addrlen != 8 && addrlen != 6) + if (addrlen != 8 && addrlen != 6) { + IF_ADDR_UNLOCK(ifp); return -1; + } /* * check for invalid MAC address - on bsdi, we see it a lot * since wildboar configures all-zero MAC on pccard before * card insertion. */ - if (bcmp(addr, allzero, addrlen) == 0) + if (bcmp(addr, allzero, addrlen) == 0) { + IF_ADDR_UNLOCK(ifp); return -1; - if (bcmp(addr, allone, addrlen) == 0) + } + if (bcmp(addr, allone, addrlen) == 0) { + IF_ADDR_UNLOCK(ifp); return -1; + } /* make EUI64 address */ if (addrlen == 8) @@ -296,10 +304,14 @@ break; case IFT_ARCNET: - if (addrlen != 1) + if (addrlen != 1) { + IF_ADDR_UNLOCK(ifp); return -1; - if (!addr[0]) + } + if (!addr[0]) { + IF_ADDR_UNLOCK(ifp); return -1; + } bzero(&in6->s6_addr[8], 8); in6->s6_addr[15] = addr[0]; @@ -321,15 +333,19 @@ * identifier source (can be renumbered). * we don't do this. */ + IF_ADDR_UNLOCK(ifp); return -1; default: + IF_ADDR_UNLOCK(ifp); return -1; } /* sanity check: g bit must not indicate "group" */ - if (EUI64_GROUP(in6)) + if (EUI64_GROUP(in6)) { + IF_ADDR_UNLOCK(ifp); return -1; + } /* convert EUI64 into IPv6 interface identifier */ EUI64_TO_IFID(in6); @@ -340,9 +356,11 @@ */ if ((in6->s6_addr[8] & ~(EUI64_GBIT | EUI64_UBIT)) == 0x00 && bcmp(&in6->s6_addr[9], allzero, 7) == 0) { + IF_ADDR_UNLOCK(ifp); return -1; } + IF_ADDR_UNLOCK(ifp); return 0; } Index: netinet6/icmp6.c =================================================================== --- netinet6/icmp6.c (revision 188604) +++ netinet6/icmp6.c (working copy) @@ -1687,6 +1687,7 @@ IFNET_RLOCK(); for (ifp = TAILQ_FIRST(&V_ifnet); ifp; ifp = TAILQ_NEXT(ifp, if_list)) { addrsofif = 0; + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) { if (ifa->ifa_addr->sa_family != AF_INET6) continue; @@ -1737,6 +1738,7 @@ } addrsofif++; /* count the address */ } + IF_ADDR_UNLOCK(ifp); if (iffound) { *ifpp = ifp; IFNET_RUNLOCK(); @@ -1772,6 +1774,7 @@ again: for (; ifp; ifp = TAILQ_NEXT(ifp, if_list)) { + IF_ADDR_LOCK(ifp); for (ifa = ifp->if_addrlist.tqh_first; ifa; ifa = ifa->ifa_list.tqe_next) { if (ifa->ifa_addr->sa_family != AF_INET6) @@ -1832,6 +1835,7 @@ * Set the truncate flag and return. */ nni6->ni_flags |= NI_NODEADDR_FLAG_TRUNCATE; + IF_ADDR_UNLOCK(ifp); IFNET_RUNLOCK(); return (copied); } @@ -1873,6 +1877,7 @@ resid -= (sizeof(struct in6_addr) + sizeof(u_int32_t)); copied += (sizeof(struct in6_addr) + sizeof(u_int32_t)); } + IF_ADDR_UNLOCK(ifp); if (ifp0) /* we need search only on the specified IF */ break; } Index: netinet6/nd6.c =================================================================== --- netinet6/nd6.c (revision 188604) +++ netinet6/nd6.c (working copy) @@ -726,6 +726,7 @@ struct in6_ifaddr *public_ifa6 = NULL; ifp = ia6->ia_ifa.ifa_ifp; + IF_ADDR_LOCK(ifp); for (ifa = ifp->if_addrlist.tqh_first; ifa; ifa = ifa->ifa_list.tqe_next) { struct in6_ifaddr *it6; @@ -770,12 +771,15 @@ int e; if ((e = in6_tmpifadd(public_ifa6, 0, 0)) != 0) { + IF_ADDR_UNLOCK(ifp); log(LOG_NOTICE, "regen_tmpaddr: failed to create a new" " tmp addr,errno=%d\n", e); return (-1); } + IF_ADDR_UNLOCK(ifp); return (0); } + IF_ADDR_UNLOCK(ifp); return (-1); } Index: netinet6/in6.c =================================================================== --- netinet6/in6.c (revision 188604) +++ netinet6/in6.c (working copy) @@ -814,7 +814,9 @@ V_in6_ifaddr = ia; ia->ia_ifa.ifa_refcnt = 1; + IF_ADDR_LOCK(ifp); TAILQ_INSERT_TAIL(&ifp->if_addrlist, &ia->ia_ifa, ifa_list); + IF_ADDR_UNLOCK(ifp); } /* update timestamp */ @@ -1174,7 +1176,9 @@ struct in6_ifaddr *oia; int s = splnet(); + IF_ADDR_LOCK(ifp); TAILQ_REMOVE(&ifp->if_addrlist, &ia->ia_ifa, ifa_list); + IF_ADDR_UNLOCK(ifp); oia = ia; if (oia == (ia = V_in6_ifaddr)) @@ -1226,12 +1230,14 @@ { struct ifaddr *ifa, *nifa; + IF_ADDR_LOCK(ifp); for (ifa = TAILQ_FIRST(&ifp->if_addrlist); ifa != NULL; ifa = nifa) { nifa = TAILQ_NEXT(ifa, ifa_list); if (ifa->ifa_addr->sa_family != AF_INET6) continue; in6_purgeaddr(ifa); } + IF_ADDR_UNLOCK(ifp); in6_ifdetach(ifp); } @@ -1409,6 +1415,7 @@ } } + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) { if (ifa->ifa_addr->sa_family != AF_INET6) continue; @@ -1429,6 +1436,7 @@ if (IN6_ARE_ADDR_EQUAL(&candidate, &match)) break; } + IF_ADDR_UNLOCK(ifp); if (!ifa) return EADDRNOTAVAIL; ia = ifa2ia6(ifa); @@ -1506,11 +1514,13 @@ * if this is its first address, * and to validate the address if necessary. */ + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) { if (ifa->ifa_addr->sa_family != AF_INET6) continue; ifacount++; } + IF_ADDR_UNLOCK(ifp); ia->ia_addr = *sin6; @@ -1622,6 +1632,7 @@ { struct ifaddr *ifa; + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) { if (ifa->ifa_addr->sa_family != AF_INET6) continue; @@ -1632,6 +1643,7 @@ break; } } + IF_ADDR_UNLOCK(ifp); return ((struct in6_ifaddr *)ifa); } @@ -1645,12 +1657,14 @@ { struct ifaddr *ifa; + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) { if (ifa->ifa_addr->sa_family != AF_INET6) continue; if (IN6_ARE_ADDR_EQUAL(addr, IFA_IN6(ifa))) break; } + IF_ADDR_UNLOCK(ifp); return ((struct in6_ifaddr *)ifa); } @@ -1849,6 +1863,7 @@ * If two or more, return one which matches the dst longest. * If none, return one of global addresses assigned other ifs. */ + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) { if (ifa->ifa_addr->sa_family != AF_INET6) continue; @@ -1880,8 +1895,10 @@ besta = (struct in6_ifaddr *)ifa; } } - if (besta) + if (besta) { + IF_ADDR_UNLOCK(ifp); return (besta); + } TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) { if (ifa->ifa_addr->sa_family != AF_INET6) @@ -1898,8 +1915,10 @@ continue; } + IF_ADDR_UNLOCK(ifp); return (struct in6_ifaddr *)ifa; } + IF_ADDR_UNLOCK(ifp); /* use the last-resort values, that are, deprecated addresses */ if (dep[0]) @@ -1919,6 +1938,7 @@ struct ifaddr *ifa; struct in6_ifaddr *ia; + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) { if (ifa->ifa_addr->sa_family != AF_INET6) continue; @@ -1934,6 +1954,7 @@ arc4random() % (MAX_RTR_SOLICITATION_DELAY * hz)); } } + IF_ADDR_UNLOCK(ifp); /* * special cases, like 6to4, are handled in in6_ifattach Index: netipsec/xform_ipip.c =================================================================== --- netipsec/xform_ipip.c (revision 188604) +++ netipsec/xform_ipip.c (working copy) @@ -309,6 +309,7 @@ V_ipip_allow != 2) { IFNET_RLOCK(); TAILQ_FOREACH(ifp, &V_ifnet, if_link) { + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { #ifdef INET if (ipo) { @@ -320,9 +321,10 @@ if (sin->sin_addr.s_addr == ipo->ip_src.s_addr) { + IF_ADDR_UNLOCK(ifp); + IFNET_RUNLOCK(); V_ipipstat.ipips_spoof++; m_freem(m); - IFNET_RUNLOCK(); return; } } @@ -337,15 +339,17 @@ sin6 = (struct sockaddr_in6 *) ifa->ifa_addr; if (IN6_ARE_ADDR_EQUAL(&sin6->sin6_addr, &ip6->ip6_src)) { + IF_ADDR_UNLOCK(ifp); + IFNET_RUNLOCK(); V_ipipstat.ipips_spoof++; m_freem(m); - IFNET_RUNLOCK(); return; } } #endif /* INET6 */ } + IF_ADDR_UNLOCK(ifp); } IFNET_RUNLOCK(); } Index: netipsec/key.c =================================================================== --- netipsec/key.c (revision 188604) +++ netipsec/key.c (working copy) @@ -3740,6 +3740,7 @@ #ifdef INET case AF_INET: sin = (struct sockaddr_in *)sa; + IN_IFADDRHEAD_RLOCK(); for (ia = V_in_ifaddrhead.tqh_first; ia; ia = ia->ia_link.tqe_next) { @@ -3747,9 +3748,11 @@ sin->sin_len == ia->ia_addr.sin_len && sin->sin_addr.s_addr == ia->ia_addr.sin_addr.s_addr) { + IN_IFADDRHEAD_RUNLOCK(); return 1; } } + IN_IFADDRHEAD_RUNLOCK(); break; #endif #ifdef INET6