- fine tuning PF_LOCK in pfioctl()

  (extra thanks to Hrvoje for testing)

OK mpi@

1 files changed, 176 insertions, 3 deletions

diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index de40e934112..f6ad2cb8c53 100644
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf_ioctl.c,v 1.322 2017/08/11 21:24:19 mpi Exp $ */
+/*	$OpenBSD: pf_ioctl.c,v 1.323 2017/10/30 22:35:50 sashan Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -1003,10 +1003,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		}
 
 	NET_LOCK();
-	PF_LOCK();
 	switch (cmd) {
 
 	case DIOCSTART:
+		PF_LOCK();
 		if (pf_status.running)
 			error = EEXIST;
 		else {
@@ -1020,9 +1020,11 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			pf_create_queues();
 			DPFPRINTF(LOG_NOTICE, "pf: started");
 		}
+		PF_UNLOCK();
 		break;
 
 	case DIOCSTOP:
+		PF_LOCK();
 		if (!pf_status.running)
 			error = ENOENT;
 		else {
@@ -1031,6 +1033,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			pf_remove_queues();
 			DPFPRINTF(LOG_NOTICE, "pf: stopped");
 		}
+		PF_UNLOCK();
 		break;
 
 	case DIOCGETQUEUES: {
@@ -1038,6 +1041,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		struct pf_queuespec	*qs;
 		u_int32_t		 nr = 0;
 
+		PF_LOCK();
 		pq->ticket = pf_main_ruleset.rules.active.ticket;
 
 		/* save state to not run over them all each time? */
@@ -1047,6 +1051,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			nr++;
 		}
 		pq->nr = nr;
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1055,8 +1060,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		struct pf_queuespec	*qs;
 		u_int32_t		 nr = 0;
 
+		PF_LOCK();
 		if (pq->ticket != pf_main_ruleset.rules.active.ticket) {
 			error = EBUSY;
+			PF_UNLOCK();
 			break;
 		}
 
@@ -1066,9 +1073,11 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			qs = TAILQ_NEXT(qs, entries);
 		if (qs == NULL) {
 			error = EBUSY;
+			PF_UNLOCK();
 			break;
 		}
 		bcopy(qs, &pq->queue, sizeof(pq->queue));
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1078,8 +1087,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		u_int32_t		 nr;
 		int			 nbytes;
 
+		PF_LOCK();
 		if (pq->ticket != pf_main_ruleset.rules.active.ticket) {
 			error = EBUSY;
+			PF_UNLOCK();
 			break;
 		}
 		nbytes = pq->nbytes;
@@ -1091,6 +1102,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			qs = TAILQ_NEXT(qs, entries);
 		if (qs == NULL) {
 			error = EBUSY;
+			PF_UNLOCK();
 			break;
 		}
 		bcopy(qs, &pq->queue, sizeof(pq->queue));
@@ -1104,6 +1116,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			    &nbytes);
 		if (error == 0)
 			pq->nbytes = nbytes;
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1111,13 +1124,16 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		struct pfioc_queue	*q = (struct pfioc_queue *)addr;
 		struct pf_queuespec	*qs;
 
+		PF_LOCK();
 		if (q->ticket != pf_main_ruleset.rules.inactive.ticket) {
 			error = EBUSY;
+			PF_UNLOCK();
 			break;
 		}
 		qs = pool_get(&pf_queue_pl, PR_WAITOK|PR_LIMITFAIL|PR_ZERO);
 		if (qs == NULL) {
 			error = ENOMEM;
+			PF_UNLOCK();
 			break;
 		}
 		bcopy(&q->queue, qs, sizeof(*qs));
@@ -1126,18 +1142,21 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		    pf_qname2qid(qs->parent, 0)) == 0) {
 			pool_put(&pf_queue_pl, qs);
 			error = ESRCH;
+			PF_UNLOCK();
 			break;
 		}
 		qs->kif = pfi_kif_get(qs->ifname);
 		if (qs->kif == NULL) {
 			pool_put(&pf_queue_pl, qs);
 			error = ESRCH;
+			PF_UNLOCK();
 			break;
 		}
 		/* XXX resolve bw percentage specs */
 		pfi_kif_ref(qs->kif, PFI_KIF_REF_RULE);
 
 		TAILQ_INSERT_TAIL(pf_queues_inactive, qs, entries);
+		PF_UNLOCK();
 
 		break;
 	}
@@ -1147,28 +1166,34 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		struct pf_ruleset	*ruleset;
 		struct pf_rule		*rule, *tail;
 
