From bd69ffad5281c1ad53bd51fcabedef9ebcab65c2 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Sun, 7 Aug 2022 09:29:46 +0000 Subject: [PATCH] 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. - Sprinkle assertions. --- sys/dev/pci/if_wm.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/sys/dev/pci/if_wm.c b/sys/dev/pci/if_wm.c index 92633e1ae2ab..864aaa28034a 100644 --- a/sys/dev/pci/if_wm.c +++ b/sys/dev/pci/if_wm.c @@ -3571,6 +3571,7 @@ wm_resume(device_t self, const pmf_qual_t *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) @@ -3589,6 +3590,7 @@ wm_resume(device_t self, const pmf_qual_t *qual) * via wm_init(). */ } + IFNET_UNLOCK(ifp); return true; } @@ -4325,6 +4327,7 @@ wm_set_filter(struct wm_softc *sc) DPRINTF(sc, WM_DEBUG_INIT, ("%s: %s called\n", device_xname(sc->sc_dev), __func__)); + KASSERT(IFNET_LOCKED(ifp)); if (sc->sc_type >= WM_T_82544) mta_reg = WMREG_CORDOVA_MTA; @@ -5256,6 +5259,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; @@ -5348,6 +5353,7 @@ wm_reset(struct wm_softc *sc) DPRINTF(sc, WM_DEBUG_INIT, ("%s: %s called\n", device_xname(sc->sc_dev), __func__)); KASSERT(sc->sc_type != 0); + KASSERT(IFNET_LOCKED(&sc->sc_ethercom.ec_if)); /* * Allocate on-chip memory according to the MTU size. @@ -7073,6 +7079,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); @@ -7098,6 +7105,7 @@ wm_stop_locked(struct ifnet *ifp, bool disable, bool wait) 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); @@ -7177,6 +7185,7 @@ wm_stop_locked(struct ifnet *ifp, bool disable, bool wait) /* 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++) { @@ -8376,9 +8385,8 @@ wm_send_common_locked(struct ifnet *ifp, struct wm_txqueue *txq, 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; @@ -8994,9 +9002,8 @@ wm_nq_send_common_locked(struct ifnet *ifp, struct wm_txqueue *txq, 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; @@ -13133,7 +13140,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) { @@ -15622,6 +15629,8 @@ wm_init_manageability(struct wm_softc *sc) 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);