On Sat, Feb 24, 2018 at 04:20:50PM -0500, Johan Huldtgren wrote:
> trying to connect to my gateway today I found the following
> panic. This is 100% reproducible anytime I connect via
> openvpn and then generate traffic. This first happened on
> the Feb 7th snap, I updated and it happens on the latest
> snap as well.

The questions is what have you used before.  I have rewritten the
code at 2017/12/29.  Was your working version before that?

Although a bit different, a simmilar assertion was there before.

> ip_output(d2574200,0,f5275000,1,0,0,0) at ip_output+0x649
> ip_forward(d2574200,d24b8000,d1f8aed8,0) at ip_forward+0x20a

> ddb> show mbuf
> mbuf 0xd05032f4

When you type "show mbuf" you must give the address of the mbuf.
The functions ip_output() and ip_forward() pass it at the first
argument.  So "show mbuf 0xd2574200" would produces reasonable
results, your command dumped arbitrary memory.

> panic: kernel diagnostic assertion "skrev->reverse == NULL" failed: file 
> "/usr/src/sys/net/pf.c", line 7277
> pf_find_state(d2486f00,f528d8e0,2,d251b900) at pf_find_state+0x28d

This functions calls pf_state_key_link_reverse(sk, pkt_sk) with
pkt_sk->reverse != NULL.

On the way to that call we went through
                pkt_sk = m->m_pkthdr.pf.statekey;
                if (pkt_sk && pf_state_key_isvalid(pkt_sk->reverse))
                        sk = pkt_sk->reverse;

We know that pkt_sk != NULL and pkt_sk->reverse != NULL and before
doing the RB_FIND() lookup we check that sk == NULL.  So
pf_state_key_isvalid(pkt_sk->reverse) must be false.

The kernel tried to use an invalid statekey.  How can that happen?
Invalid means sk->removed == 1, but pf_state_key_detach() also calls
pf_state_key_unlink_reverse().

> pf_test(2,3,d247d400,f528da64) at pf_test+0xb63
> ip_output(d251b900,0,f528dad0,1,0,0,0) at ip_output+0x649

Here we find the outgoing state.  The mbuf had a statekey before.

> ip_forward(d251b900,d247d400,d201cb48,0) at ip_forward+0x20a
> ip_input_if(f528dc64,f528dc50,4,0,d247d400) at ip_input_if+0x48e
> ipv4_input(d247d400,d251b900) at ipv4_input+0x2b

Here pf attaches the incoming statekey to the mbuf.  This is the
one with the invalid reverse.

> tun_dev_write(d247d400,f528dd98,10001) at tun_dev_write+0x222
> tunwrite(2800,f528dd98,11) at tunwrite+0x53

How does you pf config look like?  Do you have some skip on tun?
Was there unencrpyted traffic before you enabled openvpn?  Were
there matching pf states before you enabled openvpn?  Does it
immediately crash when you start openvpn and the first packet is
sent out.  Do you only use the tun interface and the outgoing
interface?  Do you have more forwarding paths?  Do you use floating
states?  Does the problem go away with if-bound states?  Is
there more stuff involved like gif(4) or bridge(4) or ....

Although I do not fully understand what is the root of the problem,
you can try this diff.  Does it prevent the panic?  Do you see the
log message I have added there?  This would at least prove that my
theory is correct.

bluhm

Index: net/pf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.1061
diff -u -p -r1.1061 pf.c
--- net/pf.c    18 Feb 2018 21:45:30 -0000      1.1061
+++ net/pf.c    26 Feb 2018 00:27:57 -0000
@@ -1070,8 +1070,20 @@ pf_find_state(struct pfi_kif *kif, struc
                        pkt_sk = NULL;
                }
 
-               if (pkt_sk && pf_state_key_isvalid(pkt_sk->reverse))
-                       sk = pkt_sk->reverse;
+               if (pkt_sk) {
+                       if (pf_state_key_isvalid(pkt_sk->reverse)) {
+                               sk = pkt_sk->reverse;
+                       } else if (pkt_sk->reverse != NULL) {
+                               log(LOG_ERR,
+                                   "pf: state key reverse invalid. "
+                                   "pkt_sk=%p, pkt_sk->reverse=%p, "
+                                   "pkt_sk->reverse->reverse=%p\n",
+                                   pkt_sk, pkt_sk->reverse,
+                                   pkt_sk->reverse->reverse);
+                               pf_mbuf_unlink_state_key(m);
+                               pkt_sk = NULL;
+                       }
+               }
 
                if (pkt_sk == NULL) {
                        /* here we deal with local outbound packet */

Reply via email to