+		PF_LOCK();
 		pr->anchor[sizeof(pr->anchor) - 1] = 0;
 		ruleset = pf_find_ruleset(pr->anchor);
 		if (ruleset == NULL) {
 			error = EINVAL;
+			PF_UNLOCK();
 			break;
 		}
 		if (pr->rule.return_icmp >> 8 > ICMP_MAXTYPE) {
 			error = EINVAL;
+			PF_UNLOCK();
 			break;
 		}
 		if (pr->ticket != ruleset->rules.inactive.ticket) {
 			error = EBUSY;
+			PF_UNLOCK();
 			break;
 		}
 		rule = pool_get(&pf_rule_pl, PR_WAITOK|PR_LIMITFAIL|PR_ZERO);
 		if (rule == NULL) {
 			error = ENOMEM;
+			PF_UNLOCK();
 			break;
 		}
 		if ((error = pf_rule_copyin(&pr->rule, rule, ruleset))) {
 			pf_rm_rule(NULL, rule);
 			rule = NULL;
+			PF_UNLOCK();
 			break;
 		}
 		rule->cuid = p->p_ucred->cr_ruid;
@@ -1187,6 +1212,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			pf_rm_rule(NULL, rule);
 			rule = NULL;
 			error = EAFNOSUPPORT;
+			PF_UNLOCK();
 			goto fail;
 		}
 		tail = TAILQ_LAST(ruleset->rules.inactive.ptr,
@@ -1221,12 +1247,14 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 
 		if (error) {
 			pf_rm_rule(NULL, rule);
+			PF_UNLOCK();
 			break;
 		}
 		TAILQ_INSERT_TAIL(ruleset->rules.inactive.ptr,
 		    rule, entries);
 		rule->ruleset = ruleset;
 		ruleset->rules.inactive.rcount++;
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1235,10 +1263,12 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		struct pf_ruleset	*ruleset;
 		struct pf_rule		*tail;
 
+		PF_LOCK();
 		pr->anchor[sizeof(pr->anchor) - 1] = 0;
 		ruleset = pf_find_ruleset(pr->anchor);
 		if (ruleset == NULL) {
 			error = EINVAL;
+			PF_UNLOCK();
 			break;
 		}
 		tail = TAILQ_LAST(ruleset->rules.active.ptr, pf_rulequeue);
@@ -1247,6 +1277,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		else
 			pr->nr = 0;
 		pr->ticket = ruleset->rules.active.ticket;
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1256,14 +1287,17 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		struct pf_rule		*rule;
 		int			 i;
 
+		PF_LOCK();
 		pr->anchor[sizeof(pr->anchor) - 1] = 0;
 		ruleset = pf_find_ruleset(pr->anchor);
 		if (ruleset == NULL) {
 			error = EINVAL;
+			PF_UNLOCK();
 			break;
 		}
 		if (pr->ticket != ruleset->rules.active.ticket) {
 			error = EBUSY;
+			PF_UNLOCK();
 			break;
 		}
 		rule = TAILQ_FIRST(ruleset->rules.active.ptr);
@@ -1271,6 +1305,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			rule = TAILQ_NEXT(rule, entries);
 		if (rule == NULL) {
 			error = EBUSY;
+			PF_UNLOCK();
 			break;
 		}
 		bcopy(rule, &pr->rule, sizeof(struct pf_rule));
@@ -1286,6 +1321,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		pr->rule.ruleset = NULL;
 		if (pf_anchor_copyout(ruleset, rule, pr)) {
 			error = EBUSY;
+			PF_UNLOCK();
 			break;
 		}
 		pf_addr_copyout(&pr->rule.src.addr);
@@ -1306,6 +1342,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			rule->bytes[0] = rule->bytes[1] = 0;
 			rule->states_tot = 0;
 		}
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1320,23 +1357,28 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = EINVAL;
 			break;
 		}
+		PF_LOCK();
 		ruleset = pf_find_ruleset(pcr->anchor);
 		if (ruleset == NULL) {
 			error = EINVAL;
+			PF_UNLOCK();
 			break;
 		}
 
 		if (pcr->action == PF_CHANGE_GET_TICKET) {
 			pcr->ticket = ++ruleset->rules.active.ticket;
+			PF_UNLOCK();
 			break;
 		} else {
 			if (pcr->ticket !=
 			    ruleset->rules.active.ticket) {
 				error = EINVAL;
+				PF_UNLOCK();
 				break;
 			}
 			if (pcr->rule.return_icmp >> 8 > ICMP_MAXTYPE) {
 				error = EINVAL;
+				PF_UNLOCK();
 				break;
 			}
 		}
