KASAN: use-after-free Write in detach_if_pending

[syzbot]

Hello,

syzkaller hit the following crash on
e7989f973ae1b90ec7c0b671c81f7f553affccbe
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master
compiler: gcc (GCC) 7.1.1 20170620
.config is attached
Raw console output is attached.
C reproducer is attached
syzkaller reproducer is attached. See https://goo.gl/kgGztJ
for information about syzkaller reproducers


BUG: KASAN: use-after-free in __write_once_size
include/linux/compiler.h:305 [inline]
BUG: KASAN: use-after-free in __hlist_del include/linux/list.h:648 [inline]
BUG: KASAN: use-after-free in detach_timer kernel/time/timer.c:791 [inline]
BUG: KASAN: use-after-free in detach_if_pending+0x557/0x610
kernel/time/timer.c:808
Write of size 8 at addr ffff8801d3bab780 by task syzkaller900516/2986

CPU: 1 PID: 2986 Comm: syzkaller900516 Not tainted 4.13.0+ #82
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:16 [inline]
dump_stack+0x194/0x257 lib/dump_stack.c:52
print_address_description+0x73/0x250 mm/kasan/report.c:252
kasan_report_error mm/kasan/report.c:351 [inline]
kasan_report+0x24e/0x340 mm/kasan/report.c:409
__asan_report_store8_noabort+0x17/0x20 mm/kasan/report.c:435
__write_once_size include/linux/compiler.h:305 [inline]
__hlist_del include/linux/list.h:648 [inline]
detach_timer kernel/time/timer.c:791 [inline]
detach_if_pending+0x557/0x610 kernel/time/timer.c:808
try_to_del_timer_sync+0xa2/0x120 kernel/time/timer.c:1182
del_timer_sync+0x18a/0x240 kernel/time/timer.c:1247
tun_flow_uninit drivers/net/tun.c:1104 [inline]
tun_free_netdev+0x105/0x1b0 drivers/net/tun.c:1776
netdev_run_todo+0x870/0xca0 net/core/dev.c:7864
rtnl_unlock+0xe/0x10 net/core/rtnetlink.c:106
tun_detach drivers/net/tun.c:588 [inline]
tun_chr_close+0x49/0x60 drivers/net/tun.c:2609
__fput+0x333/0x7f0 fs/file_table.c:210
____fput+0x15/0x20 fs/file_table.c:246
task_work_run+0x199/0x270 kernel/task_work.c:112
exit_task_work include/linux/task_work.h:21 [inline]
do_exit+0xa52/0x1b40 kernel/exit.c:865
do_group_exit+0x149/0x400 kernel/exit.c:968
SYSC_exit_group kernel/exit.c:979 [inline]
SyS_exit_group+0x1d/0x20 kernel/exit.c:977
entry_SYSCALL_64_fastpath+0x1f/0xbe
RIP: 0033:0x443a28
RSP: 002b:00007fff42800438 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 0000000000443a28
RDX: 0000000000000000 RSI: 000000000000003c RDI: 0000000000000000
RBP: 0000000000000082 R08: 00000000000000e7 R09: ffffffffffffffd4
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
R13: 00000000006d6180 R14: 0000000000000000 R15: 0000000000000000

Allocated by task 2986:
save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459 [inline]
kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:551
__do_kmalloc_node mm/slab.c:3689 [inline]
__kmalloc_node+0x47/0x70 mm/slab.c:3696
kmalloc_node include/linux/slab.h:535 [inline]
kvmalloc_node+0x64/0xd0 mm/util.c:397
kvmalloc include/linux/mm.h:529 [inline]
kvzalloc include/linux/mm.h:537 [inline]
alloc_netdev_mqs+0x16e/0xed0 net/core/dev.c:8018
tun_set_iff drivers/net/tun.c:2022 [inline]
__tun_chr_ioctl+0x12be/0x3d20 drivers/net/tun.c:2276
tun_chr_ioctl+0x2a/0x40 drivers/net/tun.c:2521
vfs_ioctl fs/ioctl.c:45 [inline]
do_vfs_ioctl+0x1b1/0x1530 fs/ioctl.c:685
SYSC_ioctl fs/ioctl.c:700 [inline]
SyS_ioctl+0x8f/0xc0 fs/ioctl.c:691
entry_SYSCALL_64_fastpath+0x1f/0xbe

