From nobody Tue Jul 09 20:07:30 2024 X-Original-To: dev-commits-src-main@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 4WJX8f6TL3z5R1Cd; Tue, 09 Jul 2024 20:07:30 +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 "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4WJX8f5g9Rz4vvF; Tue, 9 Jul 2024 20:07:30 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1720555650; 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=oLDSL38KPjE4uyORgO2IEkYh2GPSxdKJLcn+2PhnGW0=; b=PF9mvci/xxbA7QmkAPChrsWjeTOtP0smQyIdoIPWUChjJVe5+0sLGX1oSXPUlq8gpa65ch fMeziwzVfppaTU310gPfQr3OqdRySbVeft1bL9q/Ek8UTWG7KXOQQr1LZokkU0uoPAfiIo uXrluPjZBw1x3ARQap82RDnLKpM6RlTzIrLgI3U8sHJALQFqycIZGyJWtMD5EJ4NMeXKj5 LFJNZq/y2xaiT0GsfkUmzmLVn1qxFk4yrQKvzOykxvZSbV4s3nOnx6GTjZyNb7kAHZC2Hu xTHhfRl2xdrbebgY4bZ+D0wEMmA0OUIUOsh7xlygqVhdnbndwYeGNOSTxuyFZA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1720555650; a=rsa-sha256; cv=none; b=mhr/ed/3yr4Rh92C4sQv33p9EhiNIionFzJrR8Ut46sHMby7v+x/Ge7lPTaTmDPBbdtYek UpD7pHRFy2jRcKUDfo6L+DKK9p0rBw5Vug4GscsJ96zXbXEPzI66xTO8cQkBCA9yJVl4of ZWOE+RSIG5ZA3wqW9RxnsnDUyDPO54/Zgh9ZRiLV3ZyYKJHPZHinoGGEWBib0ccJejxtic M88SxhHGMSjXf5rA2dTYAK8ZmYpa5yq77ckhHnaSC8rjU4NBsNl/VC7fPX2xo8PTWMk/aV SO4g9ie/nkgapHFGhmNah0QdxgKP4LNvtfnpB8h/UdRuLW86B7R7bE7k0a2kMw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1720555650; 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=oLDSL38KPjE4uyORgO2IEkYh2GPSxdKJLcn+2PhnGW0=; b=grynlLbhCMF9Bw40igQPquLNgx8tUPzr6Rno8V0U6AkRJpEmIwte4juP6rGuiMe/6e+4YK Oxupj0/unOWeQQVACKlYkwl3cO3icEOXHS3IpWnNumXAGMLnv6ZmS3JZuMCLvnL/xLPZIT 0rLOE/O2Yy41KxIysH3Yw4TzPRPmEt0KLkC+lQZTTSKgUU6P8dQwBffuOn+GBXH26D9OP2 DOB6Yj0ycovugM8WRrxgihx+fON/R5Qat+vdri8ybNQy2MSERB97E9HQMMA0SNSGc1i2Oe 8OpdC/sdepiCjmKCgX1Pbjw9SqrWitY/g50XAGRRwh2Y8yhjOiyB+jIGz/544Q== 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 4WJX8f4xT2znW6; Tue, 9 Jul 2024 20:07:30 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 469K7UFt025247; Tue, 9 Jul 2024 20:07:30 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 469K7UqU025244; Tue, 9 Jul 2024 20:07:30 GMT (envelope-from git) Date: Tue, 9 Jul 2024 20:07:30 GMT Message-Id: <202407092007.469K7UqU025244@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: 5f75cd390a67 - main - if_pfsync: lock buckets during pfsync_drop() List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@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: 5f75cd390a67cbec06993c4c66f784f0f777c854 Auto-Submitted: auto-generated The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=5f75cd390a67cbec06993c4c66f784f0f777c854 commit 5f75cd390a67cbec06993c4c66f784f0f777c854 Author: Kristof Provost AuthorDate: 2024-07-07 14:42:48 +0000 Commit: Kristof Provost CommitDate: 2024-07-09 20:07:13 +0000 if_pfsync: lock buckets during pfsync_drop() We failed to lock buckets while dropping messages, which could potentially lead to crashes, and is the likely cause of panics like: > pfsync_drop: st->sync_state == q > # pfsync_drop > # pfsync_q_ins > # pfsync_insert_state > # pf_state_insert > ... Handle this by only handling the currently relevant (and this locked) bucket. This ensures that the bucket is locked while we manipulate it. While here also log slightly more information in the KASSERT(). MFC after: 2 weeks Sponsored by: Orange Business Services --- sys/netpfil/pf/if_pfsync.c | 70 ++++++++++++++++++++++++++++------------------ 1 file changed, 43 insertions(+), 27 deletions(-) diff --git a/sys/netpfil/pf/if_pfsync.c b/sys/netpfil/pf/if_pfsync.c index 080938700e1d..67011d16c788 100644 --- a/sys/netpfil/pf/if_pfsync.c +++ b/sys/netpfil/pf/if_pfsync.c @@ -345,7 +345,8 @@ static void pfsync_defer_tmo(void *); static void pfsync_request_update(u_int32_t, u_int64_t); static bool pfsync_update_state_req(struct pf_kstate *); -static void pfsync_drop(struct pfsync_softc *); +static void pfsync_drop_all(struct pfsync_softc *); +static void pfsync_drop(struct pfsync_softc *, int); static void pfsync_sendout(int, int); static void pfsync_send_plus(void *, size_t); @@ -485,7 +486,7 @@ pfsync_clone_destroy(struct ifnet *ifp) bpfdetach(ifp); if_detach(ifp); - pfsync_drop(sc); + pfsync_drop_all(sc); if_free(ifp); pfsync_multicast_cleanup(sc); @@ -1736,40 +1737,54 @@ pfsync_out_del_c(struct pf_kstate *st, void *buf) } static void -pfsync_drop(struct pfsync_softc *sc) +pfsync_drop_all(struct pfsync_softc *sc) { - struct pf_kstate *st, *next; - struct pfsync_upd_req_item *ur; struct pfsync_bucket *b; int c; - enum pfsync_q_id q; for (c = 0; c < pfsync_buckets; c++) { b = &sc->sc_buckets[c]; - for (q = 0; q < PFSYNC_Q_COUNT; q++) { - if (TAILQ_EMPTY(&b->b_qs[q])) - continue; - TAILQ_FOREACH_SAFE(st, &b->b_qs[q], sync_list, next) { - KASSERT(st->sync_state == pfsync_qid_sstate[q], - ("%s: st->sync_state == q", - __func__)); - st->sync_state = PFSYNC_S_NONE; - pf_release_state(st); - } - TAILQ_INIT(&b->b_qs[q]); - } + PFSYNC_BUCKET_LOCK(b); + pfsync_drop(sc, c); + PFSYNC_BUCKET_UNLOCK(b); + } +} - while ((ur = TAILQ_FIRST(&b->b_upd_req_list)) != NULL) { - TAILQ_REMOVE(&b->b_upd_req_list, ur, ur_entry); - free(ur, M_PFSYNC); +static void +pfsync_drop(struct pfsync_softc *sc, int c) +{ + struct pf_kstate *st, *next; + struct pfsync_upd_req_item *ur; + struct pfsync_bucket *b; + enum pfsync_q_id q; + + b = &sc->sc_buckets[c]; + PFSYNC_BUCKET_LOCK_ASSERT(b); + + for (q = 0; q < PFSYNC_Q_COUNT; q++) { + if (TAILQ_EMPTY(&b->b_qs[q])) + continue; + + TAILQ_FOREACH_SAFE(st, &b->b_qs[q], sync_list, next) { + KASSERT(st->sync_state == pfsync_qid_sstate[q], + ("%s: st->sync_state %d == q %d", + __func__, st->sync_state, q)); + st->sync_state = PFSYNC_S_NONE; + pf_release_state(st); } + TAILQ_INIT(&b->b_qs[q]); + } - b->b_len = PFSYNC_MINPKT; - free(b->b_plus, M_PFSYNC); - b->b_plus = NULL; - b->b_pluslen = 0; + while ((ur = TAILQ_FIRST(&b->b_upd_req_list)) != NULL) { + TAILQ_REMOVE(&b->b_upd_req_list, ur, ur_entry); + free(ur, M_PFSYNC); } + + b->b_len = PFSYNC_MINPKT; + free(b->b_plus, M_PFSYNC); + b->b_plus = NULL; + b->b_pluslen = 0; } static void @@ -1793,7 +1808,7 @@ pfsync_sendout(int schedswi, int c) PFSYNC_BUCKET_LOCK_ASSERT(b); if (!bpf_peers_present(ifp->if_bpf) && sc->sc_sync_if == NULL) { - pfsync_drop(sc); + pfsync_drop(sc, c); return; } @@ -1841,6 +1856,7 @@ pfsync_sendout(int schedswi, int c) #endif default: m_freem(m); + pfsync_drop(sc, c); return; } m->m_len = m->m_pkthdr.len = len; @@ -2401,8 +2417,8 @@ pfsync_q_ins(struct pf_kstate *st, int sync_state, bool ref) } b->b_len += nlen; - TAILQ_INSERT_TAIL(&b->b_qs[q], st, sync_list); st->sync_state = pfsync_qid_sstate[q]; + TAILQ_INSERT_TAIL(&b->b_qs[q], st, sync_list); if (ref) pf_ref_state(st); }