Re: Re: KMSAN: uninit-value in snd_midi_event_encode_byte

[syzbot <syzbot+194dffdb8b22fc5d207a@syzkaller.appspotmail.com>]

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



> Takashi

> -- 8< --
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH] ALSA: rawmidi: Initialize allocated buffers

> syzbot reported the uninitialized value exposure in certain situations
> using virmidi loop.  It's likely a very small race at writing and
> reading, and the influence is almost negligible.  But it's safer to
> paper over this just by replacing the existing kvmalloc() with
> kvzalloc().

> Reported-by: syzbot+194dffdb8b22fc5d207a@syzkaller.appspotmail.com
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>   sound/core/rawmidi.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

> diff --git a/sound/core/rawmidi.c b/sound/core/rawmidi.c
> index 69517e18ef07..08d5662039e3 100644
> --- a/sound/core/rawmidi.c
> +++ b/sound/core/rawmidi.c
> @@ -129,7 +129,7 @@ static int snd_rawmidi_runtime_create(struct  
> snd_rawmidi_substream *substream)
>   		runtime->avail = 0;
>   	else
>   		runtime->avail = runtime->buffer_size;
> -	runtime->buffer = kvmalloc(runtime->buffer_size, GFP_KERNEL);
> +	runtime->buffer = kvzalloc(runtime->buffer_size, GFP_KERNEL);
>   	if (!runtime->buffer) {
>   		kfree(runtime);
>   		return -ENOMEM;
> @@ -655,7 +655,7 @@ static int resize_runtime_buffer(struct  
> snd_rawmidi_runtime *runtime,
>   	if (params->avail_min < 1 || params->avail_min > params->buffer_size)
>   		return -EINVAL;
>   	if (params->buffer_size != runtime->buffer_size) {
> -		newbuf = kvmalloc(params->buffer_size, GFP_KERNEL);
> +		newbuf = kvzalloc(params->buffer_size, GFP_KERNEL);
>   		if (!newbuf)
>   			return -ENOMEM;
>   		spin_lock_irq(&runtime->lock);
> --
> 2.18.0