Michael S. Tsirkin
2021-Aug-19 20:58 UTC
Use of uninitialized memory with CONFIG_HW_RANDOM_VIRTIO
On Fri, Nov 13, 2020 at 06:26:16PM +0100, Alexander Potapenko wrote:> Hi Amos, Rusty, Amit, Michael, > > I am hitting something that I believe to be a minor problem in the > virtio RNG driver. > When running the kernel under KMSAN with "-device virtio-rng-pci" > passed to QEMU, I am seeing reports about rng_fillbuf in > drivers/char/hw_random/core.c being used before initialization (see > the report below). > > This can be verified by initializing rng_fillbuf with 'A' as follows: > =========================================> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c > index 8c1c47dd9f46..44d609a5796a 100644 > --- a/drivers/char/hw_random/core.c > +++ b/drivers/char/hw_random/core.c > @@ -439,8 +439,11 @@ static int hwrng_fillfn(void *unused) > if (IS_ERR(rng) || !rng) > break; > mutex_lock(&reading_mutex); > + memset(rng_fillbuf, 'A', rng_buffer_size()); > + rng_fillbuf[rng_buffer_size()-1] = 0; > rc = rng_get_data(rng, rng_fillbuf, > rng_buffer_size(), 1); > + pr_err("rng_fillbuf: %s\n", rng_fillbuf); > mutex_unlock(&reading_mutex); > put_rng(rng); > if (rc <= 0) { > =========================================> > and booting the kernel: the first call of hwrng_fillfn() will print > "AAAAAAA.." instead of random data. > > For some reason on that first iteration vi->busy is true here: > https://elixir.bootlin.com/linux/latest/source/drivers/char/hw_random/virtio-rng.c#L62, > therefore the buffer is not being sent to virtio ring. > > While probably being benign, this bug is preventing syzkaller from > finding more bugs, so it would be nice to fix it. > Perhaps the easiest solution is to kzalloc rng_fillbuf, but if it's > critical for this driver to not skip even the first read, then maybe > you have better ideas? > > KMSAN report follows: > > ====================================================> BUG: KMSAN: uninit-value in _mix_pool_bytes+0x7d2/0x950 > drivers/char/random.c:570 > CPU: 0 PID: 2711 Comm: hwrng Not tainted 5.9.0-rc8-syzkaller #0 > 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+0x21c/0x280 lib/dump_stack.c:118 > kmsan_report+0xf7/0x1e0 mm/kmsan/kmsan_report.c:122 > __msan_warning+0x5f/0xa0 mm/kmsan/kmsan_instr.c:201 > _mix_pool_bytes+0x7d2/0x950 drivers/char/random.c:570 > mix_pool_bytes+0xca/0x2a0 drivers/char/random.c:599 > add_hwgenerator_randomness+0x4ac/0x500 drivers/char/random.c:2319 > hwrng_fillfn+0x6ae/0x940 drivers/char/hw_random/core.c:452 > kthread+0x51c/0x560 kernel/kthread.c:293 > ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294 > > Uninit was created at: > kmsan_save_stack_with_flags mm/kmsan/kmsan.c:129 [inline] > kmsan_internal_poison_shadow+0x5c/0xf0 mm/kmsan/kmsan.c:112 > kmsan_slab_alloc+0x8d/0xe0 mm/kmsan/kmsan_hooks.c:80 > slab_alloc_node mm/slub.c:2903 [inline] > slab_alloc mm/slub.c:2912 [inline] > kmem_cache_alloc_trace+0x61e/0xc90 mm/slub.c:2929 > kmalloc include/linux/slab.h:554 [inline] > hwrng_modinit+0x103/0x2ef drivers/char/hw_random/core.c:621 > do_one_initcall+0x371/0x9c0 init/main.c:1208 > do_initcall_level+0x1e5/0x3c6 init/main.c:1281 > do_initcalls+0x127/0x1cb init/main.c:1297 > do_basic_setup+0x33/0x36 init/main.c:1317 > kernel_init_freeable+0x238/0x38b init/main.c:1517 > kernel_init+0x1f/0x840 init/main.c:1406 > ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294 > ====================================================> > Thanks, > AlexCc Laurent - I think he said he was going to look at virtio rng.> -- > Alexander Potapenko > Software Engineer > > Google Germany GmbH > Erika-Mann-Stra?e, 33 > 80636 M?nchen > > Gesch?ftsf?hrer: Paul Manicle, Halimah DeLaine Prado > Registergericht und -nummer: Hamburg, HRB 86891 > Sitz der Gesellschaft: Hamburg
Laurent Vivier
2021-Aug-20 16:15 UTC
Use of uninitialized memory with CONFIG_HW_RANDOM_VIRTIO
On 19/08/2021 22:58, Michael S. Tsirkin wrote:> On Fri, Nov 13, 2020 at 06:26:16PM +0100, Alexander Potapenko wrote: >> Hi Amos, Rusty, Amit, Michael, >> >> I am hitting something that I believe to be a minor problem in the >> virtio RNG driver. >> When running the kernel under KMSAN with "-device virtio-rng-pci" >> passed to QEMU, I am seeing reports about rng_fillbuf in >> drivers/char/hw_random/core.c being used before initialization (see >> the report below). >> >> This can be verified by initializing rng_fillbuf with 'A' as follows: >> =========================================>> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c >> index 8c1c47dd9f46..44d609a5796a 100644 >> --- a/drivers/char/hw_random/core.c >> +++ b/drivers/char/hw_random/core.c >> @@ -439,8 +439,11 @@ static int hwrng_fillfn(void *unused) >> if (IS_ERR(rng) || !rng) >> break; >> mutex_lock(&reading_mutex); >> + memset(rng_fillbuf, 'A', rng_buffer_size()); >> + rng_fillbuf[rng_buffer_size()-1] = 0; >> rc = rng_get_data(rng, rng_fillbuf, >> rng_buffer_size(), 1); >> + pr_err("rng_fillbuf: %s\n", rng_fillbuf); >> mutex_unlock(&reading_mutex); >> put_rng(rng); >> if (rc <= 0) { >> =========================================>> >> and booting the kernel: the first call of hwrng_fillfn() will print >> "AAAAAAA.." instead of random data. >> >> For some reason on that first iteration vi->busy is true here: >> https://elixir.bootlin.com/linux/latest/source/drivers/char/hw_random/virtio-rng.c#L62, >> therefore the buffer is not being sent to virtio ring. >> >> While probably being benign, this bug is preventing syzkaller from >> finding more bugs, so it would be nice to fix it. >> Perhaps the easiest solution is to kzalloc rng_fillbuf, but if it's >> critical for this driver to not skip even the first read, then maybe >> you have better ideas? >> >> KMSAN report follows: >> >> ====================================================>> BUG: KMSAN: uninit-value in _mix_pool_bytes+0x7d2/0x950 >> drivers/char/random.c:570 >> CPU: 0 PID: 2711 Comm: hwrng Not tainted 5.9.0-rc8-syzkaller #0 >> 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+0x21c/0x280 lib/dump_stack.c:118 >> kmsan_report+0xf7/0x1e0 mm/kmsan/kmsan_report.c:122 >> __msan_warning+0x5f/0xa0 mm/kmsan/kmsan_instr.c:201 >> _mix_pool_bytes+0x7d2/0x950 drivers/char/random.c:570 >> mix_pool_bytes+0xca/0x2a0 drivers/char/random.c:599 >> add_hwgenerator_randomness+0x4ac/0x500 drivers/char/random.c:2319 >> hwrng_fillfn+0x6ae/0x940 drivers/char/hw_random/core.c:452 >> kthread+0x51c/0x560 kernel/kthread.c:293 >> ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294 >> >> Uninit was created at: >> kmsan_save_stack_with_flags mm/kmsan/kmsan.c:129 [inline] >> kmsan_internal_poison_shadow+0x5c/0xf0 mm/kmsan/kmsan.c:112 >> kmsan_slab_alloc+0x8d/0xe0 mm/kmsan/kmsan_hooks.c:80 >> slab_alloc_node mm/slub.c:2903 [inline] >> slab_alloc mm/slub.c:2912 [inline] >> kmem_cache_alloc_trace+0x61e/0xc90 mm/slub.c:2929 >> kmalloc include/linux/slab.h:554 [inline] >> hwrng_modinit+0x103/0x2ef drivers/char/hw_random/core.c:621 >> do_one_initcall+0x371/0x9c0 init/main.c:1208 >> do_initcall_level+0x1e5/0x3c6 init/main.c:1281 >> do_initcalls+0x127/0x1cb init/main.c:1297 >> do_basic_setup+0x33/0x36 init/main.c:1317 >> kernel_init_freeable+0x238/0x38b init/main.c:1517 >> kernel_init+0x1f/0x840 init/main.c:1406 >> ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294 >> ====================================================>> >> Thanks, >> Alex > > > Cc Laurent - I think he said he was going to look at virtio rng.I will have look next week. Thanks, Laurent
Alexander Potapenko
2021-Sep-12 17:05 UTC
Use of uninitialized memory with CONFIG_HW_RANDOM_VIRTIO
Hi Laurent, Do you by any chance have an update on this? Thanks, Alex On Fri, Aug 20, 2021 at 6:15 PM Laurent Vivier <lvivier at redhat.com> wrote:> > On 19/08/2021 22:58, Michael S. Tsirkin wrote: > > On Fri, Nov 13, 2020 at 06:26:16PM +0100, Alexander Potapenko wrote: > >> Hi Amos, Rusty, Amit, Michael, > >> > >> I am hitting something that I believe to be a minor problem in the > >> virtio RNG driver. > >> When running the kernel under KMSAN with "-device virtio-rng-pci" > >> passed to QEMU, I am seeing reports about rng_fillbuf in > >> drivers/char/hw_random/core.c being used before initialization (see > >> the report below). > >> > >> This can be verified by initializing rng_fillbuf with 'A' as follows: > >> =========================================> >> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c > >> index 8c1c47dd9f46..44d609a5796a 100644 > >> --- a/drivers/char/hw_random/core.c > >> +++ b/drivers/char/hw_random/core.c > >> @@ -439,8 +439,11 @@ static int hwrng_fillfn(void *unused) > >> if (IS_ERR(rng) || !rng) > >> break; > >> mutex_lock(&reading_mutex); > >> + memset(rng_fillbuf, 'A', rng_buffer_size()); > >> + rng_fillbuf[rng_buffer_size()-1] = 0; > >> rc = rng_get_data(rng, rng_fillbuf, > >> rng_buffer_size(), 1); > >> + pr_err("rng_fillbuf: %s\n", rng_fillbuf); > >> mutex_unlock(&reading_mutex); > >> put_rng(rng); > >> if (rc <= 0) { > >> =========================================> >> > >> and booting the kernel: the first call of hwrng_fillfn() will print > >> "AAAAAAA.." instead of random data. > >> > >> For some reason on that first iteration vi->busy is true here: > >> https://elixir.bootlin.com/linux/latest/source/drivers/char/hw_random/virtio-rng.c#L62, > >> therefore the buffer is not being sent to virtio ring. > >> > >> While probably being benign, this bug is preventing syzkaller from > >> finding more bugs, so it would be nice to fix it. > >> Perhaps the easiest solution is to kzalloc rng_fillbuf, but if it's > >> critical for this driver to not skip even the first read, then maybe > >> you have better ideas? > >> > >> KMSAN report follows: > >> > >> ====================================================> >> BUG: KMSAN: uninit-value in _mix_pool_bytes+0x7d2/0x950 > >> drivers/char/random.c:570 > >> CPU: 0 PID: 2711 Comm: hwrng Not tainted 5.9.0-rc8-syzkaller #0 > >> 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+0x21c/0x280 lib/dump_stack.c:118 > >> kmsan_report+0xf7/0x1e0 mm/kmsan/kmsan_report.c:122 > >> __msan_warning+0x5f/0xa0 mm/kmsan/kmsan_instr.c:201 > >> _mix_pool_bytes+0x7d2/0x950 drivers/char/random.c:570 > >> mix_pool_bytes+0xca/0x2a0 drivers/char/random.c:599 > >> add_hwgenerator_randomness+0x4ac/0x500 drivers/char/random.c:2319 > >> hwrng_fillfn+0x6ae/0x940 drivers/char/hw_random/core.c:452 > >> kthread+0x51c/0x560 kernel/kthread.c:293 > >> ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294 > >> > >> Uninit was created at: > >> kmsan_save_stack_with_flags mm/kmsan/kmsan.c:129 [inline] > >> kmsan_internal_poison_shadow+0x5c/0xf0 mm/kmsan/kmsan.c:112 > >> kmsan_slab_alloc+0x8d/0xe0 mm/kmsan/kmsan_hooks.c:80 > >> slab_alloc_node mm/slub.c:2903 [inline] > >> slab_alloc mm/slub.c:2912 [inline] > >> kmem_cache_alloc_trace+0x61e/0xc90 mm/slub.c:2929 > >> kmalloc include/linux/slab.h:554 [inline] > >> hwrng_modinit+0x103/0x2ef drivers/char/hw_random/core.c:621 > >> do_one_initcall+0x371/0x9c0 init/main.c:1208 > >> do_initcall_level+0x1e5/0x3c6 init/main.c:1281 > >> do_initcalls+0x127/0x1cb init/main.c:1297 > >> do_basic_setup+0x33/0x36 init/main.c:1317 > >> kernel_init_freeable+0x238/0x38b init/main.c:1517 > >> kernel_init+0x1f/0x840 init/main.c:1406 > >> ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294 > >> ====================================================> >> > >> Thanks, > >> Alex > > > > > > Cc Laurent - I think he said he was going to look at virtio rng. > > I will have look next week. > > Thanks, > Laurent >-- Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Stra?e, 33 80636 M?nchen Gesch?ftsf?hrer: Paul Manicle, Halimah DeLaine Prado Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg