pf_create_state() is sometimes better to use pf_unlink_state()

[Alexandr Nedvedicky]

Hello,

snippet below comes from pf_create_state():

   3559         if (pf_state_insert(BOUND_IFACE(r, pd->kif), skw, sks, s)) {
   3560                 pf_state_key_detach(s, PF_SK_STACK);
   3561                 pf_state_key_detach(s, PF_SK_WIRE);
   3562                 *sks = *skw = NULL;
   3563                 REASON_SET(&reason, PFRES_STATEINS);
   3564                 goto csfailed;
   3565         } else
   3566                 *sm = s;
   3567 
   3568         /* attach src nodes late, otherwise cleanup on error nontrivial 
*/
   3569         for (i = 0; i < PF_SN_MAX; i++)
   3570                 if (sns[i] != NULL) {
   3571                         struct pf_sn_item       *sni;
   3572 
   3573                         sni = pool_get(&pf_sn_item_pl, PR_NOWAIT);
   3574                         if (sni == NULL) {
   3575                                 REASON_SET(&reason, PFRES_MEMORY);
   3576                                 pf_src_tree_remove_state(s);
   3577                                 STATE_DEC_COUNTERS(s);
   3578                                 pool_put(&pf_state_pl, s);
   3579                                 return (PF_DROP);
   3580                         }
   3581                         sni->sn = sns[i];
   3582                         SLIST_INSERT_HEAD(&s->src_nodes, sni, next);
   3583                         sni->sn->states++;
   3584                 }

at line 3559 PF inserts state to table. If insert operation succeeds, then
state can no longer be killed using simple pool_put() as it currently
happens at line 3578. I think PF should go for pf_unlink_state() instead.

patch below should kill the bug.

also one more off-topic question:

        would you be interested in SMP patch for PF?
        it basically introduces fine locking and reference counting
        on PF data objects, so firewall can handle more packets at
        single instance of time.

regards
sasha

===================================================================
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.913
diff -u -r1.913 pf.c
--- pf.c        11 May 2015 12:22:14 -0000      1.913
+++ pf.c        21 May 2015 15:20:01 -0000
@@ -3573,9 +3573,7 @@   
                        sni = pool_get(&pf_sn_item_pl, PR_NOWAIT);
                        if (sni == NULL) {
                                REASON_SET(&reason, PFRES_MEMORY);
-                               pf_src_tree_remove_state(s);
-                               STATE_DEC_COUNTERS(s);
-                               pool_put(&pf_state_pl, s);
+                               pf_unlink_state(s);
                                return (PF_DROP);
                        }
                        sni->sn = sns[i];