Freed by task 2986:
save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459 [inline]
kasan_slab_free+0x71/0xc0 mm/kasan/kasan.c:524
__cache_free mm/slab.c:3503 [inline]
kfree+0xca/0x250 mm/slab.c:3820
kvfree+0x36/0x60 mm/util.c:416
netdev_freemem net/core/dev.c:7970 [inline]
free_netdev+0x2cf/0x360 net/core/dev.c:8132
tun_set_iff drivers/net/tun.c:2105 [inline]
__tun_chr_ioctl+0x2cf6/0x3d20 drivers/net/tun.c:2276
tun_chr_ioctl+0x2a/0x40 drivers/net/tun.c:2521
vfs_ioctl fs/ioctl.c:45 [inline]
do_vfs_ioctl+0x1b1/0x1530 fs/ioctl.c:685
SYSC_ioctl fs/ioctl.c:700 [inline]
SyS_ioctl+0x8f/0xc0 fs/ioctl.c:691
entry_SYSCALL_64_fastpath+0x1f/0xbe

The buggy address belongs to the object at ffff8801d3ba8380
which belongs to the cache kmalloc-16384 of size 16384
The buggy address is located 13312 bytes inside of
16384-byte region [ffff8801d3ba8380, ffff8801d3bac380)
The buggy address belongs to the page:
page:ffffea00074eea00 count:1 mapcount:0 mapping:ffff8801d3ba8380 index:0x0
compound_mapcount: 0
flags: 0x200000000008100(slab|head)
raw: 0200000000008100 ffff8801d3ba8380 0000000000000000 0000000100000001
raw: ffffea0007355020 ffff8801dac01c50 ffff8801dac02200 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
ffff8801d3bab680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8801d3bab700: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff8801d3bab780: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff8801d3bab800: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8801d3bab880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================


---
This bug is generated by a dumb bot. It may contain errors.
See https://goo.gl/tpsmEJ for details.
Direct all questions to syzk...@googlegroups.com.

syzbot will keep track of this bug report.
Once a fix for this bug is committed, please reply to this email with:
#syz fix: exact-commit-title
To mark this as a duplicate of another syzbot report, please reply with:
#syz dup: exact-subject-of-another-report
If it's a one-off invalid bug report, please reply with:
#syz invalid
Note: if the crash happens again, it will cause creation of a new bug
report.

[Thomas Gleixner]

On Fri, 27 Oct 2017, syzbot wrote:

Cc'ed network folks.

> syzkaller hit the following crash on e7989f973ae1b90ec7c0b671c81f7f553affccbe
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master
> compiler: gcc (GCC) 7.1.1 20170620
> .config is attached
> Raw console output is attached.
> C reproducer is attached
> syzkaller reproducer is attached. See https://goo.gl/kgGztJ
> for information about syzkaller reproducers
>
>
> BUG: KASAN: use-after-free in __write_once_size include/linux/compiler.h:305
> [inline]
> BUG: KASAN: use-after-free in __hlist_del include/linux/list.h:648 [inline]
> BUG: KASAN: use-after-free in detach_timer kernel/time/timer.c:791 [inline]
> BUG: KASAN: use-after-free in detach_if_pending+0x557/0x610
> kernel/time/timer.c:808
> Write of size 8 at addr ffff8801d3bab780 by task syzkaller900516/2986

That's just the point where this gets detected.

> CPU: 1 PID: 2986 Comm: syzkaller900516 Not tainted 4.13.0+ #82

> __hlist_del include/linux/list.h:648 [inline]
> detach_timer kernel/time/timer.c:791 [inline]
> detach_if_pending+0x557/0x610 kernel/time/timer.c:808
> try_to_del_timer_sync+0xa2/0x120 kernel/time/timer.c:1182
> del_timer_sync+0x18a/0x240 kernel/time/timer.c:1247
> tun_flow_uninit drivers/net/tun.c:1104 [inline]
> tun_free_netdev+0x105/0x1b0 drivers/net/tun.c:1776

^^^^^^^^^^^^ This shouldn't be called I think

