Module Name:    src
Committed By:   riastradh
Date:           Fri Aug 12 10:57:07 UTC 2022

Modified Files:
        src/sys/dev/pci: if_wm.c

Log Message:
wm(4): if_flags and IFNET_LOCK audit

Don't touch if_flags without IFNET_LOCK:

- If only core lock is held, use sc_if_flags.
- If only txq lock is held, use txq_stopping.
  => Verified all paths guarantee !txq_stopping, so assert.
- Make sure sc_if_flags is updated on stop.
- Make wm_init fail once we enter wm_detach.
- Sprinkle assertions.

Author: Taylor R Campbell <riastr...@netbsd.org>


To generate a diff of this commit:
cvs rdiff -u -r1.759 -r1.760 src/sys/dev/pci/if_wm.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/dev/pci/if_wm.c
diff -u src/sys/dev/pci/if_wm.c:1.759 src/sys/dev/pci/if_wm.c:1.760
--- src/sys/dev/pci/if_wm.c:1.759	Fri Aug 12 10:55:01 2022
+++ src/sys/dev/pci/if_wm.c	Fri Aug 12 10:57:06 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_wm.c,v 1.759 2022/08/12 10:55:01 riastradh Exp $	*/
+/*	$NetBSD: if_wm.c,v 1.760 2022/08/12 10:57:06 riastradh Exp $	*/
 
 /*
  * Copyright (c) 2001, 2002, 2003, 2004 Wasabi Systems, Inc.
@@ -82,7 +82,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_wm.c,v 1.759 2022/08/12 10:55:01 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_wm.c,v 1.760 2022/08/12 10:57:06 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_net_mpsafe.h"
@@ -705,6 +705,9 @@ struct wm_softc {
 
 	struct wm_phyop phy;
 	struct wm_nvmop nvm;
+
+	bool sc_dying;
+
 #ifdef WM_DEBUG
 	uint32_t sc_debug;
 #endif
@@ -3385,7 +3388,10 @@ wm_detach(device_t self, int flags __unu
 		return 0;
 
 	/* Stop the interface. Callouts are stopped in it. */
+	IFNET_LOCK(ifp);
+	sc->sc_dying = true;
 	wm_stop(ifp, 1);
+	IFNET_UNLOCK(ifp);
 
 	pmf_device_deregister(self);
 