@@ -1346,6 +1388,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			    PR_WAITOK|PR_LIMITFAIL|PR_ZERO);
 			if (newrule == NULL) {
 				error = ENOMEM;
+				PF_UNLOCK();
 				break;
 			}
 			pf_rule_copyin(&pcr->rule, newrule, ruleset);
@@ -1364,6 +1407,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			default:
 				pool_put(&pf_rule_pl, newrule);
 				error = EAFNOSUPPORT;
+				PF_UNLOCK();
 				goto fail;
 			}
 
@@ -1384,6 +1428,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 
 			if (error) {
 				pf_rm_rule(NULL, newrule);
+				PF_UNLOCK();
 				break;
 			}
 		}
@@ -1401,6 +1446,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				if (newrule != NULL)
 					pf_rm_rule(NULL, newrule);
 				error = EINVAL;
+				PF_UNLOCK();
 				break;
 			}
 		}
@@ -1432,6 +1478,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		pf_calc_skip_steps(ruleset->rules.active.ptr);
 		pf_remove_if_empty_ruleset(ruleset);
 
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1440,6 +1487,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		struct pfioc_state_kill *psk = (struct pfioc_state_kill *)addr;
 		u_int			 killed = 0;
 
+		PF_LOCK();
 		for (s = RB_MIN(pf_state_tree_id, &tree_id); s; s = nexts) {
 			nexts = RB_NEXT(pf_state_tree_id, &tree_id, s);
 
@@ -1457,6 +1505,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 #if NPFSYNC > 0
 		pfsync_clear_states(pf_status.hostid, psk->psk_ifname);
 #endif	/* NPFSYNC > 0 */
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1474,10 +1523,12 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		if (psk->psk_pfcmp.id) {
 			if (psk->psk_pfcmp.creatorid == 0)
 				psk->psk_pfcmp.creatorid = pf_status.hostid;
+			PF_LOCK();
 			if ((s = pf_find_state_byid(&psk->psk_pfcmp))) {
 				pf_remove_state(s);
 				psk->psk_killed = 1;
 			}
+			PF_UNLOCK();
 			break;
 		}
 
@@ -1489,6 +1540,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			key.proto = psk->psk_proto;
 			key.rdomain = psk->psk_rdomain;
 
+			PF_LOCK();
 			for (i = 0; i < nitems(dirs); i++) {
 				if (dirs[i] == PF_IN) {
 					sidx = 0;
@@ -1529,9 +1581,11 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			}
 			if (killed)
 				psk->psk_killed = killed;
+			PF_UNLOCK();
 			break;
 		}
 
+		PF_LOCK();
 		for (s = RB_MIN(pf_state_tree_id, &tree_id); s;
 		    s = nexts) {
 			nexts = RB_NEXT(pf_state_tree_id, &tree_id, s);
@@ -1577,6 +1631,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			}
 		}
 		psk->psk_killed = killed;
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1589,7 +1644,9 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = EINVAL;
 			break;
 		}
+		PF_LOCK();
 		error = pfsync_state_import(sp, PFSYNC_SI_IOCTL);
+		PF_UNLOCK();
 		break;
 	}
 #endif	/* NPFSYNC > 0 */
@@ -1603,13 +1660,16 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		id_key.id = ps->state.id;
 		id_key.creatorid = ps->state.creatorid;
 
+		PF_LOCK();
 		s = pf_find_state_byid(&id_key);
 		if (s == NULL) {
 			error = ENOENT;
+			PF_UNLOCK();
 			break;
 		}
 
 		pf_state_export(&ps->state, s);
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1629,6 +1689,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 
 		p = ps->ps_states;
 
+		PF_LOCK();
 		state = TAILQ_FIRST(&state_list);
 		while (state) {
 			if (state->timeout != PFTM_UNLINKED) {
@@ -1645,6 +1706,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			}
 			state = TAILQ_NEXT(state, entry_list);
 		}
+		PF_UNLOCK();
 
 		ps->ps_len = sizeof(struct pfsync_state) * nr;
 
