From 19fc74fb60b2ccd4f1c64f561cde5d0c9835e457 Mon Sep 17 00:00:00 2001 From: Jeffrey Hsu Date: Wed, 18 Dec 2002 11:46:59 +0000 Subject: [PATCH] Lock up ifaddr reference counts. --- sys/net/if.c | 9 ++++++--- sys/net/if_var.h | 32 ++++++++++++++++++++++++-------- sys/net/net_osdep.h | 3 --- sys/net/route.c | 29 ++++++++--------------------- sys/net/rtsock.c | 12 +++++++----- sys/netatalk/at_control.c | 13 ++----------- sys/netinet/in.c | 8 +++++--- sys/netinet6/in6.c | 8 ++++---- 8 files changed, 56 insertions(+), 58 deletions(-) diff --git a/sys/net/if.c b/sys/net/if.c index 699278bd5f89..10c6c10ee413 100644 --- a/sys/net/if.c +++ b/sys/net/if.c @@ -425,6 +425,7 @@ if_attach(ifp) ifasize = sizeof(*ifa) + 2 * socksize; ifa = (struct ifaddr *)malloc(ifasize, M_IFADDR, M_WAITOK | M_ZERO); if (ifa) { + IFA_LOCK_INIT(ifa); sdl = (struct sockaddr_dl *)(ifa + 1); sdl->sdl_len = socksize; sdl->sdl_family = AF_LINK; @@ -441,6 +442,7 @@ if_attach(ifp) sdl->sdl_len = masklen; while (namelen != 0) sdl->sdl_data[--namelen] = 0xff; + ifa->ifa_refcnt = 1; TAILQ_INSERT_HEAD(&ifp->if_addrhead, ifa, ifa_link); } ifp->if_broadcastaddr = 0; /* reliably crash if used uninitialized */ @@ -830,6 +832,9 @@ if_clone_list(ifcr) return (error); } +#define equal(a1, a2) \ + (bcmp((caddr_t)(a1), (caddr_t)(a2), ((struct sockaddr *)(a1))->sa_len) == 0) + /* * Locate an interface based on a complete address. */ @@ -841,8 +846,6 @@ ifa_ifwithaddr(addr) struct ifnet *ifp; struct ifaddr *ifa; -#define equal(a1, a2) \ - (bcmp((caddr_t)(a1), (caddr_t)(a2), ((struct sockaddr *)(a1))->sa_len) == 0) TAILQ_FOREACH(ifp, &ifnet, if_link) TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { if (ifa->ifa_addr->sa_family != addr->sa_family) @@ -1051,8 +1054,8 @@ link_rtrequest(cmd, rt, info) ifa = ifaof_ifpforaddr(dst, ifp); if (ifa) { IFAFREE(rt->rt_ifa); + IFAREF(ifa); /* XXX */ rt->rt_ifa = ifa; - ifa->ifa_refcnt++; if (ifa->ifa_rtrequest && ifa->ifa_rtrequest != link_rtrequest) ifa->ifa_rtrequest(cmd, rt, info); } diff --git a/sys/net/if_var.h b/sys/net/if_var.h index 4f943bbe1d40..bf9ee0c16d91 100644 --- a/sys/net/if_var.h +++ b/sys/net/if_var.h @@ -348,13 +348,19 @@ struct ifaddr { #endif int (*ifa_claim_addr) /* check if an addr goes to this if */ (struct ifaddr *, struct sockaddr *); - + struct mtx ifa_mtx; }; #define IFA_ROUTE RTF_UP /* route installed */ /* for compatibility with other BSDs */ #define ifa_list ifa_link +#define IFA_LOCK_INIT(ifa) \ + mtx_init(&(ifa)->ifa_mtx, "ifaddr", NULL, MTX_DEF) +#define IFA_LOCK(ifa) mtx_lock(&(ifa)->ifa_mtx) +#define IFA_UNLOCK(ifa) mtx_unlock(&(ifa)->ifa_mtx) +#define IFA_DESTROY(ifa) mtx_destroy(&(ifa)->ifa_mtx) + /* * The prefix structure contains information about one prefix * of an interface. They are maintained by the different address families, @@ -385,12 +391,23 @@ struct ifmultiaddr { }; #ifdef _KERNEL -#define IFAFREE(ifa) \ - do { \ - if ((ifa)->ifa_refcnt <= 0) \ - ifafree(ifa); \ - else \ - (ifa)->ifa_refcnt--; \ +#define IFAFREE(ifa) \ + do { \ + IFA_LOCK(ifa); \ + if ((ifa)->ifa_refcnt == 0) { \ + IFA_DESTROY(ifa); \ + free(ifa, M_IFADDR); \ + } else { \ + --(ifa)->ifa_refcnt; \ + IFA_UNLOCK(ifa); \ + } \ + } while (0) + +#define IFAREF(ifa) \ + do { \ + IFA_LOCK(ifa); \ + ++(ifa)->ifa_refcnt; \ + IFA_UNLOCK(ifa); \ } while (0) struct ifindex_entry { @@ -438,7 +455,6 @@ struct ifaddr *ifa_ifwithdstaddr(struct sockaddr *); struct ifaddr *ifa_ifwithnet(struct sockaddr *); struct ifaddr *ifa_ifwithroute(int, struct sockaddr *, struct sockaddr *); struct ifaddr *ifaof_ifpforaddr(struct sockaddr *, struct ifnet *); -void ifafree(struct ifaddr *); struct ifmultiaddr *ifmaof_ifpforaddr(struct sockaddr *, struct ifnet *); int if_simloop(struct ifnet *ifp, struct mbuf *m, int af, int hlen); diff --git a/sys/net/net_osdep.h b/sys/net/net_osdep.h index 5669275476b2..62b7dc059469 100644 --- a/sys/net/net_osdep.h +++ b/sys/net/net_osdep.h @@ -304,9 +304,6 @@ extern const char *if_name(struct ifnet *); #define if_addrlist if_addrhead #define if_list if_link -/* sys/net/if.h */ -#define IFAREF(ifa) do { ++(ifa)->ifa_refcnt; } while (0) - #define WITH_CONVERT_AND_STRIP_IP_LEN #if 1 /* at this moment, all OSes do this */ diff --git a/sys/net/route.c b/sys/net/route.c index adf0b3bbc8ab..bb07949ee11e 100644 --- a/sys/net/route.c +++ b/sys/net/route.c @@ -213,7 +213,6 @@ rtfree(rt) */ register struct radix_node_head *rnh = rt_tables[rt_key(rt)->sa_family]; - register struct ifaddr *ifa; if (rt == 0 || rnh == 0) panic("rtfree"); @@ -252,11 +251,10 @@ rtfree(rt) * release references on items we hold them on.. * e.g other routes and ifaddrs. */ - if((ifa = rt->rt_ifa)) - IFAFREE(ifa); - if (rt->rt_parent) { + if (rt->rt_ifa) + IFAFREE(rt->rt_ifa); + if (rt->rt_parent) RTFREE(rt->rt_parent); - } /* * The key is separatly alloc'd so free it (see rt_setgate()). @@ -272,17 +270,7 @@ rtfree(rt) } } -void -ifafree(ifa) - register struct ifaddr *ifa; -{ - if (ifa == NULL) - panic("ifafree"); - if (ifa->ifa_refcnt == 0) - free(ifa, M_IFADDR); - else - ifa->ifa_refcnt--; -} +#define equal(a1, a2) (bcmp((caddr_t)(a1), (caddr_t)(a2), (a1)->sa_len) == 0) /* * Force a routing table entry to the specified @@ -317,7 +305,6 @@ rtredirect(dst, gateway, netmask, flags, src, rtp) * we have a routing loop, perhaps as a result of an interface * going down recently. */ -#define equal(a1, a2) (bcmp((caddr_t)(a1), (caddr_t)(a2), (a1)->sa_len) == 0) if (!(flags & RTF_DONE) && rt && (!equal(src, rt->rt_gateway) || rt->rt_ifa != ifa)) error = EINVAL; @@ -394,8 +381,8 @@ out: } /* -* Routing table ioctl interface. -*/ + * Routing table ioctl interface. + */ int rtioctl(req, data) u_long req; @@ -443,7 +430,7 @@ ifa_ifwithroute(flags, dst, gateway) struct rtentry *rt = rtalloc1(gateway, 0, 0UL); if (rt == 0) return (0); - rt->rt_refcnt--; + --rt->rt_refcnt; if ((ifa = rt->rt_ifa) == 0) return (0); } @@ -679,7 +666,7 @@ rtrequest1(req, info, ret_nrt) * This moved from below so that rnh->rnh_addaddr() can * examine the ifa and ifa->ifa_ifp if it so desires. */ - ifa->ifa_refcnt++; + IFAREF(ifa); rt->rt_ifa = ifa; rt->rt_ifp = ifa->ifa_ifp; /* XXX mtu manipulation will be done in rnh_addaddr -- itojun */ diff --git a/sys/net/rtsock.c b/sys/net/rtsock.c index 16d5c03818e1..e7fad28cfa60 100644 --- a/sys/net/rtsock.c +++ b/sys/net/rtsock.c @@ -428,12 +428,14 @@ route_output(m, so) if ((ifa = info.rti_ifa) != NULL) { register struct ifaddr *oifa = rt->rt_ifa; if (oifa != ifa) { - if (oifa && oifa->ifa_rtrequest) - oifa->ifa_rtrequest(RTM_DELETE, rt, - &info); - IFAFREE(rt->rt_ifa); + if (oifa) { + IFAFREE(oifa); + if (oifa->ifa_rtrequest) + oifa->ifa_rtrequest(RTM_DELETE, rt, + &info); + } + IFAREF(ifa); rt->rt_ifa = ifa; - ifa->ifa_refcnt++; rt->rt_ifp = info.rti_ifp; } } diff --git a/sys/netatalk/at_control.c b/sys/netatalk/at_control.c index c0e67279802c..d7e7586b648a 100644 --- a/sys/netatalk/at_control.c +++ b/sys/netatalk/at_control.c @@ -159,12 +159,6 @@ at_control(struct socket *so, u_long cmd, caddr_t data, } else { at_ifaddr = aa0; } - /* - * Don't Add a reference for the aa itself! - * I fell into this trap. IFAFREE tests for <=0 - * not <= 1 like RTFREE - */ - /* aa->aa_ifa.ifa_refcnt++; DON'T DO THIS!! */ aa = aa0; /* @@ -172,13 +166,10 @@ at_control(struct socket *so, u_long cmd, caddr_t data, * and link our new one on the end */ ifa = (struct ifaddr *)aa; + IFA_LOCK_INIT(ifa); + ifa->ifa_refcnt = 1; TAILQ_INSERT_TAIL(&ifp->if_addrhead, ifa, ifa_link); - /* - * Add a reference for the linking into the ifp_if_addrlist. - */ - ifa->ifa_refcnt++; - /* * As the at_ifaddr contains the actual sockaddrs, * and the ifaddr itself, link them al together correctly. diff --git a/sys/netinet/in.c b/sys/netinet/in.c index 8fa63ea3bafc..89c412313527 100644 --- a/sys/netinet/in.c +++ b/sys/netinet/in.c @@ -276,14 +276,16 @@ in_control(so, cmd, data, ifp, td) * while we're modifying it. */ s = splnet(); - TAILQ_INSERT_TAIL(&in_ifaddrhead, ia, ia_link); - ifa = &ia->ia_ifa; - TAILQ_INSERT_TAIL(&ifp->if_addrhead, ifa, ifa_link); + ifa = &ia->ia_ifa; + IFA_LOCK_INIT(ifa); ifa->ifa_addr = (struct sockaddr *)&ia->ia_addr; ifa->ifa_dstaddr = (struct sockaddr *)&ia->ia_dstaddr; ifa->ifa_netmask = (struct sockaddr *)&ia->ia_sockmask; + ifa->ifa_refcnt = 1; + TAILQ_INSERT_TAIL(&ifp->if_addrhead, ifa, ifa_link); + ia->ia_sockmask.sin_len = 8; ia->ia_sockmask.sin_family = AF_INET; if (ifp->if_flags & IFF_BROADCAST) { diff --git a/sys/netinet6/in6.c b/sys/netinet6/in6.c index f0e7b40900c6..bb38bde9976c 100644 --- a/sys/netinet6/in6.c +++ b/sys/netinet6/in6.c @@ -908,6 +908,7 @@ in6_update_ifa(ifp, ifra, ia) return (ENOBUFS); bzero((caddr_t)ia, sizeof(*ia)); /* Initialize the address and masks */ + IFA_LOCK_INIT(&ia->ia_ifa); ia->ia_ifa.ifa_addr = (struct sockaddr *)&ia->ia_addr; ia->ia_addr.sin6_family = AF_INET6; ia->ia_addr.sin6_len = sizeof(ia->ia_addr); @@ -921,8 +922,7 @@ in6_update_ifa(ifp, ifra, ia) } else { ia->ia_ifa.ifa_dstaddr = NULL; } - ia->ia_ifa.ifa_netmask - = (struct sockaddr *)&ia->ia_prefixmask; + ia->ia_ifa.ifa_netmask = (struct sockaddr *)&ia->ia_prefixmask; ia->ia_ifp = ifp; if ((oia = in6_ifaddr) != NULL) { @@ -932,8 +932,8 @@ in6_update_ifa(ifp, ifra, ia) } else in6_ifaddr = ia; - TAILQ_INSERT_TAIL(&ifp->if_addrlist, &ia->ia_ifa, - ifa_list); + ia->ia_ifa.ifa_refcnt = 1; + TAILQ_INSERT_TAIL(&ifp->if_addrlist, &ia->ia_ifa, ifa_list); } /* set prefix mask */