removes the pf_consistency_lock and protects the users with

NET_LOCK().  pfioctl() will need the NET_LOCK() anyway. So better keep
things simple until we're going to redesign PF for a MP world.
fixes the crash reported by Kaya Saman.
ok mpi@, bluhm@

4 files changed, 22 insertions, 54 deletions

diff --git a/sys/net/pf.c b/sys/net/pf.c
index e54e4493bcc..def59d835fe 100644
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf.c,v 1.1011 2017/01/25 06:15:50 mpi Exp $ */
+/*	$OpenBSD: pf.c,v 1.1012 2017/01/30 17:41:33 benno Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -1154,26 +1154,20 @@ pf_state_export(struct pfsync_state *sp, struct pf_state *st)
 /* END state table stuff */
 
 void
-pf_purge_expired_rules(int locked)
+pf_purge_expired_rules(void)
 {
 	struct pf_rule	*r;
 
+	NET_ASSERT_LOCKED();
+
 	if (SLIST_EMPTY(&pf_rule_gcl))
 		return;
 
-	if (!locked)
-		rw_enter_write(&pf_consistency_lock);
-	else
-		rw_assert_wrlock(&pf_consistency_lock);
-
 	while ((r = SLIST_FIRST(&pf_rule_gcl)) != NULL) {
 		SLIST_REMOVE(&pf_rule_gcl, r, pf_rule, gcle);
 		KASSERT(r->rule_flag & PFRULE_EXPIRED);
 		pf_purge_rule(r);
 	}
-
-	if (!locked)
-		rw_exit_write(&pf_consistency_lock);
 }
 
 void
@@ -1194,7 +1188,7 @@ pf_purge_thread(void *v)
 		if (++nloops >= pf_default_rule.timeout[PFTM_INTERVAL]) {
 			pf_purge_expired_fragments();
 			pf_purge_expired_src_nodes(0);
-			pf_purge_expired_rules(0);
+			pf_purge_expired_rules();
 			nloops = 0;
 		}
 
@@ -1241,27 +1235,21 @@ pf_state_expires(const struct pf_state *state)
 }
 
 void
-pf_purge_expired_src_nodes(int waslocked)
+pf_purge_expired_src_nodes(void)
 {
 	struct pf_src_node		*cur, *next;
-	int				 locked = waslocked;
+
+	NET_ASSERT_LOCKED();
 
 	for (cur = RB_MIN(pf_src_tree, &tree_src_tracking); cur; cur = next) {
 	next = RB_NEXT(pf_src_tree, &tree_src_tracking, cur);
 
 		if (cur->states == 0 && cur->expire <= time_uptime) {
-			if (! locked) {
-				rw_enter_write(&pf_consistency_lock);
-				next = RB_NEXT(pf_src_tree,
-				    &tree_src_tracking, cur);
-				locked = 1;
-			}
+			next = RB_NEXT(pf_src_tree,
+			    &tree_src_tracking, cur);
 			pf_remove_src_node(cur);
 		}
 	}
-
-	if (locked && !waslocked)
-		rw_exit_write(&pf_consistency_lock);
 }
 
 void
@@ -1334,13 +1322,12 @@ pf_remove_divert_state(struct pf_state_key *sk)
 	}
 }
 