@@ -1654,29 +1716,36 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 
 	case DIOCGETSTATUS: {
 		struct pf_status *s = (struct pf_status *)addr;
+		PF_LOCK();
 		bcopy(&pf_status, s, sizeof(struct pf_status));
 		pfi_update_status(s->ifname, s);
+		PF_UNLOCK();
 		break;
 	}
 
 	case DIOCSETSTATUSIF: {
 		struct pfioc_iface	*pi = (struct pfioc_iface *)addr;
 
+		PF_LOCK();
 		if (pi->pfiio_name[0] == 0) {
 			bzero(pf_status.ifname, IFNAMSIZ);
+			PF_UNLOCK();
 			break;
 		}
 		strlcpy(pf_trans_set.statusif, pi->pfiio_name, IFNAMSIZ);
 		pf_trans_set.mask |= PF_TSET_STATUSIF;
+		PF_UNLOCK();
 		break;
 	}
 
 	case DIOCCLRSTATUS: {
 		struct pfioc_iface	*pi = (struct pfioc_iface *)addr;
 
+		PF_LOCK();
 		/* if ifname is specified, clear counters there only */
 		if (pi->pfiio_name[0]) {
 			pfi_update_status(pi->pfiio_name, NULL);
+			PF_UNLOCK();
 			break;
 		}
 
@@ -1685,6 +1754,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		bzero(pf_status.scounters, sizeof(pf_status.scounters));
 		pf_status.since = time_uptime;
 
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1717,6 +1787,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			PF_ACPY(&key.addr[didx], &pnl->daddr, pnl->af);
 			key.port[didx] = pnl->dport;
 
+			PF_LOCK();
 			state = pf_find_state_all(&key, direction, &m);
 
 			if (m > 1)
@@ -1730,6 +1801,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				pnl->rrdomain = sk->rdomain;
 			} else
 				error = ENOENT;
+			PF_UNLOCK();
 		}
 		break;
 	}
@@ -1742,10 +1814,12 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = EINVAL;
 			goto fail;
 		}
+		PF_LOCK();
 		if (pt->timeout == PFTM_INTERVAL && pt->seconds == 0)
 			pt->seconds = 1;
 		pf_default_rule_new.timeout[pt->timeout] = pt->seconds;
 		pt->seconds = pf_default_rule.timeout[pt->timeout];
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1756,7 +1830,9 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = EINVAL;
 			goto fail;
 		}
+		PF_LOCK();
 		pt->seconds = pf_default_rule.timeout[pt->timeout];
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1767,39 +1843,48 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = EINVAL;
 			goto fail;
 		}
+		PF_LOCK();
 		pl->limit = pf_pool_limits[pl->index].limit;
+		PF_UNLOCK();
 		break;
 	}
 
 	case DIOCSETLIMIT: {
 		struct pfioc_limit	*pl = (struct pfioc_limit *)addr;
 
+		PF_LOCK();
 		if (pl->index < 0 || pl->index >= PF_LIMIT_MAX ||
 		    pf_pool_limits[pl->index].pp == NULL) {
 			error = EINVAL;
+			PF_UNLOCK();
 			goto fail;
 		}
 		if (((struct pool *)pf_pool_limits[pl->index].pp)->pr_nout >
 		    pl->limit) {
 			error = EBUSY;
+			PF_UNLOCK();
 			goto fail;
 		}
 		/* Fragments reference mbuf clusters. */
 		if (pl->index == PF_LIMIT_FRAGS && pl->limit > nmbclust) {
 			error = EINVAL;
+			PF_UNLOCK();
 			goto fail;
 		}
 
 		pf_pool_limits[pl->index].limit_new = pl->limit;
 		pl->limit = pf_pool_limits[pl->index].limit;
+		PF_UNLOCK();
 		break;
 	}
 
 	case DIOCSETDEBUG: {
 		u_int32_t	*level = (u_int32_t *)addr;
 
+		PF_LOCK();
 		pf_trans_set.debug = *level;
 		pf_trans_set.mask |= PF_TSET_DEBUG;
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1808,9 +1893,11 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		struct pf_ruleset	*ruleset;
 		struct pf_anchor	*anchor;
 
+		PF_LOCK();
 		pr->path[sizeof(pr->path) - 1] = 0;
 		if ((ruleset = pf_find_ruleset(pr->path)) == NULL) {
 			error = EINVAL;
+			PF_UNLOCK();
 			break;
 		}
 		pr->nr = 0;
@@ -1824,6 +1911,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			    &ruleset->anchor->children)
 				pr->nr++;
 		}
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1833,9 +1921,11 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		struct pf_anchor	*anchor;
 		u_int32_t		 nr = 0;
 
