Page MenuHomeFreeBSD

netinet: re-read IP length after PFIL hook
ClosedPublic

Authored by kp on Jun 2 2023, 7:27 PM.
Tags
None
Referenced Files
F86906229: D40395.id.diff
Thu, Jun 27, 7:31 AM
F86892379: D40395.diff
Thu, Jun 27, 3:26 AM
F86892378: D40395.diff
Thu, Jun 27, 3:26 AM
F86892377: D40395.diff
Thu, Jun 27, 3:26 AM
F86892376: D40395.diff
Thu, Jun 27, 3:26 AM
F86892373: D40395.diff
Thu, Jun 27, 3:26 AM
F86892371: D40395.diff
Thu, Jun 27, 3:26 AM
Unknown Object (File)
Fri, Jun 21, 11:08 PM

Details

Summary

The pfil hook may modify the packet, so before we check its length (to
decide if it needs to be fragmented or not) we should re-read that
length.

This is most likely to happen when pf is reassembling packets. In that
scenario we'd receive the last fragment, which is likely to be a short
packet, pf would reassemble it (likely exceeding the interface MTU) and
then we'd transmit it without fragmenting, because we're comparing the
MTU to the length of the last fragment, not the fully reassembled
packet.

See also: https://redmine.pfsense.org/issues/14396
MFC after: 3 weeks
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 2 2023, 7:27 PM

so the reload is only needed if hooks are present to begin with? as in it should get hoisted up

what about other fields:

ip = mtod(m, struct ip *);
ip_len = ntohs(ip->ip_len);
ip_off = ntohs(ip->ip_off);

ip is is already getting reloaded

  • move re-read of ip_len to only be done when we run pfil hooks.
cy added a subscriber: cy.

LGTM

This revision is now accepted and ready to land.Jun 5 2023, 3:20 PM
This revision was automatically updated to reflect the committed changes.