git: c0382512bfce - main - ipfw: pmod: avoid further rule processing after tcp-mod failures

From: Kyle Evans <kevans_at_FreeBSD.org>
Date: Sat, 01 Nov 2025 17:34:38 UTC
The branch main has been updated by kevans:

URL: https://cgit.FreeBSD.org/src/commit/?id=c0382512bfce872102d213b9bc2550de0bc30b67

commit c0382512bfce872102d213b9bc2550de0bc30b67
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2025-11-01 17:34:11 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2025-11-01 17:34:11 +0000

    ipfw: pmod: avoid further rule processing after tcp-mod failures
    
    m_pullup() here will have freed the mbuf chain, but we pass back an
    IP_FW_DENY without any signal that the outer loop should finish.  Thus,
    rule processing continues without an mbuf and there's a chance that we
    conclude that the packet may pass (but there's no mbuf remaining)
    depending on the rules that follow it.
    
    PR:             284606
    Reviewed by:    ae
    MFC after:      1 week
---
 sys/netpfil/ipfw/pmod/tcpmod.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/sys/netpfil/ipfw/pmod/tcpmod.c b/sys/netpfil/ipfw/pmod/tcpmod.c
index 0338dc792c64..50074ee98cca 100644
--- a/sys/netpfil/ipfw/pmod/tcpmod.c
+++ b/sys/netpfil/ipfw/pmod/tcpmod.c
@@ -57,7 +57,8 @@ VNET_DEFINE_STATIC(uint32_t, tcpmod_setmss_eid) = 0;
 #define	V_tcpmod_setmss_eid	VNET(tcpmod_setmss_eid)
 
 static int
-tcpmod_setmss(struct mbuf **mp, struct tcphdr *tcp, int tlen, uint16_t mss)
+tcpmod_setmss(struct mbuf **mp, struct tcphdr *tcp, int tlen, uint16_t mss,
+    int *done)
 {
 	struct mbuf *m;
 	u_char *cp;
@@ -72,8 +73,10 @@ tcpmod_setmss(struct mbuf **mp, struct tcphdr *tcp, int tlen, uint16_t mss)
 		 * TCP header with options.
 		 */
 		*mp = m = m_pullup(m, m->m_pkthdr.len);
-		if (m == NULL)
+		if (m == NULL) {
+			*done = 1;
 			return (ret);
+		}
 	}
 	/* Parse TCP options. */
 	for (tlen -= sizeof(struct tcphdr), cp = (u_char *)(tcp + 1);
@@ -114,7 +117,7 @@ tcpmod_setmss(struct mbuf **mp, struct tcphdr *tcp, int tlen, uint16_t mss)
 
 #ifdef INET6
 static int
-tcpmod_ipv6_setmss(struct mbuf **mp, uint16_t mss)
+tcpmod_ipv6_setmss(struct mbuf **mp, uint16_t mss, int *done)
 {
 	struct ip6_hdr *ip6;
 	struct ip6_hbh *hbh;
@@ -142,13 +145,13 @@ tcpmod_ipv6_setmss(struct mbuf **mp, uint16_t mss)
 	/* We must have TCP options and enough data in a packet. */
 	if (hlen <= sizeof(struct tcphdr) || hlen > plen)
 		return (IP_FW_DENY);
-	return (tcpmod_setmss(mp, tcp, hlen, mss));
+	return (tcpmod_setmss(mp, tcp, hlen, mss, done));
 }
 #endif /* INET6 */
 
 #ifdef INET
 static int
-tcpmod_ipv4_setmss(struct mbuf **mp, uint16_t mss)
+tcpmod_ipv4_setmss(struct mbuf **mp, uint16_t mss, int *done)
 {
 	struct tcphdr *tcp;
 	struct ip *ip;
@@ -162,7 +165,7 @@ tcpmod_ipv4_setmss(struct mbuf **mp, uint16_t mss)
 	/* We must have TCP options and enough data in a packet. */
 	if (hlen <= sizeof(struct tcphdr) || hlen > plen)
 		return (IP_FW_DENY);
-	return (tcpmod_setmss(mp, tcp, hlen, mss));
+	return (tcpmod_setmss(mp, tcp, hlen, mss, done));
 }
 #endif /* INET */
 
@@ -206,19 +209,23 @@ ipfw_tcpmod(struct ip_fw_chain *chain, struct ip_fw_args *args,
 	switch (args->f_id.addr_type) {
 #ifdef INET
 		case 4:
-			ret = tcpmod_ipv4_setmss(&args->m, htons(icmd->arg1));
+			ret = tcpmod_ipv4_setmss(&args->m, htons(icmd->arg1),
+			    done);
 			break;
 #endif
 #ifdef INET6
 		case 6:
-			ret = tcpmod_ipv6_setmss(&args->m, htons(icmd->arg1));
+			ret = tcpmod_ipv6_setmss(&args->m, htons(icmd->arg1),
+			    done);
 			break;
 #endif
 	}
 	/*
 	 * We return zero in both @ret and @done on success, and ipfw_chk()
 	 * will update rule counters. Otherwise a packet will not be matched
-	 * by rule.
+	 * by rule. We passed @done around above in case we hit a fatal error
+	 * somewhere, we'll return non-zero but signal that rule processing
+	 * cannot succeed.
 	 */
 	return (ret);
 }