Improve the safety of pf IOCTLs, taking into account that some paths can sleep.

- Introduces a rw_lock in pfioctl so that we can have concurrent readers
  but only one process performing updates at a time;

- Separates state expiry into "unlink" and "free" parts; anyone can unlink
  a state/src node from the RB trees at any time, but a state can only be
  freed whilst the write lock is held;

- Converts state_updates into list state_list containing all states,
  regardless of whether they are "linked" or "unlinked";

- Introduces a new PFTM_UNLINKED state that is used on the "unlinked" states
  to signal that they can be freed;

- Converts pf_purge_expired_state to an "unlink" state routine, which only
  unlinks the state from the RB trees.  Freeing the state/src nodes is left
  to the purge thread, which runs whilst holding a write lock, such that all
  "next" references remain valid;

- Converts pfsync_bulk_update and DIOCGETSTATES to walk state_list rather
  than the RB trees;

- Converts the purge thread to use the new state_list and perform a partial
  purge every second, with the target rate a full state table walk every
  PFTM_INTERVAL seconds.

seen by mcbride, henning, dhartmei pre-3.8, but too intrusive for then

6 files changed, 196 insertions, 79 deletions

diff --git a/sys/net/if_pfsync.c b/sys/net/if_pfsync.c
index 5814a17148d..9b6bebae796 100644
--- a/sys/net/if_pfsync.c
+++ b/sys/net/if_pfsync.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_pfsync.c,v 1.54 2005/08/18 10:28:13 pascoe Exp $	*/
+/*	$OpenBSD: if_pfsync.c,v 1.55 2005/09/28 01:46:32 pascoe Exp $	*/
 
 /*
  * Copyright (c) 2002 Michael Shalayeff
@@ -123,6 +123,8 @@ pfsyncattach(int npfsync)
 	pfsyncif.sc_sendaddr.s_addr = INADDR_PFSYNC_GROUP;
 	pfsyncif.sc_ureq_received = 0;
 	pfsyncif.sc_ureq_sent = 0;
+	pfsyncif.sc_bulk_send_next = NULL;
+	pfsyncif.sc_bulk_terminator = NULL;
 	ifp = &pfsyncif.sc_if;
 	strlcpy(ifp->if_xname, "pfsync0", sizeof ifp->if_xname);
 	ifp->if_softc = &pfsyncif;
@@ -361,8 +363,8 @@ pfsync_input(struct mbuf *m, ...)
 			    st; st = nexts) {
 				nexts = RB_NEXT(pf_state_tree_id, &tree_id, st);
 				if (st->creatorid == creatorid) {
-					st->timeout = PFTM_PURGE;
 					st->sync_flags |= PFSTATE_FROMSYNC;
+					pf_unlink_state(st);
 				}
 			}
 		} else {
@@ -375,8 +377,8 @@ pfsync_input(struct mbuf *m, ...)
 				nexts = RB_NEXT(pf_state_tree_lan_ext,
 				    &kif->pfik_lan_ext, st);
 				if (st->creatorid == creatorid) {
-					st->timeout = PFTM_PURGE;
 					st->sync_flags |= PFSTATE_FROMSYNC;
+					pf_unlink_state(st);
 				}
 			}
 		}
@@ -541,8 +543,8 @@ pfsync_input(struct mbuf *m, ...)
 				pfsyncstats.pfsyncs_badstate++;
 				continue;
 			}
-			st->timeout = PFTM_PURGE;
 			st->sync_flags |= PFSTATE_FROMSYNC;
+			pf_unlink_state(st);
 		}
 		splx(s);
 		break;
@@ -663,8 +665,8 @@ pfsync_input(struct mbuf *m, ...)
 				pfsyncstats.pfsyncs_badstate++;
 				continue;
 			}
-			st->timeout = PFTM_PURGE;
 			st->sync_flags |= PFSTATE_FROMSYNC;
+			pf_unlink_state(st);
 		}
 		splx(s);
 		break;
@@ -690,6 +692,10 @@ pfsync_input(struct mbuf *m, ...)
 
 			if (key.id == 0 && key.creatorid == 0) {
 				sc->sc_ureq_received = time_uptime;
+				if (sc->sc_bulk_send_next == NULL)
+					sc->sc_bulk_send_next = 
+					    TAILQ_FIRST(&state_list);
+				sc->sc_bulk_terminator = sc->sc_bulk_send_next;
 				if (pf_status.debug >= PF_DEBUG_MISC)
 					printf("pfsync: received "
 					    "bulk update request\n");
@@ -1093,8 +1099,6 @@ pfsync_pack_state(u_int8_t action, struct pf_state *st, int flags)
 	secs = time_second;
 
 	st->pfsync_time = time_uptime;
-	TAILQ_REMOVE(&state_updates, st, u.s.entry_updates);
-	TAILQ_INSERT_TAIL(&state_updates, st, u.s.entry_updates);
 
 	if (sp == NULL) {
 		/* not a "duplicate" update */
