KASAN: use-after-free Read in tcf_block_find

[syzbot]

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+37b877...@syzkaller.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
tc_new_tfilter+0x497/0x1d20 net/sched/cls_api.c:1331
rtnetlink_rcv_msg+0x46a/0xc20 net/core/rtnetlink.c:4730
netlink_rcv_skb+0x172/0x440 net/netlink/af_netlink.c:2447
rtnetlink_rcv+0x1c/0x20 net/core/rtnetlink.c:4748
netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
netlink_unicast+0x5a5/0x760 net/netlink/af_netlink.c:1336
netlink_sendmsg+0xa18/0xfc0 net/netlink/af_netlink.c:1901
sock_sendmsg_nosec net/socket.c:621 [inline]
sock_sendmsg+0xd5/0x120 net/socket.c:631
___sys_sendmsg+0x7fd/0x930 net/socket.c:2116
__sys_sendmsg+0x11d/0x280 net/socket.c:2154
__do_sys_sendmsg net/socket.c:2163 [inline]
__se_sys_sendmsg net/socket.c:2161 [inline]
__x64_sys_sendmsg+0x78/0xb0 net/socket.c:2161
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x441979
Code: e8 0c ac 02 00 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 fb 04 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffd8b1e9178 EFLAGS: 00000213 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 0000000000441979
RDX: 0000000000000000 RSI: 0000000020000100 RDI: 0000000000000003
RBP: 0000000000009641 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000001bb6880 R11: 0000000000000213 R12: 0000000000000000
R13: 0000000000402390 R14: 0000000000000000 R15: 0000000000000000

Allocated by task 5522:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
kasan_kmalloc+0xc7/0xe0 mm/kasan/kasan.c:553
__do_kmalloc_node mm/slab.c:3682 [inline]
__kmalloc_node+0x47/0x70 mm/slab.c:3689
kmalloc_node include/linux/slab.h:555 [inline]
kzalloc_node include/linux/slab.h:718 [inline]
qdisc_alloc+0x10f/0xb50 net/sched/sch_generic.c:820
qdisc_create_dflt+0x7a/0x1e0 net/sched/sch_generic.c:894
attach_one_default_qdisc net/sched/sch_generic.c:1041 [inline]
netdev_for_each_tx_queue include/linux/netdevice.h:2114 [inline]
attach_default_qdiscs net/sched/sch_generic.c:1060 [inline]
dev_activate+0x82f/0xcb0 net/sched/sch_generic.c:1103
__dev_open+0x2cb/0x410 net/core/dev.c:1400
__dev_change_flags+0x730/0x9b0 net/core/dev.c:7448
dev_change_flags+0x89/0x150 net/core/dev.c:7517
do_setlink+0xb5f/0x3f20 net/core/rtnetlink.c:2441
rtnl_newlink+0x136f/0x1d40 net/core/rtnetlink.c:3054
rtnetlink_rcv_msg+0x46a/0xc20 net/core/rtnetlink.c:4730
netlink_rcv_skb+0x172/0x440 net/netlink/af_netlink.c:2447
rtnetlink_rcv+0x1c/0x20 net/core/rtnetlink.c:4748
netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
netlink_unicast+0x5a5/0x760 net/netlink/af_netlink.c:1336
netlink_sendmsg+0xa18/0xfc0 net/netlink/af_netlink.c:1901
sock_sendmsg_nosec net/socket.c:621 [inline]
sock_sendmsg+0xd5/0x120 net/socket.c:631
___sys_sendmsg+0x7fd/0x930 net/socket.c:2116
__sys_sendmsg+0x11d/0x280 net/socket.c:2154
__do_sys_sendmsg net/socket.c:2163 [inline]
__se_sys_sendmsg net/socket.c:2161 [inline]
__x64_sys_sendmsg+0x78/0xb0 net/socket.c:2161
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe

Freed by task 0:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
__kasan_slab_free+0x102/0x150 mm/kasan/kasan.c:521
kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
__cache_free mm/slab.c:3498 [inline]
kfree+0xcf/0x230 mm/slab.c:3813
qdisc_free+0x89/0x100 net/sched/sch_generic.c:941
qdisc_free_cb+0x19/0x20 net/sched/sch_generic.c:948
__rcu_reclaim kernel/rcu/rcu.h:236 [inline]
rcu_do_batch kernel/rcu/tree.c:2576 [inline]
invoke_rcu_callbacks kernel/rcu/tree.c:2880 [inline]
__rcu_process_callbacks kernel/rcu/tree.c:2847 [inline]
rcu_process_callbacks+0xf23/0x2670 kernel/rcu/tree.c:2864
__do_softirq+0x30b/0xad8 kernel/softirq.c:292

