From mboxrd@z Thu Jan  1 00:00:00 1970
From: =?UTF-8?Q?Maciej_=C5=BBenczykowski?= <zenczykowski@gmail.com>
Subject: crash in xt_policy due to skb_dst_drop() in nf_ct_frag6_gather()
Date: Mon, 15 Oct 2018 21:13:25 -0700
Message-ID: <CANP3RGeX5=c=Lb+Pkg89zD7zQ_Z1T8oPJRCkorNCdghmofYXxg@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
To: Lorenzo Colitti <lorenzo@google.com>,
        Eric Dumazet <edumazet@google.com>,
        Florian Westphal <fw@strlen.de>,
        Linux NetDev <netdev@vger.kernel.org>,
        Maciej Zenczykowski <maze@google.com>,
        =?UTF-8?Q?Maciej_=C5=BBenczykowski?= <zenczykowski@gmail.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-ua1-f52.google.com ([209.85.222.52]:41280 "EHLO
        mail-ua1-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1728568AbeJPMBt (ORCPT
        <rfc822;netdev@vger.kernel.org>); Tue, 16 Oct 2018 08:01:49 -0400
Received: by mail-ua1-f52.google.com with SMTP id o17so93347uad.8
        for <netdev@vger.kernel.org>; Mon, 15 Oct 2018 21:13:26 -0700 (PDT)
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

I believe that:

commit ad8b1ffc3efae2f65080bdb11145c87d299b8f9a
Author: Florian Westphal <fw@strlen.de>
    netfilter: ipv6: nf_defrag: drop skb dst before queueing

+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -618,6 +618,8 @@ int nf_ct_frag6_gather(struct net *net, struct
sk_buff *skb, u32 user)
            fq->q.meat =3D=3D fq->q.len &&
            nf_ct_frag6_reasm(fq, skb, dev))
                ret =3D 0;
+       else
+               skb_dst_drop(skb);

 out_unlock:
        spin_unlock_bh(&fq->q.lock);

Is causing a crash on android after upgrading from 4.9.96 to 4.9.119

This is because clatd ipv4 to ipv6 translation user space daemon is
functionally equivalent to the syzkaller reproducer.
It will convert ipv4 frags it receives via tap into ipv6 frags which
it will write out via rawv6 sendmsg.

However we are also using xt_policy, after stripping cruft this is basicall=
y:

ip6tables -A OUTPUT -m policy --dir out --pol ipsec

Crash is:

match_policy_out()
const struct dst_entry *dst =3D skb_dst(skb); // returns NULL
if (dst->xfrm =3D=3D NULL) <-- dst =3D=3D NULL -> panic
[ 1136.606948] c1 2675   [<ffffff9ec38b4098>] policy_mt+0x34/0x18c
[ 1136.606954] c1 2675   [<ffffff9ec39e6af8>] ip6t_do_table+0x280/0x684
[ 1136.606961] c1 2675   [<ffffff9ec39e7250>] ip6table_filter_hook+0x20/0x2=
8
[ 1136.606969] c1 2675   [<ffffff9ec386ecc8>] nf_hook_slow+0x98/0x154
[ 1136.606977] c1 2675   [<ffffff9ec39b9b10>] rawv6_sendmsg+0xd14/0x1520
[ 1136.606985] c1 2675   [<ffffff9ec39191fc>] inet_sendmsg+0x100/0x1b0
[ 1136.606993] c1 2675   [<ffffff9ec37d3720>] ___sys_sendmsg+0x2a0/0x414
[ 1136.606999] c1 2675   [<ffffff9ec37d3d48>] SyS_sendmsg+0x94/0xe4

Just checking for NULL in xt_policy.c:match_policy_out() and returning
0 or 1 unconditionally seems to be the wrong thing to do,
since after all prior to skb_dst_drop() the skb->dst->xfrm might not
have been NULL.

Maciej =C5=BBenczykowski, Kernel Networking Developer @ Google