git: ffeab76b6855 - main - pfil: PFIL_PASS never frees the mbuf

The branch main has been updated by kp:

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

commit ffeab76b68550c347abcd7c31c3b3dfcdea732b5
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2024-01-26 12:29:31 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2024-01-29 13:10:19 +0000

    pfil: PFIL_PASS never frees the mbuf
    
    pfil hooks (i.e. firewalls) may pass, modify or free the mbuf passed
    to them. (E.g. when rejecting a packet, or when gathering up packets
    for reassembly).
    
    If the hook returns PFIL_PASS the mbuf must still be present. Assert
    this in pfil_mem_common() and ensure that ipfilter follows this
    convention. pf and ipfw already did.
    Similarly, if the hook returns PFIL_DROPPED or PFIL_CONSUMED the mbuf
    must have been freed (or now be owned by the firewall for further
    processing, like packet scheduling or reassembly).
    
    This allows us to remove a few extraneous NULL checks.
    
    Suggested by:   tuexen
    Reviewed by:    tuexen, zlei
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
    Differential Revision:  https://reviews.freebsd.org/D43617
---
 sys/net/if_ethersubr.c                        | 2 +-
 sys/net/pfil.c                                | 7 ++++++-
 sys/netinet/ip_input.c                        | 4 ----
 sys/netinet6/ip6_input.c                      | 2 --
 sys/netpfil/ipfilter/netinet/ip_fil_freebsd.c | 4 ++++
 5 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/sys/net/if_ethersubr.c b/sys/net/if_ethersubr.c
index ef0b1f705260..4332f4ce864e 100644
--- a/sys/net/if_ethersubr.c
+++ b/sys/net/if_ethersubr.c
@@ -878,7 +878,7 @@ ether_demux(struct ifnet *ifp, struct mbuf *m)
 	/* Do not grab PROMISC frames in case we are re-entered. */
 	if (PFIL_HOOKED_IN(V_link_pfil_head) && !(m->m_flags & M_PROMISC)) {
 		i = pfil_mbuf_in(V_link_pfil_head, &m, ifp, NULL);
-		if (i != 0 || m == NULL)
+		if (i != PFIL_PASS)
 			return;
 	}
 
diff --git a/sys/net/pfil.c b/sys/net/pfil.c
index 3ceffcefb758..fae150839eb3 100644
--- a/sys/net/pfil.c
+++ b/sys/net/pfil.c
@@ -211,9 +211,14 @@ pfil_mbuf_common(pfil_chain_t *pch, struct mbuf **m, struct ifnet *ifp,
 	CK_STAILQ_FOREACH(link, pch, link_chain) {
 		rv = link->link_mbuf_chk(m, ifp, flags, link->link_ruleset,
 		    inp);
-		if (rv == PFIL_DROPPED || rv == PFIL_CONSUMED)
+		if (rv == PFIL_DROPPED || rv == PFIL_CONSUMED) {
+			MPASS(*m == NULL);
 			break;
+		} else {
+			MPASS(*m != NULL);
+		}
 	}
+
 	return (rv);
 }
 
diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c
index e501c15cb7e8..82d7acdd0710 100644
--- a/sys/netinet/ip_input.c
+++ b/sys/netinet/ip_input.c
@@ -621,8 +621,6 @@ tooshort:
 	if (pfil_mbuf_in(V_inet_pfil_head, &m, ifp, NULL) !=
 	    PFIL_PASS)
 		return;
-	if (m == NULL)			/* consumed by filter */
-		return;
 
 	ip = mtod(m, struct ip *);
 	dchg = (odst.s_addr != ip->ip_dst.s_addr);
@@ -827,8 +825,6 @@ ours:
 		if (pfil_mbuf_out(V_inet_local_pfil_head, &m, V_loif, NULL) !=
 		    PFIL_PASS)
 			return;
-		if (m == NULL)			/* consumed by filter */
-			return;
 		ip = mtod(m, struct ip *);
 	}
 
diff --git a/sys/netinet6/ip6_input.c b/sys/netinet6/ip6_input.c
index 5de8f49b5483..11b92c152a1a 100644
--- a/sys/netinet6/ip6_input.c
+++ b/sys/netinet6/ip6_input.c
@@ -894,8 +894,6 @@ passin:
 		if (pfil_mbuf_out(V_inet6_local_pfil_head, &m, V_loif, NULL) !=
 		    PFIL_PASS)
 			return;
-		if (m == NULL)			/* consumed by filter */
-			return;
 		ip6 = mtod(m, struct ip6_hdr *);
 	}
 
diff --git a/sys/netpfil/ipfilter/netinet/ip_fil_freebsd.c b/sys/netpfil/ipfilter/netinet/ip_fil_freebsd.c
index 1922880e90df..bcde0d2c7323 100644
--- a/sys/netpfil/ipfilter/netinet/ip_fil_freebsd.c
+++ b/sys/netpfil/ipfilter/netinet/ip_fil_freebsd.c
@@ -133,6 +133,8 @@ ipf_check_wrapper(struct mbuf **mp, struct ifnet *ifp, int flags,
 	rv = ipf_check(&V_ipfmain, ip, ip->ip_hl << 2, ifp,
 	    !!(flags & PFIL_OUT), mp);
 	CURVNET_RESTORE();
+	if (rv == 0 && *mp == NULL)
+		return (PFIL_CONSUMED);
 	return (rv == 0 ? PFIL_PASS : PFIL_DROPPED);
 }
 
@@ -147,6 +149,8 @@ ipf_check_wrapper6(struct mbuf **mp, struct ifnet *ifp, int flags,
 	rv = ipf_check(&V_ipfmain, mtod(*mp, struct ip *),
 	    sizeof(struct ip6_hdr), ifp, !!(flags & PFIL_OUT), mp);
 	CURVNET_RESTORE();
+	if (rv == 0 && *mp == NULL)
+		return (PFIL_CONSUMED);
 
 	return (rv == 0 ? PFIL_PASS : PFIL_DROPPED);
 }