Page MenuHomeFreeBSD

iflib: Prevent kernel panic caused by loading driver with a specific interrupt configuration
ClosedPublic

Authored by erj on Jun 24 2019, 8:17 PM.

Details

Summary

If a device has only 1 MSI-X interrupt available and does not support either MSI or legacy
interrupts, iflib_device_register() will fail, leak memory and MSI resources, and the driver
will not load. Worse, if another iflib-using driver tries to unload afterwards, a kernel panic will occur
because the previous failed iflib driver loead did not properly call "taskqgroup_detach()" during it's
cleanup.

This patch is band-aid for this situation -- don't try allocating MSI or legacy interrupts if
a single MSI-X interrupt was allocated, but fail to load instead. As well, during the cleanup,
properly call taskqgroup_detach() on the admin task to prevent panics when other iflib
drivers unload.

This whole interrupt allocation process probably needs re-doing to properly support a single
MSI-X interrupt as well as supporting devices that only support MSI-X.

Signed-off-by: Eric Joyner <erj@freebsd.org>

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

erj created this revision.Jun 24 2019, 8:17 PM
marius requested changes to this revision.Jun 24 2019, 8:42 PM
marius added a subscriber: marius.

Apart from the style problem, this change looks good to me.
Have you looked at how hard it would be to actually support configurations
with just 1 MSI-X, though? I think to remember the assumption that a single
interrupt means what iflib(4) calls "legacy", i. e. INTx or MSI, in 1 or 2 places
and generally iflib(4) would need to be made less smart and not so aggressive
in setting up interrupts, but nothing which seemed that much work.

sys/net/iflib.c
4747 ↗(On Diff #58964)

Please wrap the line after "dev," so the code stays within 80 columns.

This revision now requires changes to proceed.Jun 24 2019, 8:42 PM
erj updated this revision to Diff 58976.Jun 24 2019, 11:26 PM

Wrap line to keep it under 80 cols

erj marked an inline comment as done.Jun 24 2019, 11:27 PM
marius accepted this revision.Jun 26 2019, 12:57 PM

Looks good to me now, thanks.
For committing, please extend the commit message to additionally state
that the iflib(4) interrupt allocation needs re-doing not only to also support
single-message MSI-X and MSI-X-only devices, but for none-PCI devices
and multiple interrupt vectors that are not MSI-X as well, though.

This revision is now accepted and ready to land.Jun 26 2019, 12:57 PM