DF-3681 — igb never terminates its self-re-arming timers at detach (UAF)

What

igb(4) runs two self-re-arming callouts and tears both down with callout_stop() only — never callout_terminate():

• sc->timer — igb_timer() (sys/dev/netif/igb/if_igb.c:1308-1321) takes lwkt_serialize_enter(&sc->main_serialize) (blocking) and unconditionally re-arms itself at :1318 (callout_reset_bycpu(..., hz, igb_timer, sc, ...)).
• txr->tx_gc_timer (one per TX ring) — igb_txgc_timer() (:327-350) re-arms itself at :349 (callout_reset(..., 1, ...)) on the try-serialize-failure path (goto done).

igb_stop() stops them with callout_stop() (:1436, :1447), which per sys/kern/kern_timeout.c:1087-1098 "does not interlock against a callout that is in-progress": a callback that is currently executing (or blocked) can still callout_reset() itself after callout_stop() returns. igb_detach() (:742-802) contains no callout_terminate for either timer.

Race (wide window for sc->timer)

1. CPU1: igb_timer fires; blocks in lwkt_serialize_enter(&sc->main_serialize) (detach holds all serializers via ifnet_serialize_all, :750).
2. CPU0: igb_detach → igb_stop → callout_stop(&sc->timer) — the callout is INPROG, not pending, so nothing is cancelled → ifnet_deserialize_all (:763) → ether_ifdetach (:765) → resource release (:773-799) → igb_free_rings (:784) → kfree(sc->serializes) (:791).
3. CPU1: acquires the (about-to-be-freed) serializer, runs, and re-arms sc->timer for hz later (:1318).
4. One second later the callout fires into freed memory: the callout structure lives inside the softc (freed by devclass after detach / kldunload), the callback executes igb_timer reading freed sc, lwkt_serialize_enter on freed memory, MMIO through the released mem_res mapping; under kldunload if_igb the module text itself is unmapped. Use-after-free (read + callout-internal writes on freed slab).

Same class as DF-3669 (ix). Notably, ix(4) received callout_terminate(&sc->timer) at detach (sys/dev/netif/ix/if_ix.c:693) — igb(4) lacks even that mitigation for both timer classes.

Reproduce / verify

Requires igb hardware (timers only run once the interface is up). The audit guest has none (run.log). Source-level verification only.

Expected result (on hardware)

kldunload if_igb / devctl detach racing a fired igb_timer/igb_txgc_timer → delayed panic or heap corruption in callout internals after the softc/rings are freed. With the fix, callout_terminate() after the serializers are dropped guarantees no post-detach fire.

DF-3681 VERDICT — self-re-arming timers not terminated at detach

Status: inconclusive on guest (no igb hardware) — source-likely

Root cause (path:line)

Two self-re-arming callouts; teardown uses only the non-interlocking stop:

1. igb_timer() sys/dev/netif/igb/if_igb.c:1308-1321 c 1313: lwkt_serialize_enter(&sc->main_serialize); /* BLOCKING */ ... 1318: callout_reset_bycpu(&sc->timer, hz, igb_timer, sc, sc->timer_cpuid);
2. igb_txgc_timer() :327-350 c 337: if (!lwkt_serialize_try(&txr->tx_serialize)) 338: goto done; ... 349: callout_reset(&txr->tx_gc_timer, 1, igb_txgc_timer, txr); /* done: */

Teardown: - igb_stop() :1436 callout_stop(&sc->timer); and :1447 callout_stop(&txr->tx_gc_timer); - igb_detach() :742-802 — no callout_terminate anywhere.

sys/kern/kern_timeout.c:1087-1098 documents callout_stop() as non-interlocking against an in-progress callback; callout_terminate() (:1104-1128) is the API that synchronously cancels and prevents further callout_reset() (CALLOUT_CANCEL semantics), and frees the internal _callout safely.

Race proof (sc->timer — deterministic re-arm)

igb_stop() runs under ifnet_serialize_all() (detach :750), i.e. with main_serialize held. An igb_timer callback that fired before the flag clear is guaranteed to be parked at :1313 on that serializer for the whole stop/detach critical section (which includes e1000_reset_hw, PHY ops, and up to msec_delay(IGB_I210_LINK_DELAY) = 1000 ms inside igb_update_link_status on i210 — a very wide window). callout_stop() at :1436 cannot cancel it (INPROG, not pending). When detach releases the serializers (:763), the parked callback resumes, runs its body against the stopped/reset device, and re-arms at :1318 for +1 s — after detach has already called ether_ifdetach (:765), released mem_res (:779-782), freed the rings (:784) and sc->serializes (:791).

The re-armed fire then touches: - sc->hw MMIO via sc->osdep.mem_bus_space_handle — mapping released at :779-782 (unmapped/reassignable region); - lwkt_serialize_enter(&sc->main_serialize) on freed memory; - the callout subsystem writes into sc->timer's freed slab on each cycle.

If the interface was detached via kldunload if_igb, the softc (and the module text containing igb_timer) are freed/unmapped when the timer fires → guaranteed fault/UAF; absent module unload, a perpetually re-arming timer operating on a dead softc persists.

tx_gc_timer has the same hole via the goto done re-arm (:337-349): a callback that failed lwkt_serialize_try while detach held the serializers re-arms with a 1-tick timeout and dereferences txr->sc->arpcom.ac_if after the rings are freed (:784, kfree(sc->tx_rings) inside igb_free_rings).

Cross-reference

DF-3669 filed the same class against ix(4) (ix_txgc_timer); this tree's ix(4) detach already calls callout_terminate(&sc->timer) (sys/dev/netif/ix/if_ix.c:693) — igb(4) has no equivalent for either timer, making it strictly weaker than the post-DF-3669 state of ix.

Why not reproduced on the guest

No igb hardware: the guest's only NIC is virtio vtnet0; igb_attach() never runs so the timers are never armed. See run.log. (The callout-semantics half of the argument is kernel-generic and verified against sys/kern/kern_timeout.c; the arming half needs the device.)

Fix

callout_terminate() for both timer classes in igb_detach() after the serializers are released (fix.diff). Not hardware-testable: fix_status = not_testable.

Fix status (batch validation)

Applied with the 36 sibling fix.diffs of this batch in findings/poc/_batch41/all_fixes.patch; single in-guest make -j6 nativekernel KERNCONF=X86_64_GENERIC completed RC=0 with zero errors under -Werror (full log: findings/poc/_batch41/fix_build8.log). Behavior A/B intentionally skipped for this Info-tier item.