@@ -1342,28 +1346,39 @@ pfsync_bulk_update(void *v)
 	 * Grab at most PFSYNC_BULKPACKETS worth of states which have not
 	 * been sent since the latest request was made.
 	 */
-	while ((state = TAILQ_FIRST(&state_updates)) != NULL &&
-	    ++i < (sc->sc_maxcount * PFSYNC_BULKPACKETS)) {
-		if (state->pfsync_time > sc->sc_ureq_received) {
-			/* we're done */
-			pfsync_send_bus(sc, PFSYNC_BUS_END);
-			sc->sc_ureq_received = 0;
-			timeout_del(&sc->sc_bulk_tmo);
-			if (pf_status.debug >= PF_DEBUG_MISC)
-				printf("pfsync: bulk update complete\n");
-			break;
-		} else {
-			/* send an update and move to end of list */
-			if (!state->sync_flags)
+	state = sc->sc_bulk_send_next;
+	if (state)
+		do {
+			/* send state update if syncable and not already sent */
+			if (!state->sync_flags
+			    && state->timeout < PFTM_MAX
+			    && state->pfsync_time <= sc->sc_ureq_received) {
 				pfsync_pack_state(PFSYNC_ACT_UPD, state, 0);
-			state->pfsync_time = time_uptime;
-			TAILQ_REMOVE(&state_updates, state, u.s.entry_updates);
-			TAILQ_INSERT_TAIL(&state_updates, state,
-			    u.s.entry_updates);
+				i++;
+			}
 
-			/* look again for more in a bit */
-			timeout_add(&sc->sc_bulk_tmo, 1);
-		}
+			/* figure next state to send */
+			state = TAILQ_NEXT(state, u.s.entry_list);
+
+			/* wrap to start of list if we hit the end */
+			if (!state)
+				state = TAILQ_FIRST(&state_list);
+		} while (i < sc->sc_maxcount * PFSYNC_BULKPACKETS &&
+		    state != sc->sc_bulk_terminator);
+
+	if (!state || state == sc->sc_bulk_terminator) {
+		/* we're done */
+		pfsync_send_bus(sc, PFSYNC_BUS_END);
+		sc->sc_ureq_received = 0;
+		sc->sc_bulk_send_next = NULL;
+		sc->sc_bulk_terminator = NULL;
+		timeout_del(&sc->sc_bulk_tmo);
+		if (pf_status.debug >= PF_DEBUG_MISC)
+			printf("pfsync: bulk update complete\n");
+	} else {
+		/* look again for more in a bit */
+		timeout_add(&sc->sc_bulk_tmo, 1);
+		sc->sc_bulk_send_next = state;
 	}
 	if (sc->sc_mbuf != NULL)
 		pfsync_sendout(sc);
diff --git a/sys/net/if_pfsync.h b/sys/net/if_pfsync.h
index 4d6cfebc143..8adf0a0372b 100644
--- a/sys/net/if_pfsync.h
+++ b/sys/net/if_pfsync.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_pfsync.h,v 1.25 2005/08/16 11:26:48 pascoe Exp $	*/
+/*	$OpenBSD: if_pfsync.h,v 1.26 2005/09/28 01:46:32 pascoe Exp $	*/
 
 /*
  * Copyright (c) 2001 Michael Shalayeff
@@ -178,10 +178,14 @@ struct pfsync_softc {
 	union sc_tdb_statep	 sc_statep_tdb;
 	u_int32_t		 sc_ureq_received;
 	u_int32_t		 sc_ureq_sent;
+	struct pf_state		*sc_bulk_send_next;
+	struct pf_state		*sc_bulk_terminator;
 	int			 sc_bulk_tries;
 	int			 sc_maxcount;	/* number of states in mtu */
 	int			 sc_maxupdates;	/* number of updates/state */
 };