+		PF_LOCK();
 		pr->path[sizeof(pr->path) - 1] = 0;
 		if ((ruleset = pf_find_ruleset(pr->path)) == NULL) {
 			error = EINVAL;
+			PF_UNLOCK();
 			break;
 		}
 		pr->name[0] = 0;
@@ -1845,6 +1935,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				if (anchor->parent == NULL && nr++ == pr->nr) {
 					strlcpy(pr->name, anchor->name,
 					    sizeof(pr->name));
+					PF_UNLOCK();
 					break;
 				}
 		} else {
@@ -1853,11 +1944,13 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				if (nr++ == pr->nr) {
 					strlcpy(pr->name, anchor->name,
 					    sizeof(pr->name));
+					PF_UNLOCK();
 					break;
 				}
 		}
 		if (!pr->name[0])
 			error = EBUSY;
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1868,8 +1961,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = ENODEV;
 			break;
 		}
+		PF_LOCK();
 		error = pfr_clr_tables(&io->pfrio_table, &io->pfrio_ndel,
 		    io->pfrio_flags | PFR_FLAG_USERIOCTL);
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1880,8 +1975,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = ENODEV;
 			break;
 		}
+		PF_LOCK();
 		error = pfr_add_tables(io->pfrio_buffer, io->pfrio_size,
 		    &io->pfrio_nadd, io->pfrio_flags | PFR_FLAG_USERIOCTL);
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1892,8 +1989,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = ENODEV;
 			break;
 		}
+		PF_LOCK();
 		error = pfr_del_tables(io->pfrio_buffer, io->pfrio_size,
 		    &io->pfrio_ndel, io->pfrio_flags | PFR_FLAG_USERIOCTL);
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1904,8 +2003,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = ENODEV;
 			break;
 		}
+		PF_LOCK();
 		error = pfr_get_tables(&io->pfrio_table, io->pfrio_buffer,
 		    &io->pfrio_size, io->pfrio_flags | PFR_FLAG_USERIOCTL);
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1916,8 +2017,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = ENODEV;
 			break;
 		}
+		PF_LOCK();
 		error = pfr_get_tstats(&io->pfrio_table, io->pfrio_buffer,
 		    &io->pfrio_size, io->pfrio_flags | PFR_FLAG_USERIOCTL);
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1928,8 +2031,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = ENODEV;
 			break;
 		}
+		PF_LOCK();
 		error = pfr_clr_tstats(io->pfrio_buffer, io->pfrio_size,
 		    &io->pfrio_nzero, io->pfrio_flags | PFR_FLAG_USERIOCTL);
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1940,9 +2045,11 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = ENODEV;
 			break;
 		}
+		PF_LOCK();
 		error = pfr_set_tflags(io->pfrio_buffer, io->pfrio_size,
 		    io->pfrio_setflag, io->pfrio_clrflag, &io->pfrio_nchange,
 		    &io->pfrio_ndel, io->pfrio_flags | PFR_FLAG_USERIOCTL);
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1953,8 +2060,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = ENODEV;
 			break;
 		}
+		PF_LOCK();
 		error = pfr_clr_addrs(&io->pfrio_table, &io->pfrio_ndel,
 		    io->pfrio_flags | PFR_FLAG_USERIOCTL);
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1965,9 +2074,11 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = ENODEV;
 			break;
 		}
+		PF_LOCK();
 		error = pfr_add_addrs(&io->pfrio_table, io->pfrio_buffer,
 		    io->pfrio_size, &io->pfrio_nadd, io->pfrio_flags |
 		    PFR_FLAG_USERIOCTL);
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1978,9 +2089,11 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = ENODEV;
 			break;
 		}
+		PF_LOCK();
 		error = pfr_del_addrs(&io->pfrio_table, io->pfrio_buffer,
 		    io->pfrio_size, &io->pfrio_ndel, io->pfrio_flags |
 		    PFR_FLAG_USERIOCTL);
+		PF_UNLOCK();
 		break;
 	}
 
@@ -1991,10 +2104,12 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = ENODEV;
 			break;
 		}
+		PF_LOCK();
 		error = pfr_set_addrs(&io->pfrio_table, io->pfrio_buffer,
 		    io->pfrio_size, &io->pfrio_size2, &io->pfrio_nadd,
 		    &io->pfrio_ndel, &io->pfrio_nchange, io->pfrio_flags |
 		    PFR_FLAG_USERIOCTL, 0);
+		PF_UNLOCK();
 		break;
 	}
 
@@ -2005,8 +2120,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = ENODEV;
 			break;
 		}