-/* callers should hold the write_lock on pf_consistency_lock */
 void
 pf_free_state(struct pf_state *cur)
 {
 	struct pf_rule_item *ri;
 
-	splsoftassert(IPL_SOFTNET);
+	NET_ASSERT_LOCKED();
 
 #if NPFSYNC > 0
 	if (pfsync_state_in_use(cur))
@@ -1375,7 +1362,8 @@ pf_purge_expired_states(u_int32_t maxcheck)
 {
 	static struct pf_state	*cur = NULL;
 	struct pf_state		*next;
-	int			 locked = 0;
+
+	NET_ASSERT_LOCKED();
 
 	while (maxcheck--) {
 		/* wrap to start of list when we hit the end */
@@ -1390,25 +1378,14 @@ pf_purge_expired_states(u_int32_t maxcheck)
 
 		if (cur->timeout == PFTM_UNLINKED) {
 			/* free removed state */
-			if (! locked) {
-				rw_enter_write(&pf_consistency_lock);
-				locked = 1;
-			}
 			pf_free_state(cur);
 		} else if (pf_state_expires(cur) <= time_uptime) {
 			/* remove and free expired state */
 			pf_remove_state(cur);
-			if (! locked) {
-				rw_enter_write(&pf_consistency_lock);
-				locked = 1;
-			}
 			pf_free_state(cur);
 		}
 		cur = next;
 	}
-
-	if (locked)
-		rw_exit_write(&pf_consistency_lock);
 }
 
 int
diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index 9b278c907f5..56a43a55ab8 100644
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf_ioctl.c,v 1.306 2017/01/24 10:08:30 krw Exp $ */
+/*	$OpenBSD: pf_ioctl.c,v 1.307 2017/01/30 17:41:34 benno Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -111,7 +111,6 @@ void			 pf_qid2qname(u_int16_t, char *);
 void			 pf_qid_unref(u_int16_t);
 
 struct pf_rule		 pf_default_rule, pf_default_rule_new;
-struct rwlock		 pf_consistency_lock = RWLOCK_INITIALIZER("pfcnslk");
 
 struct {
 	char		statusif[IFNAMSIZ];
@@ -987,12 +986,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			return (EACCES);
 		}
 
-	if (flags & FWRITE)
-		rw_enter_write(&pf_consistency_lock);
-	else
-		rw_enter_read(&pf_consistency_lock);
-
-	s = splsoftnet();
+	NET_LOCK(s);
 	switch (cmd) {
 
 	case DIOCSTART:
@@ -2388,11 +2382,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		break;
 	}
 fail:
-	splx(s);
-	if (flags & FWRITE)
-		rw_exit_write(&pf_consistency_lock);
-	else
-		rw_exit_read(&pf_consistency_lock);
+	NET_UNLOCK(s);
 	return (error);
 }
 
diff --git a/sys/net/pf_norm.c b/sys/net/pf_norm.c
index 6bf9681e37f..ae60ffa2317 100644
--- a/sys/net/pf_norm.c
+++ b/sys/net/pf_norm.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf_norm.c,v 1.200 2016/12/29 13:01:48 bluhm Exp $ */
+/*	$OpenBSD: pf_norm.c,v 1.201 2017/01/30 17:41:34 benno Exp $ */
 
 /*
  * Copyright 2001 Niels Provos <provos@citi.umich.edu>
@@ -176,6 +176,8 @@ pf_purge_expired_fragments(void)
 	struct pf_fragment	*frag;
 	int32_t			 expire;
 
+	NET_ASSERT_LOCKED();
+
 	expire = time_uptime - pf_default_rule.timeout[PFTM_FRAG];
 	while ((frag = TAILQ_LAST(&pf_fragqueue, pf_fragqueue)) != NULL) {
 		if (frag->fr_timeout > expire)
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index 9896bf82eca..5bb2a8ea93c 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfvar.h,v 1.447 2017/01/24 10:08:30 krw Exp $ */
+/*	$OpenBSD: pfvar.h,v 1.448 2017/01/30 17:41:34 benno Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -1614,9 +1614,9 @@ extern void			 pf_tbladdr_remove(struct pf_addr_wrap *);
 extern void			 pf_tbladdr_copyout(struct pf_addr_wrap *);
 extern void			 pf_calc_skip_steps(struct pf_rulequeue *);
 extern void			 pf_purge_thread(void *);
-extern void			 pf_purge_expired_src_nodes(int);
+extern void			 pf_purge_expired_src_nodes();
 extern void			 pf_purge_expired_states(u_int32_t);
-extern void			 pf_purge_expired_rules(int);
+extern void			 pf_purge_expired_rules();
 extern void			 pf_remove_state(struct pf_state *);
 extern void			 pf_remove_divert_state(struct pf_state_key *);
 extern void			 pf_free_state(struct pf_state *);
@@ -1790,7 +1790,6 @@ int		 pf_addr_compare(struct pf_addr *, struct pf_addr *,
 
 extern struct pf_status	pf_status;
 extern struct pool	pf_frent_pl, pf_frag_pl;
-extern struct rwlock	pf_consistency_lock;
 
 struct pf_pool_limit {
 	void		*pp;