From cad5270a74df5efdb3de91436d8eac261e433cb0 Mon Sep 17 00:00:00 2001 From: Alexandr Nedvedicky Date: Mon, 17 May 2021 23:01:27 +0000 Subject: 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@ --- sys/net/pf.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) (limited to 'sys') diff --git a/sys/net/pf.c b/sys/net/pf.c index 23eebf4a274..06fc993b4f4 100644 --- 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 -- cgit v1.2.3