+		PF_LOCK();
 		error = pfr_get_addrs(&io->pfrio_table, io->pfrio_buffer,
 		    &io->pfrio_size, io->pfrio_flags | PFR_FLAG_USERIOCTL);
+		PF_UNLOCK();
 		break;
 	}
 
@@ -2017,8 +2134,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = ENODEV;
 			break;
 		}
+		PF_LOCK();
 		error = pfr_get_astats(&io->pfrio_table, io->pfrio_buffer,
 		    &io->pfrio_size, io->pfrio_flags | PFR_FLAG_USERIOCTL);
+		PF_UNLOCK();
 		break;
 	}
 
@@ -2029,9 +2148,11 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = ENODEV;
 			break;
 		}
+		PF_LOCK();
 		error = pfr_clr_astats(&io->pfrio_table, io->pfrio_buffer,
 		    io->pfrio_size, &io->pfrio_nzero, io->pfrio_flags |
 		    PFR_FLAG_USERIOCTL);
+		PF_UNLOCK();
 		break;
 	}
 
@@ -2042,9 +2163,11 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = ENODEV;
 			break;
 		}
+		PF_LOCK();
 		error = pfr_tst_addrs(&io->pfrio_table, io->pfrio_buffer,
 		    io->pfrio_size, &io->pfrio_nmatch, io->pfrio_flags |
 		    PFR_FLAG_USERIOCTL);
+		PF_UNLOCK();
 		break;
 	}
 
@@ -2055,21 +2178,27 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = ENODEV;
 			break;
 		}
+		PF_LOCK();
 		error = pfr_ina_define(&io->pfrio_table, io->pfrio_buffer,
 		    io->pfrio_size, &io->pfrio_nadd, &io->pfrio_naddr,
 		    io->pfrio_ticket, io->pfrio_flags | PFR_FLAG_USERIOCTL);
+		PF_UNLOCK();
 		break;
 	}
 
 	case DIOCOSFPADD: {
 		struct pf_osfp_ioctl *io = (struct pf_osfp_ioctl *)addr;
+		PF_LOCK();
 		error = pf_osfp_add(io);
+		PF_UNLOCK();
 		break;
 	}
 
 	case DIOCOSFPGET: {
 		struct pf_osfp_ioctl *io = (struct pf_osfp_ioctl *)addr;
+		PF_LOCK();
 		error = pf_osfp_get(io);
+		PF_UNLOCK();
 		break;
 	}
 
@@ -2083,6 +2212,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = ENODEV;
 			goto fail;
 		}
+		PF_LOCK();
 		ioe = malloc(sizeof(*ioe), M_TEMP, M_WAITOK);
 		table = malloc(sizeof(*table), M_TEMP, M_WAITOK);
 		pf_default_rule_new = pf_default_rule;
@@ -2092,6 +2222,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				free(table, M_TEMP, sizeof(*table));
 				free(ioe, M_TEMP, sizeof(*ioe));
 				error = EFAULT;
+				PF_UNLOCK();
 				goto fail;
 			}
 			if (strnlen(ioe->anchor, sizeof(ioe->anchor)) ==
@@ -2099,6 +2230,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				free(table, M_TEMP, sizeof(*table));
 				free(ioe, M_TEMP, sizeof(*ioe));
 				error = ENAMETOOLONG;
+				PF_UNLOCK();
 				goto fail;
 			}
 			switch (ioe->type) {
@@ -2110,6 +2242,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				    &ioe->ticket, NULL, 0))) {
 					free(table, M_TEMP, sizeof(*table));
 					free(ioe, M_TEMP, sizeof(*ioe));
+					PF_UNLOCK();
 					goto fail;
 				}
 				break;
@@ -2118,6 +2251,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				    ioe->anchor))) {
 					free(table, M_TEMP, sizeof(*table));
 					free(ioe, M_TEMP, sizeof(*ioe));
+					PF_UNLOCK();
 					goto fail;
 				}
 				break;
@@ -2126,11 +2260,13 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				free(table, M_TEMP, sizeof(*table));
 				free(ioe, M_TEMP, sizeof(*ioe));
 				error = EFAULT;
+				PF_UNLOCK();
 				goto fail;
 			}
 		}
 		free(table, M_TEMP, sizeof(*table));
 		free(ioe, M_TEMP, sizeof(*ioe));
+		PF_UNLOCK();
 		break;
 	}
 
@@ -2144,6 +2280,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = ENODEV;
 			goto fail;
 		}
