Module Name:    src
Committed By:   martin
Date:           Mon Dec  1 09:02:26 UTC 2014

Modified Files:
        src/sys/net/npf [netbsd-7]: npf_nat.c npf_ruleset.c

Log Message:
Pull up following revision(s) (requested by rmind in ticket #274):
        sys/net/npf/npf_nat.c: revision 1.35
        sys/net/npf/npf_ruleset.c: revision 1.38
NPF: fix the reference counting and share the active NAT portmap correctly
when performing the reload.  Should fixes PR/49412, reported by kardel@.


To generate a diff of this commit:
cvs rdiff -u -r1.32.2.1 -r1.32.2.2 src/sys/net/npf/npf_nat.c
cvs rdiff -u -r1.37 -r1.37.2.1 src/sys/net/npf/npf_ruleset.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/net/npf/npf_nat.c
diff -u src/sys/net/npf/npf_nat.c:1.32.2.1 src/sys/net/npf/npf_nat.c:1.32.2.2
--- src/sys/net/npf/npf_nat.c:1.32.2.1	Fri Aug 29 11:14:14 2014
+++ src/sys/net/npf/npf_nat.c	Mon Dec  1 09:02:26 2014
@@ -1,4 +1,4 @@
-/*	$NetBSD: npf_nat.c,v 1.32.2.1 2014/08/29 11:14:14 martin Exp $	*/
+/*	$NetBSD: npf_nat.c,v 1.32.2.2 2014/12/01 09:02:26 martin 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.32.2.1 2014/08/29 11:14:14 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: npf_nat.c,v 1.32.2.2 2014/12/01 09:02:26 martin Exp $");
 
 #include <sys/param.h>
 #include <sys/types.h>
@@ -313,9 +313,10 @@ npf_nat_freepolicy(npf_natpolicy_t *np)
 		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 @@ npf_nat_sharepm(npf_natpolicy_t *np, npf
 	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;
 }
 

Index: src/sys/net/npf/npf_ruleset.c
diff -u src/sys/net/npf/npf_ruleset.c:1.37 src/sys/net/npf/npf_ruleset.c:1.37.2.1
--- src/sys/net/npf/npf_ruleset.c:1.37	Mon Aug 11 01:54:12 2014
+++ src/sys/net/npf/npf_ruleset.c	Mon Dec  1 09:02:26 2014
@@ -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.37.2.1 2014/12/01 09:02:26 martin 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.37.2.1 2014/12/01 09:02:26 martin Exp $");
 
 #include <sys/param.h>
 #include <sys/types.h>
@@ -421,22 +421,6 @@ npf_ruleset_gc(npf_ruleset_t *rlset)
 }
 
 /*
- * 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 @@ npf_ruleset_reload(npf_ruleset_t *newset
 			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_ruleset_sharepm(npf_ruleset_t *rlset
 	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;

Reply via email to