Mike Pattrick <[email protected]> writes: > On Fri, Aug 28, 2026 at 8:33 AM Aaron Conole via dev > <[email protected]> wrote: > > Aaron Conole <[email protected]> writes: > > > The FTP ALG handling for IPv4 and IPv6 is guarded from overwriting the > > buffer space by checking the headroom + tailroom against the rewrite > > delta. However, when a packet needs to be expanded, only the tailroom > > is actually available. The guard itself allows modifications to pass > > that may otherwise be flagged. > > > > Fix this by only checking that the tailroom has sufficient space to > > handle the packet growth. This is a bit different as we no longer > > check the delta, but just the actual expansion room available. This > > check is a bit more conservative, so there could be an edge-case > > packet that used to pass through the checks but no longer > > would. > > > > The FTP ALG unit tests are updated to correct a comment about the > > guard design, and test for the new condition. > > > > Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.") > > Reported-by: Vinícius Rodrigues <[email protected]> > > Assisted-by: Claude Sonnet 4.5 > > Signed-off-by: Aaron Conole <[email protected]> > > --- > > AUTHORS.rst | 1 + > > lib/conntrack.c | 20 +++---- > > tests/library.at | 4 ++ > > tests/test-conntrack.c | 122 ++++++++++++++++++++++++++++++++++++++++- > > 4 files changed, 134 insertions(+), 13 deletions(-) > > > > diff --git a/AUTHORS.rst b/AUTHORS.rst > > index 0f7445c80f..92500507d4 100644 > > --- a/AUTHORS.rst > > +++ b/AUTHORS.rst > > @@ -797,6 +797,7 @@ Tulio Ribeiro > [email protected] > > Tytus Kurek [email protected] > > Valentin Bud [email protected] > > Vasiliy Tolstov [email protected] > > +Vinícius Rodrigues [email protected] > > Vinllen Chen [email protected] > > Vipul Ashri [email protected] > > Vishal Swarankar [email protected] > > diff --git a/lib/conntrack.c b/lib/conntrack.c > > index f84cdd216a..0809e73f24 100644 > > --- a/lib/conntrack.c > > +++ b/lib/conntrack.c > > @@ -3294,12 +3294,12 @@ repl_ftp_v4_addr(struct dp_packet *pkt, ovs_be32 > v4_addr_rep, > > > > /* Do conservative check for pathological MTU usage. */ > > uint32_t orig_used_size = dp_packet_size(pkt); > > - if (orig_used_size + MAX_FTP_V4_NAT_DELTA > > > - dp_packet_get_allocated(pkt)) { > > - > > + if (MAX_FTP_V4_NAT_DELTA > dp_packet_tailroom(pkt)) { > > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); > > - VLOG_WARN_RL(&rl, "Unsupported effective MTU %u used with FTP V4", > > - dp_packet_get_allocated(pkt)); > > + VLOG_WARN_RL(&rl, > > + "Oversized packet detected with FTPv4 (%"PRIuSIZE > > + " vs. %"PRIu32")", > > + dp_packet_tailroom(pkt), MAX_FTP_V4_NAT_DELTA); > > return 0; > > } > > > > @@ -3674,12 +3674,12 @@ repl_ftp_v6_addr(struct dp_packet *pkt, union > ct_addr v6_addr_rep, > > > > /* Do conservative check for pathological MTU usage. */ > > uint32_t orig_used_size = dp_packet_size(pkt); > > - if (orig_used_size + MAX_FTP_V6_NAT_DELTA > > > - dp_packet_get_allocated(pkt)) { > > - > > + if (MAX_FTP_V6_NAT_DELTA > dp_packet_tailroom(pkt)) { > > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); > > - VLOG_WARN_RL(&rl, "Unsupported effective MTU %u used with FTP V6", > > - dp_packet_get_allocated(pkt)); > > + VLOG_WARN_RL(&rl, > > + "Oversized packet detected with FTPv6 (%"PRIuSIZE > > + " vs. %"PRIu32")", > > + dp_packet_tailroom(pkt), MAX_FTP_V6_NAT_DELTA); > > return 0; > > } > > > > diff --git a/tests/library.at b/tests/library.at > > index 80ebe6ed8a..83516bc502 100644 > > --- a/tests/library.at > > +++ b/tests/library.at > > @@ -296,3 +296,7 @@ AT_CLEANUP > > AT_SETUP([Conntrack Library - FTP ALG parsing]) > > AT_CHECK([ovstest test-conntrack ftp-alg-large-payload]) > > AT_CLEANUP > > + > > +AT_SETUP([Conntrack Library - FTP ALG guard]) > > +AT_CHECK([ovstest test-conntrack ftp-alg-no-tailroom]) > > ^ This is missing a squelch for the WARN message that gets generated. > For some reason I forgot to git commit --amend it in place (sorry > about that). Will fix with a v2. Sorry for the noise. If anyone > wants to review anything else, I'll wait until monday to post the new > version. > > Hello Aaron, > > The 0-day bot had some feedback too, but other than those issues the fix and > test look good to me!
Thanks Mike! I've posted a new version at: http://patchwork.ozlabs.org/project/openvswitch/list/?series=521058 > -M _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
