On Wed, Dec 23, 2015 at 05:27:58PM +0100, Stefan Kempf wrote:
> Same panic here, but with a different trace (see below).
> It looks like the pf_state_key reference count is not always
> updated. When the packet header of an mbuf is duplicated, the
> reference count is not being increased. But freeing the old and
> the duplicated mbuf would result in two pf_state_key_unref()s.
I think you are correct.
> +void pf_pkt_state_key_ref(struct mbuf *m)
> +{
We put a newline after the void.
In M_MOVE_HDR we also copy the statekey pointer. It should not
matter as M_PKTHDR is cleared. But having ref-counted pointers
dangling around seems dangerous, I recommend to reset it there.
So this diff is OK bluhm@ if someone wants to commit it. I don't
have internet access during Xmas and cannot take care of it myself.
bluhm
Index: kern/uipc_mbuf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_mbuf.c,v
retrieving revision 1.215
diff -u -p -r1.215 uipc_mbuf.c
--- kern/uipc_mbuf.c 22 Dec 2015 13:33:26 -0000 1.215
+++ kern/uipc_mbuf.c 23 Dec 2015 16:57:55 -0000
@@ -1215,6 +1215,10 @@ m_dup_pkthdr(struct mbuf *to, struct mbu
to->m_flags |= (from->m_flags & M_COPYFLAGS);
to->m_pkthdr = from->m_pkthdr;
+#if NPF > 0
+ pf_pkt_state_key_ref(to);
+#endif /* NPF > 0 */
+
SLIST_INIT(&to->m_pkthdr.ph_tags);
if ((error = m_tag_copy_chain(to, from, wait)) != 0)
Index: net/pf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.961
diff -u -p -r1.961 pf.c
--- net/pf.c 22 Dec 2015 13:33:26 -0000 1.961
+++ net/pf.c 23 Dec 2015 17:10:52 -0000
@@ -6851,6 +6851,12 @@ pf_pkt_unlink_state_key(struct mbuf *m)
}
void
+pf_pkt_state_key_ref(struct mbuf *m)
+{
+ pf_state_key_ref(m->m_pkthdr.pf.statekey);
+}
+
+void
pf_inpcb_unlink_state_key(struct inpcb *inp)
{
if (inp != NULL) {
Index: net/pfvar.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pfvar.h,v
retrieving revision 1.427
diff -u -p -r1.427 pfvar.h
--- net/pfvar.h 22 Dec 2015 13:33:26 -0000 1.427
+++ net/pfvar.h 23 Dec 2015 16:57:55 -0000
@@ -1923,6 +1923,7 @@ struct pf_state_key *pf_state_key_ref(st
void pf_state_key_unref(struct pf_state_key *);
int pf_state_key_isvalid(struct pf_state_key *);
void pf_pkt_unlink_state_key(struct mbuf *);
+void pf_pkt_state_key_ref(struct mbuf *);
#endif /* _KERNEL */
Index: sys/mbuf.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/sys/mbuf.h,v
retrieving revision 1.205
diff -u -p -r1.205 mbuf.h
--- sys/mbuf.h 21 Nov 2015 11:46:25 -0000 1.205
+++ sys/mbuf.h 23 Dec 2015 17:02:34 -0000
@@ -316,6 +316,7 @@ struct mbuf {
(to)->m_pkthdr = (from)->m_pkthdr; \
(from)->m_flags &= ~M_PKTHDR; \
SLIST_INIT(&(from)->m_pkthdr.ph_tags); \
+ (from)->m_pkthdr.pf.statekey = NULL; \
} while (/* CONSTCOND */ 0)
/*
