From owner-freebsd-net@FreeBSD.ORG Sat Oct 16 09:23:38 2004 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id F15BD16A4CE; Sat, 16 Oct 2004 09:23:37 +0000 (GMT) Received: from outbound0.sv.meer.net (outbound0.sv.meer.net [205.217.152.13]) by mx1.FreeBSD.org (Postfix) with ESMTP id A6C1A43D4C; Sat, 16 Oct 2004 09:23:37 +0000 (GMT) (envelope-from gnn@neville-neil.com) Received: from mail.meer.net (mail.meer.net [209.157.152.14]) i9G9MsUr003289; Sat, 16 Oct 2004 02:22:54 -0700 (PDT) (envelope-from gnn@neville-neil.com) Received: from minion.local.neville-neil.com (pc1.oakwoodazabu1-unet.ocn.ne.jp [220.110.140.201]) by mail.meer.net (8.12.10/8.12.2/meer) with ESMTP id i9G9Mpap033602; Sat, 16 Oct 2004 02:22:52 -0700 (PDT) (envelope-from gnn@neville-neil.com) Date: Sat, 16 Oct 2004 18:22:49 +0900 Message-ID: From: gnn@freebsd.org To: rwatson@freebsd.org User-Agent: Wanderlust/2.10.1 (Watching The Wheels) SEMI/1.14.5 (Awara-Onsen) FLIM/1.14.5 (Demachiyanagi) APEL/10.5 Emacs/21.2 (powerpc-apple-darwin) MULE/5.0 (SAKAKI) MIME-Version: 1.0 (generated by SEMI 1.14.5 - "Awara-Onsen") Content-Type: text/plain; charset=US-ASCII cc: freebsd-net@freebsd.org Subject: Locking fixes to IPv6 scope6.c X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 16 Oct 2004 09:23:38 -0000 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(); }