[v5,23/38] kmsan: printk: treat the result of vscnprintf() as initialized

Commit Message

Alexander Potapenko March 25, 2020, 4:12 p.m. UTC

In vprintk_store(), vscnprintf() may return an uninitialized text_len
value if any of its arguments are uninitialized. In that case KMSAN will
report one or more errors in vscnprintf() itself, but it doesn't make
much sense to track that value further, as it may trigger more errors in
printk. Instead, we explicitly mark it as initialized.

Signed-off-by: Alexander Potapenko <[e-mail]>
To: Alexander Potapenko <[e-mail]>
Cc: Andrew Morton <[e-mail]>
Cc: Greg Kroah-Hartman <[e-mail]>
Cc: Eric Dumazet <[e-mail]>
Cc: Wolfram Sang <[e-mail]>
Cc: Petr Mladek <[e-mail]>
Cc: Vegard Nossum <[e-mail]>
Cc: Dmitry Vyukov <[e-mail]>
Cc: Marco Elver <[e-mail]>
Cc: Andrey Konovalov <[e-mail]>
Cc: [e-mail]
Acked-by: Petr Mladek <[e-mail]>

---

This patch was split from "kmsan: call KMSAN hooks where needed", as
requested by Andrey Konovalov. Petr Mladek has previously acked the
printk part of that patch, hence the Acked-by above.

v4:
 - split this patch away

Change-Id: Ibed60b0bdd25f8ae91acee5800b5328e78e0735a
---
 kernel/printk/printk.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Andrey Konovalov April 14, 2020, 2:37 p.m. UTC | #1

On Wed, Mar 25, 2020 at 5:14 PM <[e-mail]> wrote:
>
> In vprintk_store(), vscnprintf() may return an uninitialized text_len
> value if any of its arguments are uninitialized. In that case KMSAN will
> report one or more errors in vscnprintf() itself, but it doesn't make
> much sense to track that value further, as it may trigger more errors in
> printk. Instead, we explicitly mark it as initialized.
>
> Signed-off-by: Alexander Potapenko <[e-mail]>
> To: Alexander Potapenko <[e-mail]>
> Cc: Andrew Morton <[e-mail]>
> Cc: Greg Kroah-Hartman <[e-mail]>
> Cc: Eric Dumazet <[e-mail]>
> Cc: Wolfram Sang <[e-mail]>
> Cc: Petr Mladek <[e-mail]>
> Cc: Vegard Nossum <[e-mail]>
> Cc: Dmitry Vyukov <[e-mail]>
> Cc: Marco Elver <[e-mail]>
> Cc: Andrey Konovalov <[e-mail]>
> Cc: [e-mail]
> Acked-by: Petr Mladek <[e-mail]>

Reviewed-by: Andrey Konovalov <[e-mail]>

>
> ---
>
> This patch was split from "kmsan: call KMSAN hooks where needed", as
> requested by Andrey Konovalov. Petr Mladek has previously acked the
> printk part of that patch, hence the Acked-by above.
>
> v4:
>  - split this patch away
>
> Change-Id: Ibed60b0bdd25f8ae91acee5800b5328e78e0735a
> ---
>  kernel/printk/printk.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index ad46062345452..4cadba3c1e68d 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -1913,6 +1913,12 @@ int vprintk_store(int facility, int level,
>          * prefix which might be passed-in as a parameter.
>          */
>         text_len = vscnprintf(text, sizeof(textbuf), fmt, args);
> +       /*
> +        * 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(text_len);
>
>         /* mark and strip a trailing newline */
>         if (text_len && text[text_len-1] == '\n') {
> --
> 2.25.1.696.g5e7596f4ac-goog
>

Patch

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index ad46062345452..4cadba3c1e68d 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1913,6 +1913,12 @@  int vprintk_store(int facility, int level,
 	 * prefix which might be passed-in as a parameter.
 	 */
 	text_len = vscnprintf(text, sizeof(textbuf), fmt, args);
+	/*
+	 * 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(text_len);
 
 	/* mark and strip a trailing newline */
 	if (text_len && text[text_len-1] == '\n') {