Page MenuHomeFreeBSD
Differential D6924

Make pf more VNET safe
ClosedPublic
Actions

Authored by bz on Jun 22 2016, 12:59 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Dec 16, 7:16 PM
Unknown Object (File)
Wed, Dec 4, 6:49 AM
Unknown Object (File)
Thu, Nov 28, 5:05 AM
Unknown Object (File)
Thu, Nov 28, 5:01 AM
Unknown Object (File)
Oct 14 2024, 7:22 PM
Unknown Object (File)
Oct 14 2024, 7:22 PM
Unknown Object (File)
Oct 14 2024, 7:22 PM
Unknown Object (File)
Oct 14 2024, 7:21 PM
View All Files
Subscribers
imp
robak
network

Details

Reviewers
emaste
nvass-gmx.com
kp
gnn
Commits
rS302156: Update pf(4) and pflog(4) to survive basic VNET testing, which includes
Summary

Update pf(4) and pflog(4) to survive basic VNET testing, which includes
proper virtualisation, teardown, avoiding use-after-free, race conditions,
no longer creating a thread per VNET (which could easily be a couple of
thousand threads), handling global events (e.g., eventhandlers) on teardown,
clearing various globally cached pointers and checking them before use.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 4309
Build 4353: arc lint + arc unit

Event Timeline

bz updated this revision to Diff 17762.Jun 22 2016, 12:59 PM
bz retitled this revision from to Make pf more VNET safe.
bz updated this object.
bz edited the test plan for this revision. (Show Details)
bz added reviewers: gnn, emaste, kp, nvass-gmx.com.
bz added a subscriber: network.
Herald added a subscriber: imp. · View Herald TranscriptJun 22 2016, 12:59 PM
robak added a subscriber: robak.Jun 22 2016, 1:20 PM

I can't discuss the code, I can only test it and see if it makes the difference in system stability. Any chance for this to land in 11.0?

bz added a comment.Jun 22 2016, 2:01 PM
In D6924#145160, @robak wrote:

I can't discuss the code, I can only test it and see if it makes the difference in system stability. Any chance for this to land in 11.0?

Yes, that's the plan. Needs all the testing it can get and I am still looking at pfsync and a possible very old security issue (don't run with untrusted customers in VNET yet) which I don't know if FreeBSD is vulnerable to.

kp added inline comments.Jun 22 2016, 8:15 PM
sys/netpfil/pf/pf.c
302

There's a '#define V_pf_end_threads VNET(pf_end_threads)' in net/pfvar.h.
Presumably you'll want to get rid of that too.

sys/netpfil/pf/pf_if.c
911

Minor typo here and in the others like it. 'expensie'.

sys/netpfil/pf/pf_ioctl.c
3782

When is this ever not true?
As far as I can tell you removed the only pf_end_threads++; from pf_purge_thread()

bz marked 3 inline comments as done.Jun 23 2016, 12:39 AM

Should all be addressed in the updated diff to come.

sys/netpfil/pf/pf_ioctl.c
3782

Good catch; that would be an endless loop...

bz updated this revision to Diff 17783.Jun 23 2016, 12:41 AM
bz marked an inline comment as done.

Address comments from @kristof .
Especially get rid of an endless loooCp on unload hopefully.p

kp accepted this revision.Jun 23 2016, 6:03 AM
kp edited edge metadata.
This revision is now accepted and ready to land.Jun 23 2016, 6:03 AM
Closed by commit rS302156: Update pf(4) and pflog(4) to survive basic VNET testing, which includes (authored by bz). · Explain WhyJun 23 2016, 9:34 PM
This revision was automatically updated to reflect the committed changes.
kp mentioned this in D1944: PF and VIMAGE fixes.Aug 16 2016, 9:26 AM

Revision Contents
Changeset List

PathSize
sys/
net/
4 lines
netpfil/
pf/
43 lines
77 lines
95 lines
152 lines

Diff 17783

View Options

sys/net/pfvar.h

Loading...
View Options

sys/netpfil/pf/if_pflog.c

Loading...
View Options

sys/netpfil/pf/pf.c

Loading...
View Options

sys/netpfil/pf/pf_if.c

Loading...
View Options

sys/netpfil/pf/pf_ioctl.c

Loading...