Page MenuHomeFreeBSD

altq: Fix panics on rmc_restart()
ClosedPublic

Authored by kp on Aug 23 2021, 10:12 AM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 22 2024, 11:59 PM
Unknown Object (File)
Feb 9 2024, 10:56 AM
Unknown Object (File)
Dec 30 2023, 6:51 AM
Unknown Object (File)
Dec 22 2023, 10:04 PM
Unknown Object (File)
Nov 18 2023, 10:36 PM
Unknown Object (File)
Oct 12 2023, 11:01 PM
Unknown Object (File)
Aug 17 2023, 1:46 PM
Unknown Object (File)
Aug 10 2023, 2:52 AM
Subscribers

Details

Summary

rmc_restart() is called from a timer, but can trigger traffic. This
means the curvnet context will not be set.
Use the vnet associated with the interface we're currently processing to
set it. We also have to enter net_epoch here, for the same reason.

MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kp requested review of this revision.Aug 23 2021, 10:12 AM

I think it would future-proof it to assert curvnet is not set and only set it after taking the lock.

In D31642#713694, @mjg wrote:

I think it would future-proof it to assert curvnet is not set and only set it after taking the lock.

Agreed on the second part (setting it after taking the lock), but I'm not sure how it future proofs us to assert curvnet isn't set yet. It might prevent mistakes if rmc_restart() is ever pulled out of the callback and moved into the regular data path, but that seems unlikely. It also wouldn't break anything.

There's also no precedent that I can see for anything like 'MPASS(curvnet == NULL)'.

  • move CURVNET_SET inside the lock
This revision is now accepted and ready to land.Aug 23 2021, 4:22 PM

the assumption in the patch is that vnet is not set, but i'm not going to insist on the assert

In D31642#713750, @mjg wrote:

the assumption in the patch is that vnet is not set, but i'm not going to insist on the assert

It's true that that's assumed (in that I believe that to be the cause of a panic on pfsense), but it doesn't actually matter. We can (and do, for example in if_epair.c) set a vnet while in the context of a different vnet. CURVNET_RESTORE() will then return us to the original vnet.

This revision was automatically updated to reflect the committed changes.