From nobody Wed Jul 17 21:55:26 2024 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 4WPV9V3RrMz5QT7j; Wed, 17 Jul 2024 21:55:26 +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 4WPV9V2j0Wz4p80; Wed, 17 Jul 2024 21:55:26 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1721253326; 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=quwV52xgmB9RPu0Tfq4kLyanuROsiaulqzrauYPdKDM=; b=r8YMpcnMrCDMc+Px+37ajaB1T/+9vEO8iUtKvGFAcSZqrdqeEH3OQiXAUZ3bSJkCutYIf7 rKNVO6DRxGz7buKg7vXrEEM5TTubeicvxFJer80+hOpdevHSkr2irdA+yQBKfX9J5nm9Q+ wSEj0rCGaMyM1hYczLOy6UKDg3BF1PTosAcKaZB3cDKCkDnTZcKp9Q06CwZJwHmg413GnJ ocX3vbKsGS439PAIPQr7DHZ/4+ysfzJI7EpN7BIUlyEfswlmfMMQhKsMwFy4lyQmu0M3DP 8rcwxjuXpDOsku/dA7VkrZFfvEKrhTySEUM1OLtSq3WU7gjG3yjGAASyigCjeQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1721253326; a=rsa-sha256; cv=none; b=v9HPvnNhO2kzVEWuZjyGLPOKFYltTwQ6THK4QQB4sTDLVieo3AtsvgKRCatIiqTNCqg+mz ee1LhH3CdnEDa3vofW5pLWtrgWTZZ3bF/MAUTzjsZ9lFsf7vrbgPHIWpfCytrlGKeuRNjx AGRnmi8+SlWjFSuMoeIlAM8emFKv/0MrCkEi1gnDVSPqZCrMYFjDhsiALc/esoN3/M55J5 ZMl+taZqQxUrb8zwHhxzAqZSVGTGHP+4SDFu9Jli1u3cjQEUWj6PzS3AoCVko5jgnBg2O1 qqa5Ql3ntM5Ez/BA8EqYyIk4wYIh7iswXN1pg/G/Fp5jIIZTsgPg7yc3mvR6Cw== 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=1721253326; 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=quwV52xgmB9RPu0Tfq4kLyanuROsiaulqzrauYPdKDM=; b=igd8Lh/e8L37yRYq81jVXTmS/Kv10/6HYSW2qoxLsxL8FXDMf1Kn3zEG5mZB29nVB/XU8P AX405+ZkrHXpfm1p051FXNyo2ppzP9vlokaV1ld4q/hutRst/BMsqkjUZxeS1mGPvKVJbS gg5WMvodZ9bbfIEeJhj9qUaf4x5Fc443kCFrEVAVHxvfEij8qE0na36S37m1M7fHILLfZ3 sK/DCAiCoW4Zeph6BQRE2rP2EdUmmV60yi73LEt+qQ0HPFMSmQ9qJALW5xXBhHHRIEo7Pb DNvVq+ZxLJ6Mf3DNGXKsCd8RmYZH49zs30nv8egamd/AP5x66ZuI/RqXoCgIQQ== 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 4WPV9V20MkzMB8; Wed, 17 Jul 2024 21:55:26 +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 46HLtQaC096684; Wed, 17 Jul 2024 21:55:26 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 46HLtQaY096681; Wed, 17 Jul 2024 21:55:26 GMT (envelope-from git) Date: Wed, 17 Jul 2024 21:55:26 GMT Message-Id: <202407172155.46HLtQaY096681@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Kristof Provost Subject: git: a69d075e96a6 - stable/13 - if_pfsync: lock buckets during pfsync_drop() 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: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-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/stable/13 X-Git-Reftype: branch X-Git-Commit: a69d075e96a60273a9b39005b3e765e6476a8b53 Auto-Submitted: auto-generated The branch stable/13 has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=a69d075e96a60273a9b39005b3e765e6476a8b53 commit a69d075e96a60273a9b39005b3e765e6476a8b53 Author: Kristof Provost AuthorDate: 2024-07-07 14:42:48 +0000 Commit: Kristof Provost CommitDate: 2024-07-17 21:54:35 +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 (cherry picked from commit 5f75cd390a67cbec06993c4c66f784f0f777c854) --- 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 1dde4e52e3f8..b8632e1fd61b 100644 --- a/sys/netpfil/pf/if_pfsync.c +++ b/sys/netpfil/pf/if_pfsync.c @@ -300,7 +300,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); @@ -434,7 +435,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); @@ -1532,39 +1533,54 @@ pfsync_out_del(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, q; + int c; for (c = 0; c < pfsync_buckets; c++) { b = &sc->sc_buckets[c]; - for (q = 0; q < PFSYNC_S_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 == 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; + int q; + + b = &sc->sc_buckets[c]; + PFSYNC_BUCKET_LOCK_ASSERT(b); + + for (q = 0; q < PFSYNC_S_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 == 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 @@ -1588,7 +1604,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; } @@ -2130,8 +2146,8 @@ pfsync_q_ins(struct pf_kstate *st, int q, bool ref) } b->b_len += nlen; - TAILQ_INSERT_TAIL(&b->b_qs[q], st, sync_list); st->sync_state = q; + TAILQ_INSERT_TAIL(&b->b_qs[q], st, sync_list); if (ref) pf_ref_state(st); }