The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=ffeab76b68550c347abcd7c31c3b3dfcdea732b5
commit ffeab76b68550c347abcd7c31c3b3dfcdea732b5 Author: Kristof Provost <[email protected]> AuthorDate: 2024-01-26 12:29:31 +0000 Commit: Kristof Provost <[email protected]> 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); }
