Module Name:    src
Committed By:   ozaki-r
Date:           Wed Dec 28 07:32:16 UTC 2016

Modified Files:
        src/sys/arch/x86/pci: if_vmx.c
        src/sys/dev/pci: if_vioif.c if_wm.c
        src/sys/net: if_ether.h if_ethersubr.c

Log Message:
Protect ec_multi* with mutex

The data can be accessed from sysctl, ioctl, interface watchdog
(if_slowtimo) and interrupt handlers. We need to protect the data against
parallel accesses from them.

Currently the mutex is applied to some drivers, we need to apply it to all
drivers in the future.

Note that the mutex is adaptive one for ease of implementation but some
drivers access the data in interrupt context so we cannot apply the mutex
to every drivers as is. We have two options: one is to replace the mutex
with a spin one, which requires some additional works (see
ether_multicast_sysctl), and the other is to modify the drivers to access
the data not in interrupt context somehow.


To generate a diff of this commit:
cvs rdiff -u -r1.14 -r1.15 src/sys/arch/x86/pci/if_vmx.c
cvs rdiff -u -r1.29 -r1.30 src/sys/dev/pci/if_vioif.c
cvs rdiff -u -r1.459 -r1.460 src/sys/dev/pci/if_wm.c
cvs rdiff -u -r1.65 -r1.66 src/sys/net/if_ether.h
cvs rdiff -u -r1.230 -r1.231 src/sys/net/if_ethersubr.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/arch/x86/pci/if_vmx.c
diff -u src/sys/arch/x86/pci/if_vmx.c:1.14 src/sys/arch/x86/pci/if_vmx.c:1.15
--- src/sys/arch/x86/pci/if_vmx.c:1.14	Tue Dec 27 03:09:55 2016
+++ src/sys/arch/x86/pci/if_vmx.c	Wed Dec 28 07:32:16 2016
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_vmx.c,v 1.14 2016/12/27 03:09:55 hikaru Exp $	*/
+/*	$NetBSD: if_vmx.c,v 1.15 2016/12/28 07:32:16 ozaki-r Exp $	*/
 /*	$OpenBSD: if_vmx.c,v 1.16 2014/01/22 06:04:17 brad Exp $	*/
 
 /*
@@ -19,7 +19,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_vmx.c,v 1.14 2016/12/27 03:09:55 hikaru Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_vmx.c,v 1.15 2016/12/28 07:32:16 ozaki-r Exp $");
 
 #include <sys/param.h>
 #include <sys/cpu.h>
@@ -2818,9 +2818,11 @@ vmxnet3_set_rxfilter(struct vmxnet3_soft
 		goto allmulti;
 
 	p = sc->vmx_mcast;
+	ETHER_LOCK(ec);
 	ETHER_FIRST_MULTI(step, ec, enm);
 	while (enm != NULL) {
 		if (memcmp(enm->enm_addrlo, enm->enm_addrhi, ETHER_ADDR_LEN)) {
+			ETHER_UNLOCK(ec);
 			/*
 			 * We must listen to a range of multicast addresses.
 			 * For now, just accept all multicasts, rather than
@@ -2837,6 +2839,7 @@ vmxnet3_set_rxfilter(struct vmxnet3_soft
 
 		ETHER_NEXT_MULTI(step, enm);
 	}
+	ETHER_UNLOCK(ec);
 
 	if (ec->ec_multicnt > 0) {
 		SET(mode, VMXNET3_RXMODE_MCAST);

Index: src/sys/dev/pci/if_vioif.c
diff -u src/sys/dev/pci/if_vioif.c:1.29 src/sys/dev/pci/if_vioif.c:1.30
--- src/sys/dev/pci/if_vioif.c:1.29	Thu Dec 15 09:28:05 2016
+++ src/sys/dev/pci/if_vioif.c	Wed Dec 28 07:32:16 2016
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_vioif.c,v 1.29 2016/12/15 09:28:05 ozaki-r Exp $	*/
+/*	$NetBSD: if_vioif.c,v 1.30 2016/12/28 07:32:16 ozaki-r Exp $	*/
 
 /*
  * Copyright (c) 2010 Minoura Makoto.
@@ -26,7 +26,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_vioif.c,v 1.29 2016/12/15 09:28:05 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_vioif.c,v 1.30 2016/12/28 07:32:16 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_net_mpsafe.h"
@@ -1446,6 +1446,7 @@ vioif_rx_filter(struct vioif_softc *sc)
 	}
 
 	nentries = -1;
+	ETHER_LOCK(&sc->sc_ethercom);
 	ETHER_FIRST_MULTI(step, &sc->sc_ethercom, enm);
 	while (nentries++, enm != NULL) {
 		if (nentries >= VIRTIO_NET_CTRL_MAC_MAXENTRIES) {
@@ -1464,6 +1465,8 @@ vioif_rx_filter(struct vioif_softc *sc)
 	rxfilter = 1;
 
 set:
+	ETHER_UNLOCK(&sc->sc_ethercom);
+
 	if (rxfilter) {
 		sc->sc_ctrl_mac_tbl_uc->nentries = 0;
 		sc->sc_ctrl_mac_tbl_mc->nentries = nentries;

Index: src/sys/dev/pci/if_wm.c
diff -u src/sys/dev/pci/if_wm.c:1.459 src/sys/dev/pci/if_wm.c:1.460
--- src/sys/dev/pci/if_wm.c:1.459	Mon Dec 26 07:55:00 2016
+++ src/sys/dev/pci/if_wm.c	Wed Dec 28 07:32:16 2016
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_wm.c,v 1.459 2016/12/26 07:55:00 msaitoh Exp $	*/
+/*	$NetBSD: if_wm.c,v 1.460 2016/12/28 07:32:16 ozaki-r Exp $	*/
 
 /*
  * Copyright (c) 2001, 2002, 2003, 2004 Wasabi Systems, Inc.
@@ -84,7 +84,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_wm.c,v 1.459 2016/12/26 07:55:00 msaitoh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_wm.c,v 1.460 2016/12/28 07:32:16 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_net_mpsafe.h"
@@ -3249,9 +3249,11 @@ wm_set_filter(struct wm_softc *sc)
 	for (i = 0; i < size; i++)
 		CSR_WRITE(sc, mta_reg + (i << 2), 0);
 
+	ETHER_LOCK(ec);
 	ETHER_FIRST_MULTI(step, ec, enm);
 	while (enm != NULL) {
 		if (memcmp(enm->enm_addrlo, enm->enm_addrhi, ETHER_ADDR_LEN)) {
+			ETHER_UNLOCK(ec);
 			/*
 			 * We must listen to a range of multicast addresses.
 			 * For now, just accept all multicasts, rather than
@@ -3293,6 +3295,7 @@ wm_set_filter(struct wm_softc *sc)
 
 		ETHER_NEXT_MULTI(step, enm);
 	}
+	ETHER_UNLOCK(ec);
 
 	ifp->if_flags &= ~IFF_ALLMULTI;
 	goto setit;

Index: src/sys/net/if_ether.h
diff -u src/sys/net/if_ether.h:1.65 src/sys/net/if_ether.h:1.66
--- src/sys/net/if_ether.h:1.65	Thu Nov 19 16:23:54 2015
+++ src/sys/net/if_ether.h	Wed Dec 28 07:32:16 2016
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_ether.h,v 1.65 2015/11/19 16:23:54 christos Exp $	*/
+/*	$NetBSD: if_ether.h,v 1.66 2016/12/28 07:32:16 ozaki-r Exp $	*/
 
 /*
  * Copyright (c) 1982, 1986, 1993
@@ -177,6 +177,7 @@ struct ethercom {
 	 * ec_if.if_init, 0 on success, not 0 on failure.
 	 */
 	ether_cb_t				ec_ifflags_cb;
