Module Name:    src
Committed By:   ozaki-r
Date:           Wed Nov 22 03:03:18 UTC 2017

Modified Files:
        src/sys/kern: sys_socket.c
        src/sys/net: if.c if.h if_media.c if_vlan.c

Log Message:
Hold KERNEL_LOCK on if_ioctl selectively based on IFEF_MPSAFE

If IFEF_MPSAFE is set, hold the lock and otherwise don't hold.

This change requires additions of KERNEL_LOCK to subsequence functions from
if_ioctl such as ifmedia_ioctl and ifioctl_common to protect non-MP-safe
components.

Proposed on tech-kern@ and tech-net@


To generate a diff of this commit:
cvs rdiff -u -r1.74 -r1.75 src/sys/kern/sys_socket.c
cvs rdiff -u -r1.398 -r1.399 src/sys/net/if.c
cvs rdiff -u -r1.243 -r1.244 src/sys/net/if.h
cvs rdiff -u -r1.34 -r1.35 src/sys/net/if_media.c
cvs rdiff -u -r1.108 -r1.109 src/sys/net/if_vlan.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/kern/sys_socket.c
diff -u src/sys/kern/sys_socket.c:1.74 src/sys/kern/sys_socket.c:1.75
--- src/sys/kern/sys_socket.c:1.74	Thu Jul  7 06:55:43 2016
+++ src/sys/kern/sys_socket.c	Wed Nov 22 03:03:18 2017
@@ -1,4 +1,4 @@
-/*	$NetBSD: sys_socket.c,v 1.74 2016/07/07 06:55:43 msaitoh Exp $	*/
+/*	$NetBSD: sys_socket.c,v 1.75 2017/11/22 03:03:18 ozaki-r Exp $	*/
 
 /*-
  * Copyright (c) 2008, 2009 The NetBSD Foundation, Inc.
@@ -61,7 +61,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: sys_socket.c,v 1.74 2016/07/07 06:55:43 msaitoh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sys_socket.c,v 1.75 2017/11/22 03:03:18 ozaki-r Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -197,14 +197,18 @@ soo_ioctl(file_t *fp, u_long cmd, void *
 		 * interface and routing ioctls should have a
 		 * different entry since a socket's unnecessary
 		 */
-		KERNEL_LOCK(1, NULL);
 		if (IOCGROUP(cmd) == 'i')
+			/*
+			 * KERNEL_LOCK will be held later if if_ioctl() of the
+			 * interface isn't MP-safe.
+			 */
 			error = ifioctl(so, cmd, data, curlwp);
 		else {
+			KERNEL_LOCK(1, NULL);
 			error = (*so->so_proto->pr_usrreqs->pr_ioctl)(so,
 			    cmd, data, NULL);
+			KERNEL_UNLOCK_ONE(NULL);
 		}
-		KERNEL_UNLOCK_ONE(NULL);
 		break;
 	}
 

