Page MenuHomeFreeBSD

pf: copy out rather than m_pullup() in pf_test_eth_rule()
ClosedPublic

Authored by kp on Jun 22 2022, 4:14 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Feb 10, 10:50 AM
Unknown Object (File)
Sat, Feb 8, 11:39 PM
Unknown Object (File)
Sun, Jan 26, 9:21 PM
Unknown Object (File)
Sat, Jan 18, 9:16 PM
Unknown Object (File)
Fri, Jan 17, 1:58 AM
Unknown Object (File)
Jan 11 2025, 3:52 AM
Unknown Object (File)
Jan 11 2025, 3:49 AM
Unknown Object (File)
Jan 11 2025, 12:54 AM

Details

Summary

Don't change the mbuf chain layout. We've encountered alignment issues
in the tcp syncookie code on armv7, which are triggered by the
m_pullup() here.

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.Jun 22 2022, 4:14 PM
mjg added inline comments.
sys/netpfil/pf/pf.c
3920–3921

now that m does not change this assignment is redundant

more importantly though for non v4/v6 packets src and dst will be NULL and code below will be reached. that's asking for a remotely-induced crash down the road.

sys/netpfil/pf/pf.c
3920–3921

I'll remove the redundant assignment.

src/dst are not even NULL, they're uninitialised. This hasn't actually changed with this patch, but we check af to see if we have an address we can compare to or not. It'd probably be safer to initialise src and dst to NULL, but that still wouldn't prevent a panic if we mess up the af handling.

  • remove now redundant update of *m0 and e
mjg added inline comments.
sys/netpfil/pf/pf.c
3920–3921

this definitely needs to be sorted out in a different patch, this is too error prone for any future changes

This revision is now accepted and ready to land.Jun 23 2022, 12:15 AM
sys/netpfil/pf/pf.c
3920–3921

See D35573