On Fri, Nov 8, 2019 at 1:17 PM Dmitry Vyukov <dvyukov@xxxxxxxxxx> wrote:
>
> On Fri, Nov 8, 2019 at 1:08 PM Dmitry Vyukov <dvyukov@xxxxxxxxxx> wrote:
> >
> > On Wed, Oct 30, 2019 at 3:23 PM <glider@xxxxxxxxxx> wrote:
> > >
> > > This patch adds the KernelMemorySanitizer runtime and associated files:
> > >
> > > - arch/x86/include/asm/kmsan.h: assembly definitions for hooking
> > > interrupt handlers;
> > > - include/linux/kmsan-checks.h: user API to enable/disable KMSAN,
> > > poison/unpoison memory etc.
> > > - include/linux/kmsan.h: declarations of KMSAN memory hooks to be
> > > referenced outside KMSAN runtime
> > > - lib/Kconfig.kmsan: declarations for CONFIG_KMSAN and
> > > CONFIG_TEST_KMSAN
> > > - mm/kmsan/Makefile: boilerplate Makefile
> > > - mm/kmsan/kmsan.h: internal KMSAN declarations
> > > - mm/kmsan/kmsan.c: core functions that operate with shadow and
> > > origin memory and perform checks, utility functions
> > > - mm/kmsan/kmsan_entry.c: KMSAN hooks for entry_64.S
> > > - mm/kmsan/kmsan_hooks.c: KMSAN hooks for kernel subsystems
> > > - mm/kmsan/kmsan_init.c: KMSAN initialization routines
> > > - mm/kmsan/kmsan_instr.c: functions called by KMSAN instrumentation
> > > - scripts/Makefile.kmsan: CFLAGS_KMSAN
> > >
> > > Signed-off-by: Alexander Potapenko <glider@xxxxxxxxxx>
> > > To: Alexander Potapenko <glider@xxxxxxxxxx>
> > > Cc: Vegard Nossum <vegard.nossum@xxxxxxxxxx>
> > > Cc: Dmitry Vyukov <dvyukov@xxxxxxxxxx>
> > > Cc: linux-mm@xxxxxxxxx
> >
> > >+ kmsan_pr_err("Local variable description: %s\n", descr);
> > >+ kmsan_pr_err("Variable was created at:\n");
> >
> > It would be nice to tidy up the description, it contains lots of
> > duplicate/unneeded/confusing info:
> >
> > Local variable description: ----br.i@vp7045_read_mac_addr
> > Variable was created at:
> > vp7045_read_eeprom drivers/media/usb/dvb-usb/vp7045.c:119 [inline]
> > vp7045_read_mac_addr+0x7b/0xbe0 drivers/media/usb/dvb-usb/vp7045.c:132
> > vp7045_read_eeprom drivers/media/usb/dvb-usb/vp7045.c:119 [inline]
> > vp7045_read_mac_addr+0x7b/0xbe0 drivers/media/usb/dvb-usb/vp7045.c:132
> >
> > It could be just:
> >
> > Local variable br was created at:
> > vp7045_read_eeprom drivers/media/usb/dvb-usb/vp7045.c:119 [inline]
> > vp7045_read_mac_addr+0x7b/0xbe0 drivers/media/usb/dvb-usb/vp7045.c:132
> > vp7045_read_eeprom drivers/media/usb/dvb-usb/vp7045.c:119 [inline]
> > vp7045_read_mac_addr+0x7b/0xbe0 drivers/media/usb/dvb-usb/vp7045.c:132
>
>
> >+ kmsan_pr_err("Uninit was stored to memory at:\n");
> >+ chained_nr_entries =
> >+ stack_depot_fetch(head, &chained_entries);
> >+ stack_trace_print(chained_entries, chained_nr_entries,
> >+ 0);
>
> I like how KCSAN trims all stacks of internal runtime frames. It would
> be nice to do the same for KMSAN, currently we have 3-4 uninteresting
> frames in the beginning of each stack. Besides taking space, it also
> requires to manually visually search for the actual interesting frame
> somewhere in the middle of the stack. E.g. all these frames are
> pointing into kmsan runtime guts:
>
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x1c9/0x220 lib/dump_stack.c:113
> kmsan_report+0x12d/0x290 mm/kmsan/kmsan.c:682
> __msan_warning+0x76/0xc0 mm/kmsan/kmsan_instr.c:316
>
> Uninit was stored to memory at:
> kmsan_save_stack_with_flags mm/kmsan/kmsan.c:247 [inline]
> kmsan_save_stack mm/kmsan/kmsan.c:262 [inline]
> kmsan_internal_chain_origin+0x162/0x260 mm/kmsan/kmsan.c:470
> __msan_chain_origin+0x6d/0xb0 mm/kmsan/kmsan_instr.c:197
>
> Uninit was stored to memory at:
> kmsan_save_stack_with_flags mm/kmsan/kmsan.c:247 [inline]
> kmsan_save_stack mm/kmsan/kmsan.c:262 [inline]
> kmsan_internal_chain_origin+0x162/0x260 mm/kmsan/kmsan.c:470
> __msan_chain_origin+0x6d/0xb0 mm/kmsan/kmsan_instr.c:197
>
> Uninit was stored to memory at:
> kmsan_save_stack_with_flags mm/kmsan/kmsan.c:247 [inline]
> kmsan_save_stack mm/kmsan/kmsan.c:262 [inline]
> kmsan_internal_chain_origin+0x162/0x260 mm/kmsan/kmsan.c:470
> kmsan_memcpy_memmove_metadata+0x1a9/0xf30 mm/kmsan/kmsan.c:345
> kmsan_memcpy_metadata+0xb/0x10 mm/kmsan/kmsan.c:363
> __msan_memcpy+0x61/0x70 mm/kmsan/kmsan_instr.c:148
>
> Uninit was created at:
> kmsan_save_stack_with_flags mm/kmsan/kmsan.c:247 [inline]
> kmsan_internal_poison_shadow+0x92/0x150 mm/kmsan/kmsan.c:170
> kmsan_kmalloc+0xa1/0x100 mm/kmsan/kmsan_hooks.c:179
> kmsan_slab_alloc+0xe/0x10 mm/kmsan/kmsan_hooks.c:188
> slab_post_alloc_hook mm/slab.h:446 [inline]
> slab_alloc_node mm/slub.c:2759 [inline]
> __kmalloc_node_track_caller+0xf06/0x1120 mm/slub.c:4383
Not sure this is strictly required for the initial version, but I've
added the feature to the issue tracker:
https://github.com/google/kmsan/issues/66
--
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
