svn commit: r194971 - in head/sys: netinet netinet6 netipsec

Robert Watson rwatson at FreeBSD.org
Thu Jun 25 16:35:29 UTC 2009


Author: rwatson
Date: Thu Jun 25 16:35:28 2009
New Revision: 194971
URL: http://svn.freebsd.org/changeset/base/194971

Log:
  Add address list locking for in6_ifaddrhead/ia_link: as with locking
  for in_ifaddrhead, we stick with an rwlock for the time being, which
  we will revisit in the future with a possible move to rmlocks.
  
  Some pieces of code require significant further reworking to be
  safe from all classes of writer-writer races.
  
  Reviewed by:	bz
  MFC after:	6 weeks

Modified:
  head/sys/netinet/ip_carp.c
  head/sys/netinet6/in6.c
  head/sys/netinet6/in6_ifattach.c
  head/sys/netinet6/in6_src.c
  head/sys/netinet6/in6_var.h
  head/sys/netinet6/ip6_input.c
  head/sys/netinet6/nd6.c
  head/sys/netinet6/nd6_rtr.c
  head/sys/netipsec/key.c

Modified: head/sys/netinet/ip_carp.c
==============================================================================
--- head/sys/netinet/ip_carp.c	Thu Jun 25 16:34:29 2009	(r194970)
+++ head/sys/netinet/ip_carp.c	Thu Jun 25 16:35:28 2009	(r194971)
@@ -1680,6 +1680,7 @@ carp_set_addr6(struct carp_softc *sc, st
 
 	/* we have to do it by hands to check we won't match on us */
 	ia_if = NULL; own = 0;
+	IN6_IFADDR_RLOCK();
 	TAILQ_FOREACH(ia, &V_in6_ifaddrhead, ia_link) {
 		int i;
 
@@ -1702,14 +1703,20 @@ carp_set_addr6(struct carp_softc *sc, st
 		}
 	}
 
-	if (!ia_if)
+	if (!ia_if) {
+		IN6_IFADDR_RUNLOCK();
 		return (EADDRNOTAVAIL);
+	}
 	ia = ia_if;
+	ifa_ref(&ia->ia_ifa);
+	IN6_IFADDR_RUNLOCK();
 	ifp = ia->ia_ifp;
 
 	if (ifp == NULL || (ifp->if_flags & IFF_MULTICAST) == 0 ||
-	    (im6o->im6o_multicast_ifp && im6o->im6o_multicast_ifp != ifp))
+	    (im6o->im6o_multicast_ifp && im6o->im6o_multicast_ifp != ifp)) {
+		ifa_free(&ia->ia_ifa);
 		return (EADDRNOTAVAIL);
+	}
 
 	if (!sc->sc_naddrs6) {
 		struct in6_multi *in6m;
@@ -1811,12 +1818,14 @@ carp_set_addr6(struct carp_softc *sc, st
 	carp_setrun(sc, 0);
 
 	CARP_UNLOCK(cif);
+	ifa_free(&ia->ia_ifa);	/* XXXRW: should hold reference for softc. */
 
 	return (0);
 
 cleanup:
 	if (!sc->sc_naddrs6)
 		carp_multicast6_cleanup(sc);
+	ifa_free(&ia->ia_ifa);
 	return (error);
 }
 

Modified: head/sys/netinet6/in6.c
==============================================================================
--- head/sys/netinet6/in6.c	Thu Jun 25 16:34:29 2009	(r194970)
+++ head/sys/netinet6/in6.c	Thu Jun 25 16:35:28 2009	(r194971)
@@ -831,8 +831,10 @@ in6_update_ifa(struct ifnet *ifp, struct
 		TAILQ_INSERT_TAIL(&ifp->if_addrhead, &ia->ia_ifa, ifa_link);
 		IF_ADDR_UNLOCK(ifp);
 
-		ifa_ref(&ia->ia_ifa);			/* in6_if_addrhead */
+		ifa_ref(&ia->ia_ifa);			/* in6_ifaddrhead */
+		IN6_IFADDR_WLOCK();
 		TAILQ_INSERT_TAIL(&V_in6_ifaddrhead, ia, ia_link);
+		IN6_IFADDR_WUNLOCK();
 	}
 
 	/* update timestamp */
@@ -1376,7 +1378,9 @@ in6_unlink_ifa(struct in6_ifaddr *ia, st
 	IF_ADDR_UNLOCK(ifp);
 	ifa_free(&ia->ia_ifa);			/* if_addrhead */
 
+	IN6_IFADDR_WLOCK();
 	TAILQ_REMOVE(&V_in6_ifaddrhead, ia, ia_link);
+	IN6_IFADDR_WUNLOCK();
 	ifa_free(&ia->ia_ifa);			/* in6_ifaddrhead */
 
 	/*
@@ -1917,12 +1921,15 @@ in6_localaddr(struct in6_addr *in6)
 	if (IN6_IS_ADDR_LOOPBACK(in6) || IN6_IS_ADDR_LINKLOCAL(in6))
 		return 1;
 
+	IN6_IFADDR_RLOCK();
 	TAILQ_FOREACH(ia, &V_in6_ifaddrhead, ia_link) {
 		if (IN6_ARE_MASKED_ADDR_EQUAL(in6, &ia->ia_addr.sin6_addr,
 		    &ia->ia_prefixmask.sin6_addr)) {
+			IN6_IFADDR_RUNLOCK();
 			return 1;
 		}
 	}
+	IN6_IFADDR_RUNLOCK();
 
 	return (0);
 }
@@ -1933,14 +1940,18 @@ in6_is_addr_deprecated(struct sockaddr_i
 	INIT_VNET_INET6(curvnet);
 	struct in6_ifaddr *ia;
 
+	IN6_IFADDR_RLOCK();
 	TAILQ_FOREACH(ia, &V_in6_ifaddrhead, ia_link) {
 		if (IN6_ARE_ADDR_EQUAL(&ia->ia_addr.sin6_addr,
 				       &sa6->sin6_addr) &&
-		    (ia->ia6_flags & IN6_IFF_DEPRECATED) != 0)
+		    (ia->ia6_flags & IN6_IFF_DEPRECATED) != 0) {
+			IN6_IFADDR_RUNLOCK();
 			return (1); /* true */
+		}
 
 		/* XXX: do we still have to go thru the rest of the list? */
 	}