+
+extern struct pfsync_softc pfsyncif;
 #endif
 
 
diff --git a/sys/net/pf.c b/sys/net/pf.c
index bdf1480471d..11d098bab0a 100644
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf.c,v 1.502 2005/08/22 11:54:25 dhartmei Exp $ */
+/*	$OpenBSD: pf.c,v 1.503 2005/09/28 01:46:32 pascoe Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -49,6 +49,7 @@
 #include <sys/time.h>
 #include <sys/pool.h>
 #include <sys/proc.h>
+#include <sys/rwlock.h>
 
 #include <net/if.h>
 #include <net/if_types.h>
@@ -288,7 +289,7 @@ static __inline int pf_anchor_compare(struct pf_anchor *, struct pf_anchor *);
 struct pf_src_tree tree_src_tracking;
 
 struct pf_state_tree_id tree_id;
-struct pf_state_queue state_updates;
+struct pf_state_queue state_list;
 
 RB_GENERATE(pf_src_tree, pf_src_node, entry, pf_src_compare);
 RB_GENERATE(pf_state_tree_lan_ext, pf_state,
@@ -848,8 +849,7 @@ pf_insert_state(struct pfi_kif *kif, struct pf_state *state)
 		RB_REMOVE(pf_state_tree_ext_gwy, &kif->pfik_ext_gwy, state);
 		return (-1);
 	}
-	TAILQ_INSERT_HEAD(&state_updates, state, u.s.entry_updates);
-
+	TAILQ_INSERT_TAIL(&state_list, state, u.s.entry_list);
 	pf_status.fcounters[FCNT_STATE_INSERT]++;
 	pf_status.states++;
 	pfi_kif_ref(kif, PFI_KIF_REF_STATE);
@@ -862,15 +862,24 @@ pf_insert_state(struct pfi_kif *kif, struct pf_state *state)
 void
 pf_purge_thread(void *v)
 {
-	int s;
+	int nloops = 0, s;
 
 	for (;;) {
-		tsleep(pf_purge_thread, PWAIT, "pftm",
-		    pf_default_rule.timeout[PFTM_INTERVAL] * hz);
+		tsleep(pf_purge_thread, PWAIT, "pftm", 1 * hz);
+
 		s = splsoftnet();
-		pf_purge_expired_states();
-		pf_purge_expired_fragments();
-		pf_purge_expired_src_nodes();
+
+		/* process a fraction of the state table every second */
+		pf_purge_expired_states(1 + (pf_status.states
+		    / pf_default_rule.timeout[PFTM_INTERVAL]));
+
+		/* purge other expired types every PFTM_INTERVAL seconds */
+		if (++nloops >= pf_default_rule.timeout[PFTM_INTERVAL]) {
+			pf_purge_expired_fragments();
+			pf_purge_expired_src_nodes(0);
+			nloops = 0;
+		}
+
 		splx(s);
 	}
 }
@@ -888,6 +897,7 @@ pf_state_expires(const struct pf_state *state)
 		return (time_second);
 	if (state->timeout == PFTM_UNTIL_PACKET)
 		return (0);
+	KASSERT(state->timeout != PFTM_UNLINKED);
 	KASSERT(state->timeout < PFTM_MAX);
 	timeout = state->rule.ptr->timeout[state->timeout];
 	if (!timeout)
@@ -912,14 +922,21 @@ pf_state_expires(const struct pf_state *state)
 }
 
 void
