From 4f9806359150157724d07191a512a2b7719d91d4 Mon Sep 17 00:00:00 2001
From: riastradh <riastradh@NetBSD.org>
Date: Thu, 3 Mar 2022 05:53:14 +0000
Subject: [PATCH] usbnet drivers: Stop abusing ifp->if_flags & IFF_ALLMULTI.

This legacy flag is a figment of userland's imagination.  The actual
kernel state is ec->ec_flags & ETHER_F_ALLMULTI, protected by the
ETHER_LOCK, so that multicast filter updates -- which run without
IFNET_LOCK -- need not attempt to write racily to ifp->if_flags.
---
 sys/dev/usb/if_aue.c  | 16 ++++++++--------
 sys/dev/usb/if_cue.c  | 12 ++++++------
 sys/dev/usb/if_kue.c  | 12 ++++++------
 sys/dev/usb/if_mue.c  | 13 ++++++-------
 sys/dev/usb/if_smsc.c | 12 +++++++-----
 sys/dev/usb/if_udav.c | 16 ++++++++++------
 6 files changed, 43 insertions(+), 38 deletions(-)

diff --git a/sys/dev/usb/if_aue.c b/sys/dev/usb/if_aue.c
index 6506abf49bdf..6c3c5501feb2 100644
--- a/sys/dev/usb/if_aue.c
+++ b/sys/dev/usb/if_aue.c
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_aue.c,v 1.179 2022/03/03 05:53:04 riastradh Exp $	*/
+/*	$NetBSD: if_aue.c,v 1.180 2022/03/03 05:53:14 riastradh Exp $	*/
 
 /*
  * Copyright (c) 1997, 1998, 1999, 2000
@@ -76,7 +76,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_aue.c,v 1.179 2022/03/03 05:53:04 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_aue.c,v 1.180 2022/03/03 05:53:14 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -627,21 +627,20 @@ aue_uno_mcast(struct ifnet *ifp)
 	usbnet_isowned_core(un);
 
 	if (ifp->if_flags & IFF_PROMISC) {
+		ETHER_LOCK(ec);
 allmulti:
-		ifp->if_flags |= IFF_ALLMULTI;
+		ec->ec_flags |= ETHER_F_ALLMULTI;
+		ETHER_UNLOCK(ec);
 		AUE_SETBIT(sc, AUE_CTL0, AUE_CTL0_ALLMULTI);
 		return;
 	}
 
-	AUE_CLRBIT(sc, AUE_CTL0, AUE_CTL0_ALLMULTI);
-
 	/* now program new ones */
 	ETHER_LOCK(ec);
 	ETHER_FIRST_MULTI(step, ec, enm);
 	while (enm != NULL) {
 		if (memcmp(enm->enm_addrlo,
 		    enm->enm_addrhi, ETHER_ADDR_LEN) != 0) {
-			ETHER_UNLOCK(ec);
 			goto allmulti;
 		}
 
@@ -649,13 +648,14 @@ allmulti:
 		hashtbl[h >> 3] |= 1 << (h & 0x7);
 		ETHER_NEXT_MULTI(step, enm);
 	}
+	ec->ec_flags &= ~ETHER_F_ALLMULTI;
 	ETHER_UNLOCK(ec);
 
+	AUE_CLRBIT(sc, AUE_CTL0, AUE_CTL0_ALLMULTI);
+
 	/* write the hashtable */
 	for (i = 0; i < 8; i++)
 		aue_csr_write_1(sc, AUE_MAR0 + i, hashtbl[i]);
-
-	ifp->if_flags &= ~IFF_ALLMULTI;
 }
 
 static void
