Download raw body.
pf: refcount struct pf_rule lifetime
pf_test() ends with this comment:
/*
* At the moment, we rely on NET_LOCK() to prevent removal of items
* we've collected above ('r', 'anchor' and 'ruleset'). They'll have
* to be refcounted when NET_LOCK() is gone.
*/
struct pf_anchor already has that lifetime refcount (struct refcnt ref
in pfvar.h, initialised in pf_create_anchor(), wrapped by
pf_anchor_take()/pf_anchor_rele() in pf_ruleset.c). struct pf_ruleset
is embedded in pf_anchor via anchor->ruleset, and pf_main_ruleset is
a macro for pf_main_anchor.ruleset, so ruleset lifetime tracks the
anchor for free. struct pf_rule has no refcount.
Diff below adds one to pf_rule, matching the anchor pattern:
refcnt_init() at the two pool_get(&pf_rule_pl, ...) sites in
pfioctl(), refcnt_rele() gating the three pool_put(&pf_rule_pl, ...)
sites in pf_rule_free(), pf_rm_rule() and the DIOCCHANGERULE error
path. No caller takes an extra reference yet; every refcnt_rele()
sees count 1, returns non-zero, and pool_put() runs exactly as
before. The intent is to get the field and the release gate in
place so a later diff can refcnt_take() around rule selection in
pf_test() without having to touch the lifetime paths again.
sizeof(struct pf_rule) changes, which cascades through pfioc_rule to
DIOC{ADD,GET,CHANGE}RULE; pfctl has to be rebuilt in lockstep. No
new include: pfvar.h already pulls in <sys/refcnt.h>.
Tested on amd64 GENERIC.MP against a recent snapshot:
- builds clean (-Werror, clang)
- regress/sys/net/pf_trans passes (run-regress-dev-limit with 2048
procs and 1023 /dev/pf fds each, run-regress-iocmd-limit with
DIOCGETRULES and DIOCXEND)
- 500 iterations of `pfctl -f /etc/pf.conf` under concurrent TCP
traffic: no errors, no warnings, pfrule pool stats balanced
(3518 cumulative requests, 8 in-use at end matching the loaded
ruleset)
- no panics, no splassert, no witness complaints, no leaks
ok?
Index: sys/net/pfvar.h
===================================================================
--- sys/net/pfvar.h
+++ sys/net/pfvar.h
@@ -614,6 +614,7 @@
} divert;
time_t exptime;
+ struct refcnt ref; /* [a] rule lifetime */
};
/* rule flags */
Index: sys/net/pf_ioctl.c
===================================================================
--- sys/net/pf_ioctl.c
+++ sys/net/pf_ioctl.c
@@ -355,6 +355,8 @@
pfi_kif_free(rule->nat.kif);
pfi_kif_free(rule->route.kif);
+ if (refcnt_rele(&rule->ref) == 0)
+ return;
pool_put(&pf_rule_pl, rule);
}
@@ -408,6 +410,8 @@
pfi_kif_unref(rule->nat.kif, PFI_KIF_REF_RULE);
pfi_kif_unref(rule->route.kif, PFI_KIF_REF_RULE);
pf_remove_anchor(rule);
+ if (refcnt_rele(&rule->ref) == 0)
+ return;
pool_put(&pf_rule_pl, rule);
}
@@ -2327,6 +2331,7 @@
error = ENOMEM;
goto fail;
}
+ refcnt_init(&rule->ref);
if ((error = pf_rule_copyin(&pr->rule, rule))) {
pf_rule_free(rule);
@@ -2578,10 +2583,12 @@
error = ENOMEM;
goto fail;
}
+ refcnt_init(&newrule->ref);
if (pcr->rule.return_icmp >> 8 > ICMP_MAXTYPE) {
error = EINVAL;
- pool_put(&pf_rule_pl, newrule);
+ if (refcnt_rele(&newrule->ref) != 0)
+ pool_put(&pf_rule_pl, newrule);
goto fail;
}
error = pf_rule_copyin(&pcr->rule, newrule);
pf: refcount struct pf_rule lifetime