Page MenuHomeFreeBSD

iflib: clean up correctly when device attach fails
AbandonedPublic

Authored by nick_spun.io on Jul 21 2026, 3:03 PM.
Referenced Files
Unknown Object (File)
Wed, Aug 26, 12:21 AM
Unknown Object (File)
Sun, Aug 23, 9:30 PM
Unknown Object (File)
Sun, Aug 16, 11:27 AM
Unknown Object (File)
Sat, Aug 15, 5:07 PM
Unknown Object (File)
Sat, Aug 15, 8:26 AM
Unknown Object (File)
Fri, Aug 14, 8:28 PM
Unknown Object (File)
Fri, Aug 14, 2:18 PM
Unknown Object (File)
Fri, Aug 14, 11:53 AM

Details

Reviewers
shurd
gallatin
Group Reviewers
iflib
Summary

Two leaks on the iflib_device_register() failure paths.

The device sysctls are the first thing registered, so free them last:
move sysctl_ctx_free() to the fail_unlock path that every failure
reaches. On that path only the pre-attach sysctls exist and none
reference queue memory, so freeing them after the queue teardown is
safe.

Every failure after a successful IFDI_ATTACH_PRE() then unwinds through
fail_queues, which calls IFDI_DETACH() and IFDI_QUEUES_FREE() so the
driver can release whatever it set up in attach_pre. The one exception
is iflib_queues_alloc(): it jumped straight to fail_intr_free, so the
driver never learned that attach failed. Anything it allocated in
attach_pre was leaked -- for aq(4) that is the BAR0 resource, a 512-byte
VLAN bitmap, and an initialized mutex whose freed memory stays on the
witness lock list. The per-interface admin taskqueue, created earlier
in iflib_device_register(), was leaked on that path as well.

That path cannot simply join fail_queues: iflib_queues_alloc() already
frees the queue rings and clears ifc_txqs/ifc_rxqs on failure, so
iflib_tx_structures_free() would walk a NULL array rather than leak.
Add a label after the structure teardown instead, so the queue
allocation failure gets IFDI_DETACH(), IFDI_QUEUES_FREE() and
taskqueue_free() without repeating the frees iflib_queues_alloc() has
already done. IFDI_QUEUES_FREE() is needed because the driver's TX
queue allocation may have succeeded before the RX one failed.

fail_intr_free loses its last goto and becomes a plain fallthrough, so
drop the label.

Diff Detail

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

Event Timeline

nick_spun.io retitled this revision from iflib: free the device sysctls on every attach-failure path to iflib: free the device pre-attach sysctls on every attach-failure path.Jul 21 2026, 3:13 PM
nick_spun.io edited the summary of this revision. (Show Details)
nick_spun.io edited the summary of this revision. (Show Details)

Fold in the queue-allocation attach-failure leak; both fixes are on the same iflib_device_register() failure ladder.

nick_spun.io retitled this revision from iflib: free the device pre-attach sysctls on every attach-failure path to iflib: clean up correctly when device attach fails.Jul 25 2026, 3:56 AM
nick_spun.io edited the summary of this revision. (Show Details)
adrian added a reviewer: gallatin.
adrian added a subscriber: gallatin.

@gallatin would you mind eyeballing this? ty!

This revision is now accepted and ready to land.Mon, Aug 3, 4:50 PM

I was independently dealing with similar issues while testing SR-IOV. Can you have a look at https://reviews.freebsd.org/D58721 and the linked fail(9) commit.

Superseded by D58721, which covers both fixes here on a rewritten common unwind path and adds the SR-IOV schema and core-offset refcount cases. Abandoning in favor of that.