+	kmutex_t				*ec_lock;
 #ifdef MBUFTRACE
 	struct	mowner ec_rx_mowner;		/* mbufs received */
 	struct	mowner ec_tx_mowner;		/* mbufs transmitted */
@@ -286,6 +287,9 @@ struct ether_multistep {
 
 #ifdef _KERNEL
 
+#define ETHER_LOCK(ec)		mutex_enter((ec)->ec_lock)
+#define ETHER_UNLOCK(ec)	mutex_exit((ec)->ec_lock)
+
 /*
  * Ethernet 802.1Q VLAN structures.
  */

Index: src/sys/net/if_ethersubr.c
diff -u src/sys/net/if_ethersubr.c:1.230 src/sys/net/if_ethersubr.c:1.231
--- src/sys/net/if_ethersubr.c:1.230	Wed Dec 28 07:26:24 2016
+++ src/sys/net/if_ethersubr.c	Wed Dec 28 07:32:16 2016
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_ethersubr.c,v 1.230 2016/12/28 07:26:24 ozaki-r Exp $	*/
+/*	$NetBSD: if_ethersubr.c,v 1.231 2016/12/28 07:32:16 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.230 2016/12/28 07:26:24 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_ethersubr.c,v 1.231 2016/12/28 07:32:16 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -89,6 +89,7 @@ __KERNEL_RCSID(0, "$NetBSD: if_ethersubr
 #include <sys/rnd.h>
 #include <sys/rndsource.h>
 #include <sys/cpu.h>
+#include <sys/kmem.h>
 
 #include <net/if.h>
 #include <net/netisr.h>
@@ -966,6 +967,7 @@ ether_ifattach(struct ifnet *ifp, const 
 		if_set_sadl(ifp, lla, ETHER_ADDR_LEN, !ETHER_IS_LOCAL(lla));
 
 	LIST_INIT(&ec->ec_multiaddrs);
+	ec->ec_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_NONE);
 	ifp->if_broadcastaddr = etherbroadcastaddr;
 	bpf_attach(ifp, DLT_EN10MB, sizeof(struct ether_header));
 #ifdef MBUFTRACE
@@ -1012,13 +1014,17 @@ ether_ifdetach(struct ifnet *ifp)
 #endif
 
 	s = splnet();
+	mutex_enter(ec->ec_lock);
 	while ((enm = LIST_FIRST(&ec->ec_multiaddrs)) != NULL) {
 		LIST_REMOVE(enm, enm_list);
-		free(enm, M_IFMADDR);
+		kmem_free(enm, sizeof(*enm));
 		ec->ec_multicnt--;
 	}
+	mutex_exit(ec->ec_lock);
 	splx(s);
 
+	mutex_destroy(ec->ec_lock);
+
 	ifp->if_mowner = NULL;
 	MOWNER_DETACH(&ec->ec_rx_mowner);
 	MOWNER_DETACH(&ec->ec_tx_mowner);
@@ -1224,56 +1230,62 @@ ether_multiaddr(const struct sockaddr *s
 int
 ether_addmulti(const struct sockaddr *sa, struct ethercom *ec)
 {
-	struct ether_multi *enm;
+	struct ether_multi *enm, *_enm;
 	u_char addrlo[ETHER_ADDR_LEN];
 	u_char addrhi[ETHER_ADDR_LEN];
-	int s = splnet(), error;
+	int s, error = 0;
+
+	/* Allocate out of lock */
+	enm = kmem_alloc(sizeof(*enm), KM_SLEEP);
+	if (enm == NULL)
+		return ENOBUFS;
 
+	s = splnet();
+	mutex_enter(ec->ec_lock);
 	error = ether_multiaddr(sa, addrlo, addrhi);
-	if (error != 0) {
-		splx(s);
-		return error;
-	}
+	if (error != 0)
+		goto out;
 
 	/*
 	 * Verify that we have valid Ethernet multicast addresses.
 	 */
 	if (!ETHER_IS_MULTICAST(addrlo) || !ETHER_IS_MULTICAST(addrhi)) {
-		splx(s);
-		return EINVAL;
+		error = EINVAL;
+		goto out;
 	}
 	/*
 	 * See if the address range is already in the list.
 	 */
-	ETHER_LOOKUP_MULTI(addrlo, addrhi, ec, enm);
-	if (enm != NULL) {
+	ETHER_LOOKUP_MULTI(addrlo, addrhi, ec, _enm);
+	if (_enm != NULL) {
 		/*
 		 * Found it; just increment the reference count.
 		 */
-		++enm->enm_refcount;
-		splx(s);
-		return 0;
+		++_enm->enm_refcount;
+		error = 0;
+		goto out;
 	}
 	/*
 	 * New address or range; malloc a new multicast record
 	 * and link it into the interface's multicast list.
 	 */
-	enm = (struct ether_multi *)malloc(sizeof(*enm), M_IFMADDR, M_NOWAIT);
-	if (enm == NULL) {
-		splx(s);
-		return ENOBUFS;
-	}
 	memcpy(enm->enm_addrlo, addrlo, 6);
 	memcpy(enm->enm_addrhi, addrhi, 6);
 	enm->enm_refcount = 1;
 	LIST_INSERT_HEAD(&ec->ec_multiaddrs, enm, enm_list);
 	ec->ec_multicnt++;
-	splx(s);
 	/*
 	 * Return ENETRESET to inform the driver that the list has changed
 	 * and its reception filter should be adjusted accordingly.
 	 */
-	return ENETRESET;
+	error = ENETRESET;
+	enm = NULL;
+out:
+	mutex_exit(ec->ec_lock);
+	splx(s);
+	if (enm != NULL)
+		kmem_free(enm, sizeof(*enm));
+	return error;
 }
 
 /*
@@ -1285,41 +1297,47 @@ ether_delmulti(const struct sockaddr *sa
 	struct ether_multi *enm;
 	u_char addrlo[ETHER_ADDR_LEN];
 	u_char addrhi[ETHER_ADDR_LEN];
-	int s = splnet(), error;
+	int s, error;
 
+	s = splnet();
+	mutex_enter(ec->ec_lock);
 	error = ether_multiaddr(sa, addrlo, addrhi);
-	if (error != 0) {
-		splx(s);
-		return (error);
-	}
+	if (error != 0)
+		goto error;
 
 	/*
 	 * Look ur the address in our list.
 	 */
 	ETHER_LOOKUP_MULTI(addrlo, addrhi, ec, enm);
 	if (enm == NULL) {
-		splx(s);
-		return (ENXIO);
+		error = ENXIO;
+		goto error;
 	}
 	if (--enm->enm_refcount != 0) {
 		/*
 		 * Still some claims to this record.
 		 */
-		splx(s);
-		return (0);
+		error = 0;
+		goto error;
 	}
 	/*
 	 * No remaining claims to this record; unlink and free it.
 	 */
 	LIST_REMOVE(enm, enm_list);
-	free(enm, M_IFMADDR);
 	ec->ec_multicnt--;
+	mutex_exit(ec->ec_lock);
 	splx(s);
+
+	kmem_free(enm, sizeof(*enm));
 	/*
 	 * Return ENETRESET to inform the driver that the list has changed
 	 * and its reception filter should be adjusted accordingly.
 	 */
-	return (ENETRESET);
+	return ENETRESET;
+error:
+	mutex_exit(ec->ec_lock);
+	splx(s);
+	return error;
 }
 
 void
@@ -1522,7 +1540,7 @@ ether_multicast_sysctl(SYSCTLFN_ARGS)
 	int error = 0;
 	size_t written;
 	struct psref psref;
-	int bound;
+	int bound, s;
 
 	if (namelen != 1)
 		return EINVAL;
@@ -1550,6 +1568,8 @@ ether_multicast_sysctl(SYSCTLFN_ARGS)
 	error = 0;
 	written = 0;
 
+	s = splnet();
+	mutex_enter(ec->ec_lock);
 	LIST_FOREACH(enm, &ec->ec_multiaddrs, enm_list) {
 		if (written + sizeof(addr) > *oldlenp)
 			break;
@@ -1562,6 +1582,8 @@ ether_multicast_sysctl(SYSCTLFN_ARGS)
 		written += sizeof(addr);
 		oldp = (char *)oldp + sizeof(addr);
 	}
+	mutex_exit(ec->ec_lock);
+	splx(s);
 	if_put(ifp, &psref);
 
 	*oldlenp = written;

Reply via email to