Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 5 Feb 2024 21:19:20 GMT
From:      Kristof Provost <kp@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 6d4a140acfdf - main - pf: Ensure that st->kif is obtained in a way which respects the r->rpool->mtx mutex
Message-ID:  <202402052119.415LJKSN037380@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by kp:

URL: https://cgit.FreeBSD.org/src/commit/?id=6d4a140acfdf637bb559d371c583e4db478e1549

commit 6d4a140acfdf637bb559d371c583e4db478e1549
Author:     Igor Ostapenko <pm@igoro.pro>
AuthorDate: 2024-02-05 16:22:31 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2024-02-05 21:18:11 +0000

    pf: Ensure that st->kif is obtained in a way which respects the r->rpool->mtx mutex
    
    The redirection pool stored in r->rpool.cur is used for loadbalancing
    and cur can change whenever loadbalancing happens, which is for every
    new connection. Therefore it can't be trusted outside of pf_map_addr()
    and the r->rpool->mtx mutex. After evaluating the ruleset, loadbalancing
    decission is made in pf_map_addr() called from within pf_create_state()
    and stored in the state itself.
    
    This patch modifies BOUND_IFACE() so that it only uses the information
    already stored in the state which has been obtained in a way which
    respects the r->rpool->mtx mutex.
    
    Reviewed by:    kp
    Differential Revision:  https://reviews.freebsd.org/D43741
---
 sys/netpfil/pf/pf.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 9f2c07c614b3..12b72f50aa02 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -413,25 +413,22 @@ VNET_DEFINE(struct pf_limit, pf_limits[PF_LIMIT_MAX]);
 	} while (0)
 
 static struct pfi_kkif *
-BOUND_IFACE(struct pf_krule *r, struct pfi_kkif *k, struct pf_pdesc *pd)
+BOUND_IFACE(struct pf_kstate *st, struct pfi_kkif *k)
 {
 	/* Floating unless otherwise specified. */
-	if (! (r->rule_flag & PFRULE_IFBOUND))
+	if (! (st->rule.ptr->rule_flag & PFRULE_IFBOUND))
 		return (V_pfi_all);
 
 	/* Don't overrule the interface for states created on incoming packets. */
-	if (pd->dir == PF_IN)
-		return (k);
-
-	/* No route-to, so don't overrrule. */
-	if (r->rt != PF_ROUTETO)
+	if (st->direction == PF_IN)
 		return (k);
 
-	if (r->rpool.cur == NULL)
+	/* No route-to, so don't overrule. */
+	if (st->rt != PF_ROUTETO)
 		return (k);
 
 	/* Bind to the route-to interface. */
-	return (r->rpool.cur->kif);
+	return (st->rt_kif);
 }
 
 #define	STATE_INC_COUNTERS(s)						\
@@ -5018,7 +5015,7 @@ pf_create_state(struct pf_krule *r, struct pf_krule *nr, struct pf_krule *a,
 		    __func__, nr, sk, nk));
 
 	/* Swap sk/nk for PF_OUT. */
-	if (pf_state_insert(BOUND_IFACE(r, kif, pd), kif,
+	if (pf_state_insert(BOUND_IFACE(s, kif), kif,
 	    (pd->dir == PF_IN) ? sk : nk,
 	    (pd->dir == PF_IN) ? nk : sk, s)) {
 		REASON_SET(&reason, PFRES_STATEINS);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202402052119.415LJKSN037380>