Add KernelMemorySanitizer infrastructure

Clang may replace stackdepot_memcmp() with a call to instrumented bcmp(), which is exactly what we wanted to avoid creating stackdepot_memcmp(). Add a compiler barrier() to prevent optimizations.

Quoted 3 lines · On (19/10/30 15:22), [email protected] wrote:

Clang may replace stackdepot_memcmp() with a call to instrumented bcmp(), which is exactly what we wanted to avoid creating stackdepot_memcmp(). Add a compiler barrier() to prevent optimizations.

[..]

lib/stackdepot.c int stackdepot_memcmp(const unsigned long *u1, const unsigned long *u2, · line 173

Quoted 12 lines

lib/stackdepot.c +5 -0

@@ -163,6 +163,11 @@ int stackdepot_memcmp(const unsigned long *u1, const unsigned long *u2,
                      unsigned int n)
 {
      for ( ; n-- ; u1++, u2++) {
+             /*
+              * Prevent Clang from replacing this function with a bcmp()
+              * call.
+              */
+             barrier();
              if (*u1 != *u2)
                      return 1;
      }

Would 'volatile' do the trick?

        -ss

Quoted 7 lines · On (19/10/30 15:22), [email protected] wrote:

Quoted 6 lines

lib/stackdepot.c +2 -0

@@ -163,6 +163,11 @@ int stackdepot_memcmp(const unsigned long *u1, const unsigned long *u2,
                      unsigned int n)
 {
      for ( ; n-- ; u1++, u2++) {
+             /*
+              * Prevent Clang from replacing this function with a bcmp()

Quoted 8 lines

lib/stackdepot.c int stackdepot_memcmp(const unsigned long *u1, const unsigned long *u2, · line 173

Quoted 6 lines

lib/stackdepot.c +3 -0

+              * call.
+              */
+             barrier();
              if (*u1 != *u2)
                      return 1;
      }

Would 'volatile' do the trick?

It does. I can replace the barrier with a volatile if you think that's better. However this'll add a checkpatch warning, as volatiles are discouraged for synchronization (although in this case there's no synchronization)

Quoted 15 lines · Alexander Potapenko

Quoted 15 lines · On (19/10/30 15:22), [email protected] wrote:

lib/stackdepot.c int stackdepot_memcmp(const unsigned long *u1, const unsigned long *u2, · line 173

Quoted 12 lines

lib/stackdepot.c +5 -0

@@ -163,6 +163,11 @@ int stackdepot_memcmp(const unsigned long *u1, const unsigned long *u2,
                      unsigned int n)
 {
      for ( ; n-- ; u1++, u2++) {
+             /*
+              * Prevent Clang from replacing this function with a bcmp()
+              * call.
+              */
+             barrier();
              if (*u1 != *u2)
                      return 1;
      }

Would 'volatile' do the trick?

Quoted 3 lines

It does. I can replace the barrier with a volatile if you think that's better. However this'll add a checkpatch warning, as volatiles are discouraged for synchronization (although in this case there's no synchronization)

Yeah, 'volatile' in this case will do what it sort of meant to do - prevent compiler optimizations. So, like you said, it's not a synchronization issue and we don't 'volatile' data structures.

Do you need to do barrier() on every iteration? Does clang behave if you do one barrier() instead of 'n' barrier()-s?

Quoted 21 lines · Sergey Senozhatsky

Quoted 18 lines · Alexander Potapenko

Quoted 7 lines · On (19/10/30 15:22), [email protected] wrote:

Quoted 6 lines

lib/stackdepot.c +2 -0

@@ -163,6 +163,11 @@ int stackdepot_memcmp(const unsigned long *u1, const unsigned long *u2,
                      unsigned int n)
 {
      for ( ; n-- ; u1++, u2++) {
+             /*
+              * Prevent Clang from replacing this function with a bcmp()

Quoted 8 lines

lib/stackdepot.c int stackdepot_memcmp(const unsigned long *u1, const unsigned long *u2, · line 173

Quoted 6 lines

lib/stackdepot.c +3 -0

+              * call.
+              */
+             barrier();
              if (*u1 != *u2)
                      return 1;
      }

Would 'volatile' do the trick?

It does. I can replace the barrier with a volatile if you think that's better. However this'll add a checkpatch warning, as volatiles are discouraged for synchronization (although in this case there's no synchronization)

Quoted 3 lines

Yeah, 'volatile' in this case will do what it sort of meant to do - prevent compiler optimizations. So, like you said, it's not a synchronization issue and we don't 'volatile' data structures.

The normal way to do a volatile access would be READ_ONCE()/WRITE_ONCE(), but that seems stronger than the barrier() here. I'd just stick to adding a barrier.

Quoted 2 lines

Do you need to do barrier() on every iteration? Does clang behave if you do one barrier() instead of 'n' barrier()-s?

If it does things right, it would make that a single-byte copy plus a call to bcmp(). I certainly wouldn't want to have an implementation that relies on the compiler making sub-optimal decisions ;-)

Arnd

Quoted 7 lines · Arnd Bergmann

Quoted 4 lines · Sergey Senozhatsky

Yeah, 'volatile' in this case will do what it sort of meant to do - prevent compiler optimizations. So, like you said, it's not a synchronization issue and we don't 'volatile' data structures.

Quoted 3 lines

The normal way to do a volatile access would be READ_ONCE()/WRITE_ONCE(), but that seems stronger than the barrier() here. I'd just stick to adding a barrier.

I actually like the READ_ONCE idea more, as READ_ONCE is really a documented way to prevent the compiler from merging reads, which is what we want here.

I also thought that the original barrier() statement was just a compiler barrier, which didn't introduce any additional CPU instructions.
Turns out I was wrong, and barrier() also serves as a memory barrier.

Quoted 8 lines · Alexander Potapenko

Quoted 7 lines · Arnd Bergmann

Quoted 4 lines · Sergey Senozhatsky

Yeah, 'volatile' in this case will do what it sort of meant to do - prevent compiler optimizations. So, like you said, it's not a synchronization issue and we don't 'volatile' data structures.

Quoted 6 lines

Quoted 3 lines

The normal way to do a volatile access would be READ_ONCE()/WRITE_ONCE(), but that seems stronger than the barrier() here. I'd just stick to adding a barrier.

I actually like the READ_ONCE idea more, as READ_ONCE is really a documented way to prevent the compiler from merging reads, which is what we want here.

Fair enough.

Quoted 4 lines

I also thought that the original barrier() statement was just a compiler barrier, which didn't introduce any additional CPU instructions.
Turns out I was wrong, and barrier() also serves as a memory barrier.

The definition of barrier is

#define barrier() __asm__ __volatile__("": : :"memory")

which is no actual barrier instruction but is a full barrier to the compiler.

Only for the Intel ecc compiler it is defined as an intrinsic that I don't know.

Arnd

Quoted 6 lines · Alexander Potapenko

Quoted 5 lines · Arnd Bergmann

On Thu, Nov 7, 2019 at 7:08 AM Sergey Senozhatsky <[email protected]> wrote:
The normal way to do a volatile access would be READ_ONCE()/WRITE_ONCE(), but that seems stronger than the barrier() here. I'd just stick to adding a barrier.

Quoted 3 lines

I actually like the READ_ONCE idea more, as READ_ONCE is really a documented way to prevent the compiler from merging reads, which is what we want here.

I would rather go with -fno-builtin-bcmp or maybe even -fno-builtin if that works.

Quoted 12 lines · Arnd Bergmann

Quoted 11 lines · Andrey Ryabinin

Quoted 9 lines · Alexander Potapenko

Quoted 5 lines · Arnd Bergmann

On Thu, Nov 7, 2019 at 7:08 AM Sergey Senozhatsky <[email protected]> wrote:
The normal way to do a volatile access would be READ_ONCE()/WRITE_ONCE(), but that seems stronger than the barrier() here. I'd just stick to adding a barrier.

I actually like the READ_ONCE idea more, as READ_ONCE is really a documented way to prevent the compiler from merging reads, which is what we want here.

Quoted 6 lines

Quoted 1 line

I would rather go with -fno-builtin-bcmp or maybe even -fno-builtin if that works.

The commit message for 5f074f3e192f ("lib/string.c: implement a basic bcmp") mentions that -fno-builtin-bcmp did not work for LTO when the global bcmp() help was added. I don't know whether the same applies here, but my guess is that it's the same issue.

But we don't do LTO.

--
You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/6875c6e6-2f1f-f8e6-e5d7-d451c48397ff%40virtuozzo.com.

__no_sanitize_memory is a function attribute that makes KMSAN ignore the uninitialized values coming from the function's inputs, and initialize the function's outputs.

Functions marked with this attribute can't be inlined into functions not marked with it, and vice versa.

__SANITIZE_MEMORY__ is a macro that's defined iff the file is instrumented with KMSAN. This is not the same as CONFIG_KMSAN, which is defined for every file.

Signed-off-by Alexander Potapenko <[email protected]>

To: Alexander Potapenko <[email protected]>
Cc: Vegard Nossum <[email protected]>
Cc: Dmitry Vyukov <[email protected]>
Cc: [email protected]

---

Change-Id: I1f1672652c8392f15f7ca8ac26cd4e71f9cc1e4b

---
 include/linux/compiler-clang.h | 8 ++++++++
 include/linux/compiler-gcc.h   | 5 +++++
 2 files changed, 13 insertions(+)

include/linux/compiler-clang.h +0 -0

index 333a6695a918..edba13a069a6 100644

include/linux/compiler-clang.h +4 -0

+#else
+# define __no_sanitize_memory
+#endif
+
 /*
  * Not all versions of clang implement the the type-generic versions
  * of the builtin overflow checkers. Fortunately, clang implements

include/linux/compiler-gcc.h +5 -1

index d7ee4c6bad48..e5ebc788dde4 100644
@@ -145,6 +145,11 @@
 #define __no_sanitize_address
 #endif
 
+/*
+ * GCC doesn't support KMSAN.
+ */
+#define __no_sanitize_memory
+
 #if GCC_VERSION >= 50100
 #define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
 #endif
--

2.24.0.rc0.303.g954a862665-goog

Quoted 34 lines · On Wed, Oct 30, 2019 at 3:23 PM <[email protected]> wrote:

__no_sanitize_memory is a function attribute that makes KMSAN ignore the uninitialized values coming from the function's inputs, and initialize the function's outputs.

Functions marked with this attribute can't be inlined into functions not marked with it, and vice versa.

__SANITIZE_MEMORY__ is a macro that's defined iff the file is instrumented with KMSAN. This is not the same as CONFIG_KMSAN, which is defined for every file.

Signed-off-by Alexander Potapenko <[email protected]>

To: Alexander Potapenko <[email protected]>
Cc: Vegard Nossum <[email protected]>
Cc: Dmitry Vyukov <[email protected]>
Cc: [email protected]

---

Change-Id: I1f1672652c8392f15f7ca8ac26cd4e71f9cc1e4b

---
 include/linux/compiler-clang.h | 8 ++++++++
 include/linux/compiler-gcc.h   | 5 +++++
 2 files changed, 13 insertions(+)

include/linux/compiler-clang.h +0 -0

index 333a6695a918..edba13a069a6 100644
@@ -24,6 +24,14 @@
 #define __no_sanitize_address
 #endif
 

include/linux/compiler-clang.h line 30

Quoted 4 lines

include/linux/compiler-clang.h +4 -0

+/* KMSAN is a Clang-only tool, thus putting the defines here */
+#if __has_feature(memory_sanitizer)
+# define __SANITIZE_MEMORY__
+# define __no_sanitize_memory __attribute__((no_sanitize("kernel-memory")))

For KASAN with Clang we ended up choosing to use no_sanitize("address") instead of no_sanitize("kernel-address") to make it match what GCC uses. Do we want to use no_sanitize("memory") here?

Quoted 26 lines

include/linux/compiler-clang.h +4 -0

+#else
+# define __no_sanitize_memory
+#endif
+
 /*
  * Not all versions of clang implement the the type-generic versions
  * of the builtin overflow checkers. Fortunately, clang implements

include/linux/compiler-gcc.h +5 -1

index d7ee4c6bad48..e5ebc788dde4 100644
@@ -145,6 +145,11 @@
 #define __no_sanitize_address
 #endif
 
+/*
+ * GCC doesn't support KMSAN.
+ */
+#define __no_sanitize_memory
+
 #if GCC_VERSION >= 50100
 #define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
 #endif
--

2.24.0.rc0.303.g954a862665-goog

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 <[email protected]>

To: Alexander Potapenko <[email protected]>
Cc: Vegard Nossum <[email protected]>
Cc: Dmitry Vyukov <[email protected]>
Cc: [email protected]

Quoted 27 lines · On Wed, Oct 30, 2019 at 3:23 PM <[email protected]> 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 <[email protected]>

To: Alexander Potapenko <[email protected]>
Cc: Vegard Nossum <[email protected]>
Cc: Dmitry Vyukov <[email protected]>
Cc: [email protected]

mm/kmsan/kmsan_report.c line 43

Quoted 2 lines

mm/kmsan/kmsan_report.c new file +2 -0

+                       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

Quoted 51 lines · Dmitry Vyukov

Quoted 27 lines · On Wed, Oct 30, 2019 at 3:23 PM <[email protected]> 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 <[email protected]>

To: Alexander Potapenko <[email protected]>
Cc: Vegard Nossum <[email protected]>
Cc: Dmitry Vyukov <[email protected]>
Cc: [email protected]

mm/kmsan/kmsan_report.c line 43

Quoted 2 lines

mm/kmsan/kmsan_report.c new file +2 -0

+                       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

mm/kmsan/kmsan_report.c line 56

Quoted 5 lines

mm/kmsan/kmsan_report.c new file +5 -0

+                       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

+/* Helper function to check I2C-transferred data. */

Hi,

Quoted 1 line

+/* Helper function to check I2C-transferred data. */

I assume I2C is just one of the first users and other users like SPI will be implemented as time passes? Otherwise I would wonder why I2C needs to be handled with this special function.

mm/kmsan/kmsan_hooks.c line 382

Quoted 6 lines

mm/kmsan/kmsan_hooks.c new file +6 -0

+void kmsan_handle_i2c_transfer(struct i2c_msg *msgs, int num)
+{
+     int i;
+
+     if (!msgs)
+             return;

Can go. You call this function from __i2c_transfer and this checks for valid 'msgs'.

Kind regards,

Wolfram

READ_ONCE_NOCHECK() is already used by KASAN to ignore memory accesses from e.g. stack unwinders.
Define READ_ONCE_NOCHECK() for KMSAN so that it returns initialized values. This helps defeat false positives from leftover stack contents.

Signed-off-by Alexander Potapenko <[email protected]>

To: Alexander Potapenko <[email protected]>
Cc: Vegard Nossum <[email protected]>
Cc: Dmitry Vyukov <[email protected]>
Cc: [email protected]
---

Change-Id: Ib38369ba038ab3b581d8e45b81036c3304fb79cb

---
 include/linux/compiler.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

include/linux/compiler.h +0 -0

index 5e88e7e33abe..e8c86debdb2b 100644
@@ -270,9 +270,13 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
 

include/linux/compiler.h +0 -0

 static __no_kasan_or_inline
 unsigned long read_word_at_a_time(const void *addr)

Signature · 1 line

2.24.0.rc0.303.g954a862665-goog

Quoted 23 lines · On Wed, Oct 30, 2019 at 03:22:22PM +0100, [email protected] wrote:

READ_ONCE_NOCHECK() is already used by KASAN to ignore memory accesses from e.g. stack unwinders.
Define READ_ONCE_NOCHECK() for KMSAN so that it returns initialized values. This helps defeat false positives from leftover stack contents.

Signed-off-by Alexander Potapenko <[email protected]>

To: Alexander Potapenko <[email protected]>
Cc: Vegard Nossum <[email protected]>
Cc: Dmitry Vyukov <[email protected]>
Cc: [email protected]
---

Change-Id: Ib38369ba038ab3b581d8e45b81036c3304fb79cb

---
 include/linux/compiler.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

include/linux/compiler.h +0 -0

index 5e88e7e33abe..e8c86debdb2b 100644
@@ -270,9 +270,13 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
 

include/linux/compiler.h static __always_inline void __write_once_size(volatile void *p, void *res, int s · line 279

Quoted 10 lines

include/linux/compiler.h +5 -1

 /*
  * Use READ_ONCE_NOCHECK() instead of READ_ONCE() if you need
- * to hide memory access from KASAN.
+ * to hide memory access from KASAN or KMSAN.
  */
+#ifndef CONFIG_KMSAN
 #define READ_ONCE_NOCHECK(x) __READ_ONCE(x, 0)
+#else
+#define READ_ONCE_NOCHECK(x) KMSAN_INIT_VALUE(__READ_ONCE(x, 0))
+#endif

When !CONFIG_KMSAN, we have:

| #define KMSAN_INIT_VALUE(value) (value)

... so we don't need ifdeffery here, and can simply have:

/*
* Use READ_ONCE_NOCHECK() instead of READ_ONCE() if you need
* to hide memory access from KASAN or KMSAN.
*/
#define READ_ONCE_NOCHECK(x) KMSAN_INIT_VALUE(__READ_ONCE(x, 0))

Thanks,
Mark.

Quoted 6 lines

include/linux/compiler.h +0 -1

 static __no_kasan_or_inline
 unsigned long read_word_at_a_time(const void *addr)
--

2.24.0.rc0.303.g954a862665-goog

Certain config options are currently incompatible with KMSAN, disable them.

Signed-off-by Alexander Potapenko <[email protected]>

Acked-by Harry Wentland <[email protected]>

To: Alexander Potapenko <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: Harry Wentland <[email protected]>
Cc: Vegard Nossum <[email protected]>
Cc: Dmitry Vyukov <[email protected]>
Cc: [email protected]

---

Change-Id: I4521f5c764cfaeba2fae79c954925c302c5e3bc0

---
 arch/Kconfig                        |  2 +-
 arch/x86/Kconfig                    |  1 +
 arch/x86/Kconfig.debug              |  3 +++
 crypto/Kconfig                      | 26 ++++++++++++++++++++++++++
 drivers/gpu/drm/amd/display/Kconfig |  2 +-
 lib/Kconfig.debug                   |  3 +++
 6 files changed, 35 insertions(+), 2 deletions(-)

arch/Kconfig +1 -1

index 5f8a5d84dbbe..d5ca838ae8ad 100644
@@ -843,7 +843,7 @@ config HAVE_ARCH_VMAP_STACK
 config VMAP_STACK
 	default y
 	bool "Use a virtually-mapped stack"
-	depends on HAVE_ARCH_VMAP_STACK && !KASAN
+	depends on HAVE_ARCH_VMAP_STACK && !KASAN && !KMSAN
 	---help---
 	  Enable this if you want the use virtually-mapped kernel stacks
 	  with guard pages.  This causes kernel stack overflows to be

arch/x86/Kconfig +1 -0

index 3f83a5c53808..6d2b0fb3a871 100644
@@ -2143,6 +2143,7 @@ config RELOCATABLE
 config RANDOMIZE_BASE
 	bool "Randomize the address of the kernel image (KASLR)"
 	depends on RELOCATABLE
+	depends on !KMSAN
 	default y
 	---help---
 	  In support of Kernel Address Space Layout Randomization (KASLR),

arch/x86/Kconfig.debug +3 -0

index bf9cd83de777..db3cd6147829 100644
@@ -292,6 +292,9 @@ choice
 config UNWINDER_ORC
 	bool "ORC unwinder"
 	depends on X86_64
+	# KMSAN doesn't support UNWINDER_ORC yet,
+	# see https://github.com/google/kmsan/issues/48.
+	depends on !KMSAN
 	select STACK_VALIDATION
 	---help---
 	  This option enables the ORC (Oops Rewind Capability) unwinder for

crypto/Kconfig +26 -0

index 9e524044d312..37a22711012c 100644
@@ -309,11 +309,13 @@ config CRYPTO_AEGIS128
 config CRYPTO_AEGIS128_SIMD
 	bool "Support SIMD acceleration for AEGIS-128"
 	depends on CRYPTO_AEGIS128 && ((ARM || ARM64) && KERNEL_MODE_NEON)
+	depends on !KMSAN
 	default y
 
 config CRYPTO_AEGIS128_AESNI_SSE2
 	tristate "AEGIS-128 AEAD algorithm (x86_64 AESNI+SSE2 implementation)"
 	depends on X86 && 64BIT
+	depends on !KMSAN
 	select CRYPTO_AEAD
 	select CRYPTO_SIMD
 	help

@@ -571,6 +573,7 @@ config CRYPTO_CRC32C
 config CRYPTO_CRC32C_INTEL
 	tristate "CRC32c INTEL hardware acceleration"
 	depends on X86
+	depends on !KMSAN
 	select CRYPTO_HASH
 	help
 	  In Intel processor with SSE4.2 supported, the processor will

@@ -611,6 +614,7 @@ config CRYPTO_CRC32
 config CRYPTO_CRC32_PCLMUL
 	tristate "CRC32 PCLMULQDQ hardware acceleration"
 	depends on X86
+	depends on !KMSAN
 	select CRYPTO_HASH
 	select CRC32
 	help

@@ -649,6 +653,7 @@ config CRYPTO_CRCT10DIF
 config CRYPTO_CRCT10DIF_PCLMUL
 	tristate "CRCT10DIF PCLMULQDQ hardware acceleration"
 	depends on X86 && 64BIT && CRC_T10DIF
+	depends on !KMSAN
 	select CRYPTO_HASH
 	help
 	  For x86_64 processors with SSE4.2 and PCLMULQDQ supported,

@@ -695,6 +700,7 @@ config CRYPTO_POLY1305
 config CRYPTO_POLY1305_X86_64
 	tristate "Poly1305 authenticator algorithm (x86_64/SSE2/AVX2)"
 	depends on X86 && 64BIT
+	depends on !KMSAN
 	select CRYPTO_POLY1305
 	help
 	  Poly1305 authenticator algorithm, RFC7539.

@@ -814,6 +820,7 @@ config CRYPTO_SHA1
 config CRYPTO_SHA1_SSSE3
 	tristate "SHA1 digest algorithm (SSSE3/AVX/AVX2/SHA-NI)"
 	depends on X86 && 64BIT
+	depends on !KMSAN
 	select CRYPTO_SHA1
 	select CRYPTO_HASH
 	help

@@ -825,6 +832,7 @@ config CRYPTO_SHA1_SSSE3
 config CRYPTO_SHA256_SSSE3
 	tristate "SHA256 digest algorithm (SSSE3/AVX/AVX2/SHA-NI)"
 	depends on X86 && 64BIT
+	depends on !KMSAN
 	select CRYPTO_SHA256
 	select CRYPTO_HASH
 	help

@@ -837,6 +845,7 @@ config CRYPTO_SHA256_SSSE3
 config CRYPTO_SHA512_SSSE3
 	tristate "SHA512 digest algorithm (SSSE3/AVX/AVX2)"
 	depends on X86 && 64BIT
+	depends on !KMSAN
 	select CRYPTO_SHA512
 	select CRYPTO_HASH
 	help

@@ -1011,6 +1020,7 @@ config CRYPTO_WP512
 config CRYPTO_GHASH_CLMUL_NI_INTEL
 	tristate "GHASH hash function (CLMUL-NI accelerated)"
 	depends on X86 && 64BIT
+	depends on !KMSAN
 	select CRYPTO_CRYPTD
 	help
 	  This is the x86_64 CLMUL-NI accelerated implementation of

@@ -1064,6 +1074,7 @@ config CRYPTO_AES_TI
 config CRYPTO_AES_NI_INTEL
 	tristate "AES cipher algorithms (AES-NI)"
 	depends on X86
+	depends on !KMSAN
 	select CRYPTO_AEAD
 	select CRYPTO_LIB_AES
 	select CRYPTO_ALGAPI

@@ -1190,6 +1201,7 @@ config CRYPTO_BLOWFISH_COMMON
 config CRYPTO_BLOWFISH_X86_64
 	tristate "Blowfish cipher algorithm (x86_64)"
 	depends on X86 && 64BIT
+	depends on !KMSAN
 	select CRYPTO_BLKCIPHER
 	select CRYPTO_BLOWFISH_COMMON
 	help

@@ -1221,6 +1233,7 @@ config CRYPTO_CAMELLIA_X86_64
 	tristate "Camellia cipher algorithm (x86_64)"
 	depends on X86 && 64BIT
 	depends on CRYPTO
+	depends on !KMSAN
 	select CRYPTO_BLKCIPHER
 	select CRYPTO_GLUE_HELPER_X86
 	help

@@ -1238,6 +1251,7 @@ config CRYPTO_CAMELLIA_AESNI_AVX_X86_64
 	tristate "Camellia cipher algorithm (x86_64/AES-NI/AVX)"
 	depends on X86 && 64BIT
 	depends on CRYPTO
+	depends on !KMSAN
 	select CRYPTO_BLKCIPHER
 	select CRYPTO_CAMELLIA_X86_64
 	select CRYPTO_GLUE_HELPER_X86

@@ -1258,6 +1272,7 @@ config CRYPTO_CAMELLIA_AESNI_AVX2_X86_64
 	tristate "Camellia cipher algorithm (x86_64/AES-NI/AVX2)"
 	depends on X86 && 64BIT
 	depends on CRYPTO
+	depends on !KMSAN
 	select CRYPTO_CAMELLIA_AESNI_AVX_X86_64
 	help
 	  Camellia cipher algorithm module (x86_64/AES-NI/AVX2).

@@ -1303,6 +1318,7 @@ config CRYPTO_CAST5
 config CRYPTO_CAST5_AVX_X86_64
 	tristate "CAST5 (CAST-128) cipher algorithm (x86_64/AVX)"
 	depends on X86 && 64BIT
+	depends on !KMSAN
 	select CRYPTO_BLKCIPHER
 	select CRYPTO_CAST5
 	select CRYPTO_CAST_COMMON

@@ -1325,6 +1341,7 @@ config CRYPTO_CAST6
 config CRYPTO_CAST6_AVX_X86_64
 	tristate "CAST6 (CAST-256) cipher algorithm (x86_64/AVX)"
 	depends on X86 && 64BIT
+	depends on !KMSAN
 	select CRYPTO_BLKCIPHER
 	select CRYPTO_CAST6
 	select CRYPTO_CAST_COMMON

@@ -1360,6 +1377,7 @@ config CRYPTO_DES_SPARC64
 config CRYPTO_DES3_EDE_X86_64
 	tristate "Triple DES EDE cipher algorithm (x86-64)"
 	depends on X86 && 64BIT
+	depends on !KMSAN
 	select CRYPTO_BLKCIPHER
 	select CRYPTO_LIB_DES
 	help

@@ -1426,6 +1444,7 @@ config CRYPTO_CHACHA20
 config CRYPTO_CHACHA20_X86_64
 	tristate "ChaCha stream cipher algorithms (x86_64/SSSE3/AVX2/AVX-512VL)"
 	depends on X86 && 64BIT
+	depends on !KMSAN
 	select CRYPTO_BLKCIPHER
 	select CRYPTO_CHACHA20
 	help

@@ -1462,6 +1481,7 @@ config CRYPTO_SERPENT
 config CRYPTO_SERPENT_SSE2_X86_64
 	tristate "Serpent cipher algorithm (x86_64/SSE2)"
 	depends on X86 && 64BIT
+	depends on !KMSAN
 	select CRYPTO_BLKCIPHER
 	select CRYPTO_GLUE_HELPER_X86
 	select CRYPTO_SERPENT

@@ -1481,6 +1501,7 @@ config CRYPTO_SERPENT_SSE2_X86_64
 config CRYPTO_SERPENT_SSE2_586
 	tristate "Serpent cipher algorithm (i586/SSE2)"
 	depends on X86 && !64BIT
+	depends on !KMSAN
 	select CRYPTO_BLKCIPHER
 	select CRYPTO_GLUE_HELPER_X86
 	select CRYPTO_SERPENT

@@ -1500,6 +1521,7 @@ config CRYPTO_SERPENT_SSE2_586
 config CRYPTO_SERPENT_AVX_X86_64
 	tristate "Serpent cipher algorithm (x86_64/AVX)"
 	depends on X86 && 64BIT
+	depends on !KMSAN
 	select CRYPTO_BLKCIPHER
 	select CRYPTO_GLUE_HELPER_X86
 	select CRYPTO_SERPENT

@@ -1520,6 +1542,7 @@ config CRYPTO_SERPENT_AVX_X86_64
 config CRYPTO_SERPENT_AVX2_X86_64
 	tristate "Serpent cipher algorithm (x86_64/AVX2)"
 	depends on X86 && 64BIT
+	depends on !KMSAN
 	select CRYPTO_SERPENT_AVX_X86_64
 	help
 	  Serpent cipher algorithm, by Anderson, Biham & Knudsen.

@@ -1615,6 +1638,7 @@ config CRYPTO_TWOFISH_586
 config CRYPTO_TWOFISH_X86_64
 	tristate "Twofish cipher algorithm (x86_64)"
 	depends on (X86 || UML_X86) && 64BIT
+	depends on !KMSAN
 	select CRYPTO_ALGAPI
 	select CRYPTO_TWOFISH_COMMON
 	help

@@ -1631,6 +1655,7 @@ config CRYPTO_TWOFISH_X86_64
 config CRYPTO_TWOFISH_X86_64_3WAY
 	tristate "Twofish cipher algorithm (x86_64, 3-way parallel)"
 	depends on X86 && 64BIT
+	depends on !KMSAN
 	select CRYPTO_BLKCIPHER
 	select CRYPTO_TWOFISH_COMMON
 	select CRYPTO_TWOFISH_X86_64

@@ -1652,6 +1677,7 @@ config CRYPTO_TWOFISH_X86_64_3WAY
 config CRYPTO_TWOFISH_AVX_X86_64
 	tristate "Twofish cipher algorithm (x86_64/AVX)"
 	depends on X86 && 64BIT
+	depends on !KMSAN
 	select CRYPTO_BLKCIPHER
 	select CRYPTO_GLUE_HELPER_X86
 	select CRYPTO_SIMD

drivers/gpu/drm/amd/display/Kconfig +1 -1

index 71991a28a775..99585eaf4dd0 100644
@@ -6,7 +6,7 @@ config DRM_AMD_DC
 	bool "AMD DC - Enable new display engine"
 	default y
 	select SND_HDA_COMPONENT if SND_HDA_CORE
-	select DRM_AMD_DC_DCN1_0 if X86 && !(KCOV_INSTRUMENT_ALL && KCOV_ENABLE_COMPARISONS)
+	select DRM_AMD_DC_DCN1_0 if X86 && !(KCOV_INSTRUMENT_ALL && KCOV_ENABLE_COMPARISONS) && !KMSAN
 	help
 	  Choose this option if you want to use the new display engine
 	  support for AMDGPU. This adds required support for Vega and

lib/Kconfig.debug +3 -0

index 75c36318943d..a3f6f5d68593 100644
@@ -1068,6 +1068,9 @@ menu "Lock Debugging (spinlocks, mutexes, etc...)"
 config LOCK_DEBUGGING_SUPPORT
 	bool
 	depends on TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
+	# KMSAN is incompatible with lockdep,
+	# see https://github.com/google/kmsan/issues/57.
+	depends on !KMSAN
 	default y
 
 config PROVE_LOCKING

Certain config options are currently incompatible with KMSAN, disable them.

block/blk.h +0 -0

index 47fba9362e60..9ee271a22423 100644
@@ -78,6 +78,13 @@ static inline bool biovec_phys_mergeable(struct request_queue *q,
 	phys_addr_t addr1 = page_to_phys(vec1->bv_page) + vec1->bv_offset;
 	phys_addr_t addr2 = page_to_phys(vec2->bv_page) + vec2->bv_offset;
 

In order to report uninitialized memory coming from heap allocations KMSAN has to poison them unless they're created with __GFP_ZERO.

It's handy that we need KMSAN hooks in the places where init_on_alloc/init_on_free initialization is performed.

Signed-off-by Alexander Potapenko <[email protected]>

To: Alexander Potapenko <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Vegard Nossum <[email protected]>
Cc: Dmitry Vyukov <[email protected]>
Cc: [email protected]
---

Change-Id: I51103b7981d3aabed747d0c85cbdc85568665871

---
 mm/slub.c | 37 +++++++++++++++++++++++++++++++------
 1 file changed, 31 insertions(+), 6 deletions(-)

mm/slub.c +23 -2

index b25c807a111f..8b7069812801 100644
@@ -21,6 +21,8 @@
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 #include <linux/kasan.h>
+#include <linux/kmsan.h>
+#include <linux/kmsan-checks.h> /* KMSAN_INIT_VALUE */
 #include <linux/cpu.h>
 #include <linux/cpuset.h>
 #include <linux/mempolicy.h>

@@ -285,17 +287,27 @@ static void prefetch_freepointer(const struct kmem_cache *s, void *object)
        prefetch(object + s->offset);
 }
 
+/*
+ * When running under KMSAN, get_freepointer_safe() may return an uninitialized
+ * pointer value in the case the current thread loses the race for the next
+ * memory chunk in the freelist. In that case this_cpu_cmpxchg_double() in
+ * slab_alloc_node() will fail, so the uninitialized value won't be used, but
+ * KMSAN will still check all arguments of cmpxchg because of imperfect
+ * handling of inline assembly.
+ * To work around this problem, use KMSAN_INIT_VALUE() to force initialize the
+ * return value of get_freepointer_safe().
+ */
 static inline void *get_freepointer_safe(struct kmem_cache *s, void *object)
 {
        unsigned long freepointer_addr;
        void *p;
 
        if (!debug_pagealloc_enabled())
-               return get_freepointer(s, object);
+               return KMSAN_INIT_VALUE(get_freepointer(s, object));
 
        freepointer_addr = (unsigned long)object + s->offset;
        probe_kernel_read(&p, (void **)freepointer_addr, sizeof(p));
-       return freelist_ptr(s, p, freepointer_addr);
+       return KMSAN_INIT_VALUE(freelist_ptr(s, p, freepointer_addr));
 }
 
 static inline void set_freepointer(struct kmem_cache *s, void *object, void *fp)

@@ -1390,6 +1402,7 @@ static inline void *kmalloc_large_node_hook(void *ptr, size_t size, gfp_t flags)
        ptr = kasan_kmalloc_large(ptr, size, flags);
        /* As ptr might get tagged, call kmemleak hook after KASAN. */
        kmemleak_alloc(ptr, size, 1, flags);
+       kmsan_kmalloc_large(ptr, size, flags);
        return ptr;
 }
 

@@ -1397,6 +1410,7 @@ static __always_inline void kfree_hook(void *x)
 {
        kmemleak_free(x);
        kasan_kfree_large(x, _RET_IP_);
+       kmsan_kfree_large(x);
 }
 
 static __always_inline bool slab_free_hook(struct kmem_cache *s, void *x)

@@ -1453,6 +1467,12 @@ static inline bool slab_free_freelist_hook(struct kmem_cache *s,
                } while (object != old_tail);
        }
 
+       do {
+               object = next;
+               next = get_freepointer(s, object);
+               kmsan_slab_free(s, object);
+       } while (object != old_tail);
+
 /*
  * Compiler cannot detect this function can be removed if slab_free_hook()
  * evaluates to nothing.  Thus, catch all relevant config debug options here.

@@ -2776,6 +2796,7 @@ static __always_inline void *slab_alloc_node(struct kmem_cache *s,
        if (unlikely(slab_want_init_on_alloc(gfpflags, s)) && object)
                memset(object, 0, s->object_size);
 
+       kmsan_slab_alloc(s, object, gfpflags);
        slab_post_alloc_hook(s, gfpflags, 1, &object);
 
        return object;

mm/slub.c +6 -4

        return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_node_trace);

@@ -3157,7 +3179,7 @@ int kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size,
                          void **p)
 {
        struct kmem_cache_cpu *c;
-       int i;
+       int i, j;
 
        /* memcg and kmem_cache debug support */
        s = slab_pre_alloc_hook(s, flags);

@@ -3198,11 +3220,11 @@ int kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size,
 
        /* Clear memory outside IRQ disabled fastpath loop */
        if (unlikely(slab_want_init_on_alloc(flags, s))) {
-               int j;
-
                for (j = 0; j < i; j++)
                        memset(p[j], 0, s->object_size);
        }
+       for (j = 0; j < i; j++)
+               kmsan_slab_alloc(s, p[j], flags);
 
        /* memcg and kmem_cache debug support */
        slab_post_alloc_hook(s, flags, size, p);

@@ -3803,6 +3825,7 @@ static int __init setup_slub_min_objects(char *str)
 
 __setup("slub_min_objects=", setup_slub_min_objects);
 
+__no_sanitize_memory
 void *__kmalloc(size_t size, gfp_t flags)
 {
        struct kmem_cache *s;

@@ -5717,6 +5740,7 @@ static char *create_unique_id(struct kmem_cache *s)
        p += sprintf(p, "%07u", s->size);
 
        BUG_ON(p > name + ID_STR_LENGTH - 1);
+       kmsan_unpoison_shadow(name, p - name);
        return name;
 }
 

@@ -5866,6 +5890,7 @@ static int sysfs_slab_alias(struct kmem_cache *s, const char *name)
        al->name = name;
        al->next = alias_list;
        alias_list = al;
+       kmsan_unpoison_shadow(al, sizeof(struct saved_alias));
        return 0;
 }
 
--

2.24.0.rc0.303.g954a862665-goog

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 <[email protected]>

To: Alexander Potapenko <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Wolfram Sang <[email protected]>
Cc: Petr Mladek <[email protected]>
Cc: Vegard Nossum <[email protected]>
Cc: Dmitry Vyukov <[email protected]>
Cc: [email protected]
---

v2:
- dropped call to kmsan_handle_vprintk, updated comment in printk.c

kernel/printk/printk.c +0 -0

index ca65327a6de8..4b0dbed0333a 100644

Please, separate the two comments with an empty line instead of using */ and /*.

kernel/printk/printk.c int vprintk_store(int facility, int level, · line 1923

Quoted 6 lines

kernel/printk/printk.c +6 -0

+	 * 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));

Quoted 10 lines · Petr Mladek

Please, separate the two comments with an empty line instead of using */ and /*.

Quoted 6 lines

kernel/printk/printk.c +6 -0

+    * 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));

Or maybe this can be

        text_len = vscnprintf(text, sizeof(textbuf), fmt, args);

+ KMSAN_INIT_VALUE(text_len);

        -ss

Quoted 15 lines · Sergey Senozhatsky

Quoted 10 lines · Petr Mladek

Please, separate the two comments with an empty line instead of using */ and /*.

Quoted 6 lines

kernel/printk/printk.c +6 -0

+    * 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));

Or maybe this can be

Quoted 2 lines

        text_len = vscnprintf(text, sizeof(textbuf), fmt, args);

+ KMSAN_INIT_VALUE(text_len);

This isn't how KMSAN_INIT_VALUE works. It takes a value that may be uninitialized and returns the same value marked as initialized. Shall I do:
text_len = KMSAN_INIT_VALUE(text_len);
instead?

Quoted 7 lines

Quoted 6 lines

Quoted 6 lines

kernel/printk/printk.c +6 -0

+    * 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));

Quoted 4 lines

Or maybe this can be

        text_len = vscnprintf(text, sizeof(textbuf), fmt, args);

+ KMSAN_INIT_VALUE(text_len);

This isn't how KMSAN_INIT_VALUE works. It takes a value that may be uninitialized and returns the same value marked as initialized.

Shall I do:
text_len = KMSAN_INIT_VALUE(text_len);
instead?