KMSAN: uninit-value in __dev_mc_add

KMSAN: uninit-value in __dev_mc_add

[syzbot]

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: syzbot+001516d86dbe88862cec@syzkaller.appspotmail.com

==================================================================
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 syzkaller@googlegroups.com.

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

Re: KMSAN: uninit-value in __dev_mc_add

[Vladis Dronov]

[-- Attachment #1: Type: text/plain, Size: 704 bytes --]

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/20180926093018.6646-1-vdronov@redhat.com/T/#u
https://marc.info/?l=linux-netdev&m=153795423320016&w=2

A simplified reproducer is attached.

Best regards,
Vladis Dronov

[-- Attachment #2: kmsan-hw_addr_add_ex.c --]
[-- Type: text/plain, Size: 2185 bytes --]

#define _GNU_SOURCE
#include <endian.h>
#include <errno.h>
#include <fcntl.h>
#include <linux/futex.h>
#include <pthread.h>
#include <sched.h>
#include <signal.h>
#include <stdarg.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/mman.h>
#include <sys/prctl.h>
#include <sys/resource.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <sys/time.h>
#include <sys/wait.h>
#include <unistd.h>

#include <errno.h>
#include <stdarg.h>
#include <stdint.h>
#include <stdio.h>
#include <string.h>

int main(int argc, char **argv)
{
  int ret, sockfd, tunfd;

  syscall(__NR_mmap, 0x20000000, 0x1000000, 3, 0x32, -1, 0);

  // socket(AF_PACKET, SOCK_DGRAM|SOCK_NONBLOCK, 0)
  sockfd = syscall(__NR_socket, 0x11, 0x100000802, 0);
  if (sockfd < 0) {
    perror("socket()");
    ret = 1;
    goto exit_end;
  }

  memcpy((void*)0x20000240, "/dev/net/tun", 13);
  tunfd = open((char *)0x20000240, 0);
  if (tunfd < 0) {
    perror("open()");
    ret = 2;
    goto exit_sock_close;
  }

  memcpy((void*)0x200000c0, "\x69\x67\x62\x30\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00", 16);
  *(uint16_t*)0x200000d0 = 0x4012;
  ret = syscall(__NR_ioctl, tunfd, 0x400454ca, 0x200000c0); // TUNSETIFF _IOW('T', 202, int)
  if (ret < 0) {
    perror("ioctl(TUNSETIFF)");
    ret = 3;
    goto exit_tun_close;
  }

  // TUNSETLINK _IOW('T', 205, int) / 0x30a = 778 = ARPHRD_IPGRE
  if (argc < 2)
    ret = syscall(__NR_ioctl, tunfd, 0x400454cd, 0x30a);
  else
    ret = syscall(__NR_ioctl, tunfd, 0x400454cd, atoi(argv[1]));
  if (ret < 0) {
    perror("ioctl(TUNSETLINK)");
    ret = 4;
    goto exit_tun_close;
  }

  memcpy((void*)0x20000040, "\x69\x67\x62\x30\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00", 16);
  *(uint16_t*)0x20000050 = 0xa201;
  ret = syscall(__NR_ioctl, sockfd, 0x8914, 0x20000040); // SIOCSIFFLAGS 0x8914
  if (ret < 0) {
    perror("ioctl(SIOCSIFFLAGS)");
    ret = 5;
    goto exit_tun_close;
  }

  printf("done:\n");
  system("/usr/sbin/ip -details link show igb0");

exit_tun_close:
  close(tunfd);
exit_sock_close:
  close(sockfd);
exit_end:
  munmap((void *)0x20000000, 0x1000000);
  return 0;
}

Re: KMSAN: uninit-value in __dev_mc_add

[Eric Dumazet]

On Thu, Sep 27, 2018 at 2:30 PM Vladis Dronov <vdronov@redhat.com> 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/20180926093018.6646-1-vdronov@redhat.com/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.

Re: KMSAN: uninit-value in __dev_mc_add

[Vladis Dronov]

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

Re: KMSAN: uninit-value in __dev_mc_add

[syzbot]

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: syzbot+001516d86dbe88862cec@syzkaller.appspotmail.com

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
==================================================================