FYI to all: I've committed a large patch that makes greater use of struct ifaddr refcounts to avoid a range of reader-writer and writer-writer races affecting a range of uses from simultaneous ifconfigs to ppp/tunnel/vpn endpoint reconfiguration. There are still some more address list locking changes to go in in the next 48 hours before the 8.0 freeze, which should in the medium-term significant improve stability in these areas. However, because this modifies quite a few spots in the code, it's possible we'll see two classes of bugs: - Reference leaks -- references acquired, but I missed a case and the reference is leaked. This could lead to a gradual memory leak of ifaddr's. You can track ifaddr allocation using "vmstat -m | grep ifaddr" -- if you see something you think is a leak, let me know. - Use-after-free -- in some case, a reference might not be properly acquired, but will be released, in which case memory might be used after free. In -CURRENT where we have INVARIANTS enabled, memory scrubbing generally picks this up quickly but not immediately, but watch out for new kernel page faults involving 0xdeadc0de. If you run into these problems, I'll likely send you some debugging patches that make it easier to track this down. Robert N M Watson Computer Laboratory University of Cambridge ---------- Forwarded message ---------- Date: Tue, 23 Jun 2009 20:19:09 +0000 (UTC) From: Robert Watson <rwatson_at_FreeBSD.org> To: src-committers_at_freebsd.org, svn-src-all_at_freebsd.org, svn-src-head_at_freebsd.org Subject: svn commit: r194760 - in head/sys: contrib/rdma net net80211 netinet netinet6 netipx Author: rwatson Date: Tue Jun 23 20:19:09 2009 New Revision: 194760 URL: http://svn.freebsd.org/changeset/base/194760 Log: Modify most routines returning 'struct ifaddr *' to return references rather than pointers, requiring callers to properly dispose of those references. The following routines now return references: ifaddr_byindex ifa_ifwithaddr ifa_ifwithbroadaddr ifa_ifwithdstaddr ifa_ifwithnet ifaof_ifpforaddr ifa_ifwithroute ifa_ifwithroute_fib rt_getifa rt_getifa_fib IFP_TO_IA ip_rtaddr in6_ifawithifp in6ifa_ifpforlinklocal in6ifa_ifpwithaddr in6_ifadd carp_iamatch6 ip6_getdstifaddr Remove unused macro which didn't have required referencing: IFP_TO_IA6 This closes many small races in which changes to interface or address lists while an ifaddr was in use could lead to use of freed memory (etc). In a few cases, add missing if_addr_list locking required to safely acquire references. Because of a lack of deep copying support, we accept a race in which an in6_ifaddr pointed to by mbuf tags and extracted with ip6_getdstifaddr() doesn't hold a reference while in transmit. Once we have mbuf tag deep copy support, this can be fixed. Reviewed by: bz Obtained from: Apple, Inc. (portions) MFC after: 6 weeks (portions) Modified: head/sys/contrib/rdma/rdma_addr.c head/sys/contrib/rdma/rdma_cma.c head/sys/net/if.c head/sys/net/route.c head/sys/net/rtsock.c head/sys/net80211/ieee80211.c head/sys/netinet/igmp.c head/sys/netinet/in.c head/sys/netinet/in_mcast.c head/sys/netinet/in_pcb.c head/sys/netinet/in_var.h head/sys/netinet/ip_carp.c head/sys/netinet/ip_divert.c head/sys/netinet/ip_icmp.c head/sys/netinet/ip_input.c head/sys/netinet/ip_mroute.c head/sys/netinet/ip_options.c head/sys/netinet/ip_output.c head/sys/netinet/tcp_input.c head/sys/netinet6/frag6.c head/sys/netinet6/icmp6.c head/sys/netinet6/in6.c head/sys/netinet6/in6_ifattach.c head/sys/netinet6/in6_pcb.c head/sys/netinet6/in6_src.c head/sys/netinet6/in6_var.h head/sys/netinet6/ip6_input.c head/sys/netinet6/ip6_output.c head/sys/netinet6/mld6.c head/sys/netinet6/nd6.c head/sys/netinet6/nd6_nbr.c head/sys/netinet6/nd6_rtr.c head/sys/netinet6/raw_ip6.c head/sys/netipx/ipx_pcb.c Modified: head/sys/contrib/rdma/rdma_addr.c ============================================================================== --- head/sys/contrib/rdma/rdma_addr.c Tue Jun 23 20:19:02 2009 (r194759) +++ head/sys/contrib/rdma/rdma_addr.c Tue Jun 23 20:19:09 2009 (r194760) _at__at_ -129,13 +129,16 _at__at_ int rdma_translate_ip(struct sockaddr *a struct ifaddr *ifa; struct sockaddr_in *sin = (struct sockaddr_in *)addr; uint16_t port = sin->sin_port; + int ret; sin->sin_port = 0; ifa = ifa_ifwithaddr(addr); sin->sin_port = port; if (!ifa) return (EADDRNOTAVAIL); - return rdma_copy_addr(dev_addr, ifa->ifa_ifp, NULL); + ret = rdma_copy_addr(dev_addr, ifa->ifa_ifp, NULL); + ifa_free(ifa); + return (ret); } static void queue_req(struct addr_req *req) Modified: head/sys/contrib/rdma/rdma_cma.c ============================================================================== --- head/sys/contrib/rdma/rdma_cma.c Tue Jun 23 20:19:02 2009 (r194759) +++ head/sys/contrib/rdma/rdma_cma.c Tue Jun 23 20:19:09 2009 (r194760) _at__at_ -1337,6 +1337,7 _at__at_ static int iw_conn_req_handler(struct iw } dev = ifa->ifa_ifp; ret = rdma_copy_addr(&conn_id->id.route.addr.dev_addr, dev, NULL); + ifa_free(ifa); if (ret) { cma_enable_remove(conn_id); rdma_destroy_id(new_cm_id); Modified: head/sys/net/if.c ============================================================================== --- head/sys/net/if.c Tue Jun 23 20:19:02 2009 (r194759) +++ head/sys/net/if.c Tue Jun 23 20:19:09 2009 (r194760) _at__at_ -48,6 +48,7 _at__at_ #include <sys/socket.h> #include <sys/socketvar.h> #include <sys/protosw.h> +#include <sys/kdb.h> #include <sys/kernel.h> #include <sys/lock.h> #include <sys/refcount.h> _at__at_ -261,6 +262,8 _at__at_ ifaddr_byindex(u_short idx) IFNET_RLOCK(); ifa = ifnet_byindex_locked(idx)->if_addr; + if (ifa != NULL) + ifa_ref(ifa); IFNET_RUNLOCK(); return (ifa); } _at__at_ -1464,7 +1467,7 _at__at_ ifa_free(struct ifaddr *ifa) */ /*ARGSUSED*/ static struct ifaddr * -ifa_ifwithaddr_internal(struct sockaddr *addr) +ifa_ifwithaddr_internal(struct sockaddr *addr, int getref) { INIT_VNET_NET(curvnet); struct ifnet *ifp; _at__at_ -1477,6 +1480,8 _at__at_ ifa_ifwithaddr_internal(struct sockaddr if (ifa->ifa_addr->sa_family != addr->sa_family) continue; if (sa_equal(addr, ifa->ifa_addr)) { + if (getref) + ifa_ref(ifa); IF_ADDR_UNLOCK(ifp); goto done; } _at__at_ -1485,6 +1490,8 _at__at_ ifa_ifwithaddr_internal(struct sockaddr ifa->ifa_broadaddr && ifa->ifa_broadaddr->sa_len != 0 && sa_equal(ifa->ifa_broadaddr, addr)) { + if (getref) + ifa_ref(ifa); IF_ADDR_UNLOCK(ifp); goto done; } _at__at_ -1501,14 +1508,14 _at__at_ struct ifaddr * ifa_ifwithaddr(struct sockaddr *addr) { - return (ifa_ifwithaddr_internal(addr)); + return (ifa_ifwithaddr_internal(addr, 1)); } int ifa_ifwithaddr_check(struct sockaddr *addr) { - return (ifa_ifwithaddr_internal(addr) != NULL); + return (ifa_ifwithaddr_internal(addr, 0) != NULL); } /* _at__at_ -1532,6 +1539,7 _at__at_ ifa_ifwithbroadaddr(struct sockaddr *add ifa->ifa_broadaddr && ifa->ifa_broadaddr->sa_len != 0 && sa_equal(ifa->ifa_broadaddr, addr)) { + ifa_ref(ifa); IF_ADDR_UNLOCK(ifp); goto done; } _at__at_ -1565,6 +1573,7 _at__at_ ifa_ifwithdstaddr(struct sockaddr *addr) continue; if (ifa->ifa_dstaddr != NULL && sa_equal(addr, ifa->ifa_dstaddr)) { + ifa_ref(ifa); IF_ADDR_UNLOCK(ifp); goto done; } _at__at_ -1587,7 +1596,7 _at__at_ ifa_ifwithnet(struct sockaddr *addr) INIT_VNET_NET(curvnet); struct ifnet *ifp; struct ifaddr *ifa; - struct ifaddr *ifa_maybe = (struct ifaddr *) 0; + struct ifaddr *ifa_maybe = NULL; u_int af = addr->sa_family; char *addr_data = addr->sa_data, *cplim; _at__at_ -1602,8 +1611,10 _at__at_ ifa_ifwithnet(struct sockaddr *addr) } /* - * Scan though each interface, looking for ones that have - * addresses in this address family. + * Scan though each interface, looking for ones that have addresses + * in this address family. Maintain a reference on ifa_maybe once + * we find one, as we release the IF_ADDR_LOCK() that kept it stable + * when we move onto the next interface. */ IFNET_RLOCK(); TAILQ_FOREACH(ifp, &V_ifnet, if_link) { _at__at_ -1624,6 +1635,7 _at__at_ next: continue; */ if (ifa->ifa_dstaddr != NULL && sa_equal(addr, ifa->ifa_dstaddr)) { + ifa_ref(ifa); IF_ADDR_UNLOCK(ifp); goto done; } _at__at_ -1634,6 +1646,7 _at__at_ next: continue; */ if (ifa->ifa_claim_addr) { if ((*ifa->ifa_claim_addr)(ifa, addr)) { + ifa_ref(ifa); IF_ADDR_UNLOCK(ifp); goto done; } _at__at_ -1664,17 +1677,24 _at__at_ next: continue; * before continuing to search * for an even better one. */ - if (ifa_maybe == 0 || + if (ifa_maybe == NULL || rn_refines((caddr_t)ifa->ifa_netmask, - (caddr_t)ifa_maybe->ifa_netmask)) + (caddr_t)ifa_maybe->ifa_netmask)) { + if (ifa_maybe != NULL) + ifa_free(ifa_maybe); ifa_maybe = ifa; + ifa_ref(ifa_maybe); + } } } IF_ADDR_UNLOCK(ifp); } ifa = ifa_maybe; + ifa_maybe = NULL; done: IFNET_RUNLOCK(); + if (ifa_maybe != NULL) + ifa_free(ifa_maybe); return (ifa); } _at__at_ -1688,7 +1708,7 _at__at_ ifaof_ifpforaddr(struct sockaddr *addr, struct ifaddr *ifa; char *cp, *cp2, *cp3; char *cplim; - struct ifaddr *ifa_maybe = 0; + struct ifaddr *ifa_maybe = NULL; u_int af = addr->sa_family; if (af >= AF_MAX) _at__at_ -1697,7 +1717,7 _at__at_ ifaof_ifpforaddr(struct sockaddr *addr, TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { if (ifa->ifa_addr->sa_family != af) continue; - if (ifa_maybe == 0) + if (ifa_maybe == NULL) ifa_maybe = ifa; if (ifa->ifa_netmask == 0) { if (sa_equal(addr, ifa->ifa_addr) || _at__at_ -1723,6 +1743,8 _at__at_ ifaof_ifpforaddr(struct sockaddr *addr, } ifa = ifa_maybe; done: + if (ifa != NULL) + ifa_ref(ifa); IF_ADDR_UNLOCK(ifp); return (ifa); } _at__at_ -1748,7 +1770,6 _at__at_ link_rtrequest(int cmd, struct rtentry * return; ifa = ifaof_ifpforaddr(dst, ifp); if (ifa) { - ifa_ref(ifa); /* XXX */ oifa = rt->rt_ifa; rt->rt_ifa = ifa; ifa_free(oifa); Modified: head/sys/net/route.c ============================================================================== --- head/sys/net/route.c Tue Jun 23 20:19:02 2009 (r194759) +++ head/sys/net/route.c Tue Jun 23 20:19:09 2009 (r194760) _at__at_ -559,6 +559,7 _at__at_ rtredirect_fib(struct sockaddr *dst, struct ifaddr *ifa; struct radix_node_head *rnh; + ifa = NULL; rnh = rt_tables_get_rnh(fibnum, dst->sa_family); if (rnh == NULL) { error = EAFNOSUPPORT; _at__at_ -664,6 +665,8 _at__at_ out: info.rti_info[RTAX_NETMASK] = netmask; info.rti_info[RTAX_AUTHOR] = src; rt_missmsg(RTM_REDIRECT, &info, flags, error); + if (ifa != NULL) + ifa_free(ifa); } int _at__at_ -693,6 +696,9 _at__at_ rtioctl_fib(u_long req, caddr_t data, u_ #endif /* INET */ } +/* + * For both ifa_ifwithroute() routines, 'ifa' is returned referenced. + */ struct ifaddr * ifa_ifwithroute(int flags, struct sockaddr *dst, struct sockaddr *gateway) { _at__at_ -749,11 +755,13 _at__at_ ifa_ifwithroute_fib(int flags, struct so default: break; } + if (!not_found && rt->rt_ifa != NULL) { + ifa = rt->rt_ifa; + ifa_ref(ifa); + } RT_REMREF(rt); RT_UNLOCK(rt); - if (not_found) - return (NULL); - if ((ifa = rt->rt_ifa) == NULL) + if (not_found || ifa == NULL) return (NULL); } if (ifa->ifa_addr->sa_family != dst->sa_family) { _at__at_ -761,6 +769,8 _at__at_ ifa_ifwithroute_fib(int flags, struct so ifa = ifaof_ifpforaddr(dst, ifa->ifa_ifp); if (ifa == NULL) ifa = oifa; + else + ifa_free(oifa); } return (ifa); } _at__at_ -819,6 +829,10 _at__at_ rt_getifa(struct rt_addrinfo *info) return (rt_getifa_fib(info, 0)); } +/* + * Look up rt_addrinfo for a specific fib. Note that if rti_ifa is defined, + * it will be referenced so the caller must free it. + */ int rt_getifa_fib(struct rt_addrinfo *info, u_int fibnum) { _at__at_ -831,8 +845,10 _at__at_ rt_getifa_fib(struct rt_addrinfo *info, */ if (info->rti_ifp == NULL && ifpaddr != NULL && ifpaddr->sa_family == AF_LINK && - (ifa = ifa_ifwithnet(ifpaddr)) != NULL) + (ifa = ifa_ifwithnet(ifpaddr)) != NULL) { info->rti_ifp = ifa->ifa_ifp; + ifa_free(ifa); + } if (info->rti_ifa == NULL && ifaaddr != NULL) info->rti_ifa = ifa_ifwithaddr(ifaaddr); if (info->rti_ifa == NULL) { _at__at_ -1123,12 +1139,19 _at__at_ rtrequest1_fib(int req, struct rt_addrin (gateway->sa_family != AF_UNSPEC) && (gateway->sa_family != AF_LINK)) senderr(EINVAL); - if (info->rti_ifa == NULL && (error = rt_getifa_fib(info, fibnum))) - senderr(error); + if (info->rti_ifa == NULL) { + error = rt_getifa_fib(info, fibnum); + if (error) + senderr(error); + } else + ifa_ref(info->rti_ifa); ifa = info->rti_ifa; rt = uma_zalloc(V_rtzone, M_NOWAIT | M_ZERO); - if (rt == NULL) + if (rt == NULL) { + if (ifa != NULL) + ifa_free(ifa); senderr(ENOBUFS); + } RT_LOCK_INIT(rt); rt->rt_flags = RTF_UP | flags; rt->rt_fibnum = fibnum; _at__at_ -1139,6 +1162,8 _at__at_ rtrequest1_fib(int req, struct rt_addrin RT_LOCK(rt); if ((error = rt_setgate(rt, dst, gateway)) != 0) { RT_LOCK_DESTROY(rt); + if (ifa != NULL) + ifa_free(ifa); uma_zfree(V_rtzone, rt); senderr(error); } _at__at_ -1157,11 +1182,10 _at__at_ rtrequest1_fib(int req, struct rt_addrin bcopy(dst, ndst, dst->sa_len); /* - * Note that we now have a reference to the ifa. + * We use the ifa reference returned by rt_getifa_fib(). * This moved from below so that rnh->rnh_addaddr() can * examine the ifa and ifa->ifa_ifp if it so desires. */ - ifa_ref(ifa); rt->rt_ifa = ifa; rt->rt_ifp = ifa->ifa_ifp; rt->rt_rmx.rmx_weight = 1; Modified: head/sys/net/rtsock.c ============================================================================== --- head/sys/net/rtsock.c Tue Jun 23 20:19:02 2009 (r194759) +++ head/sys/net/rtsock.c Tue Jun 23 20:19:09 2009 (r194760) _at__at_ -683,6 +683,13 _at__at_ route_output(struct mbuf *m, struct sock RT_UNLOCK(rt); RADIX_NODE_HEAD_LOCK(rnh); error = rt_getifa_fib(&info, rt->rt_fibnum); + /* + * XXXRW: Really we should release this + * reference later, but this maintains + * historical behavior. + */ + if (info.rti_ifa != NULL) + ifa_free(info.rti_ifa); RADIX_NODE_HEAD_UNLOCK(rnh); if (error != 0) senderr(error); Modified: head/sys/net80211/ieee80211.c ============================================================================== --- head/sys/net80211/ieee80211.c Tue Jun 23 20:19:02 2009 (r194759) +++ head/sys/net80211/ieee80211.c Tue Jun 23 20:19:09 2009 (r194760) _at__at_ -301,6 +301,7 _at__at_ ieee80211_ifattach(struct ieee80211com * sdl->sdl_type = IFT_ETHER; /* XXX IFT_IEEE80211? */ sdl->sdl_alen = IEEE80211_ADDR_LEN; IEEE80211_ADDR_COPY(LLADDR(sdl), macaddr); + ifa_free(ifa); } /* Modified: head/sys/netinet/igmp.c ============================================================================== --- head/sys/netinet/igmp.c Tue Jun 23 20:19:02 2009 (r194759) +++ head/sys/netinet/igmp.c Tue Jun 23 20:19:09 2009 (r194760) _at__at_ -1233,8 +1233,10 _at__at_ igmp_input_v1_report(struct ifnet *ifp, */ if (V_igmp_recvifkludge && in_nullhost(ip->ip_src)) { IFP_TO_IA(ifp, ia); - if (ia != NULL) + if (ia != NULL) { ip->ip_src.s_addr = htonl(ia->ia_subnet); + ifa_free(&ia->ia_ifa); + } } CTR3(KTR_IGMPV3, "process v1 report %s on ifp %p(%s)", _at__at_ -1326,16 +1328,23 _at__at_ igmp_input_v2_report(struct ifnet *ifp, * group. */ IFP_TO_IA(ifp, ia); - if (ia != NULL && in_hosteq(ip->ip_src, IA_SIN(ia)->sin_addr)) + if (ia != NULL && in_hosteq(ip->ip_src, IA_SIN(ia)->sin_addr)) { + ifa_free(&ia->ia_ifa); return (0); + } IGMPSTAT_INC(igps_rcv_reports); - if (ifp->if_flags & IFF_LOOPBACK) + if (ifp->if_flags & IFF_LOOPBACK) { + if (ia != NULL) + ifa_free(&ia->ia_ifa); return (0); + } if (!IN_MULTICAST(ntohl(igmp->igmp_group.s_addr)) || !in_hosteq(igmp->igmp_group, ip->ip_dst)) { + if (ia != NULL) + ifa_free(&ia->ia_ifa); IGMPSTAT_INC(igps_rcv_badreports); return (EINVAL); } _at__at_ -1351,6 +1360,8 _at__at_ igmp_input_v2_report(struct ifnet *ifp, if (ia != NULL) ip->ip_src.s_addr = htonl(ia->ia_subnet); } + if (ia != NULL) + ifa_free(&ia->ia_ifa); CTR3(KTR_IGMPV3, "process v2 report %s on ifp %p(%s)", inet_ntoa(igmp->igmp_group), ifp, ifp->if_xname); _at__at_ -3534,8 +3545,10 _at__at_ igmp_v3_encap_report(struct ifnet *ifp, struct in_ifaddr *ia; IFP_TO_IA(ifp, ia); - if (ia != NULL) + if (ia != NULL) { ip->ip_src = ia->ia_addr.sin_addr; + ifa_free(&ia->ia_ifa); + } } ip->ip_dst.s_addr = htonl(INADDR_ALLRPTS_GROUP); Modified: head/sys/netinet/in.c ============================================================================== --- head/sys/netinet/in.c Tue Jun 23 20:19:02 2009 (r194759) +++ head/sys/netinet/in.c Tue Jun 23 20:19:09 2009 (r194760) _at__at_ -219,7 +219,6 _at__at_ in_control(struct socket *so, u_long cmd register struct ifaddr *ifa; struct in_addr allhosts_addr; struct in_addr dst; - struct in_ifaddr *oia; struct in_ifinfo *ii; struct in_aliasreq *ifra = (struct in_aliasreq *)data; struct sockaddr_in oldaddr; _at__at_ -323,8 +322,10 _at__at_ in_control(struct socket *so, u_long cmd break; } } - IF_ADDR_LOCK(ifp); + if (ia != NULL) + ifa_ref(&ia->ia_ifa); 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) { _at__at_ -336,6 +337,9 _at__at_ in_control(struct socket *so, u_long cmd break; } } + if (ia != NULL) + ifa_ref(&ia->ia_ifa); + IF_ADDR_UNLOCK(ifp); } if (ia == NULL) iaIsFirst = 1; _at__at_ -345,23 +349,29 _at__at_ in_control(struct socket *so, u_long cmd case SIOCAIFADDR: case SIOCDIFADDR: if (ifra->ifra_addr.sin_family == AF_INET) { + struct in_ifaddr *oia; + 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; } + if (ia != NULL && ia != oia) + ifa_ref(&ia->ia_ifa); + if (oia != NULL && ia != oia) + ifa_free(&oia->ia_ifa); if ((ifp->if_flags & IFF_POINTOPOINT) && (cmd == SIOCAIFADDR) && (ifra->ifra_dstaddr.sin_addr.s_addr == INADDR_ANY)) { error = EDESTADDRREQ; - goto out_unlock; + goto out; } } if (cmd == SIOCDIFADDR && ia == NULL) { error = EADDRNOTAVAIL; - goto out_unlock; + goto out; } /* FALLTHROUGH */ case SIOCSIFADDR: _at__at_ -373,7 +383,7 _at__at_ in_control(struct socket *so, u_long cmd M_ZERO); if (ia == NULL) { error = ENOBUFS; - goto out_unlock; + goto out; } ifa = &ia->ia_ifa; _at__at_ -390,7 +400,11 _at__at_ in_control(struct socket *so, u_long cmd } ia->ia_ifp = ifp; + ifa_ref(ifa); /* if_addrhead */ + IF_ADDR_LOCK(ifp); TAILQ_INSERT_TAIL(&ifp->if_addrhead, ifa, ifa_link); + IF_ADDR_UNLOCK(ifp); + ifa_ref(ifa); /* in_ifaddrhead */ s = splnet(); TAILQ_INSERT_TAIL(&V_in_ifaddrhead, ia, ia_link); splx(s); _at__at_ -405,64 +419,53 _at__at_ in_control(struct socket *so, u_long cmd case SIOCGIFBRDADDR: if (ia == NULL) { error = EADDRNOTAVAIL; - goto out_unlock; + goto out; } break; } /* - * Most paths in this switch return directly or via out_unlock. Only - * paths that remove the address break in order to hit common removal - * code. - * - * XXXRW: We enter the switch with IF_ADDR_LOCK() held, but leave - * without it. This is a bug. + * Most paths in this switch return directly or via out. Only paths + * that remove the address break in order to hit common removal code. */ - IF_ADDR_LOCK_ASSERT(ifp); switch (cmd) { case SIOCGIFADDR: *((struct sockaddr_in *)&ifr->ifr_addr) = ia->ia_addr; - goto out_unlock; + goto out; case SIOCGIFBRDADDR: if ((ifp->if_flags & IFF_BROADCAST) == 0) { error = EINVAL; - goto out_unlock; + goto out; } *((struct sockaddr_in *)&ifr->ifr_dstaddr) = ia->ia_broadaddr; - goto out_unlock; + goto out; case SIOCGIFDSTADDR: if ((ifp->if_flags & IFF_POINTOPOINT) == 0) { error = EINVAL; - goto out_unlock; + goto out; } *((struct sockaddr_in *)&ifr->ifr_dstaddr) = ia->ia_dstaddr; - goto out_unlock; + goto out; case SIOCGIFNETMASK: *((struct sockaddr_in *)&ifr->ifr_addr) = ia->ia_sockmask; - goto out_unlock; + goto out; case SIOCSIFDSTADDR: if ((ifp->if_flags & IFF_POINTOPOINT) == 0) { error = EINVAL; - goto out_unlock; + goto out; } oldaddr = ia->ia_dstaddr; ia->ia_dstaddr = *(struct sockaddr_in *)&ifr->ifr_dstaddr; - IF_ADDR_UNLOCK(ifp); - - /* - * XXXRW: Locks dropped for if_ioctl and rtinit, but ia is - * still being used. - */ if (ifp->if_ioctl != NULL) { error = (*ifp->if_ioctl)(ifp, SIOCSIFDSTADDR, (caddr_t)ia); if (error) { ia->ia_dstaddr = oldaddr; - return (error); + goto out; } } if (ia->ia_flags & IFA_ROUTE) { _at__at_ -472,23 +475,17 _at__at_ in_control(struct socket *so, u_long cmd (struct sockaddr *)&ia->ia_dstaddr; rtinit(&(ia->ia_ifa), (int)RTM_ADD, RTF_HOST|RTF_UP); } - return (0); + goto out; case SIOCSIFBRDADDR: if ((ifp->if_flags & IFF_BROADCAST) == 0) { error = EINVAL; - goto out_unlock; + goto out; } ia->ia_broadaddr = *(struct sockaddr_in *)&ifr->ifr_broadaddr; - goto out_unlock; + goto out; case SIOCSIFADDR: - IF_ADDR_UNLOCK(ifp); - - /* - * XXXRW: Locks dropped for in_ifinit and in_joingroup, but ia - * is still being used. - */ error = in_ifinit(ifp, ia, (struct sockaddr_in *) &ifr->ifr_addr, 1); if (error != 0 && iaIsNew) _at__at_ -502,12 +499,13 _at__at_ in_control(struct socket *so, u_long cmd } EVENTHANDLER_INVOKE(ifaddr_event, ifp); } - return (0); + error = 0; + goto out; case SIOCSIFNETMASK: ia->ia_sockmask.sin_addr = ifra->ifra_addr.sin_addr; ia->ia_subnetmask = ntohl(ia->ia_sockmask.sin_addr.s_addr); - goto out_unlock; + goto out; case SIOCAIFADDR: maskIsNew = 0; _at__at_ -521,12 +519,6 _at__at_ in_control(struct socket *so, u_long cmd ia->ia_addr.sin_addr.s_addr) hostIsNew = 0; } - IF_ADDR_UNLOCK(ifp); - - /* - * XXXRW: Locks dropped for in_ifscrub and in_ifinit, but ia - * is still being used. - */ if (ifra->ifra_mask.sin_len) { in_ifscrub(ifp, ia); ia->ia_sockmask = ifra->ifra_mask; _at__at_ -545,7 +537,7 _at__at_ in_control(struct socket *so, u_long cmd (hostIsNew || maskIsNew)) error = in_ifinit(ifp, ia, &ifra->ifra_addr, 0); if (error != 0 && iaIsNew) - break; + goto out; if ((ifp->if_flags & IFF_BROADCAST) && (ifra->ifra_broadaddr.sin_family == AF_INET)) _at__at_ -559,15 +551,10 _at__at_ in_control(struct socket *so, u_long cmd } EVENTHANDLER_INVOKE(ifaddr_event, ifp); } - return (error); + goto out; case SIOCDIFADDR: - IF_ADDR_UNLOCK(ifp); - /* - * XXXRW: Locks dropped for in_ifscrub and in_ifadown, but ia - * is still being used. - * * in_ifscrub kills the interface route. */ in_ifscrub(ifp, ia); _at__at_ -587,25 +574,25 _at__at_ in_control(struct socket *so, u_long cmd panic("in_control: unsupported ioctl"); } - /* - * XXXRW: In a more ideal world, we would still be holding - * IF_ADDR_LOCK here. - */ IF_ADDR_LOCK(ifp); TAILQ_REMOVE(&ifp->if_addrhead, &ia->ia_ifa, ifa_link); IF_ADDR_UNLOCK(ifp); + ifa_free(&ia->ia_ifa); /* if_addrhead */ s = splnet(); TAILQ_REMOVE(&V_in_ifaddrhead, ia, ia_link); + ifa_free(&ia->ia_ifa); /* in_ifaddrhead */ if (ia->ia_addr.sin_family == AF_INET) { + struct in_ifaddr *if_ia; + LIST_REMOVE(ia, ia_hash); /* * If this is the last IPv4 address configured on this * interface, leave the all-hosts group. * No state-change report need be transmitted. */ - oia = NULL; - IFP_TO_IA(ifp, oia); - if (oia == NULL) { + if_ia = NULL; + IFP_TO_IA(ifp, if_ia); + if (if_ia == NULL) { ii = ((struct in_ifinfo *)ifp->if_afdata[AF_INET]); IN_MULTI_LOCK(); if (ii->ii_allhosts) { _at__at_ -614,15 +601,13 _at__at_ in_control(struct socket *so, u_long cmd ii->ii_allhosts = NULL; } IN_MULTI_UNLOCK(); - } + } else + ifa_free(&if_ia->ia_ifa); } - ifa_free(&ia->ia_ifa); splx(s); - - return (error); - -out_unlock: - IF_ADDR_UNLOCK(ifp); +out: + if (ia != NULL) + ifa_free(&ia->ia_ifa); return (error); } Modified: head/sys/netinet/in_mcast.c ============================================================================== --- head/sys/netinet/in_mcast.c Tue Jun 23 20:19:02 2009 (r194759) +++ head/sys/netinet/in_mcast.c Tue Jun 23 20:19:09 2009 (r194760) _at__at_ -1722,6 +1722,7 _at__at_ inp_getmoptions(struct inpcb *inp, struc if (ia != NULL) { mreqn.imr_address = IA_SIN(ia)->sin_addr; + ifa_free(&ia->ia_ifa); } } } Modified: head/sys/netinet/in_pcb.c ============================================================================== --- head/sys/netinet/in_pcb.c Tue Jun 23 20:19:02 2009 (r194759) +++ head/sys/netinet/in_pcb.c Tue Jun 23 20:19:09 2009 (r194760) _at__at_ -549,7 +549,6 _at__at_ static int in_pcbladdr(struct inpcb *inp, struct in_addr *faddr, struct in_addr *laddr, struct ucred *cred) { - struct in_ifaddr *ia; struct ifaddr *ifa; struct sockaddr *sa; struct sockaddr_in *sin; _at__at_ -559,7 +558,6 _at__at_ in_pcbladdr(struct inpcb *inp, struct in KASSERT(laddr != NULL, ("%s: laddr NULL", __func__)); error = 0; - ia = NULL; bzero(&sro, sizeof(sro)); sin = (struct sockaddr_in *)&sro.ro_dst; _at__at_ -585,6 +583,7 _at__at_ in_pcbladdr(struct inpcb *inp, struct in * the source address from. */ if (sro.ro_rt == NULL || sro.ro_rt->rt_ifp == NULL) { + struct in_ifaddr *ia; struct ifnet *ifp; ia = ifatoia(ifa_ifwithdstaddr((struct sockaddr *)sin)); _at__at_ -597,10 +596,12 _at__at_ in_pcbladdr(struct inpcb *inp, struct in if (cred == NULL || !prison_flag(cred, PR_IP4)) { laddr->s_addr = ia->ia_addr.sin_addr.s_addr; + ifa_free(&ia->ia_ifa); goto done; } ifp = ia->ia_ifp; + ifa_free(&ia->ia_ifa); ia = NULL; IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { _at__at_ -636,6 +637,7 _at__at_ in_pcbladdr(struct inpcb *inp, struct in * 3. as a last resort return the 'default' jail address. */ if ((sro.ro_rt->rt_ifp->if_flags & IFF_LOOPBACK) == 0) { + struct in_ifaddr *ia; struct ifnet *ifp; /* If not jailed, use the default returned. */ _at__at_ -658,10 +660,10 _at__at_ in_pcbladdr(struct inpcb *inp, struct in * 2. Check if we have any address on the outgoing interface * belonging to this jail. */ + ia = NULL; ifp = sro.ro_rt->rt_ifp; IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { - sa = ifa->ifa_addr; if (sa->sa_family != AF_INET) continue; _at__at_ -694,6 +696,7 _at__at_ in_pcbladdr(struct inpcb *inp, struct in */ if ((sro.ro_rt->rt_ifp->if_flags & IFF_LOOPBACK) != 0) { struct sockaddr_in sain; + struct in_ifaddr *ia; bzero(&sain, sizeof(struct sockaddr_in)); sain.sin_family = AF_INET; _at__at_ -710,6 +713,7 _at__at_ in_pcbladdr(struct inpcb *inp, struct in goto done; } laddr->s_addr = ia->ia_addr.sin_addr.s_addr; + ifa_free(&ia->ia_ifa); goto done; } _at__at_ -718,6 +722,7 _at__at_ in_pcbladdr(struct inpcb *inp, struct in struct ifnet *ifp; ifp = ia->ia_ifp; + ifa_free(&ia->ia_ifa); ia = NULL; IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { Modified: head/sys/netinet/in_var.h ============================================================================== --- head/sys/netinet/in_var.h Tue Jun 23 20:19:02 2009 (r194759) +++ head/sys/netinet/in_var.h Tue Jun 23 20:19:09 2009 (r194760) _at__at_ -146,14 +146,16 _at__at_ do { \ * Macro for finding the internet address structure (in_ifaddr) corresponding * to a given interface (ifnet structure). */ -#define IFP_TO_IA(ifp, ia) \ - /* struct ifnet *ifp; */ \ - /* struct in_ifaddr *ia; */ \ -{ \ - for ((ia) = TAILQ_FIRST(&V_in_ifaddrhead); \ - (ia) != NULL && (ia)->ia_ifp != (ifp); \ - (ia) = TAILQ_NEXT((ia), ia_link)) \ - continue; \ +#define IFP_TO_IA(ifp, ia) \ + /* struct ifnet *ifp; */ \ + /* struct in_ifaddr *ia; */ \ +{ \ + for ((ia) = TAILQ_FIRST(&V_in_ifaddrhead); \ + (ia) != NULL && (ia)->ia_ifp != (ifp); \ + (ia) = TAILQ_NEXT((ia), ia_link)) \ + continue; \ + if ((ia) != NULL) \ + ifa_ref(&(ia)->ia_ifa); \ } #endif Modified: head/sys/netinet/ip_carp.c ============================================================================== --- head/sys/netinet/ip_carp.c Tue Jun 23 20:19:02 2009 (r194759) +++ head/sys/netinet/ip_carp.c Tue Jun 23 20:19:09 2009 (r194760) _at__at_ -1239,6 +1239,7 _at__at_ carp_iamatch6(void *v, struct in6_addr * (SC2IFP(vh)->if_flags & IFF_UP) && (SC2IFP(vh)->if_drv_flags & IFF_DRV_RUNNING) && vh->sc_state == MASTER) { + ifa_ref(ifa); IF_ADDR_UNLOCK(SC2IFP(vh)); CARP_UNLOCK(cif); return (ifa); Modified: head/sys/netinet/ip_divert.c ============================================================================== --- head/sys/netinet/ip_divert.c Tue Jun 23 20:19:02 2009 (r194759) +++ head/sys/netinet/ip_divert.c Tue Jun 23 20:19:09 2009 (r194760) _at__at_ -464,6 +464,7 _at__at_ div_output(struct socket *so, struct mbu goto cantsend; } m->m_pkthdr.rcvif = ifa->ifa_ifp; + ifa_free(ifa); } #ifdef MAC mac_socket_create_mbuf(so, m); Modified: head/sys/netinet/ip_icmp.c ============================================================================== --- head/sys/netinet/ip_icmp.c Tue Jun 23 20:19:02 2009 (r194759) +++ head/sys/netinet/ip_icmp.c Tue Jun 23 20:19:09 2009 (r194760) _at__at_ -536,10 +536,12 _at__at_ icmp_input(struct mbuf *m, int off) } ia = (struct in_ifaddr *)ifaof_ifpforaddr( (struct sockaddr *)&icmpdst, m->m_pkthdr.rcvif); - if (ia == 0) + if (ia == NULL) break; - if (ia->ia_ifp == 0) + if (ia->ia_ifp == NULL) { + ifa_free(&ia->ia_ifa); break; + } icp->icmp_type = ICMP_MASKREPLY; if (V_icmpmaskfake == 0) icp->icmp_mask = ia->ia_sockmask.sin_addr.s_addr; _at__at_ -551,6 +553,7 _at__at_ icmp_input(struct mbuf *m, int off) else if (ia->ia_ifp->if_flags & IFF_POINTOPOINT) ip->ip_src = satosin(&ia->ia_dstaddr)->sin_addr; } + ifa_free(&ia->ia_ifa); reflect: ip->ip_len += hlen; /* since ip_input deducts this */ ICMPSTAT_INC(icps_reflect); _at__at_ -748,6 +751,7 _at__at_ icmp_reflect(struct mbuf *m) goto done; } t = IA_SIN(ia)->sin_addr; + ifa_free(&ia->ia_ifa); match: #ifdef MAC mac_netinet_icmp_replyinplace(m); Modified: head/sys/netinet/ip_input.c ============================================================================== --- head/sys/netinet/ip_input.c Tue Jun 23 20:19:02 2009 (r194759) +++ head/sys/netinet/ip_input.c Tue Jun 23 20:19:09 2009 (r194760) _at__at_ -622,8 +622,10 _at__at_ passin: * enabled. */ if (IA_SIN(ia)->sin_addr.s_addr == ip->ip_dst.s_addr && - (!checkif || ia->ia_ifp == ifp)) + (!checkif || ia->ia_ifp == ifp)) { + ifa_ref(&ia->ia_ifa); goto ours; + } } /* * Check for broadcast addresses. _at__at_ -641,15 +643,18 _at__at_ passin: ia = ifatoia(ifa); if (satosin(&ia->ia_broadaddr)->sin_addr.s_addr == ip->ip_dst.s_addr) { + ifa_ref(ifa); IF_ADDR_UNLOCK(ifp); goto ours; } if (ia->ia_netbroadcast.s_addr == ip->ip_dst.s_addr) { + ifa_ref(ifa); IF_ADDR_UNLOCK(ifp); goto ours; } #ifdef BOOTP_COMPAT if (IA_SIN(ia)->sin_addr.s_addr == INADDR_ANY) { + ifa_ref(ifa); IF_ADDR_UNLOCK(ifp); goto ours; } _at__at_ -742,6 +747,7 _at__at_ ours: if (ia != NULL) { ia->ia_ifa.if_ipackets++; ia->ia_ifa.if_ibytes += m->m_pkthdr.len; + ifa_free(&ia->ia_ifa); } /* _at__at_ -1335,8 +1341,8 _at__at_ ipproto_unregister(u_char ipproto) } /* - * Given address of next destination (final or next hop), - * return internet address info of interface to be used to get there. + * Given address of next destination (final or next hop), return (referenced) + * internet address info of interface to be used to get there. */ struct in_ifaddr * ip_rtaddr(struct in_addr dst, u_int fibnum) _at__at_ -1356,6 +1362,7 _at__at_ ip_rtaddr(struct in_addr dst, u_int fibn return (NULL); ifa = ifatoia(sro.ro_rt->rt_ifa); + ifa_ref(&ifa->ia_ifa); RTFREE(sro.ro_rt); return (ifa); } _at__at_ -1530,11 +1537,16 _at__at_ ip_forward(struct mbuf *m, int srcrt) else { if (mcopy) m_freem(mcopy); + if (ia != NULL) + ifa_free(&ia->ia_ifa); return; } } - if (mcopy == NULL) + if (mcopy == NULL) { + if (ia != NULL) + ifa_free(&ia->ia_ifa); return; + } switch (error) { _at__at_ -1592,6 +1604,8 _at__at_ ip_forward(struct mbuf *m, int srcrt) */ if (V_ip_sendsourcequench == 0) { m_freem(mcopy); + if (ia != NULL) + ifa_free(&ia->ia_ifa); return; } else { type = ICMP_SOURCEQUENCH; _at__at_ -1601,8 +1615,12 _at__at_ ip_forward(struct mbuf *m, int srcrt) case EACCES: /* ipfw denied packet */ m_freem(mcopy); + if (ia != NULL) + ifa_free(&ia->ia_ifa); return; } + if (ia != NULL) + ifa_free(&ia->ia_ifa); icmp_error(mcopy, type, code, dest.s_addr, mtu); } Modified: head/sys/netinet/ip_mroute.c ============================================================================== --- head/sys/netinet/ip_mroute.c Tue Jun 23 20:19:02 2009 (r194759) +++ head/sys/netinet/ip_mroute.c Tue Jun 23 20:19:09 2009 (r194760) _at__at_ -883,6 +883,7 _at__at_ add_vif(struct vifctl *vifcp) return EADDRNOTAVAIL; } ifp = ifa->ifa_ifp; + ifa_free(ifa); } if ((vifcp->vifc_flags & VIFF_TUNNEL) != 0) { Modified: head/sys/netinet/ip_options.c ============================================================================== --- head/sys/netinet/ip_options.c Tue Jun 23 20:19:02 2009 (r194759) *** DIFF OUTPUT TRUNCATED AT 1000 LINES ***Received on Tue Jun 23 2009 - 18:24:29 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:39:50 UTC