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
F105087931: D35551.diff
Thu, Dec 12, 7:06 AM
Unknown Object (File)
Sun, Dec 8, 5:16 PM
Unknown Object (File)
Tue, Dec 3, 3:24 AM
Unknown Object (File)
Mon, Dec 2, 12:49 AM
Unknown Object (File)
Oct 10 2024, 10:22 AM
Unknown Object (File)
Sep 8 2024, 10:41 PM
Unknown Object (File)
Sep 5 2024, 5:07 PM
Unknown Object (File)
Sep 3 2024, 7:09 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