Module Name:    src
Committed By:   ozaki-r
Date:           Wed Apr  5 03:47:51 UTC 2017

Modified Files:
        src/sys/net: if.c if.h if_ethersubr.c link_proto.c

Log Message:
Make sure to hold if_ioctl_lock when calling ifp->if_ioctl

Unfortunately callers of ifp->if_ioctl (if_addr_init, if_flags_set
and if_mcast_op) may or may not hold if_ioctl_lock, so we have to
hold the lock only if it's not held.


To generate a diff of this commit:
cvs rdiff -u -r1.389 -r1.390 src/sys/net/if.c
cvs rdiff -u -r1.236 -r1.237 src/sys/net/if.h
cvs rdiff -u -r1.240 -r1.241 src/sys/net/if_ethersubr.c
cvs rdiff -u -r1.34 -r1.35 src/sys/net/link_proto.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/net/if.c
diff -u src/sys/net/if.c:1.389 src/sys/net/if.c:1.390
--- src/sys/net/if.c:1.389	Tue Mar 28 08:47:19 2017
+++ src/sys/net/if.c	Wed Apr  5 03:47:51 2017
@@ -1,4 +1,4 @@
-/*	$NetBSD: if.c,v 1.389 2017/03/28 08:47:19 ozaki-r Exp $	*/
+/*	$NetBSD: if.c,v 1.390 2017/04/05 03:47:51 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.389 2017/03/28 08:47:19 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.390 2017/04/05 03:47:51 ozaki-r Exp $");
 
 #if defined(_KERNEL_OPT)
 #include "opt_inet.h"
@@ -2496,8 +2496,15 @@ if_slowtimo(void *arg)
 int
 ifpromisc(struct ifnet *ifp, int pswitch)
 {
-	int pcount, ret;
+	int pcount, ret = 0;
 	short nflags;
+	bool need_unlock = false;
+
+	/* XXX if_ioctl_lock may or may not be held here */
+	if (ifp->if_ioctl_lock != NULL && !mutex_owned(ifp->if_ioctl_lock)) {
+		mutex_enter(ifp->if_ioctl_lock);
+		need_unlock = true;
+	}
 
 	pcount = ifp->if_pcount;
 	if (pswitch) {
@@ -2507,11 +2514,11 @@ ifpromisc(struct ifnet *ifp, int pswitch
 		 * consult IFF_PROMISC when it is brought up.
 		 */
 		if (ifp->if_pcount++ != 0)
-			return 0;
+			goto out;
 		nflags = ifp->if_flags | IFF_PROMISC;
 	} else {
 		if (--ifp->if_pcount > 0)
-			return 0;
+			goto out;
 		nflags = ifp->if_flags & ~IFF_PROMISC;
 	}
 	ret = if_flags_set(ifp, nflags);
@@ -2519,6 +2526,10 @@ ifpromisc(struct ifnet *ifp, int pswitch
 	if (ret != 0) {
 		ifp->if_pcount = pcount;
 	}
+
+out:
+	if (need_unlock)
+		mutex_exit(ifp->if_ioctl_lock);
 	return ret;
 }
 
@@ -2679,6 +2690,8 @@ ifioctl_common(struct ifnet *ifp, u_long
 	struct ifcapreq *ifcr;
 	struct ifdatareq *ifdr;
 
+	KASSERT(if_ioctl_locked(ifp));
+
 	switch (cmd) {
 	case SIOCSIFCAP:
 		ifcr = data;
@@ -3056,6 +3069,15 @@ out:
 	return error;
 }
 
+bool
+if_ioctl_locked(struct ifnet *ifp)
+{
+
+	KASSERT(ifp->if_ioctl_lock != NULL);
+
+	return mutex_owned(ifp->if_ioctl_lock);
+}
+
 /*
  * Return interface configuration
  * of system.  List may be used
@@ -3299,14 +3321,23 @@ int
 if_addr_init(ifnet_t *ifp, struct ifaddr *ifa, const bool src)
 {
 	int rc;
+	bool need_unlock = false;
+
+	/* XXX if_ioctl_lock may or may not be held here */
+	if (ifp->if_ioctl_lock != NULL && !mutex_owned(ifp->if_ioctl_lock)) {
+		mutex_enter(ifp->if_ioctl_lock);
+		need_unlock = true;
+	}
 
 	if (ifp->if_initaddr != NULL)
 		rc = (*ifp->if_initaddr)(ifp, ifa, src);
 	else if (src ||
-		/* FIXME: may not hold if_ioctl_lock */
 	         (rc = (*ifp->if_ioctl)(ifp, SIOCSIFDSTADDR, ifa)) == ENOTTY)
 		rc = (*ifp->if_ioctl)(ifp, SIOCINITIFADDR, ifa);
 
+	if (need_unlock)
+		mutex_exit(ifp->if_ioctl_lock);
+
 	return rc;
 }
 
@@ -3347,6 +3378,13 @@ int
 if_flags_set(ifnet_t *ifp, const short flags)
 {
 	int rc;
+	bool need_unlock = false;
+
+	/* XXX if_ioctl_lock may or may not be held here */
+	if (ifp->if_ioctl_lock != NULL && !mutex_owned(ifp->if_ioctl_lock)) {
+		mutex_enter(ifp->if_ioctl_lock);
+		need_unlock = true;
+	}
 
 	if (ifp->if_setflags != NULL)
 		rc = (*ifp->if_setflags)(ifp, flags);
@@ -3364,19 +3402,24 @@ if_flags_set(ifnet_t *ifp, const short f
                  * setting/clearing only IFF_PROMISC if the interface
                  * isn't IFF_UP.  Uphold that tradition.
 		 */
-		if (chgdflags == IFF_PROMISC && (ifp->if_flags & IFF_UP) == 0)
-			return 0;
+		if (chgdflags == IFF_PROMISC && (ifp->if_flags & IFF_UP) == 0) {
+			rc = 0;
+			goto out;
+		}
 
 		memset(&ifr, 0, sizeof(ifr));
 
 		ifr.ifr_flags = flags & ~IFF_CANTCHANGE;
-		/* FIXME: may not hold if_ioctl_lock */
 		rc = (*ifp->if_ioctl)(ifp, SIOCSIFFLAGS, &ifr);
 
 		if (rc != 0 && cantflags != 0)
 			ifp->if_flags ^= cantflags;
 	}
 
+out:
+	if (need_unlock)
+		mutex_exit(ifp->if_ioctl_lock);
+
 	return rc;
 }
 
@@ -3385,6 +3428,13 @@ if_mcast_op(ifnet_t *ifp, const unsigned
 {
 	int rc;
 	struct ifreq ifr;
+	bool need_unlock = false;
+
+	/* XXX if_ioctl_lock may or may not be held here */
+	if (ifp->if_ioctl_lock != NULL && !mutex_owned(ifp->if_ioctl_lock)) {
+		mutex_enter(ifp->if_ioctl_lock);
+		need_unlock = true;
+	}
 
 	if (ifp->if_mcastop != NULL)
 		rc = (*ifp->if_mcastop)(ifp, cmd, sa);
@@ -3393,6 +3443,9 @@ if_mcast_op(ifnet_t *ifp, const unsigned
 		rc = (*ifp->if_ioctl)(ifp, cmd, &ifr);
 	}
 
+	if (need_unlock)
+		mutex_exit(ifp->if_ioctl_lock);
+
 	return rc;
 }
 

Index: src/sys/net/if.h
diff -u src/sys/net/if.h:1.236 src/sys/net/if.h:1.237
--- src/sys/net/if.h:1.236	Tue Mar 14 09:03:08 2017
+++ src/sys/net/if.h	Wed Apr  5 03:47:51 2017
@@ -1,4 +1,4 @@
-/*	$NetBSD: if.h,v 1.236 2017/03/14 09:03:08 ozaki-r Exp $	*/
+/*	$NetBSD: if.h,v 1.237 2017/04/05 03:47:51 ozaki-r Exp $	*/
 
 /*-
  * Copyright (c) 1999, 2000, 2001 The NetBSD Foundation, Inc.
@@ -968,6 +968,7 @@ int	if_do_dad(struct ifnet *);
 int	if_mcast_op(ifnet_t *, const unsigned long, const struct sockaddr *);
 int	if_flags_set(struct ifnet *, const short);
 int	if_clone_list(int, char *, int *);
+bool	if_ioctl_locked(struct ifnet *);
 
 struct	ifnet *ifunit(const char *);
 struct	ifnet *if_get(const char *, struct psref *);

Index: src/sys/net/if_ethersubr.c
diff -u src/sys/net/if_ethersubr.c:1.240 src/sys/net/if_ethersubr.c:1.241
--- src/sys/net/if_ethersubr.c:1.240	Fri Mar 24 09:22:46 2017
+++ src/sys/net/if_ethersubr.c	Wed Apr  5 03:47:51 2017
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_ethersubr.c,v 1.240 2017/03/24 09:22:46 ozaki-r Exp $	*/
+/*	$NetBSD: if_ethersubr.c,v 1.241 2017/04/05 03:47:51 ozaki-r Exp $	*/
 
 /*
  * Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project.
@@ -61,7 +61,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_ethersubr.c,v 1.240 2017/03/24 09:22:46 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_ethersubr.c,v 1.241 2017/04/05 03:47:51 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -1351,6 +1351,8 @@ ether_ioctl(struct ifnet *ifp, u_long cm
 	static const uint8_t zero[ETHER_ADDR_LEN];
 	int error;
 
+	KASSERT(if_ioctl_locked(ifp));
+
 	switch (cmd) {
 	case SIOCINITIFADDR:
 	    {

Index: src/sys/net/link_proto.c
diff -u src/sys/net/link_proto.c:1.34 src/sys/net/link_proto.c:1.35
--- src/sys/net/link_proto.c:1.34	Wed Jan 11 07:03:59 2017
+++ src/sys/net/link_proto.c	Wed Apr  5 03:47:51 2017
@@ -1,4 +1,4 @@
-/*	$NetBSD: link_proto.c,v 1.34 2017/01/11 07:03:59 ozaki-r Exp $	*/
+/*	$NetBSD: link_proto.c,v 1.35 2017/04/05 03:47:51 ozaki-r Exp $	*/
 
 /*-
  * Copyright (c) 1982, 1986, 1993
@@ -32,7 +32,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: link_proto.c,v 1.34 2017/01/11 07:03:59 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: link_proto.c,v 1.35 2017/04/05 03:47:51 ozaki-r Exp $");
 
 #include <sys/param.h>
 #include <sys/socket.h>
@@ -143,6 +143,9 @@ link_control(struct socket *so, unsigned
 	const struct sockaddr_dl *asdl, *nsdl;
 	struct psref psref;
 
+	if (ifp != NULL)
+		KASSERT(if_ioctl_locked(ifp));
+
 	switch (cmd) {
 	case SIOCALIFADDR:
 	case SIOCDLIFADDR:

Reply via email to