Re: KMSAN: uninit-value in snd_midi_event_encode_byte

[Takashi Iwai <tiwai@suse.de>]

On Mon, 03 Sep 2018 16:19:23 +0200,
Dmitry Vyukov wrote:
> 
> On Mon, Sep 3, 2018 at 4:04 PM, Takashi Iwai <tiwai@suse.de> wrote:
> > On Mon, 03 Sep 2018 15:41:36 +0200,
> > Dmitry Vyukov wrote:
> >>
> >> On Mon, Sep 3, 2018 at 3:37 PM, Takashi Iwai <tiwai@suse.de> wrote:
> >> > On Mon, 03 Sep 2018 15:33:12 +0200,
> >> > Dmitry Vyukov wrote:
> >> >>
> >> >> On Mon, Sep 3, 2018 at 3:29 PM, Takashi Iwai <tiwai@suse.de> wrote:
> >> >> > On Mon, 03 Sep 2018 15:22:42 +0200,
> >> >> > syzbot wrote:
> >> >> >>
> >> >> >> > On Mon, 03 Sep 2018 03:53:03 +0200,
> >> >> >> > syzbot wrote:
> >> >> >>
> >> >> >> >> syzbot has found a reproducer for the following crash on:
> >> >> >>
> >> >> >> >> HEAD commit:    28f0ca98eadf kmsan: don't instrument
> >> >> >> >> do_syscall_64() and  _..
> >> >> >> >> git tree:       https://github.com/google/kmsan.git/master
> >> >> >> >> console output: https://syzkaller.appspot.com/x/log.txt?x=10556c92400000
> >> >> >> >> kernel config:
> >> >> >> >> https://syzkaller.appspot.com/x/.config?x=3431f03869413153
> >> >> >> >> dashboard link:
> >> >> >> >> https://syzkaller.appspot.com/bug?extid=194dffdb8b22fc5d207a
> >> >> >> >> compiler:       clang version 8.0.0 (trunk 339414)
> >> >> >> >> syz repro:
> >> >> >> >> https://syzkaller.appspot.com/x/repro.syz?x=123a520a400000
> >> >> >> >> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=16068dc1400000
> >> >> >>
> >> >> >> >> IMPORTANT: if you fix the bug, please add the following tag to the
> >> >> >> >> commit:
> >> >> >> >> Reported-by: syzbot+194dffdb8b22fc5d207a@syzkaller.appspotmail.com
> >> >> >>
> >> >> >> >> ==================================================================
> >> >> >> >> BUG: KMSAN: uninit-value in snd_midi_event_encode_byte+0x569/0xff0
> >> >> >> >> sound/core/seq/seq_midi_event.c:195
> >> >> >> >> CPU: 1 PID: 1659 Comm: kworker/1:1H Not tainted 4.19.0-rc1+ #40
> >> >> >> >> Hardware name: Google Google Compute Engine/Google Compute Engine,
> >> >> >> >> BIOS  Google 01/01/2011
> >> >> >> >> Workqueue: events_highpri snd_vmidi_output_work
> >> >> >> >> Call Trace:
> >> >> >> >>   __dump_stack lib/dump_stack.c:77 [inline]
> >> >> >> >>   dump_stack+0x14b/0x190 lib/dump_stack.c:113
> >> >> >> >>   kmsan_report+0x183/0x2b0 mm/kmsan/kmsan.c:956
> >> >> >> >>   __msan_warning+0x70/0xc0 mm/kmsan/kmsan_instr.c:645
> >> >> >> >>   snd_midi_event_encode_byte+0x569/0xff0
> >> >> >> >> sound/core/seq/seq_midi_event.c:195
> >> >> >> >>   snd_vmidi_output_work+0x34e/0x5b0 sound/core/seq/seq_virmidi.c:161
> >> >> >> >>   process_one_work+0x1605/0x1f40 kernel/workqueue.c:2153
> >> >> >> >>   worker_thread+0x11a2/0x2590 kernel/workqueue.c:2296
> >> >> >> >>   kthread+0x465/0x4a0 kernel/kthread.c:247
> >> >> >> >>   ret_from_fork+0x35/0x40 arch/x86/entry/entry_64.S:416
> >> >> >>
> >> >> >> >> Uninit was stored to memory at:
> >> >> >> >>   kmsan_save_stack_with_flags mm/kmsan/kmsan.c:256 [inline]
> >> >> >> >>   kmsan_save_stack mm/kmsan/kmsan.c:271 [inline]
> >> >> >> >>   kmsan_internal_chain_origin+0x128/0x210 mm/kmsan/kmsan.c:573
> >> >> >> >>   __msan_chain_origin+0x69/0xc0 mm/kmsan/kmsan_instr.c:482
> >> >> >> >>   __snd_rawmidi_transmit_peek sound/core/rawmidi.c:1103 [inline]
> >> >> >> >>   snd_rawmidi_transmit+0xa75/0xbf0 sound/core/rawmidi.c:1228
> >> >> >> >>   snd_vmidi_output_work+0x2ac/0x5b0 sound/core/seq/seq_virmidi.c:159
> >> >> >> >>   process_one_work+0x1605/0x1f40 kernel/workqueue.c:2153
> >> >> >> >>   worker_thread+0x11a2/0x2590 kernel/workqueue.c:2296
> >> >> >> >>   kthread+0x465/0x4a0 kernel/kthread.c:247
> >> >> >> >>   ret_from_fork+0x35/0x40 arch/x86/entry/entry_64.S:416
> >> >> >>
> >> >> >> >> Uninit was created at:
> >> >> >> >>   kmsan_save_stack_with_flags mm/kmsan/kmsan.c:256 [inline]
> >> >> >> >>   kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:181
> >> >> >> >>   kmsan_kmalloc+0x98/0x100 mm/kmsan/kmsan_hooks.c:91
> >> >> >> >>   __kmalloc_node+0x7bf/0x11c0 mm/slub.c:3828
> >> >> >> >>   kmalloc_node include/linux/slab.h:555 [inline]
> >> >> >> >>   kvmalloc_node+0x19d/0x3e0 mm/util.c:423
> >> >> >> >>   kvmalloc include/linux/mm.h:577 [inline]
> >> >> >> >>   snd_rawmidi_runtime_create sound/core/rawmidi.c:132 [inline]
> >> >> >> >>   open_substream+0x3c8/0xaa0 sound/core/rawmidi.c:276
> >> >> >> >>   rawmidi_open_priv+0x347/0x1000 sound/core/rawmidi.c:327
> >> >> >> >>   snd_rawmidi_open+0x7d4/0x1120 sound/core/rawmidi.c:424
> >> >> >> >>   soundcore_open+0x9be/0xa60 sound/sound_core.c:597
> >> >> >> >>   chrdev_open+0xc26/0xdb0 fs/char_dev.c:417
> >> >> >> >>   do_dentry_open+0xce6/0x1740 fs/open.c:771
> >> >> >> >>   vfs_open+0xaf/0xe0 fs/open.c:880
> >> >> >> >>   do_last fs/namei.c:3418 [inline]
> >> >> >> >>   path_openat+0x1799/0x6870 fs/namei.c:3534
> >> >> >> >>   do_filp_open+0x259/0x610 fs/namei.c:3564
> >> >> >> >>   do_sys_open+0x630/0x940 fs/open.c:1063
> >> >> >> >>   __do_sys_open fs/open.c:1081 [inline]
> >> >> >> >>   __se_sys_open+0xad/0xc0 fs/open.c:1076
> >> >> >> >>   __x64_sys_open+0x4a/0x70 fs/open.c:1076
> >> >> >> >>   do_syscall_64+0xb8/0x100 arch/x86/entry/common.c:291
> >> >> >> >>   entry_SYSCALL_64_after_hwframe+0x63/0xe7
> >> >> >> >> ==================================================================
> >> >> >>
> >> >> >> > This looks like some small race at virmidi reading and the buffer
> >> >> >> > handling, which should be almost harmless by itself.
> >> >> >> > Nevertheless, the fix should be easy, just replacing kvmalloc() with
> >> >> >> > kvzalloc(), as below.
> >> >> >>
> >> >> >> > Let's check whether this works.
> >> >> >>
> >> >> >> > #syz test:
> >> >> >> > git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
> >> >> >> > topic/rawmidi-fixes
> >> >> >>
> >> >> >> KMSAN bugs can only be tested on https://github.com/google/kmsan.git tree
> >> >> >> because KMSAN tool is not upstreamed yet.
> >> >> >> See
> >> >> >> https://github.com/google/syzkaller/blob/master/docs/syzbot.md#kmsan-bugs
> >> >> >> for details.
> >> >> >
> >> >> > So how is the procedure to test a patch for bugs spotted by KMSAN?
> >> >> >
> >> >> > In this case, the fix is trivial, so I can put it to my tree for the
> >> >> > next pull request.  Of course, it'd be better if it can be confirmed
> >> >> > to work beforehand...
> >> >>
> >> >> https://github.com/google/syzkaller/blob/master/docs/syzbot.md#kmsan-bugs
> >> >>
> >> >> Which means you need to attach a patch because it's not in that tree.
> >> >
> >> > OK, it would have been helpful if the description there was more clear
> >> > :)
> >> >
> >> > And no automated way for that?
> >>
> >> What exactly do you mean? Please elaborate.
> >
> > Well, it's not clear which action is required to let syzbot testing
> > some patch.  The description says:
> >
> >  "KMSAN is not upstream yet, though, we want to upstream it later. For
> >   now, it lives in github.com/google/kmsan and is based on a
> >   reasonably fresh upstream tree. As the result, any patch testing
> >   requests for KMSAN bugs need to go to KMSAN tree
> >   (https://github.com/google/kmsan.git repo, master branch)."
> >
> > Here, "need to go to KMSAN tree" part doesn't explain what exactly to
> > do.  Remember that many kernel devs (including the man in the mirror)
> > don't use github, at least, not dealing github PRs for kernel
> > development itself, so it's a pretty unfamiliar process.
> 
> It's not patches that need to go into that tree (i.e. no PRs). It's
> patch testing requests.
> 
> Tried to clarify doc:
> 
> https://github.com/google/syzkaller/commit/bfe906cbb9c73c326f24327e1a89bc0cd0c314c7#diff-5b3b5ff5f03b01e1d31ec93aafd2f3d5
> 
> Does this look better?

Erm, sorry, still unclear to me.  I'd need to explicitl attch/inline
the patch for testing *to where*?  Attach the test patch to the e-mail
replying to the bug?

Also, now you've put an example referring to
https://github.com/google/kmsan.git master.  What does it mean?
I should have put that line in my reply?

> >> > I mean, I already attached patch in
> >> > the previous reply (together with syz-magic command to point the git
> >> > branch URL).  Do I need anything else?
> >>
> >> Just issue the command against "https://github.com/google/kmsan.git
> >> master" and attach the patch.
> >
> > But it's basically a fix for the upstream tree, not about KMSAN
> > itself.  A PR to that tree doesn't sound right to me...
> 
> You don't need to send a PR. You attach a patch (or inline in email)
> and request to apply and test it in KMSAN tree.

Then I must have understood "just issue the command against..."
Does it mean the "syz test: https//github.com/google/kmsan.git
master"?

I need to learn the basic for understanding the black magic...


thanks,

Takashi