pf_state_key_link_reverse() is prone to race on parallel forwarding
authorsashan <sashan@openbsd.org>
Tue, 27 Apr 2021 09:38:29 +0000 (09:38 +0000)
committersashan <sashan@openbsd.org>
Tue, 27 Apr 2021 09:38:29 +0000 (09:38 +0000)
we need to adjust assertions. at time we call pf_state_key_link_reverse()
is state_key either linked to correct reverse peer or not linked at all.
The pf_state_key_link_reverse() is being called as a reader ons tate_lock.
There might be more packets, which try to update the state key.

OK bluhm@

sys/net/pf.c

index 12d0597..23eebf4 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: pf.c,v 1.1115 2021/04/23 03:29:24 dlg Exp $ */
+/*     $OpenBSD: pf.c,v 1.1116 2021/04/27 09:38:29 sashan Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -7368,11 +7368,19 @@ pf_inp_unlink(struct inpcb *inp)
 void
 pf_state_key_link_reverse(struct pf_state_key *sk, struct pf_state_key *skrev)
 {
-       /* Note that sk and skrev may be equal, then we refcount twice. */
-       KASSERT(sk->reverse == NULL);
-       KASSERT(skrev->reverse == NULL);
-       sk->reverse = pf_state_key_ref(skrev);
-       skrev->reverse = pf_state_key_ref(sk);
+       struct pf_state_key *old_reverse;
+
+       old_reverse = atomic_cas_ptr(&sk->reverse, NULL, skrev);
+       if (old_reverse != NULL)
+               KASSERT(old_reverse == skrev);
+       else
+               pf_state_key_ref(skrev);
+
+       old_reverse = atomic_cas_ptr(&skrev->reverse, NULL, sk);
+       if (old_reverse != NULL)
+               KASSERT(old_reverse == sk);
+       else
+               pf_state_key_ref(sk);
 }
 
 #if NPFLOG > 0