On Thu, Apr 10, 2026, Alexandr Nedvedicky wrote:
> I think this needs to be moved to the free: goto target below.
The early goto free paths (strlcpy failures) never attach a
table, so the != NULL guard works at either location.
At free: the PF_LOCK is no longer held. pfr_detach_table does
a non-atomic refcount decrement, and all existing callers appear
to hold PF_LOCK (pf_free_state via pf_rm_rule, pf_commit_rules,
pf_sourcelim_rollback, etc.). Since pfr_ktable objects are
shared globally, pf_free_state from the packet path could race
on the same refcount.
At unlock: the lock is still held, no race possible.
But you know the locking better than me and maybe I missed
something about the locking.
Updated diff moves it to free: as you suggested.
Index: sys/net/pf_ioctl.c
===================================================================
--- sys/net/pf_ioctl.c
+++ sys/net/pf_ioctl.c
@@ -1576,8 +1576,8 @@ pf_sourcelim_add(const struct pfioc_sour
if (RBT_INSERT(pf_sourcelim_nm_tree,
&pf_sourcelim_nm_tree_inactive, pfsrlim) != NULL) {
- RBT_INSERT(pf_sourcelim_nm_tree,
- &pf_sourcelim_nm_tree_inactive, pfsrlim);
+ RBT_REMOVE(pf_sourcelim_id_tree,
+ &pf_sourcelim_id_tree_inactive, pfsrlim);
error = EBUSY;
goto unlock;
}
@@ -1593,6 +1593,8 @@ unlock:
NET_UNLOCK();
free:
+ if (pfsrlim->pfsrlim_overload.table != NULL)
+ pfr_detach_table(pfsrlim->pfsrlim_overload.table);
pool_put(&pf_sourcelim_pl, pfsrlim);
return (error);
