Clean up reference counting wrt state creation and destruction. Fixes

problems with adaptive timeouts, max-states limits, and rules not being
freed from memory.

Diff from Chris Pascoe.

ok henning@ dhartmei@

2 files changed, 33 insertions, 22 deletions

diff --git a/sys/net/if_pfsync.c b/sys/net/if_pfsync.c
index 66b5c5c2ee1..0bf5f902769 100644
--- a/sys/net/if_pfsync.c
+++ b/sys/net/if_pfsync.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_pfsync.c,v 1.37 2004/08/30 07:44:28 mcbride Exp $	*/
+/*	$OpenBSD: if_pfsync.c,v 1.38 2004/09/17 21:49:15 mcbride Exp $	*/
 
 /*
  * Copyright (c) 2002 Michael Shalayeff
@@ -199,6 +199,7 @@ pfsync_insert_net_state(struct pfsync_state *sp)
 	st->rule.ptr = r;
 	/* XXX get pointers to nat_rule and anchor */
 
+	/* XXX when we have nat_rule/anchors, use STATE_INC_COUNTERS */
 	r->states++;
 
 	/* fill in the rest of the state entry */
@@ -227,6 +228,8 @@ pfsync_insert_net_state(struct pfsync_state *sp)
 
 	if (pf_insert_state(kif, st)) {
 		pfi_maybe_destroy(kif);
+		/* XXX when we have nat_rule/anchors, use STATE_DEC_COUNTERS */
+		r->states--;
 		pool_put(&pf_state_pl, st);
 		return (EINVAL);
 	}
diff --git a/sys/net/pf.c b/sys/net/pf.c
index 248b5df7e3e..ac07334887e 100644
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf.c,v 1.457 2004/07/11 15:54:21 itojun Exp $ */
+/*	$OpenBSD: pf.c,v 1.458 2004/09/17 21:49:15 mcbride Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -250,6 +250,24 @@ struct pf_pool_limit pf_pool_limits[PF_LIMIT_MAX] = {
 	((r)->rule_flag & PFRULE_GRBOUND) ? (k)->pfik_parent :	       \
 	(k)->pfik_parent->pfik_parent)
 
+#define STATE_INC_COUNTERS(s)				\
+	do {						\
+		s->rule.ptr->states++;			\
+		if (s->anchor.ptr != NULL)		\
+			s->anchor.ptr->states++;	\
+		if (s->nat_rule.ptr != NULL)		\
+			s->nat_rule.ptr->states++;	\
+	} while (0)
+
+#define STATE_DEC_COUNTERS(s)				\
+	do {						\
+		if (s->nat_rule.ptr != NULL)		\
+			s->nat_rule.ptr->states--;	\
+		if (s->anchor.ptr != NULL)		\
+			s->anchor.ptr->states--;	\
+		s->rule.ptr->states--;			\
+	} while (0)
+
 static __inline int pf_src_compare(struct pf_src_node *, struct pf_src_node *);
 static __inline int pf_state_compare_lan_ext(struct pf_state *,
 	struct pf_state *);
@@ -2713,14 +2731,10 @@ cleanup:
 			return (PF_DROP);
 		}
 		bzero(s, sizeof(*s));
-		r->states++;
-		if (a != NULL)
-			a->states++;
 		s->rule.ptr = r;
 		s->nat_rule.ptr = nr;
-		if (s->nat_rule.ptr != NULL)
-			s->nat_rule.ptr->states++;
 		s->anchor.ptr = a;
+		STATE_INC_COUNTERS(s);
 		s->allow_opts = r->allow_opts;
 		s->log = r->log & 2;
 		s->proto = IPPROTO_TCP;
@@ -2799,6 +2813,7 @@ cleanup:
 		    off, pd, th, &s->src, &s->dst)) {
 			REASON_SET(&reason, PFRES_MEMORY);
 			pf_src_tree_remove_state(s);
+			STATE_DEC_COUNTERS(s);
 			pool_put(&pf_state_pl, s);
 			return (PF_DROP);
 		}
@@ -2810,6 +2825,7 @@ cleanup:
 			    ("pf_normalize_tcp_stateful failed on first pkt"));
 			pf_normalize_tcp_cleanup(s);
 			pf_src_tree_remove_state(s);
+			STATE_DEC_COUNTERS(s);
 			pool_put(&pf_state_pl, s);
 			return (PF_DROP);
 		}
@@ -2817,6 +2833,7 @@ cleanup:
 			pf_normalize_tcp_cleanup(s);
 			REASON_SET(&reason, PFRES_MEMORY);
 			pf_src_tree_remove_state(s);
+			STATE_DEC_COUNTERS(s);
 			pool_put(&pf_state_pl, s);
 			return (PF_DROP);
 		} else
@@ -3054,14 +3071,10 @@ cleanup:
 			return (PF_DROP);
 		}
 		bzero(s, sizeof(*s));
-		r->states++;
-		if (a != NULL)
-			a->states++;
 		s->rule.ptr = r;
 		s->nat_rule.ptr = nr;
-		if (s->nat_rule.ptr != NULL)
-			s->nat_rule.ptr->states++;
 		s->anchor.ptr = a;
+		STATE_INC_COUNTERS(s);
 		s->allow_opts = r->allow_opts;
 		s->log = r->log & 2;
 		s->proto = IPPROTO_UDP;
@@ -3110,6 +3123,7 @@ cleanup:
 		if (pf_insert_state(BOUND_IFACE(r, kif), s)) {
 			REASON_SET(&reason, PFRES_MEMORY);
 			pf_src_tree_remove_state(s);
+			STATE_DEC_COUNTERS(s);
 			pool_put(&pf_state_pl, s);
 			return (PF_DROP);
 		} else
@@ -3340,14 +3354,10 @@ cleanup:
 			return (PF_DROP);
 		}
 		bzero(s, sizeof(*s));
-		r->states++;
-		if (a != NULL)
-			a->states++;
 		s->rule.ptr = r;
 		s->nat_rule.ptr = nr;
-		if (s->nat_rule.ptr != NULL)
-			s->nat_rule.ptr->states++;
 		s->anchor.ptr = a;
+		STATE_INC_COUNTERS(s);
 		s->allow_opts = r->allow_opts;
 		s->log = r->log & 2;
 		s->proto = pd->proto;
@@ -3390,6 +3400,7 @@ cleanup:
 		if (pf_insert_state(BOUND_IFACE(r, kif), s)) {
 			REASON_SET(&reason, PFRES_MEMORY);
 			pf_src_tree_remove_state(s);
+			STATE_DEC_COUNTERS(s);
 			pool_put(&pf_state_pl, s);
 			return (PF_DROP);
 		} else
@@ -3606,14 +3617,10 @@ cleanup:
 			return (PF_DROP);
 		}
 		bzero(s, sizeof(*s));
-		r->states++;
-		if (a != NULL)
-			a->states++;
 		s->rule.ptr = r;
 		s->nat_rule.ptr = nr;
-		if (s->nat_rule.ptr != NULL)
-			s->nat_rule.ptr->states++;
 		s->anchor.ptr = a;
+		STATE_INC_COUNTERS(s);
 		s->allow_opts = r->allow_opts;
 		s->log = r->log & 2;
 		s->proto = pd->proto;
@@ -3652,6 +3659,7 @@ cleanup:
 		if (pf_insert_state(BOUND_IFACE(r, kif), s)) {
 			REASON_SET(&reason, PFRES_MEMORY);
 			pf_src_tree_remove_state(s);
+			STATE_DEC_COUNTERS(s);
 			pool_put(&pf_state_pl, s);
 			return (PF_DROP);
 		} else