+		PF_LOCK();
 		ioe = malloc(sizeof(*ioe), M_TEMP, M_WAITOK);
 		table = malloc(sizeof(*table), M_TEMP, M_WAITOK);
 		for (i = 0; i < io->size; i++) {
@@ -2151,6 +2288,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				free(table, M_TEMP, sizeof(*table));
 				free(ioe, M_TEMP, sizeof(*ioe));
 				error = EFAULT;
+				PF_UNLOCK();
 				goto fail;
 			}
 			if (strnlen(ioe->anchor, sizeof(ioe->anchor)) ==
@@ -2158,6 +2296,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				free(table, M_TEMP, sizeof(*table));
 				free(ioe, M_TEMP, sizeof(*ioe));
 				error = ENAMETOOLONG;
+				PF_UNLOCK();
 				goto fail;
 			}
 			switch (ioe->type) {
@@ -2169,6 +2308,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				    ioe->ticket, NULL, 0))) {
 					free(table, M_TEMP, sizeof(*table));
 					free(ioe, M_TEMP, sizeof(*ioe));
+					PF_UNLOCK();
 					goto fail; /* really bad */
 				}
 				break;
@@ -2177,6 +2317,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				    ioe->anchor))) {
 					free(table, M_TEMP, sizeof(*table));
 					free(ioe, M_TEMP, sizeof(*ioe));
+					PF_UNLOCK();
 					goto fail; /* really bad */
 				}
 				break;
@@ -2184,6 +2325,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		}
 		free(table, M_TEMP, sizeof(*table));
 		free(ioe, M_TEMP, sizeof(*ioe));
+		PF_UNLOCK();
 		break;
 	}
 
@@ -2198,6 +2340,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = ENODEV;
 			goto fail;
 		}
+		PF_LOCK();
 		ioe = malloc(sizeof(*ioe), M_TEMP, M_WAITOK);
 		table = malloc(sizeof(*table), M_TEMP, M_WAITOK);
 		/* first makes sure everything will succeed */
@@ -2206,6 +2349,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				free(table, M_TEMP, sizeof(*table));
 				free(ioe, M_TEMP, sizeof(*ioe));
 				error = EFAULT;
+				PF_UNLOCK();
 				goto fail;
 			}
 			if (strnlen(ioe->anchor, sizeof(ioe->anchor)) ==
@@ -2213,6 +2357,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				free(table, M_TEMP, sizeof(*table));
 				free(ioe, M_TEMP, sizeof(*ioe));
 				error = ENAMETOOLONG;
+				PF_UNLOCK();
 				goto fail;
 			}
 			switch (ioe->type) {
@@ -2223,6 +2368,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 					free(table, M_TEMP, sizeof(*table));
 					free(ioe, M_TEMP, sizeof(*ioe));
 					error = EBUSY;
+					PF_UNLOCK();
 					goto fail;
 				}
 				break;
@@ -2235,6 +2381,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 					free(table, M_TEMP, sizeof(*table));
 					free(ioe, M_TEMP, sizeof(*ioe));
 					error = EBUSY;
+					PF_UNLOCK();
 					goto fail;
 				}
 				break;
@@ -2251,6 +2398,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				free(table, M_TEMP, sizeof(*table));
 				free(ioe, M_TEMP, sizeof(*ioe));
 				error = EBUSY;
+				PF_UNLOCK();
 				goto fail;
 			}
 		}
@@ -2260,6 +2408,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				free(table, M_TEMP, sizeof(*table));
 				free(ioe, M_TEMP, sizeof(*ioe));
 				error = EFAULT;
+				PF_UNLOCK();
 				goto fail;
 			}
 			if (strnlen(ioe->anchor, sizeof(ioe->anchor)) ==
@@ -2267,6 +2416,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				free(table, M_TEMP, sizeof(*table));
 				free(ioe, M_TEMP, sizeof(*ioe));
 				error = ENAMETOOLONG;
+				PF_UNLOCK();
 				goto fail;
 			}
 			switch (ioe->type) {
@@ -2278,6 +2428,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				    NULL, NULL, 0))) {
 					free(table, M_TEMP, sizeof(*table));
 					free(ioe, M_TEMP, sizeof(*ioe));
+					PF_UNLOCK();
 					goto fail; /* really bad */
 				}
 				break;
@@ -2286,6 +2437,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				    ioe->anchor))) {
 					free(table, M_TEMP, sizeof(*table));
 					free(ioe, M_TEMP, sizeof(*ioe));
+					PF_UNLOCK();
 					goto fail; /* really bad */
 				}
 				break;
