2018-09-26 14:26:18

by syzbot

[permalink] [raw]
Subject: KMSAN: uninit-value in __dev_mc_add

Hello,

syzbot found the following crash on:

HEAD commit: eb2e67596de2 kmsan: minor code cleanup
git tree: https://github.com/google/kmsan.git/master
console output: https://syzkaller.appspot.com/x/log.txt?x=136e3b11400000
kernel config: https://syzkaller.appspot.com/x/.config?x=94a9ed72288f7fef
dashboard link: https://syzkaller.appspot.com/bug?extid=001516d86dbe88862cec
compiler: clang version 8.0.0 (trunk 339414)

Unfortunately, I don't have any reproducer for this crash yet.

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: [email protected]

==================================================================
BUG: KMSAN: uninit-value in memcmp+0x11d/0x180 lib/string.c:863
CPU: 0 PID: 14033 Comm: kworker/0:3 Not tainted 4.19.0-rc4+ #58
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Workqueue: ipv6_addrconf addrconf_dad_work
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x2f6/0x430 lib/dump_stack.c:113
kmsan_report+0x183/0x2b0 mm/kmsan/kmsan.c:917
__msan_warning+0x70/0xc0 mm/kmsan/kmsan_instr.c:500
memcmp+0x11d/0x180 lib/string.c:863
__hw_addr_add_ex net/core/dev_addr_lists.c:61 [inline]
__dev_mc_add+0x215/0x8c0 net/core/dev_addr_lists.c:670
dev_mc_add+0x6d/0x80 net/core/dev_addr_lists.c:687
igmp6_group_added+0x2c8/0xaa0 net/ipv6/mcast.c:676
__ipv6_dev_mc_inc+0xf4b/0x1270 net/ipv6/mcast.c:934
ipv6_dev_mc_inc+0x70/0x80 net/ipv6/mcast.c:941
addrconf_join_solict net/ipv6/addrconf.c:2098 [inline]
addrconf_dad_begin net/ipv6/addrconf.c:3879 [inline]
addrconf_dad_work+0x437/0x29b0 net/ipv6/addrconf.c:4006
process_one_work+0x197f/0x2490 kernel/workqueue.c:2153
worker_thread+0x1f6a/0x2b00 kernel/workqueue.c:2296
kthread+0x59c/0x5d0 kernel/kthread.c:247
ret_from_fork+0x35/0x40 arch/x86/entry/entry_64.S:416

Local variable description: ----buf@igmp6_group_added
Variable was created at:
igmp6_group_added+0x46/0xaa0 net/ipv6/mcast.c:664
__ipv6_dev_mc_inc+0xf4b/0x1270 net/ipv6/mcast.c:934
==================================================================


---
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 [email protected].

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.


2018-09-27 21:30:56

by Vladis Dronov

[permalink] [raw]
Subject: Re: KMSAN: uninit-value in __dev_mc_add

Hello,

This report is actually for the same bug which was reported in:

https://syzkaller.appspot.com/bug?id=088efeac32fdde781038a777a63e436c0d4d7036

The note there that the bug was fixed by "Commits: net: fix uninit-value in
__hw_addr_add_ex()" is wrong. A C-reproducer from the 2nd syzkaller report
can trigger the bug from this one.

I've researched this and a result is a proposed patch, the problem is the tun
device code allowing to set an arbitrary link type.

https://lkml.org/lkml/2018/9/26/416
https://lore.kernel.org/lkml/[email protected]/T/#u
https://marc.info/?l=linux-netdev&m=153795423320016&w=2

A simplified reproducer is attached.

Best regards,
Vladis Dronov


Attachments:
kmsan-hw_addr_add_ex.c (2.22 kB)

2018-09-27 22:23:36

by Eric Dumazet

[permalink] [raw]
Subject: Re: KMSAN: uninit-value in __dev_mc_add

On Thu, Sep 27, 2018 at 2:30 PM Vladis Dronov <[email protected]> wrote:
>
> Hello,
>
> This report is actually for the same bug which was reported in:
>
> https://syzkaller.appspot.com/bug?id=088efeac32fdde781038a777a63e436c0d4d7036
>
> The note there that the bug was fixed by "Commits: net: fix uninit-value in
> __hw_addr_add_ex()" is wrong. A C-reproducer from the 2nd syzkaller report
> can trigger the bug from this one.
>
> I've researched this and a result is a proposed patch, the problem is the tun
> device code allowing to set an arbitrary link type.
>
> https://lkml.org/lkml/2018/9/26/416
> https://lore.kernel.org/lkml/[email protected]/T/#u
> https://marc.info/?l=linux-netdev&m=153795423320016&w=2
>

I dunno, your patch looks quite not the right fix.

If TUN is able to change dev->type, how comes it does not set the
appropriate dev->addr_len at the same time ?

Really the bug seems to be deeper, and without setting proper
dev->addr_len, we'll need more 'fixes' like yours.

Thanks.

2018-09-27 23:10:52

by Vladis Dronov

[permalink] [raw]
Subject: Re: KMSAN: uninit-value in __dev_mc_add

