linux-kernel - Re: KASAN: use-after-free Read in tcf_block

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACT4Y+YbQbZ7XTPK1HLvPiR9rmSm7Cz0H2LJAFfYfAta1Y3CiQ@mail.gmail.com>
Date:   Thu, 27 Sep 2018 10:10:45 +0200
From:   Dmitry Vyukov <dvyukov@...gle.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     Eric Dumazet <eric.dumazet@...il.com>,
        syzbot+37b8770e6d5a8220a039@...kaller.appspotmail.com,
        David Miller <davem@...emloft.net>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Jiri Pirko <jiri@...nulli.us>,
        LKML <linux-kernel@...r.kernel.org>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        syzkaller-bugs <syzkaller-bugs@...glegroups.com>
Subject: Re: KASAN: use-after-free Read in tcf_block_find

On Thu, Sep 27, 2018 at 10:06 AM, Dmitry Vyukov <dvyukov@...gle.com> wrote:
> On Wed, Sep 26, 2018 at 11:55 PM, Cong Wang <xiyou.wangcong@...il.com> wrote:
>>> >> Hello,
>>> >>
>>> >> syzbot found the following crash on:
>>> >>
>>> >> HEAD commit:    4b1bd6976945 net: phy: marvell: Fix build.
>>> >> git tree:       net-next
>>> >> console output: https://syzkaller.appspot.com/x/log.txt?x=16f763fa400000
>>> >> kernel config:  https://syzkaller.appspot.com/x/.config?x=443816db871edd66
>>> >> dashboard link: https://syzkaller.appspot.com/bug?extid=37b8770e6d5a8220a039
>>> >> compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
>>> >> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=17a5614e400000
>>> >> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=141a532a400000
>>> >>
>>> >> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>>> >> Reported-by: syzbot+37b8770e6d5a8220a039@...kaller.appspotmail.com
>>> >>
>>> >> IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
>>> >> 8021q: adding VLAN 0 to HW filter on device team0
>>> >> ==================================================================
>>> >> BUG: KASAN: use-after-free in tcf_block_find+0x9d1/0xb90
>>> >> net/sched/cls_api.c:646
>>> >> Read of size 4 at addr ffff8801cc126978 by task syz-executor002/5646
>>> >>
>>> >> CPU: 1 PID: 5646 Comm: syz-executor002 Not tainted 4.19.0-rc5+ #232
>>> >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
>>> >> Google 01/01/2011
>>> >> Call Trace:
>>> >>   __dump_stack lib/dump_stack.c:77 [inline]
>>> >>   dump_stack+0x1c4/0x2b4 lib/dump_stack.c:113
>>> >>   print_address_description.cold.8+0x9/0x1ff mm/kasan/report.c:256
>>> >>   kasan_report_error mm/kasan/report.c:354 [inline]
>>> >>   kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412
>>> >>   __asan_report_load4_noabort+0x14/0x20 mm/kasan/report.c:432
>>> >>   tcf_block_find+0x9d1/0xb90 net/sched/cls_api.c:646
>>> >
>>> > Hmm. looks like missing a rcu_dereference() here:
>>> >
>>> >                 if (!*parent) {
>>> >                         *q = dev->qdisc;
>>> >                         *parent = (*q)->handle;
>>> >
>>>
>>> We hold RTNL here.
>>>
>>> Have we already done the changes allowing dev->qdisc being changed
>>> without RTNL being required ?
>>
>> That transition is not completed yet, but holding RTNL doesn't help
>> any more after we now free qdisc in rcu callback.
>>
>> More interestingly, rcu read lock is already held in this context,
>> so it seems we still have to use rcu_deference() to read dev->qdisc
>> and of course mark it with __rcu too.
>
> On hardware level rcu_deference() only affects Alpha. On software
> level, it can affect all archs due to e.g. a re-read of the variable.
> Do we see the potential for compilation that would lead to the UAF
> here? A missed rcu_deference() would not be the first thing that I
> would suspect for UAF on x86. It's more likely some bolder bug.


Would a stack trace for call_rcu be helpful here? I have this idea for
a long time, but never get around to implementing it:
https://bugzilla.kernel.org/show_bug.cgi?id=198437

Also FWIW I recently used the following hack for another net bug. It
made that other bug involving call_rcu way more likely to fire. Maybe
it will be helpful here too.

diff --git a/net/core/dst.c b/net/core/dst.c
index 81ccf20e28265..591a8d0aca545 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -187,8 +187,16 @@ void dst_release(struct dst_entry *dst)
                if (unlikely(newrefcnt < 0))
                        net_warn_ratelimited("%s: dst:%p refcnt:%d\n",
                                             __func__, dst, newrefcnt);
-               if (!newrefcnt)
-                       call_rcu(&dst->rcu_head, dst_destroy_rcu);
+               if (!newrefcnt) {
+                       if (lock_is_held(&rcu_bh_lock_map) ||
+                               lock_is_held(&rcu_lock_map) ||
+                               lock_is_held(&rcu_sched_lock_map)) {
+                               call_rcu(&dst->rcu_head, dst_destroy_rcu);
+                       } else {
+                               synchronize_rcu();
+                               dst_destroy_rcu(&dst->rcu_head);
+                       }
+               }
        }
 }