git: c25259a40e3f - stable/15 - pf: Avoid taking the pf rules write lock in a couple of ioctls
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Tue, 06 Jan 2026 15:57:04 UTC
The branch stable/15 has been updated by markj:
URL: https://cgit.FreeBSD.org/src/commit/?id=c25259a40e3f1e0e9fc85f44975ebcdeb6d0cdeb
commit c25259a40e3f1e0e9fc85f44975ebcdeb6d0cdeb
Author: Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2025-12-19 14:07:26 +0000
Commit: Mark Johnston <markj@FreeBSD.org>
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 *);