diff --git a/sys/dev/usb/if_cue.c b/sys/dev/usb/if_cue.c
index 1e6c03292b60..24488eb03206 100644
--- a/sys/dev/usb/if_cue.c
+++ b/sys/dev/usb/if_cue.c
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_cue.c,v 1.99 2022/03/03 05:53:04 riastradh Exp $	*/
+/*	$NetBSD: if_cue.c,v 1.100 2022/03/03 05:53:14 riastradh Exp $	*/
 
 /*
  * Copyright (c) 1997, 1998, 1999, 2000
@@ -57,7 +57,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_cue.c,v 1.99 2022/03/03 05:53:04 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_cue.c,v 1.100 2022/03/03 05:53:14 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -370,8 +370,10 @@ cue_uno_mcast(struct ifnet *ifp)
 	    device_xname(un->un_dev), ifp->if_flags));
 
 	if (ifp->if_flags & IFF_PROMISC) {
+		ETHER_LOCK(ec);
 allmulti:
-		ifp->if_flags |= IFF_ALLMULTI;
+		ec->ec_flags |= ETHER_F_ALLMULTI;
+		ETHER_UNLOCK(ec);
 		for (i = 0; i < CUE_MCAST_TABLE_LEN; i++)
 			sc->cue_mctab[i] = 0xFF;
 		cue_mem(un, CUE_CMD_WRITESRAM, CUE_MCAST_TABLE_ADDR,
@@ -389,7 +391,6 @@ allmulti:
 	while (enm != NULL) {
 		if (memcmp(enm->enm_addrlo,
 		    enm->enm_addrhi, ETHER_ADDR_LEN) != 0) {
-			ETHER_UNLOCK(ec);
 			goto allmulti;
 		}
 
@@ -397,10 +398,9 @@ allmulti:
 		sc->cue_mctab[h >> 3] |= 1 << (h & 0x7);
 		ETHER_NEXT_MULTI(step, enm);
 	}
+	ec->ec_flags &= ~ETHER_F_ALLMULTI;
 	ETHER_UNLOCK(ec);
 
-	ifp->if_flags &= ~IFF_ALLMULTI;
-
 	/*
 	 * Also include the broadcast address in the filter
 	 * so we can receive broadcast frames.
diff --git a/sys/dev/usb/if_kue.c b/sys/dev/usb/if_kue.c
index 286c642015de..b7b745a33ccd 100644
--- a/sys/dev/usb/if_kue.c
+++ b/sys/dev/usb/if_kue.c
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_kue.c,v 1.112 2022/03/03 05:53:04 riastradh Exp $	*/
+/*	$NetBSD: if_kue.c,v 1.113 2022/03/03 05:53:14 riastradh Exp $	*/
 
 /*
  * Copyright (c) 1997, 1998, 1999, 2000
@@ -71,7 +71,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_kue.c,v 1.112 2022/03/03 05:53:04 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_kue.c,v 1.113 2022/03/03 05:53:14 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -336,8 +336,10 @@ kue_uno_mcast(struct ifnet *ifp)
 		sc->kue_rxfilt &= ~KUE_RXFILT_PROMISC;
 
 	if (ifp->if_flags & IFF_PROMISC) {
+		ETHER_LOCK(ec);
 allmulti:
-		ifp->if_flags |= IFF_ALLMULTI;
+		ec->ec_flags |= ETHER_F_ALLMULTI;
+		ETHER_UNLOCK(ec);
 		sc->kue_rxfilt |= KUE_RXFILT_ALLMULTI|KUE_RXFILT_PROMISC;
 		sc->kue_rxfilt &= ~KUE_RXFILT_MULTICAST;
 		kue_setword(un, KUE_CMD_SET_PKT_FILTER, sc->kue_rxfilt);
@@ -353,7 +355,6 @@ allmulti:
 		if (i == KUE_MCFILTCNT(sc) ||
 		    memcmp(enm->enm_addrlo, enm->enm_addrhi,
 		    ETHER_ADDR_LEN) != 0) {
-			ETHER_UNLOCK(ec);
 			goto allmulti;
 		}
 
@@ -361,10 +362,9 @@ allmulti:
 		ETHER_NEXT_MULTI(step, enm);
 		i++;
 	}
+	ec->ec_flags &= ~ETHER_F_ALLMULTI;
 	ETHER_UNLOCK(ec);
 
-	ifp->if_flags &= ~IFF_ALLMULTI;
-
 	sc->kue_rxfilt |= KUE_RXFILT_MULTICAST;
 	kue_ctl(un, KUE_CTL_WRITE, KUE_CMD_SET_MCAST_FILTERS,
 	    i, sc->kue_mcfilters, i * ETHER_ADDR_LEN);
diff --git a/sys/dev/usb/if_mue.c b/sys/dev/usb/if_mue.c
index fac8d60d336a..54d16b065017 100644
--- a/sys/dev/usb/if_mue.c
+++ b/sys/dev/usb/if_mue.c
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_mue.c,v 1.72 2022/03/03 05:53:04 riastradh Exp $	*/
+/*	$NetBSD: if_mue.c,v 1.73 2022/03/03 05:53:14 riastradh Exp $	*/
 /*	$OpenBSD: if_mue.c,v 1.3 2018/08/04 16:42:46 jsg Exp $	*/
 
 /*
@@ -20,7 +20,7 @@
 /* Driver for Microchip LAN7500/LAN7800 chipsets. */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_mue.c,v 1.72 2022/03/03 05:53:04 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_mue.c,v 1.73 2022/03/03 05:53:14 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -1023,10 +1023,11 @@ mue_uno_mcast(struct ifnet *ifp)
 	/* Always accept broadcast frames. */
 	rxfilt |= MUE_RFE_CTL_BROADCAST;
 
