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
This commit is contained in:
Kristof Provost 2024-01-26 13:29:31 +01:00
parent 0b3f9e435f
commit ffeab76b68
5 changed files with 11 additions and 8 deletions

View file

@ -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;
}

View file

@ -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);
}

View file

@ -621,8 +621,6 @@ ip_input(struct mbuf *m)
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 @@ ip_input(struct mbuf *m)
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 *);
}

View file

@ -894,8 +894,6 @@ ip6_input(struct mbuf *m)
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 *);
}

View file

@ -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);
}