Locking fixes to IPv6 scope6.c

[gnn at freebsd.org]

Howdy,

	Here is a proposed set of diffs for locking fixes in the
	scope6.c module.  Please let me know anyone has questions or
	comments.

Thanks,
George

Index: scope6.c
===================================================================
RCS file: /Volumes/exported/FreeBSD-CVS/src/sys/netinet6/scope6.c,v
retrieving revision 1.10
diff -u -r1.10 scope6.c
--- scope6.c	22 Oct 2003 15:13:36 -0000	1.10
+++ scope6.c	16 Oct 2004 09:19:53 -0000
@@ -1,4 +1,4 @@
-/*	$FreeBSD$	*/
+/*	$FreeBSD: src/sys/netinet6/scope6.c,v 1.10 2003/10/22 15:13:36 ume Exp $	*/
 /*	$KAME: scope6.c,v 1.10 2000/07/24 13:29:31 itojun Exp $	*/
 
 /*
@@ -71,12 +71,14 @@
 scope6_ifattach(ifp)
 	struct ifnet *ifp;
 {
-	int s = splnet();
+
 	struct scope6_id *sid;
 
 	sid = (struct scope6_id *)malloc(sizeof(*sid), M_IFADDR, M_WAITOK);
 	bzero(sid, sizeof(*sid));
 
+	IFNET_WLOCK();
+
 	/*
 	 * XXX: IPV6_ADDR_SCOPE_xxx macros are not standard.
 	 * Should we rather hardcode here?
@@ -89,7 +91,7 @@
 	sid->s6id_list[IPV6_ADDR_SCOPE_ORGLOCAL] = 1;
 #endif
 
-	splx(s);
+	IFNET_WUNLOCK();
 	return sid;
 }
 
@@ -106,12 +108,24 @@
 	struct ifnet *ifp;
 	struct scope6_id *idlist;
 {
-	int i, s;
+	int i;
 	int error = 0;
-	struct scope6_id *sid = SID(ifp);
+	struct scope6_id *sid = NULL;
+
+	/* 
+	 * SID retrieves data from the afdata section of the ifnet
+	 * structure, but wwe also depend on ifp staying around for a
+	 * while so lock the list, instead of the smaller afdata lock
+	 * for the as long as we need either of them.
+	 */
+
+	IFNET_WLOCK();
+	sid = SID(ifp);
 
-	if (!sid)	/* paranoid? */
+	if (!sid) {	/* paranoid? */
+		IFNET_WUNLOCK();
 		return (EINVAL);
+	}
 
 	/*
 	 * XXX: We need more consistency checks of the relationship among
@@ -123,7 +137,9 @@
 	 * interface addresses, routing table entries, PCB entries...
 	 */
 
-	s = splnet();
+	/*
+	 * Lock the ifnet list so that our ifp does not also disappear.
+	 */
 
 	SCOPE6_LOCK();
 	for (i = 0; i < 16; i++) {
@@ -135,7 +151,8 @@
 			 */
 			if (i == IPV6_ADDR_SCOPE_INTFACELOCAL &&
 			    idlist->s6id_list[i] != ifp->if_index) {
-				splx(s);
+				IFNET_WUNLOCK();
+				SCOPE6_UNLOCK();
 				return (EINVAL);
 			}
 
@@ -147,7 +164,8 @@
 				 * IDs, but we check the consistency for
 				 * safety in later use.
 				 */
-				splx(s);
+				IFNET_WUNLOCK();
+				SCOPE6_UNLOCK();
 				return (EINVAL);
 			}
 
@@ -159,8 +177,8 @@
 			sid->s6id_list[i] = idlist->s6id_list[i];
 		}
 	}
+	IFNET_WUNLOCK();
 	SCOPE6_UNLOCK();
-	splx(s);
 
 	return (error);
 }
@@ -170,15 +188,20 @@
 	struct ifnet *ifp;
 	struct scope6_id *idlist;
 {
+	/* We only need to lock the interface's afdata for SID() to work. */
+	IF_AFDATA_LOCK(ifp);
 	struct scope6_id *sid = SID(ifp);
 
-	if (sid == NULL)	/* paranoid? */
+	if (sid == NULL) {	/* paranoid? */
+		IF_AFDATA_UNLOCK(ifp);
 		return (EINVAL);
+	}
 
 	SCOPE6_LOCK();
 	*idlist = *sid;
 	SCOPE6_UNLOCK();
 
+	IF_AFDATA_UNLOCK(ifp);
 	return (0);
 }
 
@@ -259,7 +282,15 @@
 {
 	int scope;
 	u_int32_t zoneid = 0;
-	struct scope6_id *sid = SID(ifp);
+	struct scope6_id *sid = NULL;
+
+	/* 
+	 * Need both the ifp and its afdata to stick around for 
+	 * this call. 
+	 */
+	IFNET_WLOCK();
+
+	sid = SID(ifp);
 
 #ifdef DIAGNOSTIC
 	if (sid == NULL) { /* should not happen */
@@ -277,10 +308,12 @@
 	 * interface.
 	 */
 	if (IN6_IS_ADDR_LOOPBACK(addr)) {
-		if (!(ifp->if_flags & IFF_LOOPBACK))
+		if (!(ifp->if_flags & IFF_LOOPBACK)) {
+			IFNET_WUNLOCK();
 			return (-1);
-		else {
+		} else {
 			*ret_id = 0; /* there's no ambiguity */
+			IFNET_WUNLOCK();
 			return (0);
 		}
 	}
@@ -315,6 +348,9 @@
 	SCOPE6_UNLOCK();
 
 	*ret_id = zoneid;
+	
+	IFNET_WUNLOCK();
+
 	return (0);
 }
 
@@ -328,6 +364,7 @@
 	 * We might eventually have to separate the notion of "link" from
 	 * "interface" and provide a user interface to set the default.
 	 */
+	IFNET_WLOCK();
 	SCOPE6_LOCK();
 	if (ifp) {
 		sid_default.s6id_list[IPV6_ADDR_SCOPE_INTFACELOCAL] =
@@ -338,6 +375,7 @@
 		sid_default.s6id_list[IPV6_ADDR_SCOPE_INTFACELOCAL] = 0;
 		sid_default.s6id_list[IPV6_ADDR_SCOPE_LINKLOCAL] = 0;
 	}
+	IFNET_WUNLOCK();
 	SCOPE6_UNLOCK();
 }