Page MenuHomeFreeBSD

pf: Fix panic on unload
ClosedPublic

Authored by kp on Mar 16 2017, 8:16 AM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 20 2023, 2:03 AM
Unknown Object (File)
Aug 12 2023, 4:28 PM
Unknown Object (File)
May 24 2023, 12:24 AM
Unknown Object (File)
May 9 2023, 9:28 AM
Unknown Object (File)
May 3 2023, 5:20 PM
Unknown Object (File)
Apr 25 2023, 5:34 AM
Unknown Object (File)
Feb 28 2023, 6:55 PM
Unknown Object (File)
Jan 29 2023, 2:28 PM
Subscribers

Details

Reviewers
eri
Group Reviewers
network
Commits
rS317773: pf: Fix panic on unload
Summary

vnet_pf_uninit() is called through vnet_deregister_sysuninit() and
linker_file_unload() when the pf module is unloaded. This is executed
after pf_unload() so we end up trying to take locks which have been
destroyed already.

Ensure we don't call pf_unload_vnet() from vnet_pf_uninit() for the root
vnet. We still need to do that cleanup, so call it directly from
pf_unload().

Test Plan

kldload pf
pfctl -g -f pf.conf
pfctl -e
kldunload -f pf

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

It feels a lot like a hack.
Shouldn't the proper VNET accessor be called on creation and teardown?

In D10025#207198, @eri wrote:

It feels a lot like a hack.
Shouldn't the proper VNET accessor be called on creation and teardown?

Yes, but that's essentially what's happening here. The root/default vnet is getting torn down.
The problem is the order of operations. pf_modevent() gets called before vnet_pf_uninit().

eri requested changes to this revision.Mar 17 2017, 10:01 PM

I would rather remove the pf_unload and pf_load to empty stubs and do their operation in pf_vnet_[un]init wiht DEFAULT_VNET wrapping than this.

Check ipfw(4) code and it has the same solution and makes consistency.

The other way is to use a semaphore pattern but the above is much simpler.

This revision now requires changes to proceed.Mar 17 2017, 10:01 PM
kp edited edge metadata.

I think you have a point, but I don't want to move pf_load(), because that can fail.

Use SYSUNINIT() to ensure pf_load() isn't called until after all vnet_pf_uninit()s have been called

This revision was automatically updated to reflect the committed changes.