@@ -3572,6 +3578,7 @@ wm_resume(device_t self, const pmf_qual_
 
 	if (sc->sc_type >= WM_T_PCH2)
 		wm_resume_workarounds_pchlan(sc);
+	IFNET_LOCK(ifp);
 	if ((ifp->if_flags & IFF_UP) == 0) {
 		/* >= PCH_SPT hardware workaround before reset. */
 		if (sc->sc_type >= WM_T_PCH_SPT)
@@ -3590,6 +3597,7 @@ wm_resume(device_t self, const pmf_qual_
 		 * via wm_init().
 		 */
 	}
+	IFNET_UNLOCK(ifp);
 
 	return true;
 }
@@ -4326,6 +4334,7 @@ wm_set_filter(struct wm_softc *sc)
 
 	DPRINTF(sc, WM_DEBUG_INIT, ("%s: %s called\n",
 		device_xname(sc->sc_dev), __func__));
+	KASSERT(WM_CORE_LOCKED(sc));
 
 	if (sc->sc_type >= WM_T_82544)
 		mta_reg = WMREG_CORDOVA_MTA;
@@ -4334,9 +4343,9 @@ wm_set_filter(struct wm_softc *sc)
 
 	sc->sc_rctl &= ~(RCTL_BAM | RCTL_UPE | RCTL_MPE);
 
-	if (ifp->if_flags & IFF_BROADCAST)
+	if (sc->sc_if_flags & IFF_BROADCAST)
 		sc->sc_rctl |= RCTL_BAM;
-	if (ifp->if_flags & IFF_PROMISC) {
+	if (sc->sc_if_flags & IFF_PROMISC) {
 		sc->sc_rctl |= RCTL_UPE;
 		ETHER_LOCK(ec);
 		ec->ec_flags |= ETHER_F_ALLMULTI;
@@ -5257,6 +5266,8 @@ wm_flush_desc_rings(struct wm_softc *sc)
 	int nexttx;
 	uint32_t rctl;
 
+	KASSERT(IFNET_LOCKED(&sc->sc_ethercom.ec_if));
+
 	/* First, disable MULR fix in FEXTNVM11 */
 	reg = CSR_READ(sc, WMREG_FEXTNVM11);
 	reg |= FEXTNVM11_DIS_MULRFIX;
@@ -6482,6 +6493,9 @@ wm_init(struct ifnet *ifp)
 
 	KASSERT(IFNET_LOCKED(ifp));
 
+	if (sc->sc_dying)
+		return ENXIO;
+
 	WM_CORE_LOCK(sc);
 	ret = wm_init_locked(ifp);
 	WM_CORE_UNLOCK(sc);
@@ -7081,6 +7095,7 @@ wm_stop(struct ifnet *ifp, int disable)
 	struct wm_softc *sc = ifp->if_softc;
 
 	ASSERT_SLEEPABLE();
+	KASSERT(IFNET_LOCKED(ifp));
 
 	WM_CORE_LOCK(sc);
 	wm_stop_locked(ifp, disable ? true : false, true);
@@ -7106,6 +7121,7 @@ wm_stop_locked(struct ifnet *ifp, bool d
 
 	DPRINTF(sc, WM_DEBUG_INIT, ("%s: %s called\n",
 		device_xname(sc->sc_dev), __func__));
+	KASSERT(IFNET_LOCKED(ifp));
 	KASSERT(WM_CORE_LOCKED(sc));
 
 	wm_set_stopping_flags(sc);
@@ -7185,6 +7201,7 @@ wm_stop_locked(struct ifnet *ifp, bool d
 
 	/* Mark the interface as down and cancel the watchdog timer. */
 	ifp->if_flags &= ~IFF_RUNNING;
+	sc->sc_if_flags = ifp->if_flags;
 
 	if (disable) {
 		for (i = 0; i < sc->sc_nqueues; i++) {
@@ -8384,9 +8401,8 @@ wm_send_common_locked(struct ifnet *ifp,
 	bool remap = true;
 
 	KASSERT(mutex_owned(txq->txq_lock));
+	KASSERT(!txq->txq_stopping);
 
-	if ((ifp->if_flags & IFF_RUNNING) == 0)
-		return;
 	if ((txq->txq_flags & WM_TXQ_NO_SPACE) != 0)
 		return;
 
@@ -9002,9 +9018,8 @@ wm_nq_send_common_locked(struct ifnet *i
 	bool remap = true;
 
 	KASSERT(mutex_owned(txq->txq_lock));
+	KASSERT(!txq->txq_stopping);
 
-	if ((ifp->if_flags & IFF_RUNNING) == 0)
-		return;
 	if ((txq->txq_flags & WM_TXQ_NO_SPACE) != 0)
 		return;
 
@@ -13156,7 +13171,7 @@ wm_tbi_tick(struct wm_softc *sc)
 		sc->sc_tbi_serdes_ticks = 0;
 	}
 
-	if ((sc->sc_ethercom.ec_if.if_flags & IFF_UP) == 0)
+	if ((sc->sc_if_flags & IFF_UP) == 0)
 		goto setled;
 
 	if ((status & STATUS_LU) == 0) {
@@ -15650,6 +15665,8 @@ wm_init_manageability(struct wm_softc *s
 
 	DPRINTF(sc, WM_DEBUG_INIT, ("%s: %s called\n",
 		device_xname(sc->sc_dev), __func__));
+	KASSERT(IFNET_LOCKED(&sc->sc_ethercom.ec_if));
+
 	if (sc->sc_flags & WM_F_HAS_MANAGE) {
 		uint32_t manc2h = CSR_READ(sc, WMREG_MANC2H);
 		uint32_t manc = CSR_READ(sc, WMREG_MANC);

Reply via email to