git: 94804658ab04 - main - pf: Remove dead code in pf_pull_hdr().
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Wed, 10 Sep 2025 19:52:22 UTC
The branch main has been updated by kp:
URL: https://cgit.FreeBSD.org/src/commit/?id=94804658ab045fd386c2f031186c86f686c6870a
commit 94804658ab045fd386c2f031186c86f686c6870a
Author: Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2025-08-19 11:16:51 +0000
Commit: Kristof Provost <kp@FreeBSD.org>
CommitDate: 2025-09-10 19:51:39 +0000
pf: Remove dead code in pf_pull_hdr().
pf_pull_hdr() allows to pass an action pointer parameter as output
value. This is never used, all callers pass a NULL argument. Remove
ACTION_SET() entirely.
The logic (fragoff >= len) in pf_pull_hdr() does not work since
revision 1.4. Before it was used to drop short TCP or UDP fragments
that contained only part of the header. Current code in pf_pull_hdr()
drops the packets anyway, so always set reason PFRES_FRAG.
OK kn@ sashan@
Obtained from: OpenBSD, bluhm <bluhm@openbsd.org>, 46650f23db
Sponsored by: Rubicon Communications, LLC ("Netgate")
---
sys/net/pfvar.h | 2 +-
sys/netpfil/pf/pf.c | 74 ++++++++++++++++++++++--------------------------
sys/netpfil/pf/pf_norm.c | 12 ++++----
sys/netpfil/pf/pf_osfp.c | 2 +-
4 files changed, 42 insertions(+), 48 deletions(-)
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index f73420494000..e6fb1c2c3e1b 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -2423,7 +2423,7 @@ int pf_multihome_scan_init(int, int, struct pf_pdesc *);
int pf_multihome_scan_asconf(int, int, struct pf_pdesc *);
u_int32_t pf_new_isn(struct pf_kstate *);
-void *pf_pull_hdr(const struct mbuf *, int, void *, int, u_short *, u_short *,
+void *pf_pull_hdr(const struct mbuf *, int, void *, int, u_short *,
sa_family_t);
void pf_change_a(void *, u_int16_t *, u_int32_t, u_int8_t);
void pf_change_proto_a(struct mbuf *, void *, u_int16_t *, u_int32_t,
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 3a047ea44c47..5889bb9d68e6 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -1676,7 +1676,7 @@ pf_state_key_addr_setup(struct pf_pdesc *pd,
if (multi)
return (-1);
if (!pf_pull_hdr(pd->m, pd->off, &nd, sizeof(nd), NULL,
- NULL, pd->af))
+ pd->af))
return (-1);
target = (struct pf_addr *)&nd.nd_ns_target;
daddr = target;
@@ -1685,7 +1685,7 @@ pf_state_key_addr_setup(struct pf_pdesc *pd,
if (multi)
return (-1);
if (!pf_pull_hdr(pd->m, pd->off, &nd, sizeof(nd), NULL,
- NULL, pd->af))
+ pd->af))
return (-1);
target = (struct pf_addr *)&nd.nd_ns_target;
saddr = target;
@@ -4044,7 +4044,7 @@ pf_modulate_sack(struct pf_pdesc *pd, struct tcphdr *th,
optsoff = pd->off + sizeof(struct tcphdr);
#define TCPOLEN_MINSACK (TCPOLEN_SACK + 2)
if (olen < TCPOLEN_MINSACK ||
- !pf_pull_hdr(pd->m, optsoff, opts, olen, NULL, NULL, pd->af))
+ !pf_pull_hdr(pd->m, optsoff, opts, olen, NULL, pd->af))
return (0);
eoh = opts + olen;
@@ -5107,7 +5107,7 @@ pf_get_wscale(struct pf_pdesc *pd)
olen = (pd->hdr.tcp.th_off << 2) - sizeof(struct tcphdr);
if (olen < TCPOLEN_WINDOW || !pf_pull_hdr(pd->m,
- pd->off + sizeof(struct tcphdr), opts, olen, NULL, NULL, pd->af))
+ pd->off + sizeof(struct tcphdr), opts, olen, NULL, pd->af))
return (0);
opt = opts;
@@ -5132,7 +5132,7 @@ pf_get_mss(struct pf_pdesc *pd)
olen = (pd->hdr.tcp.th_off << 2) - sizeof(struct tcphdr);
if (olen < TCPOLEN_MAXSEG || !pf_pull_hdr(pd->m,
- pd->off + sizeof(struct tcphdr), opts, olen, NULL, NULL, pd->af))
+ pd->off + sizeof(struct tcphdr), opts, olen, NULL, pd->af))
return (0);
opt = opts;
@@ -7660,7 +7660,7 @@ pf_multihome_scan(int start, int len, struct pf_pdesc *pd, int op)
while (off < len) {
struct sctp_paramhdr h;
- if (!pf_pull_hdr(pd->m, start + off, &h, sizeof(h), NULL, NULL,
+ if (!pf_pull_hdr(pd->m, start + off, &h, sizeof(h), NULL,
pd->af))
return (PF_DROP);
@@ -7680,7 +7680,7 @@ pf_multihome_scan(int start, int len, struct pf_pdesc *pd, int op)
return (PF_DROP);
if (!pf_pull_hdr(pd->m, start + off + sizeof(h), &t, sizeof(t),
- NULL, NULL, pd->af))
+ NULL, pd->af))
return (PF_DROP);
if (in_nullhost(t))
@@ -7724,7 +7724,7 @@ pf_multihome_scan(int start, int len, struct pf_pdesc *pd, int op)
return (PF_DROP);
if (!pf_pull_hdr(pd->m, start + off + sizeof(h), &t, sizeof(t),
- NULL, NULL, pd->af))
+ NULL, pd->af))
return (PF_DROP);
if (memcmp(&t, &pd->src->v6, sizeof(t)) == 0)
break;
@@ -7754,7 +7754,7 @@ pf_multihome_scan(int start, int len, struct pf_pdesc *pd, int op)
struct sctp_asconf_paramhdr ah;
if (!pf_pull_hdr(pd->m, start + off, &ah, sizeof(ah),
- NULL, NULL, pd->af))
+ NULL, pd->af))
return (PF_DROP);
ret = pf_multihome_scan(start + off + sizeof(ah),
@@ -7769,7 +7769,7 @@ pf_multihome_scan(int start, int len, struct pf_pdesc *pd, int op)
struct sctp_asconf_paramhdr ah;
if (!pf_pull_hdr(pd->m, start + off, &ah, sizeof(ah),
- NULL, NULL, pd->af))
+ NULL, pd->af))
return (PF_DROP);
ret = pf_multihome_scan(start + off + sizeof(ah),
ntohs(ah.ph.param_length) - sizeof(ah), pd,
@@ -8051,7 +8051,7 @@ pf_test_state_icmp(struct pf_kstate **state, struct pf_pdesc *pd,
ipoff2 = pd->off + ICMP_MINLEN;
if (!pf_pull_hdr(pd->m, ipoff2, &h2, sizeof(h2),
- NULL, reason, pd2.af)) {
+ reason, pd2.af)) {
DPFPRINTF(PF_DEBUG_MISC,
"pf: ICMP error message too short "
"(ip)");
@@ -8083,7 +8083,7 @@ pf_test_state_icmp(struct pf_kstate **state, struct pf_pdesc *pd,
ipoff2 = pd->off + sizeof(struct icmp6_hdr);
if (!pf_pull_hdr(pd->m, ipoff2, &h2_6, sizeof(h2_6),
- NULL, reason, pd2.af)) {
+ reason, pd2.af)) {
DPFPRINTF(PF_DEBUG_MISC,
"pf: ICMP error message too short "
"(ip6)");
@@ -8136,7 +8136,7 @@ pf_test_state_icmp(struct pf_kstate **state, struct pf_pdesc *pd,
* expected. Don't access any TCP header fields after
* th_seq, an ackskew test is not possible.
*/
- if (!pf_pull_hdr(pd->m, pd2.off, th, 8, NULL, reason,
+ if (!pf_pull_hdr(pd->m, pd2.off, th, 8, reason,
pd2.af)) {
DPFPRINTF(PF_DEBUG_MISC,
"pf: ICMP error message too short "
@@ -8332,7 +8332,7 @@ pf_test_state_icmp(struct pf_kstate **state, struct pf_pdesc *pd,
int action;
if (!pf_pull_hdr(pd->m, pd2.off, uh, sizeof(*uh),
- NULL, reason, pd2.af)) {
+ reason, pd2.af)) {
DPFPRINTF(PF_DEBUG_MISC,
"pf: ICMP error message too short "
"(udp)");
@@ -8463,7 +8463,7 @@ pf_test_state_icmp(struct pf_kstate **state, struct pf_pdesc *pd,
int copyback = 0;
int action;
- if (! pf_pull_hdr(pd->m, pd2.off, sh, sizeof(*sh), NULL, reason,
+ if (! pf_pull_hdr(pd->m, pd2.off, sh, sizeof(*sh), reason,
pd2.af)) {
DPFPRINTF(PF_DEBUG_MISC,
"pf: ICMP error message too short "
@@ -8619,7 +8619,7 @@ pf_test_state_icmp(struct pf_kstate **state, struct pf_pdesc *pd,
}
if (!pf_pull_hdr(pd->m, pd2.off, iih, ICMP_MINLEN,
- NULL, reason, pd2.af)) {
+ reason, pd2.af)) {
DPFPRINTF(PF_DEBUG_MISC,
"pf: ICMP error message too short i"
"(icmp)");
@@ -8739,7 +8739,7 @@ pf_test_state_icmp(struct pf_kstate **state, struct pf_pdesc *pd,
}
if (!pf_pull_hdr(pd->m, pd2.off, iih,
- sizeof(struct icmp6_hdr), NULL, reason, pd2.af)) {
+ sizeof(struct icmp6_hdr), reason, pd2.af)) {
DPFPRINTF(PF_DEBUG_MISC,
"pf: ICMP error message too short "
"(icmp6)");
@@ -8921,7 +8921,7 @@ pf_test_state_icmp(struct pf_kstate **state, struct pf_pdesc *pd,
*/
void *
pf_pull_hdr(const struct mbuf *m, int off, void *p, int len,
- u_short *actionp, u_short *reasonp, sa_family_t af)
+ u_short *reasonp, sa_family_t af)
{
int iplen = 0;
switch (af) {
@@ -8931,12 +8931,7 @@ pf_pull_hdr(const struct mbuf *m, int off, void *p, int len,
u_int16_t fragoff = (ntohs(h->ip_off) & IP_OFFMASK) << 3;
if (fragoff) {
- if (fragoff >= len)
- ACTION_SET(actionp, PF_PASS);
- else {
- ACTION_SET(actionp, PF_DROP);
- REASON_SET(reasonp, PFRES_FRAG);
- }
+ REASON_SET(reasonp, PFRES_FRAG);
return (NULL);
}
iplen = ntohs(h->ip_len);
@@ -8953,7 +8948,6 @@ pf_pull_hdr(const struct mbuf *m, int off, void *p, int len,
#endif /* INET6 */
}
if (m->m_pkthdr.len < off + len || iplen < off + len) {
- ACTION_SET(actionp, PF_DROP);
REASON_SET(reasonp, PFRES_SHORT);
return (NULL);
}
@@ -10052,7 +10046,7 @@ pf_walk_header(struct pf_pdesc *pd, struct ip *h, u_short *reason)
end < pd->off + sizeof(ext))
return (PF_PASS);
if (!pf_pull_hdr(pd->m, pd->off, &ext, sizeof(ext),
- NULL, reason, AF_INET)) {
+ reason, AF_INET)) {
DPFPRINTF(PF_DEBUG_MISC, "IP short exthdr");
return (PF_DROP);
}
@@ -10078,7 +10072,7 @@ pf_walk_option6(struct pf_pdesc *pd, struct ip6_hdr *h, int off, int end,
while (off < end) {
if (!pf_pull_hdr(pd->m, off, &opt.ip6o_type,
- sizeof(opt.ip6o_type), NULL, reason, AF_INET6)) {
+ sizeof(opt.ip6o_type), reason, AF_INET6)) {
DPFPRINTF(PF_DEBUG_MISC, "IPv6 short opt type");
return (PF_DROP);
}
@@ -10086,7 +10080,7 @@ pf_walk_option6(struct pf_pdesc *pd, struct ip6_hdr *h, int off, int end,
off++;
continue;
}
- if (!pf_pull_hdr(pd->m, off, &opt, sizeof(opt), NULL,
+ if (!pf_pull_hdr(pd->m, off, &opt, sizeof(opt),
reason, AF_INET6)) {
DPFPRINTF(PF_DEBUG_MISC, "IPv6 short opt");
return (PF_DROP);
@@ -10111,7 +10105,7 @@ pf_walk_option6(struct pf_pdesc *pd, struct ip6_hdr *h, int off, int end,
REASON_SET(reason, PFRES_IPOPTIONS);
return (PF_DROP);
}
- if (!pf_pull_hdr(pd->m, off, &jumbo, sizeof(jumbo), NULL,
+ if (!pf_pull_hdr(pd->m, off, &jumbo, sizeof(jumbo),
reason, AF_INET6)) {
DPFPRINTF(PF_DEBUG_MISC, "IPv6 short jumbo");
return (PF_DROP);
@@ -10160,7 +10154,7 @@ pf_walk_header6(struct pf_pdesc *pd, struct ip6_hdr *h, u_short *reason)
break;
case IPPROTO_HOPOPTS:
if (!pf_pull_hdr(pd->m, pd->off, &ext, sizeof(ext),
- NULL, reason, AF_INET6)) {
+ reason, AF_INET6)) {
DPFPRINTF(PF_DEBUG_MISC, "IPv6 short exthdr");
return (PF_DROP);
}
@@ -10187,7 +10181,7 @@ pf_walk_header6(struct pf_pdesc *pd, struct ip6_hdr *h, u_short *reason)
return (PF_DROP);
}
if (!pf_pull_hdr(pd->m, pd->off, &frag, sizeof(frag),
- NULL, reason, AF_INET6)) {
+ reason, AF_INET6)) {
DPFPRINTF(PF_DEBUG_MISC, "IPv6 short fragment");
return (PF_DROP);
}
@@ -10215,7 +10209,7 @@ pf_walk_header6(struct pf_pdesc *pd, struct ip6_hdr *h, u_short *reason)
return (PF_PASS);
}
if (!pf_pull_hdr(pd->m, pd->off, &rthdr, sizeof(rthdr),
- NULL, reason, AF_INET6)) {
+ reason, AF_INET6)) {
DPFPRINTF(PF_DEBUG_MISC, "IPv6 short rthdr");
return (PF_DROP);
}
@@ -10236,7 +10230,7 @@ pf_walk_header6(struct pf_pdesc *pd, struct ip6_hdr *h, u_short *reason)
case IPPROTO_AH:
case IPPROTO_DSTOPTS:
if (!pf_pull_hdr(pd->m, pd->off, &ext, sizeof(ext),
- NULL, reason, AF_INET6)) {
+ reason, AF_INET6)) {
DPFPRINTF(PF_DEBUG_MISC, "IPv6 short exthdr");
return (PF_DROP);
}
@@ -10269,7 +10263,7 @@ pf_walk_header6(struct pf_pdesc *pd, struct ip6_hdr *h, u_short *reason)
return (PF_PASS);
}
if (!pf_pull_hdr(pd->m, pd->off, &icmp6, sizeof(icmp6),
- NULL, reason, AF_INET6)) {
+ reason, AF_INET6)) {
DPFPRINTF(PF_DEBUG_MISC,
"IPv6 short icmp6hdr");
return (PF_DROP);
@@ -10502,7 +10496,7 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0,
case IPPROTO_TCP: {
struct tcphdr *th = &pd->hdr.tcp;
- if (!pf_pull_hdr(pd->m, pd->off, th, sizeof(*th), action,
+ if (!pf_pull_hdr(pd->m, pd->off, th, sizeof(*th),
reason, af)) {
*action = PF_DROP;
REASON_SET(reason, PFRES_SHORT);
@@ -10518,7 +10512,7 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0,
case IPPROTO_UDP: {
struct udphdr *uh = &pd->hdr.udp;
- if (!pf_pull_hdr(pd->m, pd->off, uh, sizeof(*uh), action,
+ if (!pf_pull_hdr(pd->m, pd->off, uh, sizeof(*uh),
reason, af)) {
*action = PF_DROP;
REASON_SET(reason, PFRES_SHORT);
@@ -10539,7 +10533,7 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0,
}
case IPPROTO_SCTP: {
if (!pf_pull_hdr(pd->m, pd->off, &pd->hdr.sctp, sizeof(pd->hdr.sctp),
- action, reason, af)) {
+ reason, af)) {
*action = PF_DROP;
REASON_SET(reason, PFRES_SHORT);
return (-1);
@@ -10569,7 +10563,7 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0,
}
case IPPROTO_ICMP: {
if (!pf_pull_hdr(pd->m, pd->off, &pd->hdr.icmp, ICMP_MINLEN,
- action, reason, af)) {
+ reason, af)) {
*action = PF_DROP;
REASON_SET(reason, PFRES_SHORT);
return (-1);
@@ -10583,7 +10577,7 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0,
size_t icmp_hlen = sizeof(struct icmp6_hdr);
if (!pf_pull_hdr(pd->m, pd->off, &pd->hdr.icmp6, icmp_hlen,
- action, reason, af)) {
+ reason, af)) {
*action = PF_DROP;
REASON_SET(reason, PFRES_SHORT);
return (-1);
@@ -10609,7 +10603,7 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0,
}
if (icmp_hlen > sizeof(struct icmp6_hdr) &&
!pf_pull_hdr(pd->m, pd->off, &pd->hdr.icmp6, icmp_hlen,
- action, reason, af)) {
+ reason, af)) {
*action = PF_DROP;
REASON_SET(reason, PFRES_SHORT);
return (-1);
diff --git a/sys/netpfil/pf/pf_norm.c b/sys/netpfil/pf/pf_norm.c
index a684d778ab42..56074bedbc40 100644
--- a/sys/netpfil/pf/pf_norm.c
+++ b/sys/netpfil/pf/pf_norm.c
@@ -1354,7 +1354,7 @@ pf_normalize_ip6(int off, u_short *reason,
pf_rule_to_actions(r, &pd->act);
}
- if (!pf_pull_hdr(pd->m, off, &frag, sizeof(frag), NULL, reason, AF_INET6))
+ if (!pf_pull_hdr(pd->m, off, &frag, sizeof(frag), reason, AF_INET6))
return (PF_DROP);
/* Offset now points to data portion. */
@@ -1542,7 +1542,7 @@ pf_normalize_tcp_init(struct pf_pdesc *pd, struct tcphdr *th,
olen = (th->th_off << 2) - sizeof(*th);
if (olen < TCPOLEN_TIMESTAMP || !pf_pull_hdr(pd->m,
- pd->off + sizeof(*th), opts, olen, NULL, NULL, pd->af))
+ pd->off + sizeof(*th), opts, olen, NULL, pd->af))
return (0);
opt = opts;
@@ -1645,7 +1645,7 @@ pf_normalize_tcp_stateful(struct pf_pdesc *pd,
if (olen >= TCPOLEN_TIMESTAMP &&
((src->scrub && (src->scrub->pfss_flags & PFSS_TIMESTAMP)) ||
(dst->scrub && (dst->scrub->pfss_flags & PFSS_TIMESTAMP))) &&
- pf_pull_hdr(pd->m, pd->off + sizeof(*th), opts, olen, NULL, NULL, pd->af)) {
+ pf_pull_hdr(pd->m, pd->off + sizeof(*th), opts, olen, NULL, pd->af)) {
/* Modulate the timestamps. Can be used for NAT detection, OS
* uptime determination or reboot detection.
*/
@@ -1975,7 +1975,7 @@ pf_normalize_mss(struct pf_pdesc *pd)
olen = (pd->hdr.tcp.th_off << 2) - sizeof(struct tcphdr);
optsoff = pd->off + sizeof(struct tcphdr);
if (olen < TCPOLEN_MAXSEG ||
- !pf_pull_hdr(pd->m, optsoff, opts, olen, NULL, NULL, pd->af))
+ !pf_pull_hdr(pd->m, optsoff, opts, olen, NULL, pd->af))
return (0);
opt = opts;
@@ -2009,7 +2009,7 @@ pf_scan_sctp(struct pf_pdesc *pd)
int ret;
while (pd->off + chunk_off < pd->tot_len) {
- if (!pf_pull_hdr(pd->m, pd->off + chunk_off, &ch, sizeof(ch), NULL,
+ if (!pf_pull_hdr(pd->m, pd->off + chunk_off, &ch, sizeof(ch),
NULL, pd->af))
return (PF_DROP);
@@ -2026,7 +2026,7 @@ pf_scan_sctp(struct pf_pdesc *pd)
struct sctp_init_chunk init;
if (!pf_pull_hdr(pd->m, pd->off + chunk_start, &init,
- sizeof(init), NULL, NULL, pd->af))
+ sizeof(init), NULL, pd->af))
return (PF_DROP);
/*
diff --git a/sys/netpfil/pf/pf_osfp.c b/sys/netpfil/pf/pf_osfp.c
index 150626c5f3fb..8c041d45eae8 100644
--- a/sys/netpfil/pf/pf_osfp.c
+++ b/sys/netpfil/pf/pf_osfp.c
@@ -82,7 +82,7 @@ pf_osfp_fingerprint(struct pf_pdesc *pd, const struct tcphdr *tcp)
ip6 = mtod(pd->m, struct ip6_hdr *);
break;
}
- if (!pf_pull_hdr(pd->m, pd->off, hdr, tcp->th_off << 2, NULL, NULL,
+ if (!pf_pull_hdr(pd->m, pd->off, hdr, tcp->th_off << 2, NULL,
pd->af)) return (NULL);
return (pf_osfp_fingerprint_hdr(ip, ip6, (struct tcphdr *)hdr));