Re: pf: u_int32_t conn counter underflow in pf_src_tree_remove_state()

[Alexandr Nedvedicky]

Hello Janak,

I was not able to reproduce the issue on my PC-engines APU2 router. I will
appreciate if you can give a try to diff below to check if it fixes the issue.
The diff is compile/regress tested only.

thanks and
regards
sashan

--------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)