On Fri, 09 Nov 2018 10:31:03 +0100, Alexander Potapenko wrote:
On Thu, Nov 8, 2018 at 9:38 PM syzbot syzbot+1cb36954e127c98dd037@syzkaller.appspotmail.com wrote:
Hello,
syzbot found the following crash on:
HEAD commit: 7438a3b20295 kmsan: print user address when reporting info.. git tree: https://github.com/google/kmsan.git/master console output: https://syzkaller.appspot.com/x/log.txt?x=15b42133400000 kernel config: https://syzkaller.appspot.com/x/.config?x=8df5fc509a1b351b dashboard link: https://syzkaller.appspot.com/bug?extid=1cb36954e127c98dd037 compiler: clang version 8.0.0 (trunk 343298) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12be9825400000 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13e2c425400000
IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+1cb36954e127c98dd037@syzkaller.appspotmail.com
================================================================== BUG: KMSAN: uninit-value in __arch_swab32 arch/x86/include/uapi/asm/swab.h:10 [inline] BUG: KMSAN: uninit-value in __fswab32 include/uapi/linux/swab.h:59 [inline] BUG: KMSAN: uninit-value in do_convert sound/core/oss/linear.c:50 [inline] BUG: KMSAN: uninit-value in convert sound/core/oss/linear.c:81 [inline] BUG: KMSAN: uninit-value in linear_transfer+0x92d/0xca0 sound/core/oss/linear.c:110 CPU: 1 PID: 6835 Comm: syz-executor866 Not tainted 4.19.0+ #78 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x32d/0x480 lib/dump_stack.c:113 kmsan_report+0x19f/0x300 mm/kmsan/kmsan.c:911 __msan_warning+0x76/0xd0 mm/kmsan/kmsan_instr.c:415 __arch_swab32 arch/x86/include/uapi/asm/swab.h:10 [inline] __fswab32 include/uapi/linux/swab.h:59 [inline] do_convert sound/core/oss/linear.c:50 [inline] convert sound/core/oss/linear.c:81 [inline] linear_transfer+0x92d/0xca0 sound/core/oss/linear.c:110 snd_pcm_plug_read_transfer+0x3bf/0x590 sound/core/oss/pcm_plugin.c:659 snd_pcm_oss_read2 sound/core/oss/pcm_oss.c:1480 [inline] snd_pcm_oss_read1 sound/core/oss/pcm_oss.c:1518 [inline] snd_pcm_oss_read+0xe6d/0x1cb0 sound/core/oss/pcm_oss.c:2758 __vfs_read+0x1e2/0xb10 fs/read_write.c:416 vfs_read+0x380/0x6b0 fs/read_write.c:452 ksys_read fs/read_write.c:578 [inline] __do_sys_read fs/read_write.c:588 [inline] __se_sys_read+0x17a/0x370 fs/read_write.c:586 __x64_sys_read+0x4a/0x70 fs/read_write.c:586 do_syscall_64+0xcf/0x110 arch/x86/entry/common.c:291 entry_SYSCALL_64_after_hwframe+0x63/0xe7 RIP: 0033:0x446379 Code: e8 0c e8 ff ff 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 7b 08 fc ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:00007fcd5b97cda8 EFLAGS: 00000246 ORIG_RAX: 0000000000000000 RAX: ffffffffffffffda RBX: 00000000006dac38 RCX: 0000000000446379 RDX: 0000000000000184 RSI: 0000000020002180 RDI: 0000000000000003 RBP: 00000000006dac30 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000246 R12: 00000000006dac3c R13: 7073642f7665642f R14: 00800000c0045002 R15: 0000000000000001
Uninit was stored to memory at: kmsan_save_stack_with_flags mm/kmsan/kmsan.c:252 [inline] kmsan_save_stack mm/kmsan/kmsan.c:267 [inline] kmsan_internal_chain_origin+0x136/0x240 mm/kmsan/kmsan.c:569 kmsan_memcpy_origins+0x13d/0x1b0 mm/kmsan/kmsan.c:393 __msan_memcpy+0x6f/0x80 mm/kmsan/kmsan_instr.c:242 do_convert sound/core/oss/linear.c:48 [inline] convert sound/core/oss/linear.c:81 [inline] linear_transfer+0x74c/0xca0 sound/core/oss/linear.c:110 snd_pcm_plug_read_transfer+0x3bf/0x590 sound/core/oss/pcm_plugin.c:659 snd_pcm_oss_read2 sound/core/oss/pcm_oss.c:1480 [inline] snd_pcm_oss_read1 sound/core/oss/pcm_oss.c:1518 [inline] snd_pcm_oss_read+0xe6d/0x1cb0 sound/core/oss/pcm_oss.c:2758 __vfs_read+0x1e2/0xb10 fs/read_write.c:416 vfs_read+0x380/0x6b0 fs/read_write.c:452 ksys_read fs/read_write.c:578 [inline] __do_sys_read fs/read_write.c:588 [inline] __se_sys_read+0x17a/0x370 fs/read_write.c:586 __x64_sys_read+0x4a/0x70 fs/read_write.c:586 do_syscall_64+0xcf/0x110 arch/x86/entry/common.c:291 entry_SYSCALL_64_after_hwframe+0x63/0xe7
Uninit was created at: kmsan_save_stack_with_flags mm/kmsan/kmsan.c:252 [inline] kmsan_internal_alloc_meta_for_pages+0x155/0x740 mm/kmsan/kmsan.c:689 kmsan_alloc_page+0x77/0xe0 mm/kmsan/kmsan_hooks.c:320 __alloc_pages_nodemask+0x12cc/0x6640 mm/page_alloc.c:4416 alloc_pages_current+0x584/0x7e0 mm/mempolicy.c:2093 alloc_pages include/linux/gfp.h:511 [inline] __vmalloc_area_node mm/vmalloc.c:1689 [inline] __vmalloc_node_range+0x879/0x12a0 mm/vmalloc.c:1752 __vmalloc_node mm/vmalloc.c:1797 [inline] __vmalloc_node_flags mm/vmalloc.c:1811 [inline] vmalloc+0xd8/0xf0 mm/vmalloc.c:1833 snd_pcm_plugin_alloc+0x255/0xc80 sound/core/oss/pcm_plugin.c:71 snd_pcm_plug_alloc+0x281/0x600 sound/core/oss/pcm_plugin.c:137 snd_pcm_oss_change_params_locked+0x5e40/0x6e30 sound/core/oss/pcm_oss.c:1039 snd_pcm_oss_change_params sound/core/oss/pcm_oss.c:1107 [inline] snd_pcm_oss_get_active_substream+0x4f7/0x5a0 sound/core/oss/pcm_oss.c:1124 snd_pcm_oss_get_rate sound/core/oss/pcm_oss.c:1774 [inline] snd_pcm_oss_set_rate sound/core/oss/pcm_oss.c:1766 [inline] snd_pcm_oss_ioctl+0x4adb/0x8860 sound/core/oss/pcm_oss.c:2614 do_vfs_ioctl+0xf77/0x2d30 fs/ioctl.c:46 ksys_ioctl fs/ioctl.c:702 [inline] __do_sys_ioctl fs/ioctl.c:709 [inline] __se_sys_ioctl+0x1da/0x270 fs/ioctl.c:707 __x64_sys_ioctl+0x4a/0x70 fs/ioctl.c:707 do_syscall_64+0xcf/0x110 arch/x86/entry/common.c:291 entry_SYSCALL_64_after_hwframe+0x63/0xe7 ==================================================================
I am actually unsure whether this is a true positive or not. snd_pcm_plugin_alloc() allocates plugin->buf using vmalloc(size) (see https://elixir.bootlin.com/linux/v4.20-rc1/source/sound/core/oss/pcm_plugin....) KMSAN doesn't know where this buffer is initialized (any help finding this out is welcome!), so we mark |size| bytes starting from plugin->buf as initialized: https://github.com/google/kmsan/blob/master/sound/core/oss/pcm_plugin.c#L78 But when |size| is not page-aligned vmalloc() still allocates whole page, and in the report above linear_transfer() appears to run past |size| bytes and read the tail bytes. KASAN doesn't detect this bug, because it doesn't handle vmalloc'ed (and these bytes are addressable anyway).
It's still strange that the conversion function gets called for the uninitialized source. But we should clear the vmalloc page in anyway for avoiding such a problem. And even better would be to use kvzalloc() for a better performance.
Could you check whether the patch works?
I forgot the way to trigger the test for kmsan stuff. IIRC, just pushing to my tree and triggering syz test won't work for KMSAN, right?
thanks,
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: oss: Use kvzalloc() for local buffer allocations
PCM OSS layer may allocate a few temporary buffers, one for the core read/write and another for the conversions via plugins. Currently both are allocated via vmalloc(). But as the allocation size is equivalent with the PCM period size, the required size might be quite small, depending on the application.
This patch replaces these vmalloc() calls with kvzalloc() for covering small period sizes better. Also, we use "z"-alloc variant here for addressing the possible uninitialized access reported by syzkaller.
[ More reported-by and tested-by here in the final version ]
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/oss/pcm_oss.c | 6 +++--- sound/core/oss/pcm_plugin.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c index f8d4a419f3af..467039b342b5 100644 --- a/sound/core/oss/pcm_oss.c +++ b/sound/core/oss/pcm_oss.c @@ -1062,8 +1062,8 @@ static int snd_pcm_oss_change_params_locked(struct snd_pcm_substream *substream) runtime->oss.channels = params_channels(params); runtime->oss.rate = params_rate(params);
- vfree(runtime->oss.buffer); - runtime->oss.buffer = vmalloc(runtime->oss.period_bytes); + kvfree(runtime->oss.buffer); + runtime->oss.buffer = kvzalloc(runtime->oss.period_bytes, GFP_KERNEL); if (!runtime->oss.buffer) { err = -ENOMEM; goto failure; @@ -2328,7 +2328,7 @@ static void snd_pcm_oss_release_substream(struct snd_pcm_substream *substream) { struct snd_pcm_runtime *runtime; runtime = substream->runtime; - vfree(runtime->oss.buffer); + kvfree(runtime->oss.buffer); runtime->oss.buffer = NULL; #ifdef CONFIG_SND_PCM_OSS_PLUGINS snd_pcm_oss_plugin_clear(substream); diff --git a/sound/core/oss/pcm_plugin.c b/sound/core/oss/pcm_plugin.c index 141c5f3a9575..31cb2acf8afc 100644 --- a/sound/core/oss/pcm_plugin.c +++ b/sound/core/oss/pcm_plugin.c @@ -66,8 +66,8 @@ static int snd_pcm_plugin_alloc(struct snd_pcm_plugin *plugin, snd_pcm_uframes_t return -ENXIO; size /= 8; if (plugin->buf_frames < frames) { - vfree(plugin->buf); - plugin->buf = vmalloc(size); + kvfree(plugin->buf); + plugin->buf = kvzalloc(size, GFP_KERNEL); plugin->buf_frames = frames; } if (!plugin->buf) { @@ -191,7 +191,7 @@ int snd_pcm_plugin_free(struct snd_pcm_plugin *plugin) if (plugin->private_free) plugin->private_free(plugin); kfree(plugin->buf_channels); - vfree(plugin->buf); + kvfree(plugin->buf); kfree(plugin); return 0; }