⬢ DragonFlyBSD Kernel Audit
DF-4327 / fix.diff
← back to finding ↓ download raw
--- a/sys/dev/netif/sf/if_sf.c
+++ b/sys/dev/netif/sf/if_sf.c
@@ -793,8 +793,14 @@ sf_detach(device_t dev)
 	if (device_is_attached(dev)) {
 		lwkt_serialize_enter(ifp->if_serializer);
 		sf_stop(sc);
 		bus_teardown_intr(dev, sc->sf_irq, sc->sf_intrhand);
 		lwkt_serialize_exit(ifp->if_serializer);
 
 		ether_ifdetach(ifp);
+		/*
+		 * sf_stop() only issued callout_stop_async(); now that
+		 * the serializer is free, drain any in-flight callback.
+		 * (The !IFF_RUNNING guard in sf_stats_update makes it
+		 * return immediately, so this cannot deadlock.)
+		 */
+		callout_drain(&sc->sf_stat_timer);
 	}
@@ -1341,11 +1347,24 @@ sf_stop(struct sf_softc *sc)
 	struct ifnet		*ifp;
 
 	ifp = &sc->arpcom.ac_if;
 
+	/*
+	 * Mark the interface down FIRST so any running/queued
+	 * sf_stats_update() callback observes it right after
+	 * acquiring the serializer and neither touches the chip
+	 * nor re-arms itself.
+	 *
+	 * NEVER use the blocking callout_stop() here while holding
+	 * the serializer: the callback blocks acquiring this very
+	 * serializer, so callout_stop's synchronous "costp" wait
+	 * (kern_timeout.c) and the callback form an AB-BA deadlock
+	 * (DF-3715 family).  Use callout_stop_async(); sf_detach()
+	 * drains the callback after releasing the serializer.
+	 */
+	ifp->if_flags &= ~IFF_RUNNING;
+	callout_stop_async(&sc->sf_stat_timer);
-	callout_stop(&sc->sf_stat_timer);
 
 	csr_write_4(sc, SF_GEN_ETH_CTL, 0);
 	csr_write_4(sc, SF_CQ_CONSIDX, 0);
@@ -1386,6 +1405,12 @@ sf_stats_update(void *xsc)
 
 	lwkt_serialize_enter(ifp->if_serializer);
 
+	/* Stopped or stopping: do not touch the chip, do not re-arm. */
+	if ((ifp->if_flags & IFF_RUNNING) == 0) {
+		lwkt_serialize_exit(ifp->if_serializer);
+		return;
+	}
+
 	ptr = (u_int32_t *)&stats;
 	for (i = 0; i < sizeof(stats)/sizeof(u_int32_t); i++)
 		ptr[i] = csr_read_4(sc, SF_STATS_BASE +
@@ -1419,7 +1444,6 @@ sf_stats_update(void *xsc)
 
 	callout_reset(&sc->sf_stat_timer, hz, sf_stats_update, sc);
 
-	ifp->if_flags &= ~IFF_RUNNING;	(no-op note: this hunk belongs to
-					 sf_stop; sf_stop's own trailing
-					 clear at old line 1373 becomes
-					 redundant and is dropped)
 	lwkt_serialize_exit(ifp->if_serializer);
 }