-pf_purge_expired_src_nodes(void)
+pf_purge_expired_src_nodes(int waslocked)
 {
 	 struct pf_src_node		*cur, *next;
+	 int				 locked = waslocked;
 
 	 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_second) {
+			 if (! locked) {
+				 rw_enter_write(&pf_consistency_lock);
+			 	 next = RB_NEXT(pf_src_tree,
+				     &tree_src_tracking, cur);
+				 locked = 1;
+			 }
 			 if (cur->rule.ptr != NULL) {
 				 cur->rule.ptr->src_nodes--;
 				 if (cur->rule.ptr->states <= 0 &&
@@ -932,6 +949,9 @@ pf_purge_expired_src_nodes(void)
 			 pool_put(&pf_src_tree_pl, cur);
 		 }
 	 }
+
+	 if (locked && !waslocked)
+		rw_exit_write(&pf_consistency_lock);
 }
 
 void
@@ -964,8 +984,9 @@ pf_src_tree_remove_state(struct pf_state *s)
 	s->src_node = s->nat_src_node = NULL;
 }
 
+/* callers should be at splsoftnet */
 void
-pf_purge_expired_state(struct pf_state *cur)
+pf_unlink_state(struct pf_state *cur)
 {
 	if (cur->src.state == PF_TCPS_PROXY_DST)
 		pf_send_tcp(cur->rule.ptr, cur->af,
@@ -979,9 +1000,24 @@ pf_purge_expired_state(struct pf_state *cur)
 	    &cur->u.s.kif->pfik_lan_ext, cur);
 	RB_REMOVE(pf_state_tree_id, &tree_id, cur);
 #if NPFSYNC
-	pfsync_delete_state(cur);
+	if (cur->creatorid == pf_status.hostid)
+		pfsync_delete_state(cur);
 #endif
+	cur->timeout = PFTM_UNLINKED;
 	pf_src_tree_remove_state(cur);
+}
+
+/* callers should be at splsoftnet and hold the
+ * write_lock on pf_consistency_lock */
+void
+pf_free_state(struct pf_state *cur)
+{
+#if NPFSYNC
+	if (pfsyncif.sc_bulk_send_next == cur ||
+	    pfsyncif.sc_bulk_terminator == cur)
+		return;
+#endif
+	KASSERT(cur->timeout == PFTM_UNLINKED);
 	if (--cur->rule.ptr->states <= 0 &&
 	    cur->rule.ptr->src_nodes <= 0)
 		pf_rm_rule(NULL, cur->rule.ptr);
@@ -994,7 +1030,7 @@ pf_purge_expired_state(struct pf_state *cur)
 			pf_rm_rule(NULL, cur->anchor.ptr);
 	pf_normalize_tcp_cleanup(cur);
 	pfi_kif_unref(cur->u.s.kif, PFI_KIF_REF_STATE);
-	TAILQ_REMOVE(&state_updates, cur, u.s.entry_updates);
+	TAILQ_REMOVE(&state_list, cur, u.s.entry_list);
 	if (cur->tag)
 		pf_tag_unref(cur->tag);
 	pool_put(&pf_state_pl, cur);
@@ -1003,16 +1039,44 @@ pf_purge_expired_state(struct pf_state *cur)
 }
 
 void
