netdev - crash in xt_policy due to skb_dst_drop() in nf_ct_frag6

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANP3RGeX5=c=Lb+Pkg89zD7zQ_Z1T8oPJRCkorNCdghmofYXxg@mail.gmail.com>
Date:   Mon, 15 Oct 2018 21:13:25 -0700
From:   Maciej Żenczykowski <zenczykowski@...il.com>
To:     Lorenzo Colitti <lorenzo@...gle.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Florian Westphal <fw@...len.de>,
        Linux NetDev <netdev@...r.kernel.org>,
        Maciej Zenczykowski <maze@...gle.com>,
        Maciej Żenczykowski <zenczykowski@...il.com>
Subject: crash in xt_policy due to skb_dst_drop() in nf_ct_frag6_gather()

I believe that:

commit ad8b1ffc3efae2f65080bdb11145c87d299b8f9a
Author: Florian Westphal <fw@...len.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 == fq->q.len &&
            nf_ct_frag6_reasm(fq, skb, dev))
                ret = 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 basically:

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

Crash is:

match_policy_out()
const struct dst_entry *dst = skb_dst(skb); // returns NULL
if (dst->xfrm == NULL) <-- dst == 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/0x28
[ 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 Żenczykowski, Kernel Networking Developer @ Google