aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKristof Provost <kp@FreeBSD.org>2025-01-06 06:56:24 +0000
committerKristof Provost <kp@FreeBSD.org>2025-01-17 16:00:08 +0000
commit5d28f4cab8d5919aba1365e885a91a96c0655b59 (patch)
treef5287b9b29c54df4edfe0a78a64825df919f0c0f
parentd355c28a0954de487e45c4349631c2b9449ba28d (diff)
pf: clean up mbuf passing for reassembly
When we call pf_normalize_ip() or pf_normalize_ip6() we passed the mbuf twice. Once as m0, and once inside the struct pf_pdesc. Remove the former to avoid confusion when we free *m0, but don't update pd->m. This could lead to use-after-free errors e.g. if reassembly failed. PR: 283705 Reported by: Yichen Chai <yichen.chai@gmail.com>, Zhuo Ying Jiang Li <zyj20@cl.cam.ac.uk> MFC after: 3 days Sponsored by: Rubicon Communications, LLC ("Netgate")
-rw-r--r--sys/net/pfvar.h4
-rw-r--r--sys/netpfil/pf/pf.c10
-rw-r--r--sys/netpfil/pf/pf_norm.c17
3 files changed, 13 insertions, 18 deletions
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index 6c9d75bb1365..6e9418f59aef 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -2390,11 +2390,11 @@ int pf_test(sa_family_t, int, int, struct ifnet *, struct mbuf **, struct inpcb
struct pf_rule_actions *);
#endif
#ifdef INET
-int pf_normalize_ip(struct mbuf **, u_short *, struct pf_pdesc *);
+int pf_normalize_ip(u_short *, struct pf_pdesc *);
#endif /* INET */
#ifdef INET6
-int pf_normalize_ip6(struct mbuf **, int, u_short *, struct pf_pdesc *);
+int pf_normalize_ip6(int, u_short *, struct pf_pdesc *);
void pf_poolmask(struct pf_addr *, struct pf_addr*,
struct pf_addr *, struct pf_addr *, sa_family_t);
void pf_addr_inc(struct pf_addr *, sa_family_t);
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 07b7aa543dbd..3b9c71296a2b 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -9918,12 +9918,13 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0,
return (-1);
}
- if (pf_normalize_ip(m0, reason, pd) != PF_PASS) {
+ if (pf_normalize_ip(reason, pd) != PF_PASS) {
/* We do IP header normalization and packet reassembly here */
+ *m0 = pd->m;
*action = PF_DROP;
return (-1);
}
- pd->m = *m0;
+ *m0 = pd->m;
h = mtod(pd->m, struct ip *);
pd->off = h->ip_hl << 2;
@@ -9994,12 +9995,13 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0,
}
/* We do IP header normalization and packet reassembly here */
- if (pf_normalize_ip6(m0, pd->fragoff, reason, pd) !=
+ if (pf_normalize_ip6(pd->fragoff, reason, pd) !=
PF_PASS) {
+ *m0 = pd->m;
*action = PF_DROP;
return (-1);
}
- pd->m = *m0;
+ *m0 = pd->m;
if (pd->m == NULL) {
/* packet sits in reassembly queue, no error */
*action = PF_PASS;
diff --git a/sys/netpfil/pf/pf_norm.c b/sys/netpfil/pf/pf_norm.c
index c9a7f7d2df04..39afde04a6d1 100644
--- a/sys/netpfil/pf/pf_norm.c
+++ b/sys/netpfil/pf/pf_norm.c
@@ -1093,11 +1093,10 @@ pf_refragment6(struct ifnet *ifp, struct mbuf **m0, struct m_tag *mtag,
#ifdef INET
int
-pf_normalize_ip(struct mbuf **m0, u_short *reason,
- struct pf_pdesc *pd)
+pf_normalize_ip(u_short *reason, struct pf_pdesc *pd)
{
struct pf_krule *r;
- struct ip *h = mtod(*m0, struct ip *);
+ struct ip *h = mtod(pd->m, struct ip *);
int mff = (ntohs(h->ip_off) & IP_MF);
int hlen = h->ip_hl << 2;
u_int16_t fragoff = (ntohs(h->ip_off) & IP_OFFMASK) << 3;
@@ -1109,8 +1108,6 @@ pf_normalize_ip(struct mbuf **m0, u_short *reason,
PF_RULES_RASSERT();
- MPASS(pd->m == *m0);
-
r = TAILQ_FIRST(pf_main_ruleset.rules[PF_RULESET_SCRUB].active.ptr);
/*
* Check if there are any scrub rules, matching or not.
@@ -1219,13 +1216,12 @@ pf_normalize_ip(struct mbuf **m0, u_short *reason,
* Might return a completely reassembled mbuf, or NULL */
PF_FRAG_LOCK();
DPFPRINTF(("reass frag %d @ %d-%d\n", h->ip_id, fragoff, max));
- verdict = pf_reassemble(m0, pd->dir, reason);
+ verdict = pf_reassemble(&pd->m, pd->dir, reason);
PF_FRAG_UNLOCK();
if (verdict != PF_PASS)
return (PF_DROP);
- pd->m = *m0;
if (pd->m == NULL)
return (PF_DROP);
@@ -1257,7 +1253,7 @@ pf_normalize_ip(struct mbuf **m0, u_short *reason,
#ifdef INET6
int
-pf_normalize_ip6(struct mbuf **m0, int off, u_short *reason,
+pf_normalize_ip6(int off, u_short *reason,
struct pf_pdesc *pd)
{
struct pf_krule *r;
@@ -1267,8 +1263,6 @@ pf_normalize_ip6(struct mbuf **m0, int off, u_short *reason,
PF_RULES_RASSERT();
- pd->m = *m0;
-
r = TAILQ_FIRST(pf_main_ruleset.rules[PF_RULESET_SCRUB].active.ptr);
/*
* Check if there are any scrub rules, matching or not.
@@ -1323,9 +1317,8 @@ pf_normalize_ip6(struct mbuf **m0, int off, u_short *reason,
if (pd->virtual_proto == PF_VPROTO_FRAGMENT) {
/* Returns PF_DROP or *m0 is NULL or completely reassembled
* mbuf. */
- if (pf_reassemble6(m0, &frag, off, pd->extoff, reason) != PF_PASS)
+ if (pf_reassemble6(&pd->m, &frag, off, pd->extoff, reason) != PF_PASS)
return (PF_DROP);
- pd->m = *m0;
if (pd->m == NULL)
return (PF_DROP);
h = mtod(pd->m, struct ip6_hdr *);