From nobody Mon Oct 12 16:49:17 2026 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 4j3Nj93x0Cz6w6qk for ; Mon, 12 Oct 2026 16:49:17 +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 "YR2" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4j3Nj92rDdz4Ngr for ; Mon, 12 Oct 2026 16:49:17 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1791823757; 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=d4lxecAB9nAcB1SWuUqDi5TSSgykIKc/SZ4YYano1YQ=; b=hIeQU9TGWPtzut/eiPJEBpIPsO+uy8uS5943J8lryJrlbVH9kxxJwnwlmGR0Hb6qKD9ZFp zRTEGQ88CDYqMffwRj/9l6lptrPI6UZ/Fw1ZsdY8/9xJeqhpGQJy4C5yYAL9ZsDfQZ46qH G3b2an672fz6VfHV86Zh34cYTelqqXtMNX9MkBtsTtt7jhBICHky7FVPNtdZl79+Buy+Rd Q877O/19tp3gsb/lQCM5v933pPDfnifO9K3fvFI2NNj/8RUstvBaO8DTDm6WtA8sLXnZIO DLI4Vl+x5k026RUSETOKJPpSbgzKYRGZyHmPEfs6StHgToq7G8pumdHluUHdzg== ARC-Seal: i=1; a=rsa-sha256; d=freebsd.org; s=dkim; cv=none; t=1791823757; b=VEk0BYDSFgvQVRdCmj7e0koPYy/5DQ+f5yd97Iv3M/iYbbYYMHC6RYXBvoH2UqaTVyoePN qpdC1Y+RfMh6Z47gAXT7CqWHEaQnP9cuJxZYwR4TcKHvpEPEc/gnyAUP0PJp+Uh9m+xmOK qjiQn3RAiKOI6+JdAOHylpVpcwfujgjIYS96A16s9O4lIirhGMopNVQcsWvrnWLypn3WWR X/6cQPc2TNOTzEtnUifRpeXzOkNU7UCiMV84koP8Y/v0XVMq3Vtc4AR8QHkCXXbRFK+2FG w05vUcBxgF+LQnl/0VPfjH78aQLs5o0itzR0VgonBovvnAUcKjjqI4jdAP96Zw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1791823757; 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=d4lxecAB9nAcB1SWuUqDi5TSSgykIKc/SZ4YYano1YQ=; b=iAZCS2xym9QgkYs+bA+d+USY/0xJ2BeRcD2PhJmV3/MHbRYgBUogRve3CIVZ747jfAaueX 0tD1YDQr3dSW+Lsf7ethoU2xeJ4CUr5wJyaaq+T+0JR2Z8OKRxEJbogYngy0yinjUN8gPS 9+aWkwQXemwykFBb3VDMkJagaN4wvKg0IUi1Kn4Nhw9pG9cXOAvl0CsTt5DIAxA+eh7bFF zsY+PVssRW+8RpR/1KgCN8U8osmPHD1vFkSsmsLoap3TrmYK0kuJcrbW/0S0tK/E4ZszDY w0nx9/E50/V2tH4NnpFNoTTdFjKxxkXCZKYSZUrPozboeby7RyXndPEZAuRidg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) by mxrelay.nyi.freebsd.org (Postfix) with ESMTP id 4j3Nj91bs0zdWM for ; Mon, 12 Oct 2026 16:49:17 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 239da by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Mon, 12 Oct 2026 16:49:17 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: R. Christian McDonald Subject: git: 6bc57500b8b7 - main - pf: Change source limiter tables from a task List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org List-Id: Commit messages for the main branch of the src repository List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: rcm X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 6bc57500b8b700a3d9998fd417fdee653f93366d Auto-Submitted: auto-generated Date: Mon, 12 Oct 2026 16:49:17 +0000 Message-Id: <6acd0f8d.239da.26d85aac@gitrepo.freebsd.org> The branch main has been updated by rcm: URL: https://cgit.FreeBSD.org/src/commit/?id=6bc57500b8b700a3d9998fd417fdee653f93366d commit 6bc57500b8b700a3d9998fd417fdee653f93366d Author: R. Christian McDonald AuthorDate: 2026-10-12 16:47:21 +0000 Commit: R. Christian McDonald CommitDate: 2026-10-12 16:47:21 +0000 pf: Change source limiter tables from a task pf_source_used() puts a source in the table of its source limiter when the source has more states than the high-water mark, and pf_source_rele() takes it out again below the low-water mark. Both changed the table in place: pf_source_used() from pf_create_state(), with the rules lock held for reading, and pf_source_rele() from pf_remove_state(), with no rules lock at all. A table may only be changed with the rules lock held for writing, as packets look addresses up in its radix tree at the same time. With INVARIANTS the first insertion panics ("Lock pf rulesets not exclusively locked" in pfr_route_kentry()); without, the tree can be corrupted. Queue the changes instead, as pf_overload_task() does for overload tables, and make them from a task with the rules lock held for writing. An entry names the limiter by its id and remembers its table: the limiter, or its table, may be gone or replaced by the time the task runs, and then the entry is dropped. pfsr_intable changes only once the change is queued, so a failed allocation is retried the next time the source crosses a mark; a removal is also retried when the source is purged. Drain the task when a vnet goes away, after its last state. Add a regression test. Reviewed by: kp Fixes: 461648121230 ("pf: introduce source and state limiters") Sponsored by: Rubicon Communications, LLC ("Netgate") Differential Revision: https://reviews.freebsd.org/D60526 --- sys/netpfil/pf/pf.c | 134 ++++++++++++++++++++++++++++++++++----- tests/sys/netpfil/pf/limiters.sh | 60 ++++++++++++++++++ 2 files changed, 177 insertions(+), 17 deletions(-) diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c index dffaabf77434..817790ba0ef0 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -286,6 +286,31 @@ MTX_SYSINIT(pf_overloadqueue_mtx, &pf_overloadqueue_mtx, #define PF_OVERLOADQ_LOCK() mtx_lock(&pf_overloadqueue_mtx) #define PF_OVERLOADQ_UNLOCK() mtx_unlock(&pf_overloadqueue_mtx) +/* + * Source limiter overload table changes, applied by pf_source_table_task(). + * Tables can only be modified with the rules lock held exclusively, which + * packet processing does not hold. + */ +struct pf_source_table_entry { + STAILQ_ENTRY(pf_source_table_entry) next; + struct pfr_addr addr; + struct pfr_ktable *table; /* compared only */ + uint32_t id; + bool insert; +}; + +STAILQ_HEAD(pf_source_table_head, pf_source_table_entry); +VNET_DEFINE_STATIC(struct pf_source_table_head, pf_source_tablequeue); +#define V_pf_source_tablequeue VNET(pf_source_tablequeue) +VNET_DEFINE_STATIC(struct task, pf_source_tabletask); +#define V_pf_source_tabletask VNET(pf_source_tabletask) + +static struct mtx_padalign pf_source_tablequeue_mtx; +MTX_SYSINIT(pf_source_tablequeue_mtx, &pf_source_tablequeue_mtx, + "pf source limiter table queue", MTX_DEF); +#define PF_SOURCE_TABLEQ_LOCK() mtx_lock(&pf_source_tablequeue_mtx) +#define PF_SOURCE_TABLEQ_UNLOCK() mtx_unlock(&pf_source_tablequeue_mtx) + VNET_DEFINE(struct pf_krulequeue, pf_unlinked_rules); struct mtx_padalign pf_unlnkdrules_mtx; MTX_SYSINIT(pf_unlnkdrules_mtx, &pf_unlnkdrules_mtx, "pf unlinked rules", @@ -402,6 +427,9 @@ static void pf_log_matches(struct pf_pdesc *, struct pf_krule *, struct pf_krule *, struct pf_kruleset *, struct pf_krule_slist *); static void pf_overload_task(void *v, int pending); +static void pf_source_table_task(void *v, int pending); +static bool pf_source_table_enqueue(struct pf_source *sr, + bool insert); static u_short pf_insert_src_node(struct pf_ksrc_node *[PF_SN_MAX], struct pf_srchash *[PF_SN_MAX], struct pf_krule *, struct pf_addr *, sa_family_t, struct pf_addr *, @@ -571,6 +599,15 @@ pf_source_purge(void) srlim->pfsrlim_rate.seconds + 1) continue; + /* Retry a removal pf_source_rele() failed to queue. */ + if (sr->pfsr_intable && + srlim->pfsrlim_overload.table != NULL && + srlim->pfsrlim_overload.lwm > 0) { + if (!pf_source_table_enqueue(sr, false)) + continue; + sr->pfsr_intable = 0; + } + TAILQ_REMOVE(&pf_source_gc, sr, pfsr_empty_gc); RB_REMOVE(pf_source_tree, &srlim->pfsrlim_sources, sr); @@ -603,11 +640,79 @@ pf_source_pfr_addr(struct pfr_addr *p, const struct pf_source *sr) } } +/* + * Queue an overload table change for pf_source_table_task(). The entry + * identifies the limiter by id and remembers its table: either may be gone + * or replaced by the time the task runs, in which case the change is + * dropped. + */ +static bool +pf_source_table_enqueue(struct pf_source *sr, bool insert) +{ + struct pf_source_table_entry *pfste; + + pfste = malloc(sizeof(*pfste), M_PFTEMP, M_NOWAIT); + if (pfste == NULL) + return (false); + + pf_source_pfr_addr(&pfste->addr, sr); + pfste->table = sr->pfsr_parent->pfsrlim_overload.table; + pfste->id = sr->pfsr_parent->pfsrlim_id; + pfste->insert = insert; + + PF_SOURCE_TABLEQ_LOCK(); + STAILQ_INSERT_TAIL(&V_pf_source_tablequeue, pfste, next); + PF_SOURCE_TABLEQ_UNLOCK(); + taskqueue_enqueue(taskqueue_swi, &V_pf_source_tabletask); + + return (true); +} + +static void +pf_source_table_task(void *v, int pending __unused) +{ + struct pf_source_table_head queue = STAILQ_HEAD_INITIALIZER(queue); + struct pf_source_table_entry *pfste, *npfste; + struct pf_sourcelim *srlim; + struct pfr_ktable *t; + + CURVNET_SET((struct vnet *)v); + + PF_SOURCE_TABLEQ_LOCK(); + STAILQ_CONCAT(&queue, &V_pf_source_tablequeue); + PF_SOURCE_TABLEQ_UNLOCK(); + + /* A previous run may already have processed the queue. */ + if (STAILQ_EMPTY(&queue)) { + CURVNET_RESTORE(); + return; + } + + PF_RULES_WLOCK(); + STAILQ_FOREACH(pfste, &queue, next) { + srlim = pf_sourcelim_find(pfste->id); + if (srlim == NULL || + (t = srlim->pfsrlim_overload.table) == NULL || + t != pfste->table) + continue; + + if (pfste->insert) + pfr_insert_kentry(t, &pfste->addr, time_second); + else + pfr_remove_kentry(t, &pfste->addr); + } + PF_RULES_WUNLOCK(); + + STAILQ_FOREACH_SAFE(pfste, &queue, next, npfste) + free(pfste, M_PFTEMP); + + CURVNET_RESTORE(); +} + static void pf_source_used(struct pf_source *sr) { struct pf_sourcelim *srlim = sr->pfsr_parent; - struct pfr_ktable *t; unsigned int used; used = sr->pfsr_inuse++; @@ -615,14 +720,10 @@ pf_source_used(struct pf_source *sr) if (used == 0) TAILQ_REMOVE(&pf_source_gc, sr, pfsr_empty_gc); - else if ((t = srlim->pfsrlim_overload.table) != NULL && + else if (srlim->pfsrlim_overload.table != NULL && used >= srlim->pfsrlim_overload.hwm && !sr->pfsr_intable) { - struct pfr_addr p; - - pf_source_pfr_addr(&p, sr); - - pfr_insert_kentry(t, &p, time_second); - sr->pfsr_intable = 1; + if (pf_source_table_enqueue(sr, true)) + sr->pfsr_intable = 1; } } @@ -630,20 +731,14 @@ static void pf_source_rele(struct pf_source *sr) { struct pf_sourcelim *srlim = sr->pfsr_parent; - struct pfr_ktable *t; unsigned int used; used = --sr->pfsr_inuse; - t = srlim->pfsrlim_overload.table; - if (t != NULL && sr->pfsr_intable && + if (srlim->pfsrlim_overload.table != NULL && sr->pfsr_intable && used < srlim->pfsrlim_overload.lwm) { - struct pfr_addr p; - - pf_source_pfr_addr(&p, sr); - - pfr_remove_kentry(t, &p); - sr->pfsr_intable = 0; + if (pf_source_table_enqueue(sr, false)) + sr->pfsr_intable = 0; } if (used == 0) { @@ -1543,6 +1638,8 @@ retry_waitok3: STAILQ_INIT(&V_pf_sendqueue); SLIST_INIT(&V_pf_overloadqueue); TASK_INIT(&V_pf_overloadtask, 0, pf_overload_task, curvnet); + STAILQ_INIT(&V_pf_source_tablequeue); + TASK_INIT(&V_pf_source_tabletask, 0, pf_source_table_task, curvnet); /* Unlinked, but may be referenced rules. */ TAILQ_INIT(&V_pf_unlinked_rules); @@ -2980,6 +3077,9 @@ pf_unload_vnet_purge(void) pf_purge_expired_src_nodes(); pf_source_purge(); + /* All states are gone, so no more table changes can be queued. */ + taskqueue_drain(taskqueue_swi, &V_pf_source_tabletask); + /* * Now all kifs & rules should be unreferenced, * thus should be successfully freed. diff --git a/tests/sys/netpfil/pf/limiters.sh b/tests/sys/netpfil/pf/limiters.sh index c8ecc517e26c..d109fa424d22 100644 --- a/tests/sys/netpfil/pf/limiters.sh +++ b/tests/sys/netpfil/pf/limiters.sh @@ -306,6 +306,65 @@ source_basic_cleanup() pft_cleanup } +atf_test_case "source_table" "cleanup" +source_table_head() +{ + atf_set descr 'Test the table of a source limiter' + atf_set require.user root +} + +source_table_body() +{ + pft_init + + epair=$(vnet_mkepair) + + ifconfig ${epair}a 192.0.2.2/24 up + + vnet_mkjail alcatraz ${epair}b + jexec alcatraz ifconfig ${epair}b 192.0.2.1/24 up + + # Sanity check + atf_check -s exit:0 -o ignore \ + ping -c 1 192.0.2.1 + + jexec alcatraz pfctl -e + + # This used to panic a kernel with INVARIANTS: the table was changed + # while the packet was handled, with the rules lock held for reading. + pft_set_rules alcatraz \ + "set timeout icmp.error 120" \ + "table persist" \ + "source limiter \"server\" id 1 entries 128 limit 100 table above 2 below 2" \ + "pass in proto icmp source limiter \"server\"" + + # Each ping creates a state. Two states do not exceed the mark. + atf_check -s exit:0 -o ignore ping -c 1 192.0.2.1 + atf_check -s exit:0 -o ignore ping -c 1 192.0.2.1 + sleep 1 + atf_check -s exit:0 -o empty \ + jexec alcatraz pfctl -t many -T show + + atf_check -s exit:0 -o ignore ping -c 1 192.0.2.1 + atf_check -s exit:0 -o ignore ping -c 1 192.0.2.1 + # The address is added to the table by a task. + sleep 1 + atf_check -s exit:0 -o "match:192.0.2.2" \ + jexec alcatraz pfctl -t many -T show + + # Once the states are gone the source is below the mark and is removed. + atf_check -s exit:0 -e ignore \ + jexec alcatraz pfctl -F states + sleep 1 + atf_check -s exit:0 -o empty \ + jexec alcatraz pfctl -t many -T show +} + +source_table_cleanup() +{ + pft_cleanup +} + atf_test_case "out" "cleanup" out_head() { @@ -358,5 +417,6 @@ atf_init_test_cases() atf_add_test_case "state_block" atf_add_test_case "state_multiple" atf_add_test_case "source_basic" + atf_add_test_case "source_table" atf_add_test_case "out" }