Date: Sat, 16 Oct 2004 18:22:49 +0900 From: gnn@freebsd.org To: rwatson@freebsd.org Cc: freebsd-net@freebsd.org Subject: Locking fixes to IPv6 scope6.c Message-ID: <m2is9b9fva.wl@minion.local.neville-neil.com>
next in thread | raw e-mail | index | archive | help
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();
}
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?m2is9b9fva.wl>