+	IN6_IFADDR_RUNLOCK();
 
 	return (0);		/* false */
 }
@@ -2074,7 +2085,9 @@ in6_ifawithifp(struct ifnet *ifp, struct
 		IF_ADDR_UNLOCK(ifp);
 		return (besta);
 	}
+	IF_ADDR_UNLOCK(ifp);
 
+	IN6_IFADDR_RLOCK();
 	TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) {
 		if (ifa->ifa_addr->sa_family != AF_INET6)
 			continue;
@@ -2092,10 +2105,10 @@ in6_ifawithifp(struct ifnet *ifp, struct
 
 		if (ifa != NULL)
 			ifa_ref(ifa);
-		IF_ADDR_UNLOCK(ifp);
+		IN6_IFADDR_RUNLOCK();
 		return (struct in6_ifaddr *)ifa;
 	}
-	IF_ADDR_UNLOCK(ifp);
+	IN6_IFADDR_RUNLOCK();
 
 	/* use the last-resort values, that are, deprecated addresses */
 	if (dep[0])

Modified: head/sys/netinet6/in6_ifattach.c
==============================================================================
--- head/sys/netinet6/in6_ifattach.c	Thu Jun 25 16:34:29 2009	(r194970)
+++ head/sys/netinet6/in6_ifattach.c	Thu Jun 25 16:35:28 2009	(r194971)
@@ -836,7 +836,9 @@ in6_ifdetach(struct ifnet *ifp)
 		IF_ADDR_UNLOCK(ifp);
 		ifa_free(ifa);				/* if_addrhead */
 
+		IN6_IFADDR_WLOCK();
 		TAILQ_REMOVE(&V_in6_ifaddrhead, ia, ia_link);
+		IN6_IFADDR_WUNLOCK();
 		ifa_free(ifa);
 	}
 

