On Wed 2019-10-30 15:22:30, 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; > - initialize result of vscnprintf() in vprintk_store(); > - 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 > --- > > v2: > - dropped call to kmsan_handle_vprintk, updated comment in printk.c > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index ca65327a6de8..4b0dbed0333a 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -1914,7 +1914,13 @@ 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); > + /* Please, separate the two comments with an empty line instead of using */ and /*. > + * If any of vscnprintf() arguments is uninitialized, KMSAN will report > + * one or more errors and also probably mark text_len as uninitialized. > + * Initialize |text_len| to prevent the errors from spreading further. > + */ > + text_len = KMSAN_INIT_VALUE(vscnprintf(text, sizeof(textbuf), fmt, > + args)); Please, keep it on a single line. This seems to be the case where the 80-characters limit rule just breaks readability. I still think that KMSAN should report only the first use of an uninitialized value. It should _not_ report locations where the value is spread. The root of the problem must be fixed. Everything else looks like an unnecessary noise. Well, this fake initialization is added only 4 times in this patchset. So, it is not a disaster. I could live with the change. And if people like/want this behavior... With the two cosmetic changes, feel free to use: Acked-by: Petr Mladek <pmladek@xxxxxxxx> # printk part Best Regards, Petr
