Skip site navigation (1)Skip section navigation (2) 
Date:      Mon, 20 Mar 2023 16:39:31 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: 53247cdf1244 - main - pfsync: fix pfsync_undefer_state() locking
Message-ID:  <202303201639.32KGdVl5071213@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=53247cdf12449e90f6736ae563e4cce8315c923f

commit 53247cdf12449e90f6736ae563e4cce8315c923f
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2023-03-20 13:29:55 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2023-03-20 15:39:14 +0000

    pfsync: fix pfsync_undefer_state() locking
    
    pfsync_undefer_state() takes the bucket lock, but could get called from
    places (e.g. from pfsync_update_state() or pfsync_delete_state()) where
    we already held the lock.
    
    As it can also be called from places where we don't yet hold the lock
    create new locked variant for use when the lock is already held. Keep
    using pfsync_undefer_state() where the lock must still be taken.
    
    PR:             268246
    MFC after:      1 week
    Sponsored by:   Rubicon Communications, LLC (Netgate)
---
 sys/netpfil/pf/if_pfsync.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/sys/netpfil/pf/if_pfsync.c b/sys/netpfil/pf/if_pfsync.c
index 4c834c7f0315..ae3a2e46e26b 100644
--- a/sys/netpfil/pf/if_pfsync.c
+++ b/sys/netpfil/pf/if_pfsync.c
@@ -297,6 +297,7 @@ static int	pfsyncioctl(struct ifnet *, u_long, caddr_t);
 
 static int	pfsync_defer(struct pf_kstate *, struct mbuf *);
 static void	pfsync_undefer(struct pfsync_deferral *, int);
+static void	pfsync_undefer_state_locked(struct pf_kstate *, int);
 static void	pfsync_undefer_state(struct pf_kstate *, int);
 static void	pfsync_defer_tmo(void *);
 
@@ -1851,28 +1852,37 @@ pfsync_defer_tmo(void *arg)
 }
 
 static void
-pfsync_undefer_state(struct pf_kstate *st, int drop)
+pfsync_undefer_state_locked(struct pf_kstate *st, int drop)
 {
 	struct pfsync_softc *sc = V_pfsyncif;
 	struct pfsync_deferral *pd;
 	struct pfsync_bucket *b = pfsync_get_bucket(sc, st);
 
-	PFSYNC_BUCKET_LOCK(b);
+	PFSYNC_BUCKET_LOCK_ASSERT(b);
 
 	TAILQ_FOREACH(pd, &b->b_deferrals, pd_entry) {
 		 if (pd->pd_st == st) {
 			if (callout_stop(&pd->pd_tmo) > 0)
 				pfsync_undefer(pd, drop);
 
-			PFSYNC_BUCKET_UNLOCK(b);
 			return;
 		}
 	}
-	PFSYNC_BUCKET_UNLOCK(b);
 
 	panic("%s: unable to find deferred state", __func__);
 }
 
+static void
+pfsync_undefer_state(struct pf_kstate *st, int drop)
+{
+	struct pfsync_softc *sc = V_pfsyncif;
+	struct pfsync_bucket *b = pfsync_get_bucket(sc, st);
+
+	PFSYNC_BUCKET_LOCK(b);
+	pfsync_undefer_state_locked(st, drop);
+	PFSYNC_BUCKET_UNLOCK(b);
+}
+
 static struct pfsync_bucket*
 pfsync_get_bucket(struct pfsync_softc *sc, struct pf_kstate *st)
 {
@@ -1891,7 +1901,7 @@ pfsync_update_state(struct pf_kstate *st)
 	PFSYNC_BUCKET_LOCK(b);
 
 	if (st->state_flags & PFSTATE_ACK)
-		pfsync_undefer_state(st, 0);
+		pfsync_undefer_state_locked(st, 0);
 	if (st->state_flags & PFSTATE_NOSYNC) {
 		if (st->sync_state != PFSYNC_S_NONE)
 			pfsync_q_del(st, true, b);
@@ -2034,7 +2044,7 @@ pfsync_delete_state(struct pf_kstate *st)
 
 	PFSYNC_BUCKET_LOCK(b);
 	if (st->state_flags & PFSTATE_ACK)
-		pfsync_undefer_state(st, 1);
+		pfsync_undefer_state_locked(st, 1);
 	if (st->state_flags & PFSTATE_NOSYNC) {
 		if (st->sync_state != PFSYNC_S_NONE)
 			pfsync_q_del(st, true, b);

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