git: 674701e2b6d7 - main - pf: Remove some state pointer indirection
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Sat, 29 Mar 2025 08:53:19 UTC
The branch main has been updated by markj:
URL: https://cgit.FreeBSD.org/src/commit/?id=674701e2b6d74b5604ec00961333c3ab33f6ec9c
commit 674701e2b6d74b5604ec00961333c3ab33f6ec9c
Author: Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2025-03-29 08:51:38 +0000
Commit: Mark Johnston <markj@FreeBSD.org>
CommitDate: 2025-03-29 08:52:25 +0000
pf: Remove some state pointer indirection
Several subroutines take a pointer to a pointer to a pf state, but never
modify the input pointer. As in commit 9f9cf83f114a, let's remove the
indirection, making the code easier to read. No functional change
intended.
Reviewed by: kp, glebius
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D49519
---
sys/netpfil/pf/pf.c | 206 ++++++++++++++++++++++++++--------------------------
1 file changed, 103 insertions(+), 103 deletions(-)
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 002fff9b81be..d3c857a66b85 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -350,11 +350,11 @@ static int pf_create_state(struct pf_krule *, struct pf_krule *,
struct pf_krule_slist *, struct pf_udp_mapping *);
static int pf_state_key_addr_setup(struct pf_pdesc *,
struct pf_state_key_cmp *, int);
-static int pf_tcp_track_full(struct pf_kstate **,
+static int pf_tcp_track_full(struct pf_kstate *,
struct pf_pdesc *, u_short *, int *,
struct pf_state_peer *, struct pf_state_peer *,
u_int8_t, u_int8_t);
-static int pf_tcp_track_sloppy(struct pf_kstate **,
+static int pf_tcp_track_sloppy(struct pf_kstate *,
struct pf_pdesc *, u_short *,
struct pf_state_peer *, struct pf_state_peer *,
u_int8_t, u_int8_t);
@@ -6419,7 +6419,7 @@ pf_translate(struct pf_pdesc *pd, struct pf_addr *saddr, u_int16_t sport,
}
static int
-pf_tcp_track_full(struct pf_kstate **state, struct pf_pdesc *pd,
+pf_tcp_track_full(struct pf_kstate *state, struct pf_pdesc *pd,
u_short *reason, int *copyback, struct pf_state_peer *src,
struct pf_state_peer *dst, u_int8_t psrc, u_int8_t pdst)
{
@@ -6445,7 +6445,7 @@ pf_tcp_track_full(struct pf_kstate **state, struct pf_pdesc *pd,
if (src->seqlo == 0) {
/* First packet from this end. Set its state */
- if (((*state)->state_flags & PFSTATE_SCRUB_TCP || dst->scrub) &&
+ if ((state->state_flags & PFSTATE_SCRUB_TCP || dst->scrub) &&
src->scrub == NULL) {
if (pf_normalize_tcp_init(pd, th, src, dst)) {
REASON_SET(reason, PFRES_MEMORY);
@@ -6495,7 +6495,7 @@ pf_tcp_track_full(struct pf_kstate **state, struct pf_pdesc *pd,
src->seqlo = seq;
if (src->state < TCPS_SYN_SENT)
- pf_set_protostate(*state, psrc, TCPS_SYN_SENT);
+ pf_set_protostate(state, psrc, TCPS_SYN_SENT);
/*
* May need to slide the window (seqhi may have been set by
@@ -6579,7 +6579,7 @@ pf_tcp_track_full(struct pf_kstate **state, struct pf_pdesc *pd,
if (dst->scrub || src->scrub) {
if (pf_normalize_tcp_stateful(pd, reason, th,
- *state, src, dst, copyback))
+ state, src, dst, copyback))
return (PF_DROP);
}
@@ -6596,43 +6596,43 @@ pf_tcp_track_full(struct pf_kstate **state, struct pf_pdesc *pd,
/* update states */
if (tcp_get_flags(th) & TH_SYN)
if (src->state < TCPS_SYN_SENT)
- pf_set_protostate(*state, psrc, TCPS_SYN_SENT);
+ pf_set_protostate(state, psrc, TCPS_SYN_SENT);
if (tcp_get_flags(th) & TH_FIN)
if (src->state < TCPS_CLOSING)
- pf_set_protostate(*state, psrc, TCPS_CLOSING);
+ pf_set_protostate(state, psrc, TCPS_CLOSING);
if (tcp_get_flags(th) & TH_ACK) {
if (dst->state == TCPS_SYN_SENT) {
- pf_set_protostate(*state, pdst,
+ pf_set_protostate(state, pdst,
TCPS_ESTABLISHED);
if (src->state == TCPS_ESTABLISHED &&
- (*state)->sns[PF_SN_LIMIT] != NULL &&
- pf_src_connlimit(*state)) {
+ state->sns[PF_SN_LIMIT] != NULL &&
+ pf_src_connlimit(state)) {
REASON_SET(reason, PFRES_SRCLIMIT);
return (PF_DROP);
}
} else if (dst->state == TCPS_CLOSING)
- pf_set_protostate(*state, pdst,
+ pf_set_protostate(state, pdst,
TCPS_FIN_WAIT_2);
}
if (tcp_get_flags(th) & TH_RST)
- pf_set_protostate(*state, PF_PEER_BOTH, TCPS_TIME_WAIT);
+ pf_set_protostate(state, PF_PEER_BOTH, TCPS_TIME_WAIT);
/* update expire time */
- (*state)->expire = pf_get_uptime();
+ state->expire = pf_get_uptime();
if (src->state >= TCPS_FIN_WAIT_2 &&
dst->state >= TCPS_FIN_WAIT_2)
- (*state)->timeout = PFTM_TCP_CLOSED;
+ state->timeout = PFTM_TCP_CLOSED;
else if (src->state >= TCPS_CLOSING &&
dst->state >= TCPS_CLOSING)
- (*state)->timeout = PFTM_TCP_FIN_WAIT;
+ state->timeout = PFTM_TCP_FIN_WAIT;
else if (src->state < TCPS_ESTABLISHED ||
dst->state < TCPS_ESTABLISHED)
- (*state)->timeout = PFTM_TCP_OPENING;
+ state->timeout = PFTM_TCP_OPENING;
else if (src->state >= TCPS_CLOSING ||
dst->state >= TCPS_CLOSING)
- (*state)->timeout = PFTM_TCP_CLOSING;
+ state->timeout = PFTM_TCP_CLOSING;
else
- (*state)->timeout = PFTM_TCP_ESTABLISHED;
+ state->timeout = PFTM_TCP_ESTABLISHED;
/* Fall through to PASS packet */
@@ -6667,19 +6667,19 @@ pf_tcp_track_full(struct pf_kstate **state, struct pf_pdesc *pd,
if (V_pf_status.debug >= PF_DEBUG_MISC) {
printf("pf: loose state match: ");
- pf_print_state(*state);
+ pf_print_state(state);
pf_print_flags(tcp_get_flags(th));
printf(" seq=%u (%u) ack=%u len=%u ackskew=%d "
"pkts=%llu:%llu dir=%s,%s\n", seq, orig_seq, ack,
- pd->p_len, ackskew, (unsigned long long)(*state)->packets[0],
- (unsigned long long)(*state)->packets[1],
+ pd->p_len, ackskew, (unsigned long long)state->packets[0],
+ (unsigned long long)state->packets[1],
pd->dir == PF_IN ? "in" : "out",
- pd->dir == (*state)->direction ? "fwd" : "rev");
+ pd->dir == state->direction ? "fwd" : "rev");
}
if (dst->scrub || src->scrub) {
if (pf_normalize_tcp_stateful(pd, reason, th,
- *state, src, dst, copyback))
+ state, src, dst, copyback))
return (PF_DROP);
}
@@ -6700,37 +6700,37 @@ pf_tcp_track_full(struct pf_kstate **state, struct pf_pdesc *pd,
if (tcp_get_flags(th) & TH_FIN)
if (src->state < TCPS_CLOSING)
- pf_set_protostate(*state, psrc, TCPS_CLOSING);
+ pf_set_protostate(state, psrc, TCPS_CLOSING);
if (tcp_get_flags(th) & TH_RST)
- pf_set_protostate(*state, PF_PEER_BOTH, TCPS_TIME_WAIT);
+ pf_set_protostate(state, PF_PEER_BOTH, TCPS_TIME_WAIT);
/* Fall through to PASS packet */
} else {
- if ((*state)->dst.state == TCPS_SYN_SENT &&
- (*state)->src.state == TCPS_SYN_SENT) {
+ if (state->dst.state == TCPS_SYN_SENT &&
+ state->src.state == TCPS_SYN_SENT) {
/* Send RST for state mismatches during handshake */
if (!(tcp_get_flags(th) & TH_RST))
- pf_send_tcp((*state)->rule, pd->af,
+ pf_send_tcp(state->rule, pd->af,
pd->dst, pd->src, th->th_dport,
th->th_sport, ntohl(th->th_ack), 0,
TH_RST, 0, 0,
- (*state)->rule->return_ttl, M_SKIP_FIREWALL,
- 0, 0, (*state)->act.rtableid);
+ state->rule->return_ttl, M_SKIP_FIREWALL,
+ 0, 0, state->act.rtableid);
src->seqlo = 0;
src->seqhi = 1;
src->max_win = 1;
} else if (V_pf_status.debug >= PF_DEBUG_MISC) {
printf("pf: BAD state: ");
- pf_print_state(*state);
+ pf_print_state(state);
pf_print_flags(tcp_get_flags(th));
printf(" seq=%u (%u) ack=%u len=%u ackskew=%d "
"pkts=%llu:%llu dir=%s,%s\n",
seq, orig_seq, ack, pd->p_len, ackskew,
- (unsigned long long)(*state)->packets[0],
- (unsigned long long)(*state)->packets[1],
+ (unsigned long long)state->packets[0],
+ (unsigned long long)state->packets[1],
pd->dir == PF_IN ? "in" : "out",
- pd->dir == (*state)->direction ? "fwd" : "rev");
+ pd->dir == state->direction ? "fwd" : "rev");
printf("pf: State failure on: %c %c %c %c | %c %c\n",
SEQ_GEQ(src->seqhi, data_end) ? ' ' : '1',
SEQ_GEQ(seq, src->seqlo - (dst->max_win << dws)) ?
@@ -6748,7 +6748,7 @@ pf_tcp_track_full(struct pf_kstate **state, struct pf_pdesc *pd,
}
static int
-pf_tcp_track_sloppy(struct pf_kstate **state, struct pf_pdesc *pd,
+pf_tcp_track_sloppy(struct pf_kstate *state, struct pf_pdesc *pd,
u_short *reason, struct pf_state_peer *src, struct pf_state_peer *dst,
u_int8_t psrc, u_int8_t pdst)
{
@@ -6756,21 +6756,21 @@ pf_tcp_track_sloppy(struct pf_kstate **state, struct pf_pdesc *pd,
if (tcp_get_flags(th) & TH_SYN)
if (src->state < TCPS_SYN_SENT)
- pf_set_protostate(*state, psrc, TCPS_SYN_SENT);
+ pf_set_protostate(state, psrc, TCPS_SYN_SENT);
if (tcp_get_flags(th) & TH_FIN)
if (src->state < TCPS_CLOSING)
- pf_set_protostate(*state, psrc, TCPS_CLOSING);
+ pf_set_protostate(state, psrc, TCPS_CLOSING);
if (tcp_get_flags(th) & TH_ACK) {
if (dst->state == TCPS_SYN_SENT) {
- pf_set_protostate(*state, pdst, TCPS_ESTABLISHED);
+ pf_set_protostate(state, pdst, TCPS_ESTABLISHED);
if (src->state == TCPS_ESTABLISHED &&
- (*state)->sns[PF_SN_LIMIT] != NULL &&
- pf_src_connlimit(*state)) {
+ state->sns[PF_SN_LIMIT] != NULL &&
+ pf_src_connlimit(state)) {
REASON_SET(reason, PFRES_SRCLIMIT);
return (PF_DROP);
}
} else if (dst->state == TCPS_CLOSING) {
- pf_set_protostate(*state, pdst, TCPS_FIN_WAIT_2);
+ pf_set_protostate(state, pdst, TCPS_FIN_WAIT_2);
} else if (src->state == TCPS_SYN_SENT &&
dst->state < TCPS_SYN_SENT) {
/*
@@ -6779,11 +6779,11 @@ pf_tcp_track_sloppy(struct pf_kstate **state, struct pf_pdesc *pd,
* the initial SYN without ever seeing a packet from
* the destination, set the connection to established.
*/
- pf_set_protostate(*state, PF_PEER_BOTH,
+ pf_set_protostate(state, PF_PEER_BOTH,
TCPS_ESTABLISHED);
dst->state = src->state = TCPS_ESTABLISHED;
- if ((*state)->sns[PF_SN_LIMIT] != NULL &&
- pf_src_connlimit(*state)) {
+ if (state->sns[PF_SN_LIMIT] != NULL &&
+ pf_src_connlimit(state)) {
REASON_SET(reason, PFRES_SRCLIMIT);
return (PF_DROP);
}
@@ -6795,117 +6795,117 @@ pf_tcp_track_sloppy(struct pf_kstate **state, struct pf_pdesc *pd,
* don't see the full bidirectional FIN/ACK+ACK
* handshake.
*/
- pf_set_protostate(*state, pdst, TCPS_CLOSING);
+ pf_set_protostate(state, pdst, TCPS_CLOSING);
}
}
if (tcp_get_flags(th) & TH_RST)
- pf_set_protostate(*state, PF_PEER_BOTH, TCPS_TIME_WAIT);
+ pf_set_protostate(state, PF_PEER_BOTH, TCPS_TIME_WAIT);
/* update expire time */
- (*state)->expire = pf_get_uptime();
+ state->expire = pf_get_uptime();
if (src->state >= TCPS_FIN_WAIT_2 &&
dst->state >= TCPS_FIN_WAIT_2)
- (*state)->timeout = PFTM_TCP_CLOSED;
+ state->timeout = PFTM_TCP_CLOSED;
else if (src->state >= TCPS_CLOSING &&
dst->state >= TCPS_CLOSING)
- (*state)->timeout = PFTM_TCP_FIN_WAIT;
+ state->timeout = PFTM_TCP_FIN_WAIT;
else if (src->state < TCPS_ESTABLISHED ||
dst->state < TCPS_ESTABLISHED)
- (*state)->timeout = PFTM_TCP_OPENING;
+ state->timeout = PFTM_TCP_OPENING;
else if (src->state >= TCPS_CLOSING ||
dst->state >= TCPS_CLOSING)
- (*state)->timeout = PFTM_TCP_CLOSING;
+ state->timeout = PFTM_TCP_CLOSING;
else
- (*state)->timeout = PFTM_TCP_ESTABLISHED;
+ state->timeout = PFTM_TCP_ESTABLISHED;
return (PF_PASS);
}
static int
-pf_synproxy(struct pf_pdesc *pd, struct pf_kstate **state, u_short *reason)
+pf_synproxy(struct pf_pdesc *pd, struct pf_kstate *state, u_short *reason)
{
- struct pf_state_key *sk = (*state)->key[pd->didx];
+ struct pf_state_key *sk = state->key[pd->didx];
struct tcphdr *th = &pd->hdr.tcp;
- if ((*state)->src.state == PF_TCPS_PROXY_SRC) {
- if (pd->dir != (*state)->direction) {
+ if (state->src.state == PF_TCPS_PROXY_SRC) {
+ if (pd->dir != state->direction) {
REASON_SET(reason, PFRES_SYNPROXY);
return (PF_SYNPROXY_DROP);
}
if (tcp_get_flags(th) & TH_SYN) {
- if (ntohl(th->th_seq) != (*state)->src.seqlo) {
+ if (ntohl(th->th_seq) != state->src.seqlo) {
REASON_SET(reason, PFRES_SYNPROXY);
return (PF_DROP);
}
- pf_send_tcp((*state)->rule, pd->af, pd->dst,
+ pf_send_tcp(state->rule, pd->af, pd->dst,
pd->src, th->th_dport, th->th_sport,
- (*state)->src.seqhi, ntohl(th->th_seq) + 1,
- TH_SYN|TH_ACK, 0, (*state)->src.mss, 0,
- M_SKIP_FIREWALL, 0, 0, (*state)->act.rtableid);
+ state->src.seqhi, ntohl(th->th_seq) + 1,
+ TH_SYN|TH_ACK, 0, state->src.mss, 0,
+ M_SKIP_FIREWALL, 0, 0, state->act.rtableid);
REASON_SET(reason, PFRES_SYNPROXY);
return (PF_SYNPROXY_DROP);
} else if ((tcp_get_flags(th) & (TH_ACK|TH_RST|TH_FIN)) != TH_ACK ||
- (ntohl(th->th_ack) != (*state)->src.seqhi + 1) ||
- (ntohl(th->th_seq) != (*state)->src.seqlo + 1)) {
+ (ntohl(th->th_ack) != state->src.seqhi + 1) ||
+ (ntohl(th->th_seq) != state->src.seqlo + 1)) {
REASON_SET(reason, PFRES_SYNPROXY);
return (PF_DROP);
- } else if ((*state)->sns[PF_SN_LIMIT] != NULL &&
- pf_src_connlimit(*state)) {
+ } else if (state->sns[PF_SN_LIMIT] != NULL &&
+ pf_src_connlimit(state)) {
REASON_SET(reason, PFRES_SRCLIMIT);
return (PF_DROP);
} else
- pf_set_protostate(*state, PF_PEER_SRC,
+ pf_set_protostate(state, PF_PEER_SRC,
PF_TCPS_PROXY_DST);
}
- if ((*state)->src.state == PF_TCPS_PROXY_DST) {
- if (pd->dir == (*state)->direction) {
+ if (state->src.state == PF_TCPS_PROXY_DST) {
+ if (pd->dir == state->direction) {
if (((tcp_get_flags(th) & (TH_SYN|TH_ACK)) != TH_ACK) ||
- (ntohl(th->th_ack) != (*state)->src.seqhi + 1) ||
- (ntohl(th->th_seq) != (*state)->src.seqlo + 1)) {
+ (ntohl(th->th_ack) != state->src.seqhi + 1) ||
+ (ntohl(th->th_seq) != state->src.seqlo + 1)) {
REASON_SET(reason, PFRES_SYNPROXY);
return (PF_DROP);
}
- (*state)->src.max_win = MAX(ntohs(th->th_win), 1);
- if ((*state)->dst.seqhi == 1)
- (*state)->dst.seqhi = htonl(arc4random());
- pf_send_tcp((*state)->rule, pd->af,
+ state->src.max_win = MAX(ntohs(th->th_win), 1);
+ if (state->dst.seqhi == 1)
+ state->dst.seqhi = htonl(arc4random());
+ pf_send_tcp(state->rule, pd->af,
&sk->addr[pd->sidx], &sk->addr[pd->didx],
sk->port[pd->sidx], sk->port[pd->didx],
- (*state)->dst.seqhi, 0, TH_SYN, 0,
- (*state)->src.mss, 0,
- (*state)->orig_kif->pfik_ifp == V_loif ? M_LOOP : 0,
- (*state)->tag, 0, (*state)->act.rtableid);
+ state->dst.seqhi, 0, TH_SYN, 0,
+ state->src.mss, 0,
+ state->orig_kif->pfik_ifp == V_loif ? M_LOOP : 0,
+ state->tag, 0, state->act.rtableid);
REASON_SET(reason, PFRES_SYNPROXY);
return (PF_SYNPROXY_DROP);
} else if (((tcp_get_flags(th) & (TH_SYN|TH_ACK)) !=
(TH_SYN|TH_ACK)) ||
- (ntohl(th->th_ack) != (*state)->dst.seqhi + 1)) {
+ (ntohl(th->th_ack) != state->dst.seqhi + 1)) {
REASON_SET(reason, PFRES_SYNPROXY);
return (PF_DROP);
} else {
- (*state)->dst.max_win = MAX(ntohs(th->th_win), 1);
- (*state)->dst.seqlo = ntohl(th->th_seq);
- pf_send_tcp((*state)->rule, pd->af, pd->dst,
+ state->dst.max_win = MAX(ntohs(th->th_win), 1);
+ state->dst.seqlo = ntohl(th->th_seq);
+ pf_send_tcp(state->rule, pd->af, pd->dst,
pd->src, th->th_dport, th->th_sport,
ntohl(th->th_ack), ntohl(th->th_seq) + 1,
- TH_ACK, (*state)->src.max_win, 0, 0, 0,
- (*state)->tag, 0, (*state)->act.rtableid);
- pf_send_tcp((*state)->rule, pd->af,
+ TH_ACK, state->src.max_win, 0, 0, 0,
+ state->tag, 0, state->act.rtableid);
+ pf_send_tcp(state->rule, pd->af,
&sk->addr[pd->sidx], &sk->addr[pd->didx],
sk->port[pd->sidx], sk->port[pd->didx],
- (*state)->src.seqhi + 1, (*state)->src.seqlo + 1,
- TH_ACK, (*state)->dst.max_win, 0, 0,
- M_SKIP_FIREWALL, 0, 0, (*state)->act.rtableid);
- (*state)->src.seqdiff = (*state)->dst.seqhi -
- (*state)->src.seqlo;
- (*state)->dst.seqdiff = (*state)->src.seqhi -
- (*state)->dst.seqlo;
- (*state)->src.seqhi = (*state)->src.seqlo +
- (*state)->dst.max_win;
- (*state)->dst.seqhi = (*state)->dst.seqlo +
- (*state)->src.max_win;
- (*state)->src.wscale = (*state)->dst.wscale = 0;
- pf_set_protostate(*state, PF_PEER_BOTH,
+ state->src.seqhi + 1, state->src.seqlo + 1,
+ TH_ACK, state->dst.max_win, 0, 0,
+ M_SKIP_FIREWALL, 0, 0, state->act.rtableid);
+ state->src.seqdiff = state->dst.seqhi -
+ state->src.seqlo;
+ state->dst.seqdiff = state->src.seqhi -
+ state->dst.seqlo;
+ state->src.seqhi = state->src.seqlo +
+ state->dst.max_win;
+ state->dst.seqhi = state->dst.seqlo +
+ state->src.max_win;
+ state->src.wscale = state->dst.wscale = 0;
+ pf_set_protostate(state, PF_PEER_BOTH,
TCPS_ESTABLISHED);
REASON_SET(reason, PFRES_SYNPROXY);
return (PF_SYNPROXY_DROP);
@@ -6964,7 +6964,7 @@ pf_test_state(struct pf_kstate **state, struct pf_pdesc *pd, u_short *reason)
case IPPROTO_TCP: {
struct tcphdr *th = &pd->hdr.tcp;
- if ((action = pf_synproxy(pd, state, reason)) != PF_PASS)
+ if ((action = pf_synproxy(pd, *state, reason)) != PF_PASS)
return (action);
if ((*state)->src.state >= TCPS_FIN_WAIT_2 &&
(*state)->dst.state >= TCPS_FIN_WAIT_2 &&
@@ -6984,13 +6984,13 @@ pf_test_state(struct pf_kstate **state, struct pf_pdesc *pd, u_short *reason)
return (PF_DROP);
}
if ((*state)->state_flags & PFSTATE_SLOPPY) {
- if (pf_tcp_track_sloppy(state, pd, reason, src, dst,
+ if (pf_tcp_track_sloppy(*state, pd, reason, src, dst,
psrc, pdst) == PF_DROP)
return (PF_DROP);
} else {
int ret;
- ret = pf_tcp_track_full(state, pd, reason,
+ ret = pf_tcp_track_full(*state, pd, reason,
©back, src, dst, psrc, pdst);
if (ret == PF_DROP)
return (PF_DROP);
@@ -10414,7 +10414,7 @@ pf_test(sa_family_t af, int dir, int pflags, struct ifnet *ifp, struct mbuf **m0
s->src.seqhi = ntohl(pd.hdr.tcp.th_ack) - 1;
s->src.seqlo = ntohl(pd.hdr.tcp.th_seq) - 1;
pf_set_protostate(s, PF_PEER_SRC, PF_TCPS_PROXY_DST);
- action = pf_synproxy(&pd, &s, &reason);
+ action = pf_synproxy(&pd, s, &reason);
break;
} else {
action = pf_test_rule(&r, &s, &pd,