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.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 12, 9:10 AM
Unknown Object (File)
Tue, Dec 3, 11:32 AM
Unknown Object (File)
Thu, Nov 28, 10:45 AM
Unknown Object (File)
Nov 7 2024, 10:45 AM
Unknown Object (File)
Nov 7 2024, 7:41 AM
Unknown Object (File)
Oct 27 2024, 4:38 AM
Unknown Object (File)
Oct 25 2024, 7:49 AM
Unknown Object (File)
Oct 5 2024, 8:25 AM
Subscribers

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 - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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

Wrap line to keep it under 80 cols

erj marked an inline comment as done.Jun 24 2019, 11:27 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