Modified: head/sys/netinet6/in6_src.c
==============================================================================
--- head/sys/netinet6/in6_src.c	Thu Jun 25 16:34:29 2009	(r194970)
+++ head/sys/netinet6/in6_src.c	Thu Jun 25 16:35:28 2009	(r194971)
@@ -289,6 +289,7 @@ in6_selectsrc(struct sockaddr_in6 *dstso
 	if (error)
 		return (error);
 
+	IN6_IFADDR_RLOCK();
 	TAILQ_FOREACH(ia, &V_in6_ifaddrhead, ia_link) {
 		int new_scope = -1, new_matchlen = -1;
 		struct in6_addrpolicy *new_policy = NULL;
@@ -466,13 +467,16 @@ in6_selectsrc(struct sockaddr_in6 *dstso
 		break;
 	}
 
-	if ((ia = ia_best) == NULL)
+	if ((ia = ia_best) == NULL) {
+		IN6_IFADDR_RUNLOCK();
 		return (EADDRNOTAVAIL);
+	}
 
 	if (ifpp)
 		*ifpp = ifp;
 
 	bcopy(&ia->ia_addr.sin6_addr, srcp, sizeof(*srcp));
+	IN6_IFADDR_RUNLOCK();
 	return (0);
 }
 

Modified: head/sys/netinet6/in6_var.h
==============================================================================
--- head/sys/netinet6/in6_var.h	Thu Jun 25 16:34:29 2009	(r194970)
+++ head/sys/netinet6/in6_var.h	Thu Jun 25 16:35:28 2009	(r194971)
@@ -493,6 +493,16 @@ extern struct icmp6stat icmp6stat;
 
 extern unsigned long in6_maxmtu;
 #endif /* VIMAGE_GLOBALS */
+
+extern struct rwlock in6_ifaddr_lock;
+#define	IN6_IFADDR_LOCK_ASSERT(	)	rw_assert(&in6_ifaddr_lock, RA_LOCKED)
+#define	IN6_IFADDR_RLOCK()		rw_rlock(&in6_ifaddr_lock)
+#define	IN6_IFADDR_RLOCK_ASSERT()	rw_assert(&in6_ifaddr_lock, RA_RLOCKED)
+#define	IN6_IFADDR_RUNLOCK()		rw_runlock(&in6_ifaddr_lock)
+#define	IN6_IFADDR_WLOCK()		rw_wlock(&in6_ifaddr_lock)
+#define	IN6_IFADDR_WLOCK_ASSERT()	rw_assert(&in6_ifaddr_lock, RA_WLOCKED)
+#define	IN6_IFADDR_WUNLOCK()		rw_wunlock(&in6_ifaddr_lock)
+
 #define in6_ifstat_inc(ifp, tag) \
 do {								\
 	if (ifp)						\

Modified: head/sys/netinet6/ip6_input.c
==============================================================================
--- head/sys/netinet6/ip6_input.c	Thu Jun 25 16:34:29 2009	(r194970)
+++ head/sys/netinet6/ip6_input.c	Thu Jun 25 16:35:28 2009	(r194971)
@@ -150,6 +150,9 @@ extern int udp6_sendspace;
 extern int udp6_recvspace;
 #endif
 
+struct rwlock in6_ifaddr_lock;
+RW_SYSINIT(in6_ifaddr_lock, &in6_ifaddr_lock, "in6_ifaddr_lock");
+
 struct pfil_head inet6_pfil_hook;
 
 static void ip6_init2(void *);

Modified: head/sys/netinet6/nd6.c
==============================================================================
--- head/sys/netinet6/nd6.c	Thu Jun 25 16:34:29 2009	(r194970)
+++ head/sys/netinet6/nd6.c	Thu Jun 25 16:35:28 2009	(r194971)
@@ -624,6 +624,8 @@ nd6_timer(void *arg)
 	 * in the past the loop was inside prefix expiry processing.
 	 * However, from a stricter speci-confrmance standpoint, we should
 	 * rather separate address lifetimes and prefix lifetimes.
+	 *
+	 * XXXRW: in6_ifaddrhead locking.
 	 */
   addrloop:
 	TAILQ_FOREACH_SAFE(ia6, &V_in6_ifaddrhead, ia_link, nia6) {
@@ -1328,6 +1330,7 @@ nd6_ioctl(u_long cmd, caddr_t data, stru
 				continue; /* XXX */
 
 			/* do we really have to remove addresses as well? */
+			/* XXXRW: in6_ifaddrhead locking. */
 			TAILQ_FOREACH_SAFE(ia, &V_in6_ifaddrhead, ia_link,
 			    ia_next) {
 				if ((ia->ia6_flags & IN6_IFF_AUTOCONF) == 0)

Modified: head/sys/netinet6/nd6_rtr.c
==============================================================================
--- head/sys/netinet6/nd6_rtr.c	Thu Jun 25 16:34:29 2009	(r194970)
+++ head/sys/netinet6/nd6_rtr.c	Thu Jun 25 16:35:28 2009	(r194971)
@@ -1500,6 +1500,8 @@ pfxlist_onlink_check()
 	 * detached.  Note, however, that a manually configured address should
 	 * always be attached.
 	 * The precise detection logic is same as the one for prefixes.
+	 *
+	 * XXXRW: in6_ifaddrhead locking.
 	 */
 	TAILQ_FOREACH(ifa, &V_in6_ifaddrhead, ia_link) {
 		if (!(ifa->ia6_flags & IN6_IFF_AUTOCONF))
@@ -1949,10 +1951,12 @@ in6_tmpifadd(const struct in6_ifaddr *ia
 	 * there may be a time lag between generation of the ID and generation
 	 * of the address.  So, we'll do one more sanity check.
 	 */
+	IN6_IFADDR_RLOCK();
 	TAILQ_FOREACH(ia, &V_in6_ifaddrhead, ia_link) {
 		if (IN6_ARE_ADDR_EQUAL(&ia->ia_addr.sin6_addr,
 		    &ifra.ifra_addr.sin6_addr)) {
 			if (trylimit-- == 0) {
+				IN6_IFADDR_RUNLOCK();
 				/*
 				 * Give up.  Something strange should have
 				 * happened.
@@ -1961,10 +1965,12 @@ in6_tmpifadd(const struct in6_ifaddr *ia
 				    "find a unique random IFID\n"));
 				return (EEXIST);
 			}
+			IN6_IFADDR_RUNLOCK();
 			forcegen = 1;
 			goto again;
 		}
 	}
+	IN6_IFADDR_RUNLOCK();
 
 	/*
 	 * The Valid Lifetime is the lower of the Valid Lifetime of the

Modified: head/sys/netipsec/key.c
==============================================================================
--- head/sys/netipsec/key.c	Thu Jun 25 16:34:29 2009	(r194970)
+++ head/sys/netipsec/key.c	Thu Jun 25 16:35:28 2009	(r194971)
@@ -3982,10 +3982,13 @@ key_ismyaddr6(sin6)
 	struct in6_multi *in6m;
 #endif
 
+	IN6_IFADDR_RLOCK();
 	TAILQ_FOREACH(ia, &V_in6_ifaddrhead, ia_link) {
 		if (key_sockaddrcmp((struct sockaddr *)&sin6,
-		    (struct sockaddr *)&ia->ia_addr, 0) == 0)
+		    (struct sockaddr *)&ia->ia_addr, 0) == 0) {
+			IN6_IFADDR_RUNLOCK();
 			return 1;
+		}
 
 #if 0
 		/*
@@ -3996,10 +3999,13 @@ key_ismyaddr6(sin6)
 		 */
 		in6m = NULL;
 		IN6_LOOKUP_MULTI(sin6->sin6_addr, ia->ia_ifp, in6m);
-		if (in6m)
+		if (in6m) {
+			IN6_IFADDR_RUNLOCK();
 			return 1;
+		}
 #endif
 	}
+	IN6_IFADDR_RUNLOCK();
 
 	/* loopback, just for safety */
 	if (IN6_IS_ADDR_LOOPBACK(&sin6->sin6_addr))


More information about the svn-src-all mailing list