Index: src/sys/net/if.c
diff -u src/sys/net/if.c:1.398 src/sys/net/if.c:1.399
--- src/sys/net/if.c:1.398	Sun Nov 19 16:59:25 2017
+++ src/sys/net/if.c	Wed Nov 22 03:03:18 2017
@@ -1,4 +1,4 @@
-/*	$NetBSD: if.c,v 1.398 2017/11/19 16:59:25 christos Exp $	*/
+/*	$NetBSD: if.c,v 1.399 2017/11/22 03:03:18 ozaki-r Exp $	*/
 
 /*-
  * Copyright (c) 1999, 2000, 2001, 2008 The NetBSD Foundation, Inc.
@@ -90,7 +90,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.398 2017/11/19 16:59:25 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.399 2017/11/22 03:03:18 ozaki-r Exp $");
 
 #if defined(_KERNEL_OPT)
 #include "opt_inet.h"
@@ -2766,6 +2766,11 @@ ifioctl_common(struct ifnet *ifp, u_long
 		return 0;
 	case SIOCSIFFLAGS:
 		ifr = data;
+		/*
+		 * If if_is_mpsafe(ifp), KERNEL_LOCK isn't held here, but if_up
+		 * and if_down aren't MP-safe yet, so we must hold the lock.
+		 */
+		KERNEL_LOCK_IF_IFP_MPSAFE(ifp);
 		if (ifp->if_flags & IFF_UP && (ifr->ifr_flags & IFF_UP) == 0) {
 			s = splsoftnet();
 			if_down(ifp);
@@ -2776,6 +2781,7 @@ ifioctl_common(struct ifnet *ifp, u_long
 			if_up(ifp);
 			splx(s);
 		}
+		KERNEL_UNLOCK_IF_IFP_MPSAFE(ifp);
 		ifp->if_flags = (ifp->if_flags & IFF_CANTCHANGE) |
 			(ifr->ifr_flags &~ IFF_CANTCHANGE);
 		break;
@@ -2847,8 +2853,10 @@ ifioctl_common(struct ifnet *ifp, u_long
 		 * If the link MTU changed, do network layer specific procedure.
 		 */
 #ifdef INET6
+		KERNEL_LOCK_UNLESS_NET_MPSAFE();
 		if (in6_present)
 			nd6_setmtu(ifp);
+		KERNEL_UNLOCK_UNLESS_NET_MPSAFE();
 #endif
 		return ENETRESET;
 	default:
@@ -2992,11 +3000,13 @@ doifioctl(struct socket *so, u_long cmd,
 				return error;
 			}
 		}
+		KERNEL_LOCK_UNLESS_NET_MPSAFE();
 		mutex_enter(&if_clone_mtx);
 		r = (cmd == SIOCIFCREATE) ?
 			if_clone_create(ifr->ifr_name) :
 			if_clone_destroy(ifr->ifr_name);
 		mutex_exit(&if_clone_mtx);
+		KERNEL_UNLOCK_UNLESS_NET_MPSAFE();
 		curlwp_bindx(bound);
 		return r;
 
@@ -3054,6 +3064,7 @@ doifioctl(struct socket *so, u_long cmd,
 
 	oif_flags = ifp->if_flags;
 
+	KERNEL_LOCK_UNLESS_IFP_MPSAFE(ifp);
 	mutex_enter(ifp->if_ioctl_lock);
 
 	error = (*ifp->if_ioctl)(ifp, cmd, data);
@@ -3062,6 +3073,7 @@ doifioctl(struct socket *so, u_long cmd,
 	else if (so->so_proto == NULL)
 		error = EOPNOTSUPP;
 	else {
+		KERNEL_LOCK_IF_IFP_MPSAFE(ifp);
 #ifdef COMPAT_OSOCK
 		if (vec_compat_ifioctl != NULL)
 			error = (*vec_compat_ifioctl)(so, ocmd, cmd, data, l);
@@ -3069,6 +3081,7 @@ doifioctl(struct socket *so, u_long cmd,
 #endif
 			error = (*so->so_proto->pr_usrreqs->pr_ioctl)(so,
 			    cmd, data, ifp);
+		KERNEL_UNLOCK_IF_IFP_MPSAFE(ifp);
 	}
 
 	if (((oif_flags ^ ifp->if_flags) & IFF_UP) != 0) {
@@ -3084,6 +3097,7 @@ doifioctl(struct socket *so, u_long cmd,
 #endif
 
 	mutex_exit(ifp->if_ioctl_lock);
+	KERNEL_UNLOCK_UNLESS_IFP_MPSAFE(ifp);
 out:
 	if_put(ifp, &psref);
 	curlwp_bindx(bound);

Index: src/sys/net/if.h
diff -u src/sys/net/if.h:1.243 src/sys/net/if.h:1.244
--- src/sys/net/if.h:1.243	Fri Nov 17 07:37:12 2017
+++ src/sys/net/if.h	Wed Nov 22 03:03:18 2017
@@ -1,4 +1,4 @@
-/*	$NetBSD: if.h,v 1.243 2017/11/17 07:37:12 ozaki-r Exp $	*/
+/*	$NetBSD: if.h,v 1.244 2017/11/22 03:03:18 ozaki-r Exp $	*/
 
 /*-
  * Copyright (c) 1999, 2000, 2001 The NetBSD Foundation, Inc.
@@ -391,9 +391,11 @@ typedef struct ifnet {
 #define	IFEF_NO_LINK_STATE_CHANGE	__BIT(1)	/* doesn't use link state interrupts */
 
 /*
- * if_XXX() handlers that IFEF_MPSAFE suppresses KERNEL_LOCK:
+ * The following if_XXX() handlers don't take KERNEL_LOCK if the interface
+ * is set IFEF_MPSAFE:
  *   - if_start
  *   - if_output
+ *   - if_ioctl
  */
 
 #ifdef _KERNEL
@@ -441,6 +443,16 @@ if_is_link_state_changeable(struct ifnet
 	return ((ifp->if_extflags & IFEF_NO_LINK_STATE_CHANGE) == 0);
 }
 
+#define KERNEL_LOCK_IF_IFP_MPSAFE(ifp)					\
+	do { if (if_is_mpsafe(ifp)) { KERNEL_LOCK(1, NULL); } } while (0)
+#define KERNEL_UNLOCK_IF_IFP_MPSAFE(ifp)				\
+	do { if (if_is_mpsafe(ifp)) { KERNEL_UNLOCK_ONE(NULL); } } while (0)
+
+#define KERNEL_LOCK_UNLESS_IFP_MPSAFE(ifp)				\
+	do { if (!if_is_mpsafe(ifp)) { KERNEL_LOCK(1, NULL); } } while (0)
+#define KERNEL_UNLOCK_UNLESS_IFP_MPSAFE(ifp)				\
+	do { if (!if_is_mpsafe(ifp)) { KERNEL_UNLOCK_ONE(NULL); } } while (0)
+
 #ifdef _KERNEL_OPT
 #include "opt_net_mpsafe.h"
 #endif

Index: src/sys/net/if_media.c
diff -u src/sys/net/if_media.c:1.34 src/sys/net/if_media.c:1.35
--- src/sys/net/if_media.c:1.34	Mon Oct 23 03:54:40 2017
+++ src/sys/net/if_media.c	Wed Nov 22 03:03:18 2017
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_media.c,v 1.34 2017/10/23 03:54:40 msaitoh Exp $	*/
+/*	$NetBSD: if_media.c,v 1.35 2017/11/22 03:03:18 ozaki-r Exp $	*/
 
 /*-
  * Copyright (c) 1998 The NetBSD Foundation, Inc.
@@ -76,7 +76,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_media.c,v 1.34 2017/10/23 03:54:40 msaitoh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_media.c,v 1.35 2017/11/22 03:03:18 ozaki-r Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -225,8 +225,8 @@ ifmedia_set(struct ifmedia *ifm, int tar
 /*
  * Device-independent media ioctl support function.
  */
-int
-ifmedia_ioctl(struct ifnet *ifp, struct ifreq *ifr, struct ifmedia *ifm,
+static int
+_ifmedia_ioctl(struct ifnet *ifp, struct ifreq *ifr, struct ifmedia *ifm,
     u_long cmd)
 {
 	struct ifmedia_entry *match;
@@ -360,6 +360,22 @@ ifmedia_ioctl(struct ifnet *ifp, struct 
 	return error;
 }
 
+int
+ifmedia_ioctl(struct ifnet *ifp, struct ifreq *ifr, struct ifmedia *ifm,
+    u_long cmd)
+{
+	int e;
+
+	/*
+	 * If if_is_mpsafe(ifp), KERNEL_LOCK isn't held here, but _ifmedia_ioctl
+	 * isn't MP-safe yet, so we must hold the lock.
+	 */
+	KERNEL_LOCK_IF_IFP_MPSAFE(ifp);
+	e = _ifmedia_ioctl(ifp, ifr, ifm, cmd);
+	KERNEL_UNLOCK_IF_IFP_MPSAFE(ifp);
+	return e;
+}
+
 /*
  * Find media entry matching a given ifm word.
  */

Index: src/sys/net/if_vlan.c
diff -u src/sys/net/if_vlan.c:1.108 src/sys/net/if_vlan.c:1.109
--- src/sys/net/if_vlan.c:1.108	Wed Nov 22 02:35:24 2017
+++ src/sys/net/if_vlan.c	Wed Nov 22 03:03:18 2017
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_vlan.c,v 1.108 2017/11/22 02:35:24 msaitoh Exp $	*/
+/*	$NetBSD: if_vlan.c,v 1.109 2017/11/22 03:03:18 ozaki-r Exp $	*/
 
 /*-
  * Copyright (c) 2000, 2001 The NetBSD Foundation, Inc.
@@ -78,7 +78,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_vlan.c,v 1.108 2017/11/22 02:35:24 msaitoh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_vlan.c,v 1.109 2017/11/22 03:03:18 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -245,6 +245,16 @@ struct if_clone vlan_cloner =
 /* Used to pad ethernet frames with < ETHER_MIN_LEN bytes */
 static char vlan_zero_pad_buff[ETHER_MIN_LEN];
 
+static inline int
+vlan_safe_ifpromisc(struct ifnet *ifp, int pswitch)
+{
+	int e;
+	KERNEL_LOCK_UNLESS_NET_MPSAFE();
+	e = ifpromisc(ifp, pswitch);
+	KERNEL_UNLOCK_UNLESS_NET_MPSAFE();
+	return e;
+}
+
 void
 vlanattach(int n)
 {
@@ -612,13 +622,15 @@ vlan_unconfig_locked(struct ifvlan *ifv,
 	kmem_free(omib, sizeof(*omib));
 
 #ifdef INET6
+	KERNEL_LOCK_UNLESS_NET_MPSAFE();
 	/* To delete v6 link local addresses */
 	if (in6_present)
 		in6_ifdetach(ifp);
+	KERNEL_UNLOCK_UNLESS_NET_MPSAFE();
 #endif
 
 	if ((ifp->if_flags & IFF_PROMISC) != 0)
-		ifpromisc(ifp, 0);
+		vlan_safe_ifpromisc(ifp, 0);
 	if_down(ifp);
 	ifp->if_flags &= ~(IFF_UP|IFF_RUNNING);
 	ifp->if_capabilities = 0;
@@ -834,13 +846,13 @@ vlan_set_promisc(struct ifnet *ifp)
 
 	if ((ifp->if_flags & IFF_PROMISC) != 0) {
 		if ((ifv->ifv_flags & IFVF_PROMISC) == 0) {
-			error = ifpromisc(mib->ifvm_p, 1);
+			error = vlan_safe_ifpromisc(mib->ifvm_p, 1);
 			if (error == 0)
 				ifv->ifv_flags |= IFVF_PROMISC;
 		}
 	} else {
 		if ((ifv->ifv_flags & IFVF_PROMISC) != 0) {
-			error = ifpromisc(mib->ifvm_p, 0);
+			error = vlan_safe_ifpromisc(mib->ifvm_p, 0);
 			if (error == 0)
 				ifv->ifv_flags &= ~IFVF_PROMISC;
 		}
@@ -917,7 +929,7 @@ vlan_ioctl(struct ifnet *ifp, u_long cmd
 
 			if (mib->ifvm_p != NULL &&
 			    (ifp->if_flags & IFF_PROMISC) != 0)
-				error = ifpromisc(mib->ifvm_p, 0);
+				error = vlan_safe_ifpromisc(mib->ifvm_p, 0);
 
 			vlan_putref_linkmib(mib, &psref);
 			curlwp_bindx(bound);
@@ -1114,7 +1126,10 @@ vlan_ether_addmulti(struct ifvlan *ifv, 
 	LIST_INSERT_HEAD(&ifv->ifv_mc_listhead, mc, mc_entries);
 
 	mib = ifv->ifv_mib;
+
+	KERNEL_LOCK_UNLESS_IFP_MPSAFE(mib->ifvm_p);
 	error = if_mcast_op(mib->ifvm_p, SIOCADDMULTI, sa);
+	KERNEL_UNLOCK_UNLESS_IFP_MPSAFE(mib->ifvm_p);
 
 	if (error != 0)
 		goto ioctl_failed;

Reply via email to