@@ -2299,6 +2451,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				free(table, M_TEMP, sizeof(*table));
 				free(ioe, M_TEMP, sizeof(*ioe));
 				error = EBUSY;
+				PF_UNLOCK();
 				goto fail; /* really bad */
 			}
 			pf_pool_limits[i].limit = pf_pool_limits[i].limit_new;
@@ -2316,6 +2469,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		pf_trans_set_commit();
 		free(table, M_TEMP, sizeof(*table));
 		free(ioe, M_TEMP, sizeof(*ioe));
+		PF_UNLOCK();
 		break;
 	}
 
@@ -2325,10 +2479,12 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		u_int32_t		 nr = 0;
 		int			 space = psn->psn_len;
 
+		PF_LOCK();
 		if (space == 0) {
 			RB_FOREACH(n, pf_src_tree, &tree_src_tracking)
 				nr++;
 			psn->psn_len = sizeof(struct pf_src_node) * nr;
+			PF_UNLOCK();
 			break;
 		}
 
@@ -2364,6 +2520,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = copyout(pstore, p, sizeof(*p));
 			if (error) {
 				free(pstore, M_TEMP, sizeof(*pstore));
+				PF_UNLOCK();
 				goto fail;
 			}
 			p++;
@@ -2372,6 +2529,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		psn->psn_len = sizeof(struct pf_src_node) * nr;
 
 		free(pstore, M_TEMP, sizeof(*pstore));
+		PF_UNLOCK();
 		break;
 	}
 
@@ -2379,11 +2537,13 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		struct pf_src_node	*n;
 		struct pf_state		*state;
 
+		PF_LOCK();
 		RB_FOREACH(state, pf_state_tree_id, &tree_id)
 			pf_src_tree_remove_state(state);
 		RB_FOREACH(n, pf_src_tree, &tree_src_tracking)
 			n->expire = 1;
 		pf_purge_expired_src_nodes(1);
+		PF_UNLOCK();
 		break;
 	}
 
@@ -2394,6 +2554,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		    (struct pfioc_src_node_kill *)addr;
 		u_int			killed = 0;
 
+		PF_LOCK();
 		RB_FOREACH(sn, pf_src_tree, &tree_src_tracking) {
 			if (PF_MATCHA(psnk->psnk_src.neg,
 				&psnk->psnk_src.addr.v.a.addr,
@@ -2417,22 +2578,27 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			pf_purge_expired_src_nodes(1);
 
 		psnk->psnk_killed = killed;
+		PF_UNLOCK();
 		break;
 	}
 
 	case DIOCSETHOSTID: {
 		u_int32_t	*hostid = (u_int32_t *)addr;
 
+		PF_LOCK();
 		if (*hostid == 0)
 			pf_trans_set.hostid = arc4random();
 		else
 			pf_trans_set.hostid = *hostid;
 		pf_trans_set.mask |= PF_TSET_HOSTID;
+		PF_UNLOCK();
 		break;
 	}
 
 	case DIOCOSFPFLUSH:
+		PF_LOCK();
 		pf_osfp_flush();
+		PF_UNLOCK();
 		break;
 
 	case DIOCIGETIFACES: {
@@ -2442,30 +2608,38 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			error = ENODEV;
 			break;
 		}
+		PF_LOCK();
 		error = pfi_get_ifaces(io->pfiio_name, io->pfiio_buffer,
 		    &io->pfiio_size);
+		PF_UNLOCK();
 		break;
 	}
 
 	case DIOCSETIFFLAG: {
 		struct pfioc_iface *io = (struct pfioc_iface *)addr;
 
+		PF_LOCK();
 		error = pfi_set_flags(io->pfiio_name, io->pfiio_flags);
+		PF_UNLOCK();
 		break;
 	}
 
 	case DIOCCLRIFFLAG: {
 		struct pfioc_iface *io = (struct pfioc_iface *)addr;
 
+		PF_LOCK();
 		error = pfi_clear_flags(io->pfiio_name, io->pfiio_flags);
+		PF_UNLOCK();
 		break;
 	}
 
 	case DIOCSETREASS: {
 		u_int32_t	*reass = (u_int32_t *)addr;
 
+		PF_LOCK();
 		pf_trans_set.reass = *reass;
 		pf_trans_set.mask |= PF_TSET_REASS;
+		PF_UNLOCK();
 		break;
 	}
 
@@ -2474,7 +2648,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		break;
 	}
 fail:
-	PF_UNLOCK();
 	NET_UNLOCK();
 	return (error);
 }