Hello, Eric, all,

> I dunno, your patch looks quite not the right fix.

I agree, it looks more like a dirty hack. Unfortunately, I lack the deep
expertise in the network stack subsystem, so I've posted the patch to,
sort of, start a discussion and probably get some hints.

> If TUN is able to change dev->type, how comes it does not set the
> appropriate dev->addr_len at the same time ?

Well,... probably, nobody cared to do so:

[drivers/net/tun.c]
case TUNSETLINK:
...
tun->dev->type = (int) arg; //<--- that's all!
tun_debug(KERN_INFO, tun, "linktype set to %d\n",
tun->dev->type);
ret = 0;
}
break;

> Really the bug seems to be deeper, and without setting proper
> dev->addr_len, we'll need more 'fixes' like yours.

Absolutely. Unfortunately, I wasn't able to just write such deeper patch.
Let me share what I have found and let me hope to get an advise.

- So setting just the tun->dev->type makes the dev struct inconsistent.

- There are more field to adjust, at least dev->broadcast. Also, there are
a number of *_ops fields which are all set for the Ethernet type, most
probably they must be adjusted also.

- There is no get_addr_len_by_link_type() or a simple way to get link layer
properties by dev->type. Such settings are scattered in *_setup and
*_init functions, like ipgre_tunnel_init() { ... dev->addr_len = 4; ...}

Having these, I can imagine 2 ways for a proper fix.

1) Destroy the net_device in question and recreate it when changing a link
type. This way all the dev fields are set right. Create it in a similar way
as rtnl_newlink() does. Again, we do not have get_X_by_link_type(), so it
probably will be some large switch()/case:

$ grep '^#define ARPHRD_' include/uapi/linux/if_arp.h | wc -l
59

2) Leave tun an Ethernet device, add some tun->pretend_to_be_this_link_type
field and change only it on TUNSETLINK. And use this field in cases for which
TUNSETLINK was invented in the first place. Unfortunately, I do not have such
a list. The initial the commit ff4cc3ac93e1 says:

For use with
wireless and other networking types it should be possible to set the
ARP type via an ioctl.

Surely, there can be something else which I do not see. Could anyone suggest
an advice on this?

Best regards,
Vladis Dronov | Red Hat, Inc. | Product Security Engineer

2018-10-08 13:06:58

by syzbot

[permalink] [raw]
Subject: Re: KMSAN: uninit-value in __dev_mc_add

syzbot has found a reproducer for the following crash on:

HEAD commit: 43c85fe5a0ee kmsan: suppress false positives in KVM
git tree: https://github.com/google/kmsan.git/master
console output: https://syzkaller.appspot.com/x/log.txt?x=15ffd5b9400000
kernel config: https://syzkaller.appspot.com/x/.config?x=3ff9630e1f32e076
dashboard link: https://syzkaller.appspot.com/bug?extid=001516d86dbe88862cec
compiler: clang version 8.0.0 (trunk 339414)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10adf491400000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=100c8159400000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: [email protected]

random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
IPVS: ftp: loaded support on port[0] = 21
==================================================================
BUG: KMSAN: uninit-value in memcmp+0x117/0x180 lib/string.c:863
CPU: 1 PID: 18 Comm: kworker/1:0 Not tainted 4.19.0-rc4+ #64
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Workqueue: ipv6_addrconf addrconf_dad_work
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x306/0x460 lib/dump_stack.c:113
kmsan_report+0x1a2/0x2e0 mm/kmsan/kmsan.c:917
__msan_warning+0x7c/0xe0 mm/kmsan/kmsan_instr.c:500
memcmp+0x117/0x180 lib/string.c:863
__hw_addr_add_ex net/core/dev_addr_lists.c:61 [inline]
__dev_mc_add+0x1f9/0x8b0 net/core/dev_addr_lists.c:670
dev_mc_add+0x6d/0x80 net/core/dev_addr_lists.c:687
igmp6_group_added+0x2d7/0xab0 net/ipv6/mcast.c:676
__ipv6_dev_mc_inc+0xeff/0x10f0 net/ipv6/mcast.c:934
ipv6_dev_mc_inc+0x70/0x80 net/ipv6/mcast.c:941
addrconf_join_solict net/ipv6/addrconf.c:2098 [inline]
addrconf_dad_begin net/ipv6/addrconf.c:3879 [inline]
addrconf_dad_work+0x3e7/0x2690 net/ipv6/addrconf.c:4006
process_one_work+0x19c4/0x24f0 kernel/workqueue.c:2153
worker_thread+0x206d/0x2b30 kernel/workqueue.c:2296
kthread+0x59c/0x5d0 kernel/kthread.c:247
ret_from_fork+0x35/0x40 arch/x86/entry/entry_64.S:416

Local variable description: ----buf@igmp6_group_added
Variable was created at:
igmp6_group_added+0x57/0xab0 net/ipv6/mcast.c:664
__ipv6_dev_mc_inc+0xeff/0x10f0 net/ipv6/mcast.c:934
==================================================================