fix state key reference underflow, when sk == skrev

the bug has been reported by Sebastien and Olivier Cherrier.
it has turned out the pf_state_key_link_reverse() does not
grab enough references when both state keys (sk and skrev)
are identical. This makes pf to trip assert later, when
references are being dropped:

panic(ffffffff81dfbc8e) at panic+0x11d
__assert(ffffffff81e64b54,ffffffff81e0a6ee,33a,ffffffff81e03b7f)
refcnt_rele(fffffd810bf02458) at refcnt_rele+0x6f
pf_state_key_unref(fffffd810bf023f0) at pf_state_key_unref+0x21
pf_remove_state(fffffd810c0c4578) at pf_remove_state+0x1fa
pf_purge_expired_states(2) at pf_purge_expired_states+0x232
pf_purge(ffffffff82236a30) at pf_purge+0x33
taskq_thread(ffff800000032080) at taskq_thread+0x81

fixed tested by Olivier Cherrier and semarie@

OK semarie@

diff --git a/sys/net/pf.c b/sys/net/pf.c
index 23eebf4..06fc993 100644 (file)
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: pf.c,v 1.1116 2021/04/27 09:38:29 sashan Exp $ */
+/*     $OpenBSD: pf.c,v 1.1117 2021/05/17 23:01:26 sashan Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -7373,14 +7373,21 @@ pf_state_key_link_reverse(struct pf_state_key *sk, struct pf_state_key *skrev)
        old_reverse = atomic_cas_ptr(&sk->reverse, NULL, skrev);
        if (old_reverse != NULL)
                KASSERT(old_reverse == skrev);
-       else
+       else {
                pf_state_key_ref(skrev);
 
-       old_reverse = atomic_cas_ptr(&skrev->reverse, NULL, sk);
-       if (old_reverse != NULL)
-               KASSERT(old_reverse == sk);
-       else
+               /*
+                * NOTE: if sk == skrev, then KASSERT() below holds true, we
+                * still want to grab a reference in such case, because
+                * pf_state_key_unlink_reverse() does not check whether keys
+                * are identical or not.
+                */
+               old_reverse = atomic_cas_ptr(&skrev->reverse, NULL, sk);
+               if (old_reverse != NULL)
+                       KASSERT(old_reverse == sk);
+
                pf_state_key_ref(sk);
+       }
 }
 
 #if NPFLOG > 0