On Wed, Jun 21, 2017 at 01:21:50AM +0200, Alexander Bluhm wrote: > Hi, > > I saw a crash on an OpenBSD 6.1 based system when a kassert in > pf_state_key_unref() was triggert. > > kernel diagnostic assertion "(sk->inp == NULL) || (sk->inp->inp_pf_sk == > NULL)" > failed: file "../../../../../net/pf.c", line 7155 > > > panic() at panic+0xfe > > __assert() at __assert+0x25 > > pf_state_key_unref() at pf_state_key_unref+0xc6 > > pf_pkt_unlink_state_key() at pf_pkt_unlink_state_key+0x15 > > m_free() at m_free+0xc0 > > soreceive() at soreceive+0xb5d > > recvit() at recvit+0x13a > > sys_recvmsg() at sys_recvmsg+0x107 > > syscall() at syscall+0x2df > > > The problem is that setting the inp pointer in the statekey to NULL > is delayed until the statekey refcounter reaches 0. So the inp > could get linked to another statekey while the mbuf in the socket > buffer was keeping the refcounter at 1. > > The sk->inp should be set to NULL immediately, then the kassert can > get even stricter. > > ok?
ok!! dhill@ I hit this too. See bugs@ archives :) > > bluhm > > Index: net/pf.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v > retrieving revision 1.1034 > diff -u -p -r1.1034 pf.c > --- net/pf.c 5 Jun 2017 22:18:28 -0000 1.1034 > +++ net/pf.c 20 Jun 2017 22:37:43 -0000 > @@ -779,6 +779,7 @@ pf_state_key_detach(struct pf_state *s, > sk->removed = 1; > pf_state_key_unlink_reverse(sk); > pf_inpcb_unlink_state_key(sk->inp); > + sk->inp = NULL; > pf_state_key_unref(sk); > } > } > @@ -7147,8 +7148,7 @@ pf_state_key_unref(struct pf_state_key * > /* state key must be unlinked from reverse key */ > KASSERT(sk->reverse == NULL); > /* state key must be unlinked from socket */ > - KASSERT((sk->inp == NULL) || (sk->inp->inp_pf_sk == NULL)); > - sk->inp = NULL; > + KASSERT(sk->inp == NULL); > pool_put(&pf_state_key_pl, sk); > } > } >
