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();
 }
 
_______________________________________________
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to