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?
+ 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.
What is the exact meaning of KMSAN_INIT_VALUE(), please?
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
