From: Alexandr Nedvedicky Subject: pf: u_int32_t conn counter underflow in pf_src_tree_remove_state() To: tech@openbsd.org Date: Sat, 15 Aug 2026 12:10:03 +0200 Hello, resending patch from bugs [1], the mail is part of this thread [2]. the pf(4) needs to use aotmic ops to bump connection counter at source node. This happens in function pf_src_connlimit() which is called on behalf of pf_test_state() via pf_tcp_track_*() functions. the pf_test_state() itself is being called from pf_test() here: 8511 8512 PF_STATE_ENTER_READ(); 8513 action = pf_find_state(&pd, &key, &st); 8514 st = pf_state_ref(st); 8515 PF_STATE_EXIT_READ(); 8516 8517 /* check for syncookies if tcp ack and no active state */ .... 8546 8547 if (action == PF_MATCH) 8548 action = pf_test_state(&pd, &st, &reason); the thing is that pf_test_state() uses a reference to state. the function itself is running without locks. It may happen two packets try to update connection counter at the same source node entry. If that happens in parallel then counter may miss update. This later leads to underflow. The reporter (Mr. Janak Trivedi) confirms patch below works. I think he replied off-list. OK to commit diff below? thanks and regards sashan [1] https://marc.info/?l=openbsd-bugs&m=178627901979767&w=2 [2] https://marc.info/?t=178601035500001&r=1&w=2 --------8<---------------8<---------------8<------------------8<-------- diff --git a/sys/net/pf.c b/sys/net/pf.c index 0fd00c0dbf3..ca28eb0b98f 100644 --- a/sys/net/pf.c +++ b/sys/net/pf.c @@ -760,7 +760,13 @@ pf_src_connlimit(struct pf_state **stp) if ((sn = pf_get_src_node((*stp), PF_SN_NONE)) == NULL) return (0); - sn->conn++; + /* + * Note: conn limit is bumped on SYN_SENT->ESTBLISHED + * state transition. packet does not hold any locks + * when running here, therefore atomic is needed. + */ + atomic_inc_int((int *)&sn->conn); + (*stp)->src.tcp_est = 1; pf_add_threshold(&sn->conn_rate); @@ -2051,10 +2057,16 @@ pf_src_tree_remove_state(struct pf_state *st) u_int32_t timeout; struct pf_sn_item *sni; + PF_ASSERT_LOCKED(); + while ((sni = SLIST_FIRST(&st->src_nodes)) != NULL) { SLIST_REMOVE_HEAD(&st->src_nodes, next); - if (st->src.tcp_est) + if (st->src.tcp_est) { + /* + * atomic not needed here, because of PF_LOCK() + */ --sni->sn->conn; + } if (--sni->sn->states == 0) { timeout = st->rule.ptr->timeout[PFTM_SRC_NODE]; if (!timeout)