Page MenuHomeFreeBSD

iflib: drain the admin task before detaching the interface
Needs ReviewPublic

Authored by nick_spun.io on Sat, Jul 25, 4:08 AM.

Details

Reviewers
shurd
Group Reviewers
iflib
Summary

iflib_device_deregister() sets IFC_IN_DETACH and relies on
_task_fn_admin() observing it, but a task that has already passed that
check keeps running to completion. It can still call
iflib_link_state_change(), which re-arms if_linktask after
ether_ifdetach() has already drained it. The link state change is then
left pending across iflib_rx_structures_free().

When the pending state is LINK_STATE_UP and the kernel has DEBUGNET, the
resulting ifnet_link_event reaches iflib_debugnet_init(), which
dereferences ctx->ifc_rxqs[0].ifr_fl after it has been freed:

panic: page fault, virtual address 0x12
iflib_debugnet_init()
debugnet_any_ifnet_update()
do_link_state_change()

0x12 is offsetof(struct iflib_fl, ifl_size). Reproduced by unloading
if_aq while the interface was administratively down but the PHY still
had link, so the admin task kept re-announcing LINK_STATE_UP.

Drain the admin task before ether_ifdetach(), so the if_linktask drain
that it performs is the last one. IFC_IN_DETACH is already set at that
point, so a task enqueued afterwards returns without touching the
context.

The register failure path grew the same drain in 439132310ae1; this
applies it to the normal detach path, which that change had used as its
reference for correct teardown ordering.

Signed-off-by: Nick Price <nick@spun.io>

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 75120
Build 72003: arc lint + arc unit