On Mon, Oct 21, 2019 at 11:25 AM Petr Mladek <pmladek@xxxxxxxx> wrote: > > On Fri 2019-10-18 11:42:57, glider@xxxxxxxxxx wrote: > > Insert KMSAN hooks that check for potential memory errors and/or make > > necessary bookkeeping changes: > > - allocate/split/deallocate metadata pages in > > alloc_pages()/split_page()/free_page(); > > - clear page shadow and origins in clear_page(), copy_user_highpage(); > > - copy page metadata in copy_highpage(), wp_page_copy(); > > - handle vmap()/vunmap()/iounmap(); > > - handle task creation and deletion; > > - handle vprintk(); > > I looked only at the printk part. > > > - call softirq entry/exit hooks in kernel/softirq.c; > > - check/initialize memory sent to/read from USB, I2C, and network > > > > Signed-off-by: Alexander Potapenko <glider@xxxxxxxxxx> > > To: Alexander Potapenko <glider@xxxxxxxxxx> > > Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > > Cc: Eric Dumazet <edumazet@xxxxxxxxxx> > > Cc: Wolfram Sang <wsa@xxxxxxxxxxxxx> > > Cc: Petr Mladek <pmladek@xxxxxxxx> > > Cc: Vegard Nossum <vegard.nossum@xxxxxxxxxx> > > Cc: Dmitry Vyukov <dvyukov@xxxxxxxxxx> > > Cc: linux-mm@xxxxxxxxx > > Could you please add into CC also the other printk co-maitainers? Will do in the next version of this patch. > + Sergey Senozhatsky <sergey.senozhatsky@xxxxxxxxx> > + Steven Rostedt <rostedt@xxxxxxxxxxx> > > > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > > index ca65327a6de8..f77fdcb5f861 100644 > > --- a/kernel/printk/printk.c > > +++ b/kernel/printk/printk.c > > @@ -1914,7 +1914,12 @@ int vprintk_store(int facility, int level, > > * The printf needs to come first; we need the syslog > > * prefix which might be passed-in as a parameter. > > */ > > - text_len = vscnprintf(text, sizeof(textbuf), fmt, args); > > + /* > > + * We've checked the printk arguments in vprintk_emit() already. > > + * Initialize |text_len| to prevent the errors from spreading. > > + */ > > + text_len = KMSAN_INIT_VALUE(vscnprintf(text, sizeof(textbuf), fmt, > > + args)); > > I am a bit confused by the comment. Fixed in v2. > What is the exact meaning of KMSAN_INIT_VALUE(), please? Hope it'll be clear from the "kmsan: add KMSAN runtime" patch. In fact KMSAN_INIT_VALUE is a wrapper that turns an uninitialized value into an initialized one: https://github.com/google/kmsan/blob/3207d604ad68f7e5defdbbd6e63cb97750f380c1/include/linux/kmsan-checks.h#L58 It passes the value into an identity function that always returns unpoisoned values. As a result, from now on text_len is considered initialized, regardless of what vscnprintf() returned. Without KMSAN_INIT_VALUE the tool will report a huge number of errors when text_len is used in vprintk_store() and log_output(). > Does it prevent checking fmt again? > Does make the text_len variable special? In which way? > > > > /* mark and strip a trailing newline */ > > if (text_len && text[text_len-1] == '\n') { > > @@ -1972,6 +1977,7 @@ asmlinkage int vprintk_emit(int facility, int level, > > boot_delay_msec(level); > > printk_delay(); > > > > + kmsan_handle_vprintk(&fmt, args); > > What does this function, please? > Could I find more details in another patch? > > > /* This stops the holder of console_sem just where we want him */ > > logbuf_lock_irqsave(flags); > > curr_log_seq = log_next_seq; > > Best Regards, > Petr -- 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
