Date: Tue, 23 Jun 2009 21:24:28 +0100 (BST) From: Robert Watson <rwatson@FreeBSD.org> To: current@FreeBSD.org Subject: Warning: ifaddr refcount use patch (svn commit: r194760 - in head/sys: contrib/rdma net net80211 netinet netinet6 netipx (fwd)) Message-ID: <alpine.BSF.2.00.0906232120400.13224@fledge.watson.org>
next in thread | raw e-mail | index | archive | help
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@FreeBSD.org>
To: src-committers@freebsd.org, svn-src-all@freebsd.org,
svn-src-head@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)
@@ -129,13 +129,16 @@ 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)
@@ -1337,6 +1337,7 @@ 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)
@@ -48,6 +48,7 @@
#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>
@@ -261,6 +262,8 @@ ifaddr_byindex(u_short idx)
IFNET_RLOCK();
ifa = ifnet_byindex_locked(idx)->if_addr;
+ if (ifa != NULL)
+ ifa_ref(ifa);
IFNET_RUNLOCK();
return (ifa);
}
@@ -1464,7 +1467,7 @@ 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;
@@ -1477,6 +1480,8 @@ 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;
}
@@ -1485,6 +1490,8 @@ 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;
}
@@ -1501,14 +1508,14 @@ 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);
}
/*
@@ -1532,6 +1539,7 @@ 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;
}
@@ -1565,6 +1573,7 @@ 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;
}
@@ -1587,7 +1596,7 @@ 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;
@@ -1602,8 +1611,10 @@ 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) {
@@ -1624,6 +1635,7 @@ next: continue;
*/
if (ifa->ifa_dstaddr != NULL &&
sa_equal(addr, ifa->ifa_dstaddr)) {
+ ifa_ref(ifa);
IF_ADDR_UNLOCK(ifp);
goto done;
}
@@ -1634,6 +1646,7 @@ next: continue;
*/
if (ifa->ifa_claim_addr) {
if ((*ifa->ifa_claim_addr)(ifa, addr)) {
+ ifa_ref(ifa);
IF_ADDR_UNLOCK(ifp);
goto done;
}
@@ -1664,17 +1677,24 @@ 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);
}
@@ -1688,7 +1708,7 @@ 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)
@@ -1697,7 +1717,7 @@ 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) ||
@@ -1723,6 +1743,8 @@ ifaof_ifpforaddr(struct sockaddr *addr,
}
ifa = ifa_maybe;
done:
+ if (ifa != NULL)
+ ifa_ref(ifa);
IF_ADDR_UNLOCK(ifp);
return (ifa);
}
@@ -1748,7 +1770,6 @@ 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)
@@ -559,6 +559,7 @@ 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;
@@ -664,6 +665,8 @@ 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
@@ -693,6 +696,9 @@ 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)
{
@@ -749,11 +755,13 @@ 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) {
@@ -761,6 +769,8 @@ 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);
}
@@ -819,6 +829,10 @@ 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)
{
@@ -831,8 +845,10 @@ 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) {
@@ -1123,12 +1139,19 @@ 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;
@@ -1139,6 +1162,8 @@ 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);
}
@@ -1157,11 +1182,10 @@ 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)
@@ -683,6 +683,13 @@ 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)
@@ -301,6 +301,7 @@ 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)
@@ -1233,8 +1233,10 @@ 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)",
@@ -1326,16 +1328,23 @@ 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);
}
@@ -1351,6 +1360,8 @@ 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);
@@ -3534,8 +3545,10 @@ 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)
@@ -219,7 +219,6 @@ 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;
@@ -323,8 +322,10 @@ 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) {
@@ -336,6 +337,9 @@ 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;
@@ -345,23 +349,29 @@ 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:
@@ -373,7 +383,7 @@ in_control(struct socket *so, u_long cmd
M_ZERO);
if (ia == NULL) {
error = ENOBUFS;
- goto out_unlock;
+ goto out;
}
ifa = &ia->ia_ifa;
@@ -390,7 +400,11 @@ 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);
@@ -405,64 +419,53 @@ 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) {
@@ -472,23 +475,17 @@ 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)
@@ -502,12 +499,13 @@ 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;
@@ -521,12 +519,6 @@ 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;
@@ -545,7 +537,7 @@ 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))
@@ -559,15 +551,10 @@ 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);
@@ -587,25 +574,25 @@ 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) {
@@ -614,15 +601,13 @@ 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)
@@ -1722,6 +1722,7 @@ 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)
@@ -549,7 +549,6 @@ 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;
@@ -559,7 +558,6 @@ 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;
@@ -585,6 +583,7 @@ 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));
@@ -597,10 +596,12 @@ 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) {
@@ -636,6 +637,7 @@ 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. */
@@ -658,10 +660,10 @@ 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;
@@ -694,6 +696,7 @@ 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;
@@ -710,6 +713,7 @@ 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;
}
@@ -718,6 +722,7 @@ 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)
@@ -146,14 +146,16 @@ 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)
@@ -1239,6 +1239,7 @@ 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)
@@ -464,6 +464,7 @@ 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)
@@ -536,10 +536,12 @@ 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;
@@ -551,6 +553,7 @@ 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);
@@ -748,6 +751,7 @@ 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)
@@ -622,8 +622,10 @@ 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.
@@ -641,15 +643,18 @@ 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;
}
@@ -742,6 +747,7 @@ ours:
if (ia != NULL) {
ia->ia_ifa.if_ipackets++;
ia->ia_ifa.if_ibytes += m->m_pkthdr.len;
+ ifa_free(&ia->ia_ifa);
}
/*
@@ -1335,8 +1341,8 @@ 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)
@@ -1356,6 +1362,7 @@ 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);
}
@@ -1530,11 +1537,16 @@ 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) {
@@ -1592,6 +1604,8 @@ 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;
@@ -1601,8 +1615,12 @@ 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)
@@ -883,6 +883,7 @@ 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 ***
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?alpine.BSF.2.00.0906232120400.13224>