The branch main has been updated by kp:
URL:
https://cgit.FreeBSD.org/src/commit/?id=16303d2ba6b099afa8ec8f2e97deca2785caa082
commit 16303d2ba6b099afa8ec8f2e97deca2785caa082
Author: Kajetan Staszkiewicz <veg...@tuxpowered.net>
AuthorDate: 2023-05-03 08:31:05 +0000
Commit: Kristof Provost <k...@freebsd.org>
CommitDate: 2023-05-03 08:31:05 +0000
pf: improve source node error handling
Functions manipulating source nodes can fail due to various reasons like
memory allocation errors, hitting configured limits or lack of
redirection targets. Ensure those errors are properly caught and
propagated in the code. Increase the error counters not only when
parsing the main ruleset but the NAT ruleset too.
Cherry-picked from development of D39880
Reviewed by: kp
Sponsored by: InnoGames GmbH
Differential Revision: https://reviews.freebsd.org/D39940
---
sys/net/pfvar.h | 2 +-
sys/netpfil/pf/pf.c | 50 ++++++++++++++++++++++++++++++--------------------
sys/netpfil/pf/pf_lb.c | 47 ++++++++++++++++++++++++++++-------------------
3 files changed, 59 insertions(+), 40 deletions(-)
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index 412453f8dd25..f72c3980438d 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -2403,7 +2403,7 @@ int
pf_step_out_of_keth_anchor(struct pf_keth_anchor_stackframe *,
struct pf_keth_rule **, struct pf_keth_rule **,
int *);
-int pf_map_addr(u_int8_t, struct pf_krule *,
+u_short pf_map_addr(u_int8_t, struct pf_krule *,
struct pf_addr *, struct pf_addr *,
struct pf_addr *, struct pf_ksrc_node **);
struct pf_krule *pf_get_translation(struct pf_pdesc *, struct
mbuf *,
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index e5f2a5ea57a0..ee17df0c866f 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -329,7 +329,7 @@ static struct pf_kstate *pf_find_state(struct pfi_kkif
*,
struct pf_state_key_cmp *, u_int);
static int pf_src_connlimit(struct pf_kstate **);
static void pf_overload_task(void *v, int pending);
-static int pf_insert_src_node(struct pf_ksrc_node **,
+static u_short pf_insert_src_node(struct pf_ksrc_node **,
struct pf_krule *, struct pf_addr *, sa_family_t);
static u_int pf_purge_expired_states(u_int, int);
static void pf_purge_unlinked_rules(void);
@@ -865,11 +865,12 @@ pf_free_src_node(struct pf_ksrc_node *sn)
uma_zfree(V_pf_sources_z, sn);
}
-static int
+static u_short
pf_insert_src_node(struct pf_ksrc_node **sn, struct pf_krule *rule,
struct pf_addr *src, sa_family_t af)
{
- struct pf_srchash *sh = NULL;
+ u_short reason = 0;
+ struct pf_srchash *sh = NULL;
KASSERT((rule->rule_flag & PFRULE_SRCTRACK ||
rule->rpool.opts & PF_POOL_STICKYADDR),
@@ -881,15 +882,19 @@ pf_insert_src_node(struct pf_ksrc_node **sn, struct
pf_krule *rule,
if (*sn == NULL) {
PF_HASHROW_ASSERT(sh);
- if (!rule->max_src_nodes ||
- counter_u64_fetch(rule->src_nodes) < rule->max_src_nodes)
- (*sn) = uma_zalloc(V_pf_sources_z, M_NOWAIT | M_ZERO);
- else
- counter_u64_add(V_pf_status.lcounters[LCNT_SRCNODES],
- 1);
+ if (rule->max_src_nodes &&
+ counter_u64_fetch(rule->src_nodes) >= rule->max_src_nodes) {
+ counter_u64_add(V_pf_status.lcounters[LCNT_SRCNODES],
1);
+ PF_HASHROW_UNLOCK(sh);
+ reason = PFRES_SRCLIMIT;
+ goto done;
+ }
+
+ (*sn) = uma_zalloc(V_pf_sources_z, M_NOWAIT | M_ZERO);
if ((*sn) == NULL) {
PF_HASHROW_UNLOCK(sh);
- return (-1);
+ reason = PFRES_MEMORY;
+ goto done;
}
for (int i = 0; i < 2; i++) {
@@ -899,7 +904,8 @@ pf_insert_src_node(struct pf_ksrc_node **sn, struct
pf_krule *rule,
if ((*sn)->bytes[i] == NULL || (*sn)->packets[i] ==
NULL) {
pf_free_src_node(*sn);
PF_HASHROW_UNLOCK(sh);
- return (-1);
+ reason = PFRES_MEMORY;
+ goto done;
}
}
@@ -926,10 +932,12 @@ pf_insert_src_node(struct pf_ksrc_node **sn, struct
pf_krule *rule,
(*sn)->states >= rule->max_src_states) {
counter_u64_add(V_pf_status.lcounters[LCNT_SRCSTATES],
1);
- return (-1);
+ reason = PFRES_SRCLIMIT;
+ goto done;
}
}
- return (0);
+done:
+ return (reason);
}
void
@@ -4581,7 +4589,7 @@ pf_create_state(struct pf_krule *r, struct pf_krule *nr,
struct pf_krule *a,
struct pf_ksrc_node *sn = NULL;
struct tcphdr *th = &pd->hdr.tcp;
u_int16_t mss = V_tcp_mssdflt;
- u_short reason;
+ u_short reason, sn_reason;
/* check maximums */
if (r->max_states &&
@@ -4593,14 +4601,15 @@ pf_create_state(struct pf_krule *r, struct pf_krule
*nr, struct pf_krule *a,
/* src node for filter rule */
if ((r->rule_flag & PFRULE_SRCTRACK ||
r->rpool.opts & PF_POOL_STICKYADDR) &&
- pf_insert_src_node(&sn, r, pd->src, pd->af) != 0) {
- REASON_SET(&reason, PFRES_SRCLIMIT);
+ (sn_reason = pf_insert_src_node(&sn, r, pd->src, pd->af)) != 0) {
+ REASON_SET(&reason, sn_reason);
goto csfailed;
}
/* src node for translation rule */
if (nr != NULL && (nr->rpool.opts & PF_POOL_STICKYADDR) &&
- pf_insert_src_node(&nsn, nr, &sk->addr[pd->sidx], pd->af)) {
- REASON_SET(&reason, PFRES_SRCLIMIT);
+ (sn_reason = pf_insert_src_node(&nsn, nr, &sk->addr[pd->sidx],
+ pd->af)) != 0 ) {
+ REASON_SET(&reason, sn_reason);
goto csfailed;
}
s = pf_alloc_state(M_NOWAIT);
@@ -4689,8 +4698,9 @@ pf_create_state(struct pf_krule *r, struct pf_krule *nr,
struct pf_krule *a,
}
if (r->rt) {
- if (pf_map_addr(pd->af, r, pd->src, &s->rt_addr, NULL, &sn)) {
- REASON_SET(&reason, PFRES_MAPFAILED);
+ /* pf_map_addr increases the reason counters */
+ if ((reason = pf_map_addr(pd->af, r, pd->src, &s->rt_addr, NULL,
+ &sn)) != 0) {
pf_src_tree_remove_state(s);
s->timeout = PFTM_UNLINKED;
STATE_DEC_COUNTERS(s);
diff --git a/sys/netpfil/pf/pf_lb.c b/sys/netpfil/pf/pf_lb.c
index ca8593a1c37d..a2f0224f842c 100644
--- a/sys/netpfil/pf/pf_lb.c
+++ b/sys/netpfil/pf/pf_lb.c
@@ -342,10 +342,11 @@ pf_get_mape_sport(sa_family_t af, u_int8_t proto, struct
pf_krule *r,
return (1);
}
-int
+u_short
pf_map_addr(sa_family_t af, struct pf_krule *r, struct pf_addr *saddr,
struct pf_addr *naddr, struct pf_addr *init_addr, struct pf_ksrc_node **sn)
{
+ u_short reason = 0;
struct pf_kpool *rpool = &r->rpool;
struct pf_addr *raddr = NULL, *rmask = NULL;
struct pf_srchash *sh = NULL;
@@ -364,8 +365,10 @@ pf_map_addr(sa_family_t af, struct pf_krule *r, struct
pf_addr *saddr,
/* If the supplied address is the same as the current one we've
* been asked before, so tell the caller that there's no other
* address to be had. */
- if (PF_AEQ(naddr, &(*sn)->raddr, af))
- return (1);
+ if (PF_AEQ(naddr, &(*sn)->raddr, af)) {
+ reason = PFRES_MAPFAILED;
+ goto done;
+ }
PF_ACPY(naddr, &(*sn)->raddr, af);
if (V_pf_status.debug >= PF_DEBUG_NOISY) {
@@ -375,15 +378,15 @@ pf_map_addr(sa_family_t af, struct pf_krule *r, struct
pf_addr *saddr,
pf_print_host(naddr, 0, af);
printf("\n");
}
- return (0);
+ goto done;
}
mtx_lock(&rpool->mtx);
/* Find the route using chosen algorithm. Store the found route
in src_node if it was given or found. */
if (rpool->cur->addr.type == PF_ADDR_NOROUTE) {
- mtx_unlock(&rpool->mtx);
- return (1);
+ reason = PFRES_MAPFAILED;
+ goto done_pool_mtx;
}
if (rpool->cur->addr.type == PF_ADDR_DYNIFTL) {
switch (af) {
@@ -392,8 +395,8 @@ pf_map_addr(sa_family_t af, struct pf_krule *r, struct
pf_addr *saddr,
if (rpool->cur->addr.p.dyn->pfid_acnt4 < 1 &&
(rpool->opts & PF_POOL_TYPEMASK) !=
PF_POOL_ROUNDROBIN) {
- mtx_unlock(&rpool->mtx);
- return (1);
+ reason = PFRES_MAPFAILED;
+ goto done_pool_mtx;
}
raddr = &rpool->cur->addr.p.dyn->pfid_addr4;
rmask = &rpool->cur->addr.p.dyn->pfid_mask4;
@@ -404,8 +407,8 @@ pf_map_addr(sa_family_t af, struct pf_krule *r, struct
pf_addr *saddr,
if (rpool->cur->addr.p.dyn->pfid_acnt6 < 1 &&
(rpool->opts & PF_POOL_TYPEMASK) !=
PF_POOL_ROUNDROBIN) {
- mtx_unlock(&rpool->mtx);
- return (1);
+ reason = PFRES_MAPFAILED;
+ goto done_pool_mtx;
}
raddr = &rpool->cur->addr.p.dyn->pfid_addr6;
rmask = &rpool->cur->addr.p.dyn->pfid_mask6;
@@ -414,8 +417,8 @@ pf_map_addr(sa_family_t af, struct pf_krule *r, struct
pf_addr *saddr,
}
} else if (rpool->cur->addr.type == PF_ADDR_TABLE) {
if ((rpool->opts & PF_POOL_TYPEMASK) != PF_POOL_ROUNDROBIN) {
- mtx_unlock(&rpool->mtx);
- return (1); /* unsupported */
+ reason = PFRES_MAPFAILED;
+ goto done_pool_mtx; /* unsupported */
}
} else {
raddr = &rpool->cur->addr.v.a.addr;
@@ -503,8 +506,8 @@ pf_map_addr(sa_family_t af, struct pf_krule *r, struct
pf_addr *saddr,
/* table contains no address of type 'af' */
if (rpool->cur != acur)
goto try_next;
- mtx_unlock(&rpool->mtx);
- return (1);
+ reason = PFRES_MAPFAILED;
+ goto done_pool_mtx;
}
} else if (rpool->cur->addr.type == PF_ADDR_DYNIFTL) {
rpool->tblidx = -1;
@@ -513,8 +516,8 @@ pf_map_addr(sa_family_t af, struct pf_krule *r, struct
pf_addr *saddr,
/* table contains no address of type 'af' */
if (rpool->cur != acur)
goto try_next;
- mtx_unlock(&rpool->mtx);
- return (1);
+ reason = PFRES_MAPFAILED;
+ goto done_pool_mtx;
}
} else {
raddr = &rpool->cur->addr.v.a.addr;
@@ -533,8 +536,6 @@ pf_map_addr(sa_family_t af, struct pf_krule *r, struct
pf_addr *saddr,
if (*sn != NULL)
PF_ACPY(&(*sn)->raddr, naddr, af);
- mtx_unlock(&rpool->mtx);
-
if (V_pf_status.debug >= PF_DEBUG_NOISY &&
(rpool->opts & PF_POOL_TYPEMASK) != PF_POOL_NONE) {
printf("pf_map_addr: selected address ");
@@ -542,7 +543,15 @@ pf_map_addr(sa_family_t af, struct pf_krule *r, struct
pf_addr *saddr,
printf("\n");
}
- return (0);
+done_pool_mtx:
+ mtx_unlock(&rpool->mtx);
+
+done:
+ if (reason) {
+ counter_u64_add(V_pf_status.counters[reason], 1);
+ }
+
+ return (reason);
}
struct pf_krule *