> netdev_run_todo+0x870/0xca0 net/core/dev.c:7864
> rtnl_unlock+0xe/0x10 net/core/rtnetlink.c:106
> tun_detach drivers/net/tun.c:588 [inline]
> tun_chr_close+0x49/0x60 drivers/net/tun.c:2609
> __fput+0x333/0x7f0 fs/file_table.c:210
> ____fput+0x15/0x20 fs/file_table.c:246
> task_work_run+0x199/0x270 kernel/task_work.c:112
> exit_task_work include/linux/task_work.h:21 [inline]
> do_exit+0xa52/0x1b40 kernel/exit.c:865

Here is the allocation path

> alloc_netdev_mqs+0x16e/0xed0 net/core/dev.c:8018
> tun_set_iff drivers/net/tun.c:2022 [inline]
> __tun_chr_ioctl+0x12be/0x3d20 drivers/net/tun.c:2276
> tun_chr_ioctl+0x2a/0x40 drivers/net/tun.c:2521
> vfs_ioctl fs/ioctl.c:45 [inline]
> do_vfs_ioctl+0x1b1/0x1530 fs/ioctl.c:685
> SYSC_ioctl fs/ioctl.c:700 [inline]
> SyS_ioctl+0x8f/0xc0 fs/ioctl.c:691
> entry_SYSCALL_64_fastpath+0x1f/0xbe

And this is free.

> netdev_freemem net/core/dev.c:7970 [inline]
> free_netdev+0x2cf/0x360 net/core/dev.c:8132
> tun_set_iff drivers/net/tun.c:2105 [inline]

err_free_flow:
tun_flow_uninit(tun); <--------

> __tun_chr_ioctl+0x2cf6/0x3d20 drivers/net/tun.c:2276
> tun_chr_ioctl+0x2a/0x40 drivers/net/tun.c:2521
> vfs_ioctl fs/ioctl.c:45 [inline]
> do_vfs_ioctl+0x1b1/0x1530 fs/ioctl.c:685
> SYSC_ioctl fs/ioctl.c:700 [inline]
> SyS_ioctl+0x8f/0xc0 fs/ioctl.c:691
> entry_SYSCALL_64_fastpath+0x1f/0xbe

So it's the TUNSETIFF ioctl which first allocates and then frees in the
errorpath of tun_set_iff.

But for some reason this sticks and the exit of that task does it again,
which triggers KASAN in the innocent timer code.

Thanks,

tglx

[Eric Dumazet]

Pretty old story, already fixed in David Miller trees.

net-next tree :

$ git log --oneline e7989f973ae1b90ec7c0b671c81.. -- drivers/net/tun.c
f8ddadc4db6c7b7029b6d0e0d9af24f74ad27ca2 Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net
ee74d9967b829232723939cb7c9b100b29f6ec98 tun: do not arm flow_gc_timer in tun_flow_init()
81d98fa4df3d1683b3ef21e8a7a0ccac7874f0de tun: avoid extra timer schedule in tun_flow_cleanup()
7dbfb4ef77db5666f0f3a425e7db93ca30ff4285 tun: do not block BH again in tun_flow_cleanup()
aec72f3392b1d598a979e89c4fdb131965ae0ab3 net-tun: fix panics at dismantle time
010f245b9dd734adda6386c494a4ace953ea8dc4 tun: relax check on eth_get_headlen() return value
0ad646c81b2182f7fa67ec0c8c825e0ee165696d tun: call dev_get_valid_name() before register_netdevice()
53954cf8c5d205624167a2bfd117cc0c1a5f3c6d Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net
2580c4c17aee3ad58e9751012bad278dd074ccae tun: bail out from tun_get_user() if the skb is empty
de8f3a83b0a0fddb2cf56e7a718127e9619ea3da bpf: add meta pointer for direct access
9484dc74fcf0750cd6726c9aa27edf97223916a8 tun: delete original tun_get() and rename __tun_get() to tun_get()
90e33d45940793def6f773b2d528e9f3c84ffdc7 tun: enable napi_gro_frags() for TUN/TAP driver
943170998b200190f99d3fe7e771437e2c51f319 tun: enable NAPI for TUN/TAP driver

net tree :

