From nobody Tue Jan 6 15:57:04 2026 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 4dlwlh6K0Bz6NnGN for ; Tue, 06 Jan 2026 15:57:04 +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 "R13" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4dlwlh3n78z3s2W for ; Tue, 06 Jan 2026 15:57:04 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1767715024; 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=JOyuhRM16rqg9Y5RSYuUo2axwgn7uhn1OKNDhpLYkeY=; b=uS9rg79zN3ATGM5uaJOD47atRZ7ARqAwqvw8ezmghQXM2xnLIQ6xHnyrHXWTZK2QnxN0a3 hVvIRZi5lNzA/bEUIQHLOb9iGDSjFbesW7K1MMRWizzyvdAIb3DyzqJmaWw7ZJ/Y3v07NZ sGym/+XCJPOLCeeGfs7WzmO8o2uPOabGImfdbXfkUcxv+v0zguSdeq/ViY9p8euu/LXNdI sugl3Sgz/CM5EHYFJ24lhzjxlKd2R9b188HXLyBZcLtLfcLMCekhFG9/1jXMIXLd/vQjtC tSa8eyHYmvSCkddOov+kSS0PFprWtwQ91kMke9SYxMBDC5lYRMuLrJM6YBgqKQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1767715024; 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=JOyuhRM16rqg9Y5RSYuUo2axwgn7uhn1OKNDhpLYkeY=; b=WQWm1MuCMIAosWtz9py2MsQ0Fv0SkHOmVUosMNp52bBDAtzEiRpG0ffVbJItglRegVayha gDOpYJv3ZOKcAq3HHsWnGbdf6P29Ng62cgoIxs7Jf9E8H+RVNEGsEgE8NoX0hFocYwEOFW FZBzctu2ScGVUjfvNFf+Bv0wS0eMLNAcdvgcvEbQzJ6MT91rH+SK5S0Why9t+Bssn0CUs+ tdyr5ciw6OLUqpc37rR63+tNJ0+xEMzKkjRYFfklbh98gpxUmJ+A0mEvxgqQqqWK412Fa2 RSCx+AMapevKYVOyJl2shxtmzpDBDmCL+ZWgicnd0IsH1aZWtm3OCLQ/Cjmggg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1767715024; a=rsa-sha256; cv=none; b=OBn34jeSKyguoDFXyOGNS9REqgUKOzcGah/xvPPzMjTzHiMh0Beso2e+dypn4nsrtPB+P8 pHPVIWBmt326xH9mI+imwdFAqm9LdBDiyGBGayTeSMC1T5W1QCsUd2CSdz1Yj5zUQEKGy5 FiVVbMAWWTHPPyl1j3UykHR5s0ROwSjrZNIaiWeyjrYVUowfAnNgUtn0XbZpzk0GRv3dp+ n0mDc0r/kndzxB4UZ9s6nygrTcR2oQWxnPdCqySqy9ph8c9pORdJeVARmkY3DfSQhtEDlt DaRBDIakFA/evAqdl6hl97kKiKxzJMEfZJb+zSDLHPWzYiiS9WH/Zr2DQ39Cnw== 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 4dlwlh3LXgz1CLp for ; Tue, 06 Jan 2026 15:57:04 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 25b9b by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Tue, 06 Jan 2026 15:57:04 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Mark Johnston Subject: git: c25259a40e3f - stable/15 - pf: Avoid taking the pf rules write lock in a couple of ioctls 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: markj X-Git-Repository: src X-Git-Refname: refs/heads/stable/15 X-Git-Reftype: branch X-Git-Commit: c25259a40e3f1e0e9fc85f44975ebcdeb6d0cdeb Auto-Submitted: auto-generated Date: Tue, 06 Jan 2026 15:57:04 +0000 Message-Id: <695d30d0.25b9b.12981706@gitrepo.freebsd.org> The branch stable/15 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=c25259a40e3f1e0e9fc85f44975ebcdeb6d0cdeb commit c25259a40e3f1e0e9fc85f44975ebcdeb6d0cdeb Author: Mark Johnston AuthorDate: 2025-12-19 14:07:26 +0000 Commit: Mark Johnston CommitDate: 2026-01-06 15:56:49 +0000 pf: Avoid taking the pf rules write lock in a couple of ioctls The DIOCGETRULES ioctl handlers has taken the write lock ever since fine-grained locking was merged to pf, but I believe it's unneeded. Use the read lock instead. DIOCGETRULENV takes the write lock as well but I believe this is only required when clearing rule counters. Acquire the read lock if that is not the case. Reviewed by: kp, allanjude MFC after: 2 weeks Sponsored by: OPNsense Sponsored by: Klara, Inc. Differential Revision: https://reviews.freebsd.org/D54292 (cherry picked from commit ae96ff302f8ae50903a96d3a1857f9acf243f3c4) --- sys/netpfil/pf/pf_ioctl.c | 91 ++++++++++++++++++++++------------------------- sys/netpfil/pf/pf_nv.c | 2 +- sys/netpfil/pf/pf_nv.h | 2 +- 3 files changed, 45 insertions(+), 50 deletions(-) diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c index e2b63965d1e1..a6e234fcac53 100644 --- a/sys/netpfil/pf/pf_ioctl.c +++ b/sys/netpfil/pf/pf_ioctl.c @@ -2085,19 +2085,20 @@ pf_rule_to_krule(const struct pf_rule *rule, struct pf_krule *krule) int pf_ioctl_getrules(struct pfioc_rule *pr) { + PF_RULES_RLOCK_TRACKER; struct pf_kruleset *ruleset; struct pf_krule *tail; int rs_num; - PF_RULES_WLOCK(); + PF_RULES_RLOCK(); ruleset = pf_find_kruleset(pr->anchor); if (ruleset == NULL) { - PF_RULES_WUNLOCK(); + PF_RULES_RUNLOCK(); return (EINVAL); } rs_num = pf_get_ruleset_number(pr->rule.action); if (rs_num >= PF_RULESET_MAX) { - PF_RULES_WUNLOCK(); + PF_RULES_RUNLOCK(); return (EINVAL); } tail = TAILQ_LAST(ruleset->rules[rs_num].active.ptr, @@ -2107,7 +2108,7 @@ pf_ioctl_getrules(struct pfioc_rule *pr) else pr->nr = 0; pr->ticket = ruleset->rules[rs_num].active.ticket; - PF_RULES_WUNLOCK(); + PF_RULES_RUNLOCK(); return (0); } @@ -3694,6 +3695,7 @@ DIOCADDRULENV_error: } case DIOCGETRULENV: { + PF_RULES_RLOCK_TRACKER; struct pfioc_nv *nv = (struct pfioc_nv *)addr; nvlist_t *nvrule = NULL; nvlist_t *nvl = NULL; @@ -3704,6 +3706,13 @@ DIOCADDRULENV_error: bool clear_counter = false; #define ERROUT(x) ERROUT_IOCTL(DIOCGETRULENV_error, x) +#define ERROUT_LOCKED(x) do { \ + if (clear_counter) \ + PF_RULES_WUNLOCK(); \ + else \ + PF_RULES_RUNLOCK(); \ + ERROUT(x); \ +} while (0) if (nv->len > pf_ioctl_maxcount) ERROUT(ENOMEM); @@ -3735,78 +3744,64 @@ DIOCADDRULENV_error: nr = nvlist_get_number(nvl, "nr"); - PF_RULES_WLOCK(); + if (clear_counter) + PF_RULES_WLOCK(); + else + PF_RULES_RLOCK(); ruleset = pf_find_kruleset(nvlist_get_string(nvl, "anchor")); - if (ruleset == NULL) { - PF_RULES_WUNLOCK(); - ERROUT(ENOENT); - } + if (ruleset == NULL) + ERROUT_LOCKED(ENOENT); rs_num = pf_get_ruleset_number(nvlist_get_number(nvl, "ruleset")); - if (rs_num >= PF_RULESET_MAX) { - PF_RULES_WUNLOCK(); - ERROUT(EINVAL); - } + if (rs_num >= PF_RULESET_MAX) + ERROUT_LOCKED(EINVAL); if (nvlist_get_number(nvl, "ticket") != - ruleset->rules[rs_num].active.ticket) { - PF_RULES_WUNLOCK(); - ERROUT(EBUSY); - } + ruleset->rules[rs_num].active.ticket) + ERROUT_LOCKED(EBUSY); - if ((error = nvlist_error(nvl))) { - PF_RULES_WUNLOCK(); - ERROUT(error); - } + if ((error = nvlist_error(nvl))) + ERROUT_LOCKED(error); rule = TAILQ_FIRST(ruleset->rules[rs_num].active.ptr); while ((rule != NULL) && (rule->nr != nr)) rule = TAILQ_NEXT(rule, entries); - if (rule == NULL) { - PF_RULES_WUNLOCK(); - ERROUT(EBUSY); - } + if (rule == NULL) + ERROUT_LOCKED(EBUSY); nvrule = pf_krule_to_nvrule(rule); nvlist_destroy(nvl); nvl = nvlist_create(0); - if (nvl == NULL) { - PF_RULES_WUNLOCK(); - ERROUT(ENOMEM); - } + if (nvl == NULL) + ERROUT_LOCKED(ENOMEM); nvlist_add_number(nvl, "nr", nr); nvlist_add_nvlist(nvl, "rule", nvrule); nvlist_destroy(nvrule); nvrule = NULL; - if (pf_kanchor_nvcopyout(ruleset, rule, nvl)) { - PF_RULES_WUNLOCK(); - ERROUT(EBUSY); - } + if (pf_kanchor_nvcopyout(ruleset, rule, nvl)) + ERROUT_LOCKED(EBUSY); free(nvlpacked, M_NVLIST); nvlpacked = nvlist_pack(nvl, &nv->len); - if (nvlpacked == NULL) { - PF_RULES_WUNLOCK(); - ERROUT(ENOMEM); - } + if (nvlpacked == NULL) + ERROUT_LOCKED(ENOMEM); - if (nv->size == 0) { - PF_RULES_WUNLOCK(); - ERROUT(0); - } - else if (nv->size < nv->len) { - PF_RULES_WUNLOCK(); - ERROUT(ENOSPC); - } + if (nv->size == 0) + ERROUT_LOCKED(0); + else if (nv->size < nv->len) + ERROUT_LOCKED(ENOSPC); - if (clear_counter) + if (clear_counter) { pf_krule_clear_counters(rule); - - PF_RULES_WUNLOCK(); + PF_RULES_WUNLOCK(); + } else { + PF_RULES_RUNLOCK(); + } error = copyout(nvlpacked, nv->data, nv->len); +#undef ERROUT_LOCKED #undef ERROUT DIOCGETRULENV_error: free(nvlpacked, M_NVLIST); diff --git a/sys/netpfil/pf/pf_nv.c b/sys/netpfil/pf/pf_nv.c index 2f484e2dabc6..3e741dd39974 100644 --- a/sys/netpfil/pf/pf_nv.c +++ b/sys/netpfil/pf/pf_nv.c @@ -684,7 +684,7 @@ error: } nvlist_t * -pf_krule_to_nvrule(struct pf_krule *rule) +pf_krule_to_nvrule(const struct pf_krule *rule) { nvlist_t *nvl, *tmp; u_int64_t src_nodes_total = 0; diff --git a/sys/netpfil/pf/pf_nv.h b/sys/netpfil/pf/pf_nv.h index cf9fbf8bcf5b..9e43ff1e642a 100644 --- a/sys/netpfil/pf/pf_nv.h +++ b/sys/netpfil/pf/pf_nv.h @@ -78,7 +78,7 @@ int pf_nvstring(const nvlist_t *, const char *, char *, size_t); int pf_check_rule_addr(const struct pf_rule_addr *); -nvlist_t *pf_krule_to_nvrule(struct pf_krule *); +nvlist_t *pf_krule_to_nvrule(const struct pf_krule *); int pf_nvrule_to_krule(const nvlist_t *, struct pf_krule *); int pf_nvstate_kill_to_kstate_kill(const nvlist_t *, struct pf_kstate_kill *);