-pf_purge_expired_states(void)
+pf_purge_expired_states(u_int32_t maxcheck)
 {
-	struct pf_state		*cur, *next;
-
-	for (cur = RB_MIN(pf_state_tree_id, &tree_id);
-	    cur; cur = next) {
-		next = RB_NEXT(pf_state_tree_id, &tree_id, cur);
-		if (pf_state_expires(cur) <= time_second)
-			pf_purge_expired_state(cur);
+	static struct pf_state	*cur = NULL;
+	struct pf_state		*next;
+	int 			 locked = 0;
+
+	while (maxcheck--) {
+		/* wrap to start of list when we hit the end */
+		if (cur == NULL) {
+			cur = TAILQ_FIRST(&state_list);
+			if (cur == NULL)
+				break;	/* list empty */
+		}
+
+		/* get next state, as cur may get deleted */
+		next = TAILQ_NEXT(cur, u.s.entry_list);
+
+		if (cur->timeout == PFTM_UNLINKED) {
+			/* free unlinked state */
+			if (! locked) {
+				rw_enter_write(&pf_consistency_lock);
+				locked = 1;
+			}
+			pf_free_state(cur);
+		} else if (pf_state_expires(cur) <= time_second) {
+			/* unlink and free expired state */
+			pf_unlink_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_if.c b/sys/net/pf_if.c
index 549182217a5..25f83e6df1b 100644
--- a/sys/net/pf_if.c
+++ b/sys/net/pf_if.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf_if.c,v 1.43 2005/08/18 10:28:14 pascoe Exp $ */
+/*	$OpenBSD: pf_if.c,v 1.44 2005/09/28 01:46:32 pascoe Exp $ */
 
 /*
  * Copyright 2005 Henning Brauer <henning@openbsd.org>
@@ -653,20 +653,25 @@ pfi_clr_istats(const char *name)
 int
 pfi_get_ifaces(const char *name, struct pfi_kif *buf, int *size)
 {
-	struct pfi_kif	*p;
+	struct pfi_kif	*p, *nextp;
 	int		 s, n = 0;
 
 	s = splsoftnet();
-	RB_FOREACH(p, pfi_ifhead, &pfi_ifs) {
+	for (p = RB_MIN(pfi_ifhead, &pfi_ifs); p; p = nextp) {
+		nextp = RB_NEXT(pfi_ifhead, &pfi_ifs, p);
 		if (pfi_skip_if(name, p))
 			continue;
 		if (*size > n++) {
 			if (!p->pfik_tzero)
 				p->pfik_tzero = time_second;
+			pfi_kif_ref(p, PFI_KIF_REF_RULE);
 			if (copyout(p, buf++, sizeof(*buf))) {
+				pfi_kif_unref(p, PFI_KIF_REF_RULE);
 				splx(s);
 				return (EFAULT);
 			}
+			nextp = RB_NEXT(pfi_ifhead, &pfi_ifs, p);
+			pfi_kif_unref(p, PFI_KIF_REF_RULE);
 		}
 	}
 	splx(s);
diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index fa4dd38e577..c5d94203715 100644
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf_ioctl.c,v 1.158 2005/09/05 14:51:08 dhartmei Exp $ */
+/*	$OpenBSD: pf_ioctl.c,v 1.159 2005/09/28 01:46:32 pascoe Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -51,6 +51,7 @@
 #include <sys/proc.h>
 #include <sys/malloc.h>
 #include <sys/kthread.h>
+#include <sys/rwlock.h>
 
 #include <net/if.h>
 #include <net/if_types.h>
@@ -112,6 +113,7 @@ void			 pf_hash_rule_addr(MD5_CTX *, struct pf_rule_addr *);
 int			 pf_commit_rules(u_int32_t, int, char *);
 
 struct pf_rule		 pf_default_rule;
+struct rwlock		 pf_consistency_lock = RWLOCK_INITIALIZER;
 #ifdef ALTQ
 static int		 pf_altq_running;
 #endif
@@ -162,7 +164,7 @@ pfattach(int num)
 	TAILQ_INIT(&pf_pabuf);
 	pf_altqs_active = &pf_altqs[0];
 	pf_altqs_inactive = &pf_altqs[1];
-	TAILQ_INIT(&state_updates);
+	TAILQ_INIT(&state_list);
 
 	/* default rule should never be garbage collected */
 	pf_default_rule.entries.tqe_prev = &pf_default_rule.entries.tqe_next;
@@ -1229,13 +1231,20 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		case DIOCRSETADDRS:
 		case DIOCRSETTFLAGS:
 			if (((struct pfioc_table *)addr)->pfrio_flags &
-			    PFR_FLAG_DUMMY)
+			    PFR_FLAG_DUMMY) {
+				flags |= FWRITE; /* need write lock for dummy */
 				break; /* dummy operation ok */
+			}
 			return (EACCES);
 		default:
 			return (EACCES);
 		}
 
+	if (flags & FWRITE)
+		rw_enter_write(&pf_consistency_lock);
+	else
+		rw_enter_read(&pf_consistency_lock);
+
 	s = splsoftnet();
 	switch (cmd) {
 
@@ -1683,22 +1692,24 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 	}
 
 	case DIOCCLRSTATES: {
-		struct pf_state		*state;
+		struct pf_state		*state, *nexts;
 		struct pfioc_state_kill *psk = (struct pfioc_state_kill *)addr;
 		int			 killed = 0;
 
-		RB_FOREACH(state, pf_state_tree_id, &tree_id) {
+		for (state = RB_MIN(pf_state_tree_id, &tree_id); state;
+		    state = nexts) {
+			nexts = RB_NEXT(pf_state_tree_id, &tree_id, state);
+
 			if (!psk->psk_ifname[0] || !strcmp(psk->psk_ifname,
 			    state->u.s.kif->pfik_name)) {
-				state->timeout = PFTM_PURGE;
 #if NPFSYNC
 				/* don't send out individual delete messages */
 				state->sync_flags = PFSTATE_NOSYNC;
 #endif
+				pf_unlink_state(state);
 				killed++;
 			}
 		}
-		pf_purge_expired_states();
 		psk->psk_af = killed;
 #if NPFSYNC
 		pfsync_clear_states(pf_status.hostid, psk->psk_ifname);
@@ -1707,12 +1718,15 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 	}
 
 	case DIOCKILLSTATES: {
-		struct pf_state		*state;
+		struct pf_state		*state, *nexts;
 		struct pf_state_host	*src, *dst;
 		struct pfioc_state_kill	*psk = (struct pfioc_state_kill *)addr;
 		int			 killed = 0;
 
-		RB_FOREACH(state, pf_state_tree_id, &tree_id) {
+		for (state = RB_MIN(pf_state_tree_id, &tree_id); state;
+		    state = nexts) {
+			nexts = RB_NEXT(pf_state_tree_id, &tree_id, state);
+
 			if (state->direction == PF_OUT) {
 				src = &state->lan;
 				dst = &state->ext;
@@ -1741,11 +1755,15 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			    dst->port)) &&
 			    (!psk->psk_ifname[0] || !strcmp(psk->psk_ifname,
 			    state->u.s.kif->pfik_name))) {
-				state->timeout = PFTM_PURGE;
+#if NPFSYNC > 0
+				/* send immediate delete of state */
+				pfsync_delete_state(state);
+				state->sync_flags |= PFSTATE_NOSYNC;
+#endif
+				pf_unlink_state(state);
 				killed++;
 			}
 		}
-		pf_purge_expired_states();
 		psk->psk_af = killed;
 		break;
 	}
@@ -1828,13 +1846,11 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		struct pfioc_states	*ps = (struct pfioc_states *)addr;
 		struct pf_state		*state;
 		struct pf_state		*p, *pstore;
-		struct pfi_kif		*kif;
 		u_int32_t		 nr = 0;
 		int			 space = ps->ps_len;
 
 		if (space == 0) {
-			TAILQ_FOREACH(kif, &pfi_statehead, pfik_w_states)
-				nr += kif->pfik_states;
+			nr = pf_status.states;
 			ps->ps_len = sizeof(struct pf_state) * nr;
 			break;
 		}
@@ -1842,16 +1858,18 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		pstore = malloc(sizeof(*pstore), M_TEMP, M_WAITOK);
 
 		p = ps->ps_states;
-		TAILQ_FOREACH(kif, &pfi_statehead, pfik_w_states)
-			RB_FOREACH(state, pf_state_tree_ext_gwy,
-			    &kif->pfik_ext_gwy) {
+
+		state = TAILQ_FIRST(&state_list);
+		while (state) {
+			if (state->timeout != PFTM_UNLINKED) {
 				int	secs = time_second;
 
 				if ((nr+1) * sizeof(*p) > (unsigned)ps->ps_len)
 					break;
 
 				bcopy(state, pstore, sizeof(*pstore));
-				strlcpy(pstore->u.ifname, kif->pfik_name,
+				strlcpy(pstore->u.ifname,
+				    state->u.s.kif->pfik_name,
 				    sizeof(pstore->u.ifname));
 				pstore->rule.nr = state->rule.ptr->nr;
 				pstore->nat_rule.nr = (state->nat_rule.ptr ==
@@ -1872,6 +1890,9 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				p++;
 				nr++;
 			}
+			state = TAILQ_NEXT(state, u.s.entry_list);
+		}
+
 		ps->ps_len = sizeof(struct pf_state) * nr;
 
 		free(pstore, M_TEMP);
@@ -2991,7 +3012,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			n->expire = 1;
 			n->states = 0;
 		}
-		pf_purge_expired_src_nodes();
+		pf_purge_expired_src_nodes(1);
 		pf_status.src_nodes = 0;
 		break;
 	}
@@ -3042,5 +3063,9 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 	}
 fail:
 	splx(s);
+	if (flags & FWRITE)
+		rw_exit_write(&pf_consistency_lock);
+	else
+		rw_exit_read(&pf_consistency_lock);
 	return (error);
 }
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index d67b499c2fc..b458696b2d2 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfvar.h,v 1.229 2005/08/18 10:28:14 pascoe Exp $ */
+/*	$OpenBSD: pfvar.h,v 1.230 2005/09/28 01:46:33 pascoe Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -37,6 +37,7 @@
 #include <sys/types.h>
 #include <sys/queue.h>
 #include <sys/tree.h>
+#include <sys/rwlock.h>
 
 #include <net/radix.h>
 #include <net/route.h>
@@ -79,7 +80,8 @@ enum	{ PFTM_TCP_FIRST_PACKET, PFTM_TCP_OPENING, PFTM_TCP_ESTABLISHED,
 	  PFTM_OTHER_FIRST_PACKET, PFTM_OTHER_SINGLE,
 	  PFTM_OTHER_MULTIPLE, PFTM_FRAG, PFTM_INTERVAL,
 	  PFTM_ADAPTIVE_START, PFTM_ADAPTIVE_END, PFTM_SRC_NODE,
-	  PFTM_TS_DIFF, PFTM_MAX, PFTM_PURGE, PFTM_UNTIL_PACKET };
+	  PFTM_TS_DIFF, PFTM_MAX, PFTM_PURGE, PFTM_UNLINKED,
+	  PFTM_UNTIL_PACKET };
 
 /* PFTM default values */
 #define PFTM_TCP_FIRST_PACKET_VAL	120	/* First TCP packet */
@@ -708,7 +710,7 @@ struct pf_state {
 			RB_ENTRY(pf_state)	 entry_lan_ext;
 			RB_ENTRY(pf_state)	 entry_ext_gwy;
 			RB_ENTRY(pf_state)	 entry_id;
-			TAILQ_ENTRY(pf_state)	 entry_updates;
+			TAILQ_ENTRY(pf_state)	 entry_list;
 			struct pfi_kif		*kif;
 		} s;
 		char	 ifname[IFNAMSIZ];
@@ -1398,7 +1400,7 @@ RB_HEAD(pf_state_tree_id, pf_state);
 RB_PROTOTYPE(pf_state_tree_id, pf_state,
     entry_id, pf_state_compare_id);
 extern struct pf_state_tree_id tree_id;
-extern struct pf_state_queue state_updates;
+extern struct pf_state_queue state_list;
 
 extern struct pf_anchor_global		  pf_anchors;
 extern struct pf_ruleset		  pf_main_ruleset;
@@ -1425,9 +1427,10 @@ extern struct pool		 pf_src_tree_pl, pf_rule_pl;
 extern struct pool		 pf_state_pl, pf_altq_pl, pf_pooladdr_pl;
 extern struct pool		 pf_state_scrub_pl;
 extern void			 pf_purge_thread(void *);
-extern void			 pf_purge_expired_src_nodes(void);
-extern void			 pf_purge_expired_states(void);
-extern void			 pf_purge_expired_state(struct pf_state *);
+extern void			 pf_purge_expired_src_nodes(int);
+extern void			 pf_purge_expired_states(u_int32_t);
+extern void			 pf_unlink_state(struct pf_state *);
+extern void			 pf_free_state(struct pf_state *);
 extern int			 pf_insert_state(struct pfi_kif *,
 				    struct pf_state *);
 extern int			 pf_insert_src_node(struct pf_src_node **,
@@ -1569,6 +1572,7 @@ void		pf_qid_unref(u_int32_t);
 
 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;