From nobody Wed Jul 17 21:55:26 2024 X-Original-To: dev-commits-src-branches@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 4WPV9W0kshz5QTDP; Wed, 17 Jul 2024 21:55: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 "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4WPV9V6kgfz4pW4; 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=Ts72YOqV5zNudTs78j5L60Uhs6u46SHfSXpt+ux98Ls=; b=grmJYbsuM5o0G2r9B+rnxWavG/MT772l+t0sKBmDXdRyvOCfqzTR6wXJ8W7vd07Wejp3xn jn50t61nIYaKLVBVnDN21keJy9auzFYWu7REPQDYEMYN6+JERLxoI6PCuP7E7r23NzdYLj 4yu12VyYzWuVZ9tmpMAvZLfu9yNKjeCOe6/iVGCjis7lISRyzZ/DSdJDy8w8iuHqQdsPie Bt5YxzxCndS+4xZaYUdQZntOTCqoWjwYtfYVKYkLTzoir0olwRdvcu1vsmbNmmnokbsEEe 8gfjZ/0LiO8oHmtXlVvRethXZAhdAb3rFdMJg2VUVSGK15/x13NJssJVE2sndg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1721253326; a=rsa-sha256; cv=none; b=n8GFz/hX7DmMIx0W+aR/JkSC9oFWTzy/QfB8VUncmQrKkhdeuP37L4D3Fi7QcuXAyd/Kvz B9yS0TY8CrH7hBYsjuxXgOKeub0ooAnYN1NxDyNV33caPosPntbq9r0VD3t3SYQJLYLjsL af/3YEPvzUdSJ6cKLILCYpUF1G6YM4wG1NaqWvqOHSxQ7Hw/qVyPjaDg3I02iV9Riwulkn RBcz1LP0SYDP120UNAkCo/1AW6RhhYF6bKxkQpQivARzWc6nQszKPZSTiWJ2gTCYyfLxm9 JxQCWbH3Pef6fSuWVizODrtytBGnH0bC2ciyjiztCIbkaL5tgL1K55ho1+onMg== 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=Ts72YOqV5zNudTs78j5L60Uhs6u46SHfSXpt+ux98Ls=; b=GtaVy5Ygk2V38r3Tlwt6VfQpEkwHAyJeDpii+ASXGiqPOQBtERSpA4kh4mpyeMtb7ZPWXi mzKQqTfClZ8leEeSFle7NKRYPXgtjsrgKc/jeRQU40yCzP1JfibNn+Jk6yoJ/wMpgvcGgv z1FEkOyF2DwJRhtLNdAEUPd9OKlu8xbzXR9eTOP5HhBP2BLcwBJkAbltRjetf1RaoUl5Uh HXeMFcSEW6NA4XX3kR3ksN2xjfKR6oDiWgQdKrkYDBOwZx4yhiYAq/UCcDB6SRg1GBtBcf 8CuNBue5iWweM2D6rjExxj+YXn+6YTEmxjJEwVfWk0EWjGbsnnE36F5JsABhgQ== 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 4WPV9V6LJ9zMB9; 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 46HLtQtD096790; 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 46HLtQr4096787; Wed, 17 Jul 2024 21:55:26 GMT (envelope-from git) Date: Wed, 17 Jul 2024 21:55:26 GMT Message-Id: <202407172155.46HLtQr4096787@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: e99c76951e10 - stable/14 - if_pfsync: lock buckets during pfsync_drop() List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-branches@freebsd.org Sender: owner-dev-commits-src-branches@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/14 X-Git-Reftype: branch X-Git-Commit: e99c76951e1092464037cd4bcbce1e05a3589acc Auto-Submitted: auto-generated The branch stable/14 has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=e99c76951e1092464037cd4bcbce1e05a3589acc commit e99c76951e1092464037cd4bcbce1e05a3589acc Author: Kristof Provost AuthorDate: 2024-07-07 14:42:48 +0000 Commit: Kristof Provost CommitDate: 2024-07-17 15:23:48 +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 9aad44ccaf99..7af9ce8f468d 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); }