Re: KMSAN: uninit-value in linear_transfer (2)
From: Takashi Iwai
Date: Fri Nov 09 2018 - 06:18:43 EST
On Fri, 09 Nov 2018 10:31:03 +0100,
Alexander Potapenko wrote:
>
> On Thu, Nov 8, 2018 at 9:38 PM syzbot
> <syzbot+1cb36954e127c98dd037@xxxxxxxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxxxxxx
> >
> > ==================================================================
> > 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.c#L70)
> 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@xxxxxxx>
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@xxxxxxx>
---
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;
}
--
2.19.1