[src/trunk]: src/sys/net/npf NPF: fix the reference counting and share the ac...

[rmind <rmind%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/f39b67ee757c
branches:  trunk
changeset: 334022:f39b67ee757c
user:      rmind <rmind%NetBSD.org@localhost>
date:      Wed Nov 26 21:25:35 2014 +0000

description:
NPF: fix the reference counting and share the active NAT portmap correctly
when performing the reload.  Should fixes PR/49412, reported by kardel@.

diffstat:

 sys/net/npf/npf_nat.c     |  23 ++++++++++++--------
 sys/net/npf/npf_ruleset.c |  51 +++++++++++++++++++---------------------------
 2 files changed, 35 insertions(+), 39 deletions(-)

diffs (151 lines):

diff -r c761e9f30916 -r f39b67ee757c sys/net/npf/npf_nat.c
--- a/sys/net/npf/npf_nat.c     Wed Nov 26 20:46:46 2014 +0000
+++ b/sys/net/npf/npf_nat.c     Wed Nov 26 21:25:35 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: npf_nat.c,v 1.34 2014/08/24 20:36:30 rmind Exp $       */
+/*     $NetBSD: npf_nat.c,v 1.35 2014/11/26 21:25:35 rmind Exp $       */
 
 /*-
  * Copyright (c) 2014 Mindaugas Rasiukevicius <rmind at netbsd org>
@@ -71,7 +71,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: npf_nat.c,v 1.34 2014/08/24 20:36:30 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: npf_nat.c,v 1.35 2014/11/26 21:25:35 rmind Exp $");
 
 #include <sys/param.h>
 #include <sys/types.h>
@@ -313,9 +313,10 @@
                kpause("npfgcnat", false, 1, NULL);
        }
        KASSERT(LIST_EMPTY(&np->n_nat_list));
+       KASSERT(pm == NULL || pm->p_refcnt > 0);
 
        /* Destroy the port map, on last reference. */
-       if (pm && --pm->p_refcnt == 0) {
+       if (pm && atomic_dec_uint_nv(&pm->p_refcnt) == 0) {
                KASSERT((np->n_flags & NPF_NAT_PORTMAP) != 0);
                kmem_free(pm, PORTMAP_MEM_SIZE);
        }
@@ -373,17 +374,21 @@
        if (memcmp(&np->n_taddr, &mnp->n_taddr, np->n_alen) != 0) {
                return false;
        }
-       /* If NAT policy has an old port map - drop the reference. */
        mpm = mnp->n_portmap;
-       if (mpm) {
-               /* Note: at this point we cannot hold a last reference. */
-               KASSERT(mpm->p_refcnt > 1);
-               mpm->p_refcnt--;
+       KASSERT(mpm == NULL || mpm->p_refcnt > 0);
+
+       /*
+        * If NAT policy has an old port map - drop the reference
+        * and destroy the port map if it was the last.
+        */
+       if (mpm && atomic_dec_uint_nv(&mpm->p_refcnt) == 0) {
+               kmem_free(mpm, PORTMAP_MEM_SIZE);
        }
+
        /* Share the port map. */
        pm = np->n_portmap;
+       atomic_inc_uint(&pm->p_refcnt);
        mnp->n_portmap = pm;
-       pm->p_refcnt++;
        return true;
 }
 
diff -r c761e9f30916 -r f39b67ee757c sys/net/npf/npf_ruleset.c
--- a/sys/net/npf/npf_ruleset.c Wed Nov 26 20:46:46 2014 +0000
+++ b/sys/net/npf/npf_ruleset.c Wed Nov 26 21:25:35 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: npf_ruleset.c,v 1.37 2014/08/11 01:54:12 rmind Exp $   */
+/*     $NetBSD: npf_ruleset.c,v 1.38 2014/11/26 21:25:35 rmind Exp $   */
 
 /*-
  * Copyright (c) 2009-2013 The NetBSD Foundation, Inc.
@@ -34,7 +34,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: npf_ruleset.c,v 1.37 2014/08/11 01:54:12 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: npf_ruleset.c,v 1.38 2014/11/26 21:25:35 rmind Exp $");
 
 #include <sys/param.h>
 #include <sys/types.h>
@@ -421,22 +421,6 @@
 }
 
 /*
- * npf_ruleset_cmpnat: find a matching NAT policy in the ruleset.
- */
-static inline npf_rule_t *
-npf_ruleset_cmpnat(npf_ruleset_t *rlset, npf_natpolicy_t *mnp)
-{
-       npf_rule_t *rl;
-
-       /* Find a matching NAT policy in the old ruleset. */
-       LIST_FOREACH(rl, &rlset->rs_all, r_aentry) {
-               if (rl->r_natp && npf_nat_cmppolicy(rl->r_natp, mnp))
-                       break;
-       }
-       return rl;
-}
-
-/*
  * npf_ruleset_reload: prepare the new ruleset by scanning the active
  * ruleset and 1) sharing the dynamic rules 2) sharing NAT policies.
  *
@@ -492,18 +476,30 @@
                        continue;
                }
 
+               /*
+                * First, try to share the active port map.  If this
+                * policy will be unused, npf_nat_freepolicy() will
+                * drop the reference.
+                */
+               npf_ruleset_sharepm(oldset, np);
+
                /* Does it match with any policy in the active ruleset? */
-               if ((actrl = npf_ruleset_cmpnat(oldset, np)) == NULL) {
+               LIST_FOREACH(actrl, &oldset->rs_all, r_aentry) {
+                       if (!actrl->r_natp)
+                               continue;
+                       if ((actrl->r_attr & NPF_RULE_KEEPNAT) != 0)
+                               continue;
+                       if (npf_nat_cmppolicy(actrl->r_natp, np))
+                               break;
+               }
+               if (!actrl) {
+                       /* No: just set the ID and continue. */
                        npf_nat_setid(np, ++nid);
                        continue;
                }
 
-               /*
-                * Inherit the matching NAT policy and check other ones
-                * in the new ruleset for sharing the portmap.
-                */
+               /* Yes: inherit the matching NAT policy. */
                rl->r_natp = actrl->r_natp;
-               npf_ruleset_sharepm(newset, rl->r_natp);
                npf_nat_setid(rl->r_natp, ++nid);
 
                /*
@@ -525,13 +521,8 @@
        npf_natpolicy_t *np;
        npf_rule_t *rl;
 
-       /* Find a matching NAT policy in the old ruleset. */
+       /* Find a matching NAT policy in the old ruleset; skip the self. */
        LIST_FOREACH(rl, &rlset->rs_all, r_aentry) {
-               /*
-                * NAT policy might not yet be set during the creation of
-                * the ruleset (in such case, rule is for our policy), or
-                * policies might be equal due to rule exchange on reload.
-                */
                np = rl->r_natp;
                if (np == NULL || np == mnp)
                        continue;