On Tue, 23 Jun 2009, Robert Watson wrote: > However, because this modifies quite a few spots in the code, it's possible > we'll see two classes of bugs: Indeed, we've run into a couple of problems so far relating to multicast input with these changes; symptoms consisted of panics when receiving multicast IP input. Many users will want to skip stright to r194837 or later, and skip revisions r194760-r194736. Thanks to Sam Leffler and Lawrence Stewart for reports! Robert N M Watson Computer Laboratory University of Cambridge > > - 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 *** > _______________________________________________ > freebsd-current_at_freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to "freebsd-current-unsubscribe_at_freebsd.org" >Received on Wed Jun 24 2009 - 13:11:16 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:39:50 UTC