The buggy address belongs to the object at ffff8801cc126940
which belongs to the cache kmalloc-1024 of size 1024
The buggy address is located 56 bytes inside of
1024-byte region [ffff8801cc126940, ffff8801cc126d40)
The buggy address belongs to the page:
page:ffffea0007304980 count:1 mapcount:0 mapping:ffff8801da800ac0 index:0x0
compound_mapcount: 0
flags: 0x2fffc0000008100(slab|head)
raw: 02fffc0000008100 ffffea0007304808 ffffea0007302788 ffff8801da800ac0
raw: 0000000000000000 ffff8801cc126040 0000000100000007 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
ffff8801cc126800: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff8801cc126880: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff8801cc126900: fc fc fc fc fc fc fc fc fb fb fb fb fb fb fb fb
^
ffff8801cc126980: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8801cc126a00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzk...@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

[Cong Wang]

Hmm. looks like missing a rcu_dereference() here:

if (!*parent) {
*q = dev->qdisc;
*parent = (*q)->handle;

[Eric Dumazet]

We hold RTNL here.

Have we already done the changes allowing dev->qdisc being changed
without RTNL being required ?

[Cong Wang]

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.

[Dmitry Vyukov]

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.

[Dmitry Vyukov]

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);
+ }
+ }
}
}

[Eric Dumazet]

On 09/27/2018 01:10 AM, Dmitry Vyukov wrote:

>
> 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_release() can be called in context we hold a spinlock, this would be bad to reschedule here.

[Dmitry Vyukov]

I am not suggesting to commit this. This is just a hack for debugging.
It in fact lead to some warnings, but still allowed me to reproduce
the bug reliably.

[Eric Dumazet]

On 09/27/2018 06:02 AM, Dmitry Vyukov wrote:

> I am not suggesting to commit this. This is just a hack for debugging.
> It in fact lead to some warnings, but still allowed me to reproduce
> the bug reliably.
>

Had you got more meaningful stack traces ?

(Showing which context was actually doing the dst_release())

>>> + dst_destroy_rcu(&dst->rcu_head);
>>> + }
>>> + }
>>> }
>>> }

Thanks.

[Dmitry Vyukov]

On Thu, Sep 27, 2018 at 3:24 PM, Eric Dumazet <eric.d...@gmail.com> wrote:
> On 09/27/2018 06:02 AM, Dmitry Vyukov wrote:
>
>> I am not suggesting to commit this. This is just a hack for debugging.
>> It in fact lead to some warnings, but still allowed me to reproduce
>> the bug reliably.
>>
>
> Had you got more meaningful stack traces ?
>
> (Showing which context was actually doing the dst_release())

nope... I just handed off the instructions to Wei since she said she
can't reproduce it.

[Cong Wang]

On Thu, Sep 27, 2018 at 1:11 AM Dmitry Vyukov <dvy...@google.com> wrote:
>
> 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

