git: 301ec2cebb6a - main - pf: always mark states as unlinked before detaching them

From: Kristof Provost <kp_at_FreeBSD.org>
Date: Wed, 08 May 2024 11:20:01 UTC
The branch main has been updated by kp:

URL: https://cgit.FreeBSD.org/src/commit/?id=301ec2cebb6a3e472af39eb7b98c3b7ff88e04b0

commit 301ec2cebb6a3e472af39eb7b98c3b7ff88e04b0
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2024-05-06 07:43:49 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2024-05-08 11:19:05 +0000

    pf: always mark states as unlinked before detaching them
    
    Users have reported crashes in pf_test_state_udp() where at least one state key
    is NULL.
    
    That suggests that pf_detach_state() ran concurrently with pf_test_state_udp().
    pf_test_state_udp() holds the state lock (aka the id lock), but
    pf_detach_state() does not.
    The intent is that detached states are not returned by STATE_LOOKUP/
    pf_find_state(), as the state's timeout is set to PFTM_UNLINKED and thus
    pf_find_state() does not find the state.
    
    There are other paths to pf_detach_state() (outside of pf_unlink_state())
    though, where we did not set the timeout to PFTM_UNLINKED. Fix those, and assert
    that the timeout is set correctly when we enter pf_detach_state().
    
    MFC after:      1 week
    See also:       https://redmine.pfsense.org/issues/15413
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
    Differential Revision:  https://reviews.freebsd.org/D45101
---
 sys/netpfil/pf/pf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 0dd4da5f271a..195b5d49db7a 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -1353,6 +1353,7 @@ keyattach:
 						    sk : NULL);
 						printf("\n");
 					}
+					s->timeout = PFTM_UNLINKED;
 					PF_HASHROW_UNLOCK(ih);
 					KEYS_UNLOCK();
 					uma_zfree(V_pf_state_key_z, sk);
@@ -1421,6 +1422,7 @@ pf_detach_state(struct pf_kstate *s)
 	struct pf_keyhash *kh;
 
 	NET_EPOCH_ASSERT();
+	MPASS(s->timeout >= PFTM_MAX);
 
 	pf_sctp_multihome_detach_addr(s);
 
@@ -1552,6 +1554,7 @@ pf_state_insert(struct pfi_kkif *kif, struct pfi_kkif *orig_kif,
 			break;
 
 	if (cur != NULL) {
+		s->timeout = PFTM_UNLINKED;
 		PF_HASHROW_UNLOCK(ih);
 		if (V_pf_status.debug >= PF_DEBUG_MISC) {
 			printf("pf: state ID collision: "