pfsync_undefer_notify needs to be careful before dereferecing state keys.

pfsync_undefer_notify uses the state keys to look up the address
family, which is used to figure out if it should call ipv4 or ipv6
functions. however, the pf state purge code can unlink a state from
the trees (ie, the state keys get removed) while the pfsync defer
code is holding a reference to it and expects to be able to send
the deferred packet in the future. we can test if the state keys
are set by checking if the timeout state is PFTM_UNLINK or not.

this currently relies on both pf_remove_state and pfsync_undefer_notify
being called with the NET_LOCK held. this probably needs to be
rethought later but is good enough for now.

found the hard way on a production firewall at work.

1 files changed, 12 insertions, 1 deletions

diff --git a/sys/net/if_pfsync.c b/sys/net/if_pfsync.c
index 9a3baf0ce52..cc8fa3953fa 100644
--- a/sys/net/if_pfsync.c
+++ b/sys/net/if_pfsync.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_pfsync.c,v 1.293 2021/06/17 00:18:09 dlg Exp $	*/
+/*	$OpenBSD: if_pfsync.c,v 1.294 2021/06/23 05:43:53 dlg Exp $	*/
 
 /*
  * Copyright (c) 2002 Michael Shalayeff
@@ -1989,6 +1989,17 @@ pfsync_undefer_notify(struct pfsync_deferral *pd)
 	struct pf_pdesc pdesc;
 	struct pf_state *st = pd->pd_st;
 
+	/*
+	 * pf_remove_state removes the state keys and sets st->timeout
+	 * to PFTM_UNLINKED. this is done under NET_LOCK which should
+	 * be held here, so we can use PFTM_UNLINKED as a test for
+	 * whether the state keys are set for the address family
+	 * lookup.
+	 */
+
+	if (st->timeout == PFTM_UNLINKED)
+		return;
+
 	if (st->rt == PF_ROUTETO) {
 		if (pf_setup_pdesc(&pdesc, st->key[PF_SK_WIRE]->af,
 		    st->direction, st->kif, pd->pd_m, NULL) != PF_PASS)