$ git log --oneline e7989f973ae1b90ec7c0b671c81.. -- drivers/net/tun.c
63b9ab65bd76e5de6479bb14b4014b64aa1a317a tuntap: properly align skb->head before building skb
5c25f65fd1e42685f7ccd80e0621829c105785d9 tun: allow positive return values on dev_get_valid_name() call
0ad646c81b2182f7fa67ec0c8c825e0ee165696d tun: call dev_get_valid_name() before register_netdevice()
2580c4c17aee3ad58e9751012bad278dd074ccae tun: bail out from tun_get_user() if the skb is empty

Pick the fixes, they are at least 2 patches that addressed the issue.

[Dmitry Vyukov]

Let's try the last one in net-next that touches timers:

#syz fix: tun: do not arm flow_gc_timer in tun_flow_init()

[Eric Dumazet]

Note that is is common to have multiple patches sharing a same title, so
your tag is ambiguous.

Why don't you update your bot to catch up standard SHA1 title format,
so that we do not have to ?

[Dmitry Vyukov]

Yes, but hashes in random trees also don't tell much. A tree can be
rebased so the hash will be lost. It can be a tree unknown to the
system. Even if we find the commit by hash, in order to match it
against other trees we will have to use the title anyway (or are there
better options?), so using hashes becomes pointless.

> Why don't you update your bot to catch up standard SHA1 title format,
> so that we do not have to ?

Because as our internal practice shows, developers reference commits
for other reasons (a commit that introduced the bug, a commit that
should have been prevented the bug but as the report shows it does
not, a commit that may fix the bug but one does not sure, etc). It
looks reasonable for effectively a bug tracking system to explicitly
say what fixes what.

[Eric Dumazet]

On Mon, 2017-10-30 at 18:06 +0100, Dmitry Vyukov wrote:

> Yes, but hashes in random trees also don't tell much. A tree can be
> rebased so the hash will be lost. It can be a tree unknown to the
> system. Even if we find the commit by hash, in order to match it
> against other trees we will have to use the title anyway (or are there
> better options?), so using hashes becomes pointless.

We do not send hashes on random trees, but official SHA1 in David Miller
trees. They will be the same SHA1 in official Linus Torvalds tree.

Really, you make our life more difficult by pretending that hashes are
not the proper way.

They are reasons we use Fixes: tags all over the places, they are unique
in Linus tree.

Since syzbot gives a SHA1 itself, it must be using a tree, right ?

So a SHA1 that is guaranteed to enter the same tree is correct.

Please fix your bot.

[syzbot]

> They don't necessary enter the same tree (that's more of an exception
> than the rule). For bugs that we find in Linus tree, fixes enter usb,
> kvm, block, sound, linux-next and a bunch of other trees that I never
> heard of. At the very least we will need a git repo address + commit
> hash. But then for say linux-next hashes disappear. And mm which is
> not a git tree at all (no hashes).
> And still the hashes will need to be explicitly marked as fixes (with
> #syz fix or something else). So that would look like:

unknown command "fix"

> ##syz fix: git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git
> e7989f973ae1b90ec7c0b671c81f7f553affccbe
> which does not look much better than:
> ##syz fix: tun: do not arm flow_gc_timer in tun_flow_init()
> which also I think makes it easier for humans to ensure that they
> actually reference what they meant to reference (and maybe find the
> fix in other trees).

[Dmitry Vyukov]

They don't necessary enter the same tree (that's more of an exception
than the rule). For bugs that we find in Linus tree, fixes enter usb,
kvm, block, sound, linux-next and a bunch of other trees that I never
heard of. At the very least we will need a git repo address + commit
hash. But then for say linux-next hashes disappear. And mm which is
not a git tree at all (no hashes).
And still the hashes will need to be explicitly marked as fixes (with
#syz fix or something else). So that would look like:

[Eric Dumazet]

I suggested that syzbot catches up on standard way :

<SHA1> patch title

It contains way more information than :

sys fix: patch title

I never suggested to only use <SHA1>

[Dmitry Vyukov]

But people reference patches for other reasons (I've sent you examples
offline). If you mean

syz fix: <SHA1> patch title

then that's doable. If we agree on this format, then I am ready to
implement this.

[Eric Dumazet]

On Mon, 2017-10-30 at 20:52 +0300, Dmitry Vyukov wrote:

> syz fix: <SHA1> patch title
>
> then that's doable. If we agree on this format, then I am ready to
> implement this.

Yes please.

David Miller and Linus never rebase their trees, for good reasons.