Yes. Generally speaking, showing backtrace of call_rcu()
or schedule_work(0 etc. is very helpful, we are more interested
in who calls call_rcu() than what that RCU callback does.

BTW, yesterday I asked syzbot to test this:
https://github.com/congwang/linux/commit/b7815584cf1c0bbb79e8f6fe3e4b66ba10375560
I still don't get any result.

For this specific bug, we should hold a refcnt in dev->qdisc, I don't
even see how call_rcu() could be invoked, unless of course we mess
up with qdisc refcnt.

[Dmitry Vyukov]

On Thu, Sep 27, 2018 at 7:50 PM, Cong Wang <xiyou.w...@gmail.com> wrote:
> On Thu, Sep 27, 2018 at 1:11 AM Dmitry Vyukov <dvy...@google.com> wrote:
>>
>> 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
>
> Yes. Generally speaking, showing backtrace of call_rcu()
> or schedule_work(0 etc. is very helpful, we are more interested
> in who calls call_rcu() than what that RCU callback does.
>
> BTW, yesterday I asked syzbot to test this:
> https://github.com/congwang/linux/commit/b7815584cf1c0bbb79e8f6fe3e4b66ba10375560
> I still don't get any result.

I see that test job. It's in some dead loop, now trying to run for 37'th time.
I've just pushed a fix a bug that could have caused it (fuzzing the
fuzzer, we should go deeper!):
https://github.com/google/syzkaller/commit/98b28ead6ceaf22064b9715cc1950848d2bdef0b
If it won't help, I will take a look tomorrow.

[syzbot]

Hello,

syzbot has tested the proposed patch but the reproducer still triggered
crash:
KASAN: use-after-free Read in tcf_block_find

8021q: adding VLAN 0 to HW filter on device team0
8021q: adding VLAN 0 to HW filter on device team0
8021q: adding VLAN 0 to HW filter on device team0
8021q: adding VLAN 0 to HW filter on device team0
==================================================================

BUG: KASAN: use-after-free in qdisc_refcount_inc_nz
include/net/sch_generic.h:125 [inline]
BUG: KASAN: use-after-free in tcf_block_find+0xa17/0xbf0
net/sched/cls_api.c:655
Read of size 4 at addr ffff8801c15d6950 by task syz-executor0/6963

CPU: 0 PID: 6963 Comm: syz-executor0 Not tainted 4.19.0-rc5+ #1

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

qdisc_refcount_inc_nz include/net/sch_generic.h:125 [inline]
tcf_block_find+0xa17/0xbf0 net/sched/cls_api.c:655
tc_new_tfilter+0x497/0x1d20 net/sched/cls_api.c:1333

rtnetlink_rcv_msg+0x46a/0xc20 net/core/rtnetlink.c:4730
netlink_rcv_skb+0x172/0x440 net/netlink/af_netlink.c:2447
rtnetlink_rcv+0x1c/0x20 net/core/rtnetlink.c:4748
netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
netlink_unicast+0x5a5/0x760 net/netlink/af_netlink.c:1336
netlink_sendmsg+0xa18/0xfc0 net/netlink/af_netlink.c:1901
sock_sendmsg_nosec net/socket.c:621 [inline]
sock_sendmsg+0xd5/0x120 net/socket.c:631
___sys_sendmsg+0x7fd/0x930 net/socket.c:2116
__sys_sendmsg+0x11d/0x280 net/socket.c:2154
__do_sys_sendmsg net/socket.c:2163 [inline]
__se_sys_sendmsg net/socket.c:2161 [inline]
__x64_sys_sendmsg+0x78/0xb0 net/socket.c:2161
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe

RIP: 0033:0x457579
Code: 1d b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7

48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff

ff 0f 83 eb b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fe7b12d3c78 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000457579

RDX: 0000000000000000 RSI: 0000000020000100 RDI: 0000000000000003

RBP: 000000000072bf00 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007fe7b12d46d4
R13: 00000000004c3536 R14: 00000000004d5328 R15: 00000000ffffffff

Allocated by task 6316:

Freed by task 5461:

save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
__kasan_slab_free+0x102/0x150 mm/kasan/kasan.c:521
kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
__cache_free mm/slab.c:3498 [inline]
kfree+0xcf/0x230 mm/slab.c:3813
qdisc_free+0x89/0x100 net/sched/sch_generic.c:941
qdisc_free_cb+0x19/0x20 net/sched/sch_generic.c:948
__rcu_reclaim kernel/rcu/rcu.h:236 [inline]
rcu_do_batch kernel/rcu/tree.c:2576 [inline]
invoke_rcu_callbacks kernel/rcu/tree.c:2880 [inline]
__rcu_process_callbacks kernel/rcu/tree.c:2847 [inline]
rcu_process_callbacks+0xf23/0x2670 kernel/rcu/tree.c:2864
__do_softirq+0x30b/0xad8 kernel/softirq.c:292

The buggy address belongs to the object at ffff8801c15d6940

which belongs to the cache kmalloc-1024 of size 1024

The buggy address is located 16 bytes inside of
1024-byte region [ffff8801c15d6940, ffff8801c15d6d40)

The buggy address belongs to the page:

page:ffffea0007057580 count:1 mapcount:0 mapping:ffff8801da800ac0 index:0x0

compound_mapcount: 0
flags: 0x2fffc0000008100(slab|head)

raw: 02fffc0000008100 ffffea0007089908 ffffea0007041388 ffff8801da800ac0
raw: 0000000000000000 ffff8801c15d6040 0000000100000007 0000000000000000

page dumped because: kasan: bad access detected

Memory state around the buggy address:

ffff8801c15d6800: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8801c15d6880: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
> ffff8801c15d6900: fc fc fc fc fc fc fc fc fb fb fb fb fb fb fb fb
^
ffff8801c15d6980: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8801c15d6a00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================


Tested on:

commit: b7815584cf1c net_sched: read qdisc after getting a refcount
git tree: https://github.com/congwang/linux.git qdisc-test
console output: https://syzkaller.appspot.com/x/log.txt?x=1684bd1a400000
kernel config: https://syzkaller.appspot.com/x/.config?x=443816db871edd66