On Tue, Feb 27, 2018 at 11:15:22AM -0300, Johan Huldtgren wrote:
> Patch does not change anything, it panics as soon as I've connected and
> I try to ping something on the inside. I couldn't find any logging for
> this, I assume it would be in messages or daemon?
When it crashes, there is no log. syslogd(8) has no chance to write
it, the panic is faster.
I can reproduce it and know how it happens. When linking the
incoming and outgoing state, the forward and reverse statekey may
be equal. This happens when the source and destination address is
the same.
nc -u -s 10.188.236.74 -p 12345 10.188.236.74 12345
I guess in your case this is not the ping packet but some other
protocol without port.
I did not consider this possiblity when I commited the cleanup.
----------------------------
revision 1.1053
date: 2017/12/29 17:05:25; author: bluhm; state: Exp; lines: +59 -34;
commitid: 75zQrAJWxZAn90Ue;
Make the functions which link the pf state keys to mbufs, inpcbs,
or other states more consistent.
OK visa@ sashan@ on a previous version
----------------------------
So let's add a comment and move the kassert to the beginning where
it was before.
Johan, does this fix your crash?
ok?
bluhm
Index: net/pf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.1062
diff -u -p -r1.1062 pf.c
--- net/pf.c 27 Feb 2018 09:24:56 -0000 1.1062
+++ net/pf.c 1 Mar 2018 19:45:07 -0000
@@ -7273,12 +7273,10 @@ pf_inp_unlink(struct inpcb *inp)
void
pf_state_key_link_reverse(struct pf_state_key *sk, struct pf_state_key *skrev)
{
- /*
- * Assert will not wire as long as we are called by pf_find_state()
- */
+ /* Note that sk and skrev may be equal, then we refcount twice. */
KASSERT(sk->reverse == NULL);
- sk->reverse = pf_state_key_ref(skrev);
KASSERT(skrev->reverse == NULL);
+ sk->reverse = pf_state_key_ref(skrev);
skrev->reverse = pf_state_key_ref(sk);
}
@@ -7386,6 +7384,7 @@ pf_state_key_unlink_reverse(struct pf_st
{
struct pf_state_key *skrev = sk->reverse;
+ /* Note that sk and skrev may be equal, then we unref twice. */
if (skrev != NULL) {
KASSERT(skrev->reverse == sk);
sk->reverse = NULL;