+	ETHER_LOCK(ec);
 	if (ifp->if_flags & IFF_PROMISC) {
 		rxfilt |= MUE_RFE_CTL_UNICAST;
 allmulti:	rxfilt |= MUE_RFE_CTL_MULTICAST;
-		ifp->if_flags |= IFF_ALLMULTI;
+		ec->ec_flags |= ETHER_F_ALLMULTI;
 		if (ifp->if_flags & IFF_PROMISC)
 			DPRINTF(un, "promisc\n");
 		else
@@ -1036,7 +1037,6 @@ allmulti:	rxfilt |= MUE_RFE_CTL_MULTICAST;
 		pfiltbl[0][0] = MUE_ENADDR_HI(enaddr) | MUE_ADDR_FILTX_VALID;
 		pfiltbl[0][1] = MUE_ENADDR_LO(enaddr);
 		i = 1;
-		ETHER_LOCK(ec);
 		ETHER_FIRST_MULTI(step, ec, enm);
 		while (enm != NULL) {
 			if (memcmp(enm->enm_addrlo, enm->enm_addrhi,
@@ -1044,7 +1044,6 @@ allmulti:	rxfilt |= MUE_RFE_CTL_MULTICAST;
 				memset(pfiltbl, 0, sizeof(pfiltbl));
 				memset(hashtbl, 0, sizeof(hashtbl));
 				rxfilt &= ~MUE_RFE_CTL_MULTICAST_HASH;
-				ETHER_UNLOCK(ec);
 				goto allmulti;
 			}
 			if (i < MUE_NUM_ADDR_FILTX) {
@@ -1062,14 +1061,14 @@ allmulti:	rxfilt |= MUE_RFE_CTL_MULTICAST;
 			i++;
 			ETHER_NEXT_MULTI(step, enm);
 		}
-		ETHER_UNLOCK(ec);
+		ec->ec_flags &= ~ETHER_F_ALLMULTI;
 		rxfilt |= MUE_RFE_CTL_PERFECT;
-		ifp->if_flags &= ~IFF_ALLMULTI;
 		if (rxfilt & MUE_RFE_CTL_MULTICAST_HASH)
 			DPRINTF(un, "perfect filter and hash tables\n");
 		else
 			DPRINTF(un, "perfect filter\n");
 	}
+	ETHER_UNLOCK(ec);
 
 	for (i = 0; i < MUE_NUM_ADDR_FILTX; i++) {
 		hireg = (un->un_flags & LAN7500) ?
diff --git a/sys/dev/usb/if_smsc.c b/sys/dev/usb/if_smsc.c
index fb03ad1d46fc..43e03491b6f7 100644
--- a/sys/dev/usb/if_smsc.c
+++ b/sys/dev/usb/if_smsc.c
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_smsc.c,v 1.81 2022/03/03 05:53:04 riastradh Exp $	*/
+/*	$NetBSD: if_smsc.c,v 1.82 2022/03/03 05:53:14 riastradh Exp $	*/
 
 /*	$OpenBSD: if_smsc.c,v 1.4 2012/09/27 12:38:11 jsg Exp $	*/
 /*	$FreeBSD: src/sys/dev/usb/net/if_smsc.c,v 1.1 2012/08/15 04:03:55 gonzo Exp $ */
@@ -61,7 +61,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_smsc.c,v 1.81 2022/03/03 05:53:04 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_smsc.c,v 1.82 2022/03/03 05:53:14 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -427,8 +427,11 @@ smsc_uno_mcast(struct ifnet *ifp)
 	if (usbnet_isdying(un))
 		return;
 
-	if (ifp->if_flags & (IFF_ALLMULTI | IFF_PROMISC)) {
+	if (ifp->if_flags & IFF_PROMISC) {
+		ETHER_LOCK(ec);
 allmulti:
+		ec->ec_flags |= ETHER_F_ALLMULTI;
+		ETHER_UNLOCK(ec);
 		DPRINTF("receive all multicast enabled", 0, 0, 0, 0);
 		sc->sc_mac_csr |= SMSC_MAC_CSR_MCPAS;
 		sc->sc_mac_csr &= ~SMSC_MAC_CSR_HPFILT;
@@ -443,7 +446,6 @@ allmulti:
 	ETHER_FIRST_MULTI(step, ec, enm);
 	while (enm != NULL) {
 		if (memcmp(enm->enm_addrlo, enm->enm_addrhi, ETHER_ADDR_LEN)) {
-			ETHER_UNLOCK(ec);
 			goto allmulti;
 		}
 
@@ -451,6 +453,7 @@ allmulti:
 		hashtbl[hash >> 5] |= 1 << (hash & 0x1F);
 		ETHER_NEXT_MULTI(step, enm);
 	}
+	ec->ec_flags &= ~ETHER_F_ALLMULTI;
 	ETHER_UNLOCK(ec);
 
 	/* Debug */
@@ -463,7 +466,6 @@ allmulti:
 	/* Write the hash table and mac control registers */
 
 	//XXX should we be doing this?
-	ifp->if_flags &= ~IFF_ALLMULTI;
 	smsc_writereg(un, SMSC_HASHH, hashtbl[1]);
 	smsc_writereg(un, SMSC_HASHL, hashtbl[0]);
 	smsc_writereg(un, SMSC_MAC_CSR, sc->sc_mac_csr);
diff --git a/sys/dev/usb/if_udav.c b/sys/dev/usb/if_udav.c
index 3b7c0ac5176c..aed305f83f4c 100644
--- a/sys/dev/usb/if_udav.c
+++ b/sys/dev/usb/if_udav.c
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_udav.c,v 1.86 2022/03/03 05:53:04 riastradh Exp $	*/
+/*	$NetBSD: if_udav.c,v 1.87 2022/03/03 05:53:14 riastradh Exp $	*/
 /*	$nabe: if_udav.c,v 1.3 2003/08/21 16:57:19 nabe Exp $	*/
 
 /*
@@ -45,7 +45,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_udav.c,v 1.86 2022/03/03 05:53:04 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_udav.c,v 1.87 2022/03/03 05:53:14 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -598,11 +598,16 @@ udav_uno_mcast(struct ifnet *ifp)
 	}
 
 	if (ifp->if_flags & IFF_PROMISC) {
+		ETHER_LOCK(ec);
+		ec->ec_flags |= ETHER_F_ALLMULTI;
+		ETHER_UNLOCK(ec);
 		UDAV_SETBIT(un, UDAV_RCR, UDAV_RCR_ALL | UDAV_RCR_PRMSC);
 		return;
-	} else if (ifp->if_flags & IFF_ALLMULTI) {
+	} else if (ifp->if_flags & IFF_ALLMULTI) { /* XXX ??? Can't happen? */
+		ETHER_LOCK(ec);
 allmulti:
-		ifp->if_flags |= IFF_ALLMULTI;
+		ec->ec_flags |= ETHER_F_ALLMULTI;
+		ETHER_UNLOCK(ec);
 		UDAV_SETBIT(un, UDAV_RCR, UDAV_RCR_ALL);
 		UDAV_CLRBIT(un, UDAV_RCR, UDAV_RCR_PRMSC);
 		return;
@@ -619,7 +624,6 @@ allmulti:
 	while (enm != NULL) {
 		if (memcmp(enm->enm_addrlo, enm->enm_addrhi,
 		    ETHER_ADDR_LEN) != 0) {
-			ETHER_UNLOCK(ec);
 			goto allmulti;
 		}
 
@@ -627,10 +631,10 @@ allmulti:
 		hashes[h>>3] |= 1 << (h & 0x7);
 		ETHER_NEXT_MULTI(step, enm);
 	}
+	ec->ec_flags &= ~ETHER_F_ALLMULTI;
 	ETHER_UNLOCK(ec);
 
 	/* disable all multicast */
-	ifp->if_flags &= ~IFF_ALLMULTI;
 	UDAV_CLRBIT(un, UDAV_RCR, UDAV_RCR_ALL);
 
 	/* write hash value to the register */