pf_sourcelim_add() in sys/net/pf_ioctl.c has two bugs in the error
path when the second RBT_INSERT (into pf_sourcelim_nm_tree_inactive)
fails.
1) Lines 1579-1580 attempt to undo the first insert but do the
wrong operation on the wrong tree:
RBT_INSERT(pf_sourcelim_nm_tree,
&pf_sourcelim_nm_tree_inactive, pfsrlim);
This is a no-op (the nm_tree insert already failed because a
duplicate exists). It should remove from the id_tree where
the first insert succeeded:
RBT_REMOVE(pf_sourcelim_id_tree,
&pf_sourcelim_id_tree_inactive, pfsrlim);
pf_statelim_add() in the same file has the correct pattern
at lines 1046-1047. Possible copy/paste error.
After the failed cleanup, pool_put at line 1596 frees the
entry while it is still linked in pf_sourcelim_id_tree_inactive.
Any subsequent DIOCADDSOURCELIM traverses the tree and
dereferences the freed node.
2) The unlock error path does not detach the overload table
attached at lines 1561-1564. The entry is never added to
the TAILQ (line 1585 is skipped), so pf_sourcelim_rollback()
will not find it either. The table reference leaks.
Both bugs introduced on 2025-11-11 (dlg, "introduce source and
state limiters in pf").
Tested on OpenBSD 7.9-beta amd64: DIOCADDSOURCELIM with duplicate
name then subsequent inserts. Stock kernel: freed node left in
tree (UAF on next traversal). Patched kernel: 20 inserts after
the error all succeed, rollback clean, no kernel messages.
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;
}
@@ -1590,6 +1590,8 @@ pf_sourcelim_add(const struct pfioc_sour
return (0);
unlock:
+ if (pfsrlim->pfsrlim_overload.table != NULL)
+ pfr_detach_table(pfsrlim->pfsrlim_overload.table);
PF_UNLOCK();
NET_UNLOCK();
free:
