From nobody Wed May 03 09:59:27 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4QBC8v4hWpz499ST; Wed, 3 May 2023 09:59:27 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4QBC8v2G5wz3Ld1; Wed, 3 May 2023 09:59:27 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1683107967; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=tJAEtJfshjJ3mL8saCMLUNDKH/7a9yOPUbQUPYDIxtQ=; b=Tku05NJGOVmjj/btgigXgSddDh+BpIxQMXJUIPYZ90sCu9cci6m8ujC/JZ6yNcCqxJHr4I dncLJQVfArPF6UJUUTdVOMd+Cw7czBNADId8BqJM57lz2hmT7JDWiBoZGs9cWEle2HZb2C aDLLB8blzaqGSpiqhGDXqdihZt5jL4ay6zjaCGZ7iZRWmaa5l4+s9ddVP0BZgxybZQA+Db RQ2UmQNynDGfXg1WwMhIpyn/yNVIcZANYL1o/iwOHRiNVgRh5SrUzCGAd7XgMqTRUATygZ CmtbmcHIHmklC12HU6qJXW2bcLaJbEuHZrPjIZ/D8Ev8xFyJjWkNjpn4zz5A0w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1683107967; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=tJAEtJfshjJ3mL8saCMLUNDKH/7a9yOPUbQUPYDIxtQ=; b=iJUSNF+JCHwqAsTEDUjczqBZw0iMM75J7kzNLqg9kwhrs1UUjPDIP284jszQuSQlDr+sLn L+bTjJQRRdJtkFiyCjehTXdLQbqmB7MmNWQ5V13mZAUS9YQFvHv17mqkVp3eb5DPO4gpJ7 6WmFDl0NtOyLkBwFZ5bH8s+uk6dCLZKgTRaplUrRiJjiY5cbcbEBfh2/UFMh29B+gfujSg wq7bZ6hSHXqNHhLO3keXrz0EbkIhbSctQxDt33u4oAyaXDfQToildNfN3Hn11eiPXdMPNZ Eo1DOgS9InqazoetHGjhajHmOqKzPeD50gf4Ms7Ry0WeD7d2FH9rVuj47yGwPQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1683107967; a=rsa-sha256; cv=none; b=VrVYgY4I6gSKO8+uvG+u1xy1sBrRKQE/EIHjQHsaGefaRjRJhlB0eqP7J462qKEAp3aYqB O3s6vHc+Z57q52qLz7GTXY9VlULcv3u2sFr3Es3M11xjWHQtrmn6xAtlygCU3eOeA4gStK SOiq5ZIxbCQ12Lg/ZRQLOKGxuzfdNNiKoC3EWqEVPa2wgkDfNXBTubKnUHiTIl/QT4BLwG YS1bwCnX1BU6HWHrNodvRyciPKmH63xh79zyE7XMQhP42ftsfK14eXMZMj4TWt1+T73OBf P+oVo2RXDpL6QDTueNkyy0P/Z4dUIJrcgtMz8J7xbf3ou7czCFTInYaY6EjpGw== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4QBC8v13tDzQyG; Wed, 3 May 2023 09:59:27 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 3439xRuJ014220; Wed, 3 May 2023 09:59:27 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 3439xR8T014219; Wed, 3 May 2023 09:59:27 GMT (envelope-from git) Date: Wed, 3 May 2023 09:59:27 GMT Message-Id: <202305030959.3439xR8T014219@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kristof Provost Subject: git: 16303d2ba6b0 - main - pf: improve source node error handling List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kp X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 16303d2ba6b099afa8ec8f2e97deca2785caa082 Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=16303d2ba6b099afa8ec8f2e97deca2785caa082 commit 16303d2ba6b099afa8ec8f2e97deca2785caa082 Author: Kajetan Staszkiewicz AuthorDate: 2023-05-03 08:31:05 +0000 Commit: Kristof Provost 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 *