Page MenuHomeFreeBSD

pfsync: support deferring IPv6 packets
ClosedPublic

Authored by kp on Feb 14 2023, 10:57 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, May 29, 8:28 PM
Unknown Object (File)
Wed, May 29, 8:28 PM
Unknown Object (File)
Wed, May 29, 8:28 PM
Unknown Object (File)
Apr 27 2024, 9:24 AM
Unknown Object (File)
Apr 27 2024, 9:24 AM
Unknown Object (File)
Apr 27 2024, 9:24 AM
Unknown Object (File)
Apr 27 2024, 9:24 AM
Unknown Object (File)
Apr 27 2024, 9:24 AM

Details

Summary

When we send out a deferred packet we must make sure to call
ip6_output() for IPv6 packets. If not we might end up attempting to
ip_fragment() an IPv6 packet, which could lead to us reading outside of
the mbuf.

PR: 268246
MFC after: 2 weeks

Diff Detail

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

Event Timeline

kp requested review of this revision.Feb 14 2023, 10:57 AM
sys/netpfil/pf/if_pfsync.c
2403

From the readability POV, I'd vote for moving the mbuf handling and sending code into the dedicated function.

Factor out sending the mbuf into its own function.

This revision is now accepted and ready to land.Feb 15 2023, 9:41 AM
zlei added inline comments.
sys/netpfil/pf/if_pfsync.c
2404

A little confused by this check ip->ip_v == IPVERSION.

I see only two functions pfsync_sendout() and pf_test_rule() -> pfsync_defer() -> pfsync_undefer() are calling _IF_ENQUEUE(&b->b_snd, m);, and pf_test() and pf_test6() call pf_test_rule().

So is it possible that pf_test6() enqueues IPv6 packets without correct version ip6->ip6_vfc |= IPV6_VERSION ?

If in doubt then we want to assert the version.

(ip->ip_v == IPVERSION || ip->ip_v == IPV6_VERSION >> 4)
sys/netpfil/pf/if_pfsync.c
2404

Your understanding is correct. We'd only expect to see IPv4 or IPv6 here. Right now the only path for IPv6 packets to end up here is via pf_test6() -> pfsync_defer(). That's the path that can cause panics (see 268246).
At some point we may also add support for pfsync over IPv6. Someone was working on that, but I've not seen updates in a while.

An assert is a reasonable suggestion. I'll see about adding that.

Assert that packets are IPv4 or IPv6.

This revision now requires review to proceed.Feb 15 2023, 11:32 AM

Also fix the same issue in pfsync_defer_tmo()

Looks good to me.

sys/netpfil/pf/if_pfsync.c
1840

Definitely this should be fixed ;)

2333

Style(9), remove extra blanks after >>

This revision is now accepted and ready to land.Feb 16 2023, 12:57 AM
This revision was automatically updated to reflect the committed changes.