This diff fixes panic tripped by KASSERT(st->sync_state == PFSYNC_S_NONE)
authorsashan <sashan@openbsd.org>
Tue, 8 Nov 2022 16:20:26 +0000 (16:20 +0000)
committersashan <sashan@openbsd.org>
Tue, 8 Nov 2022 16:20:26 +0000 (16:20 +0000)
found in pfsync_insert_state(). It is caused by two packets which happen
to belong to the same session. Think of UDP stream or two TCP SYN packets
transmitted almost simultaneously. The first such packet wins a state lock
and inserts state to table. The second packet waits for state lock
as a reader. As soon as the first packet is done with state creation
it drops the lock and is going to sent S_INS message to its peer via
pfsync. The second update meanwhile obtains the state lock as a reader.
It finds a state created by the first packet. Later the second packet
also finds out the state needs to be updated, because sync_state
is still set to PFSYNC_S_NONE. The second packet puts state to snapshot
list marking it as S_UPD. All this happens before the first packet has
a chance to make a progress. Think of the first packet loses cpu after
dropping a write lock. Once the first packet gets running again it
trips KASSERT() because sync_state is set to S_UPD.

tested by hrvoje@

OK dlg@

sys/net/pf.c

index c42f76d..2c13c99 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: pf.c,v 1.1144 2022/11/07 16:35:11 dlg Exp $ */
+/*     $OpenBSD: pf.c,v 1.1145 2022/11/08 16:20:26 sashan Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -1023,10 +1023,10 @@ pf_state_insert(struct pfi_kif *kif, struct pf_state_key **skw,
        pf_status.fcounters[FCNT_STATE_INSERT]++;
        pf_status.states++;
        pfi_kif_ref(kif, PFI_KIF_REF_STATE);
-       PF_STATE_EXIT_WRITE();
 #if NPFSYNC > 0
        pfsync_insert_state(s);
 #endif /* NPFSYNC > 0 */
+       PF_STATE_EXIT_WRITE();
        return (0);
 }