Add KernelMemorySanitizer infrastructure

Petr Mladek suggested we use
this_cpu_read(printk_context) & PRINTK_SAFE_CONTEXT_MASK
instead of checking the spinlock status in kmsan_pr_err()

This appears to be less intrusive, although we'll need to declare printk_context in some printk header.

Quoted 3 lines · On Fri 2019-10-18 11:42:43, [email protected] wrote:

Petr Mladek suggested we use
this_cpu_read(printk_context) & PRINTK_SAFE_CONTEXT_MASK
instead of checking the spinlock status in kmsan_pr_err()

I would like to understand why the check is needed.

My guess is that it prevents a infinite recursion. Is this the case? It might be possible to debug this using trace_printk().

It is important. If it is the recursion then there are two possibilities how to prevent it. Either prevent calling the recursive printk(). Or prevent calling the memory checker recursive. I am not sure what makes more sense.

Is printk() the only piece of code where you need to avoid recursion? I wonder how it is prevented in the other cases.

Quoted 2 lines

This appears to be less intrusive, although we'll need to declare printk_context in some printk header.

It is easier than the original approach. But the main motivation is that it is more reliable. The spinlock is a global lock. But it seems that it is enough to check the state of the local CPU.

Finally, could you please CC me to the patch(es) that are using this variable? I would actually prefer to be in CC of entire patchsets so that I see the full context.

Best Regards,
Petr

Quoted 11 lines · Petr Mladek

Quoted 3 lines · On Fri 2019-10-18 11:42:43, [email protected] wrote:

Petr Mladek suggested we use
this_cpu_read(printk_context) & PRINTK_SAFE_CONTEXT_MASK
instead of checking the spinlock status in kmsan_pr_err()

I would like to understand why the check is needed.

My guess is that it prevents a infinite recursion. Is this the case? It might be possible to debug this using trace_printk().

This indeed used to prevent infinite recursion, however looks like the check isn't needed anymore.
When I remove it, KMSAN doesn't hang, probably because printk is doing a better job at deadlock prevention.

What I'm seeing now is that e.g. in the following case:
ptr = kmalloc(sizeof(int), GFP_KERNEL);
if (ptr)

    pr_info("true\n");
  else
    pr_info("false\n");

KMSAN detects errors within pr_info(), namely in vprintk_store(). If I understand correctly, printing from that point causes printk to use the per-cpu buffer, which is flushed once we're done with the original printing:

[   58.984971][ T8390] BUG: KMSAN: uninit-value in
kmsan_handle_vprintk+0xa0/0xf0
...
[   59.061976][    C0] BUG: KMSAN: uninit-value in vsnprintf+0x3276/0x32b0
...
[   59.062457][    C0] BUG: KMSAN: uninit-value in format_decode+0x17f/0x1900
...
[   59.062961][    C0] BUG: KMSAN: uninit-value in format_decode+0x17f/0x1900
...
[   59.063338][    C0] Lost 6207 message(s)!

So it turns out we'd better disable reporting under logbuf_lock, otherwise these lost messages will confuse the developers. Because the first pr_info() isn't called by KMSAN itself, the tool still needs a way to know it's running inside printk.

Quoted 20 lines

It is important. If it is the recursion then there are two possibilities how to prevent it. Either prevent calling the recursive printk(). Or prevent calling the memory checker recursive. I am not sure what makes more sense.

Is printk() the only piece of code where you need to avoid recursion? I wonder how it is prevented in the other cases.

Quoted 2 lines

This appears to be less intrusive, although we'll need to declare printk_context in some printk header.

It is easier than the original approach. But the main motivation is that it is more reliable. The spinlock is a global lock. But it seems that it is enough to check the state of the local CPU.

Finally, could you please CC me to the patch(es) that are using this variable? I would actually prefer to be in CC of entire patchsets so that I see the full context.

Quoted 2 lines

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

Quoted 8 lines · Petr Mladek

Quoted 3 lines · On Fri 2019-10-18 11:42:43, [email protected] wrote:

Petr Mladek suggested we use
this_cpu_read(printk_context) & PRINTK_SAFE_CONTEXT_MASK
instead of checking the spinlock status in kmsan_pr_err()

I would like to understand why the check is needed.

Quoted 3 lines

My guess is that it prevents a infinite recursion. Is this the case? It might be possible to debug this using trace_printk().

This indeed used to prevent infinite recursion, however looks like the check isn't needed anymore.
When I remove it, KMSAN doesn't hang, probably because printk is doing a better job at deadlock prevention.

What I'm seeing now is that e.g. in the following case:
ptr = kmalloc(sizeof(int), GFP_KERNEL);
if (ptr)

    pr_info("true\n");
  else
    pr_info("false\n");

KMSAN detects errors within pr_info(), namely in vprintk_store(). If I understand correctly, printing from that point causes printk to use the per-cpu buffer, which is flushed once we're done with the original printing:

[   58.984971][ T8390] BUG: KMSAN: uninit-value in
kmsan_handle_vprintk+0xa0/0xf0
...
[   59.061976][    C0] BUG: KMSAN: uninit-value in vsnprintf+0x3276/0x32b0
...
[   59.062457][    C0] BUG: KMSAN: uninit-value in format_decode+0x17f/0x1900
...
[   59.062961][    C0] BUG: KMSAN: uninit-value in format_decode+0x17f/0x1900

...

[   59.063338][    C0] Lost 6207 message(s)!

So it turns out we'd better disable reporting under logbuf_lock, otherwise these lost messages will confuse the developers. Because the first pr_info() isn't called by KMSAN itself, the tool still needs a way to know it's running inside printk.

Quoted 9 lines · Alexander Potapenko

Quoted 8 lines · Petr Mladek

Quoted 3 lines · On Fri 2019-10-18 11:42:43, [email protected] wrote:

Petr Mladek suggested we use
this_cpu_read(printk_context) & PRINTK_SAFE_CONTEXT_MASK
instead of checking the spinlock status in kmsan_pr_err()

I would like to understand why the check is needed.

Quoted 7 lines

Quoted 3 lines

My guess is that it prevents a infinite recursion. Is this the case? It might be possible to debug this using trace_printk().

This indeed used to prevent infinite recursion, however looks like the check isn't needed anymore.
When I remove it, KMSAN doesn't hang, probably because printk is doing a better job at deadlock prevention.

Great.

Quoted 12 lines

What I'm seeing now is that e.g. in the following case:
ptr = kmalloc(sizeof(int), GFP_KERNEL);
if (ptr)

    pr_info("true\n");
  else
    pr_info("false\n");

KMSAN detects errors within pr_info(), namely in vprintk_store(). If I understand correctly, printing from that point causes printk to use the per-cpu buffer, which is flushed once we're done with the original printing:

Quoted 8 lines

[   58.984971][ T8390] BUG: KMSAN: uninit-value in
kmsan_handle_vprintk+0xa0/0xf0
...
[   59.061976][    C0] BUG: KMSAN: uninit-value in vsnprintf+0x3276/0x32b0
...
[   59.062457][    C0] BUG: KMSAN: uninit-value in format_decode+0x17f/0x1900
...
[   59.062961][    C0] BUG: KMSAN: uninit-value in format_decode+0x17f/0x1900

Please, do you have an explanation where the uninitialized values come from? Is it a false positive? Or is there really a bug how the printk() parameters are passed?

Quoted 2 lines

...

[   59.063338][    C0] Lost 6207 message(s)!

This message comes from report_message_lost(). It means that many recursive messages did not fit into the limited per-CPU buffers.

Quoted 4 lines

So it turns out we'd better disable reporting under logbuf_lock, otherwise these lost messages will confuse the developers. Because the first pr_info() isn't called by KMSAN itself, the tool still needs a way to know it's running inside printk.

OK. Well, I still would like to understand why the recursive messages are printed at all. Is it a bug in the printk() code or a false positive?

There should be no recursive messages if we fix the bug or make some annotations to prevent the false positives.

That said, I am not completely against disabling the recursive messages. But it would be pity to hide some real bugs.

Best Regards,
Petr

Quoted 22 lines · Petr Mladek

Quoted 20 lines · Alexander Potapenko

What I'm seeing now is that e.g. in the following case:
ptr = kmalloc(sizeof(int), GFP_KERNEL);
if (ptr)

    pr_info("true\n");
  else
    pr_info("false\n");

KMSAN detects errors within pr_info(), namely in vprintk_store(). If I understand correctly, printing from that point causes printk to use the per-cpu buffer, which is flushed once we're done with the original printing:

[   58.984971][ T8390] BUG: KMSAN: uninit-value in
kmsan_handle_vprintk+0xa0/0xf0
...
[   59.061976][    C0] BUG: KMSAN: uninit-value in vsnprintf+0x3276/0x32b0
...
[   59.062457][    C0] BUG: KMSAN: uninit-value in format_decode+0x17f/0x1900
...
[   59.062961][    C0] BUG: KMSAN: uninit-value in format_decode+0x17f/0x1900

Quoted 3 lines

Please, do you have an explanation where the uninitialized values come from? Is it a false positive? Or is there really a bug how the printk() parameters are passed?

I believe these are true bugs.
The problem is, when we pass an uninitialized argument into printk(), KMSAN will report an error every time this uninitialized argument is used.

E.g. for an uninitialized format string there'll be at least strlen(fmt) reports in format_decode(), followed by several reports in vsnprintf().
Although these reports seem to be real, printing only the first of them should be more than enough.

In the future we'll actually want KMSAN to check every printk() argument (which will require parsing the format string to figure out the arguments' lengths), but disable reports within printk.

Quoted 23 lines · Alexander Potapenko

Quoted 22 lines · Petr Mladek

Quoted 20 lines · Alexander Potapenko

What I'm seeing now is that e.g. in the following case:
ptr = kmalloc(sizeof(int), GFP_KERNEL);
if (ptr)

    pr_info("true\n");
  else
    pr_info("false\n");

KMSAN detects errors within pr_info(), namely in vprintk_store(). If I understand correctly, printing from that point causes printk to use the per-cpu buffer, which is flushed once we're done with the original printing:

[   58.984971][ T8390] BUG: KMSAN: uninit-value in
kmsan_handle_vprintk+0xa0/0xf0
...
[   59.061976][    C0] BUG: KMSAN: uninit-value in vsnprintf+0x3276/0x32b0
...
[   59.062457][    C0] BUG: KMSAN: uninit-value in format_decode+0x17f/0x1900
...
[   59.062961][    C0] BUG: KMSAN: uninit-value in format_decode+0x17f/0x1900

Quoted 7 lines

Quoted 3 lines

Please, do you have an explanation where the uninitialized values come from? Is it a false positive? Or is there really a bug how the printk() parameters are passed?

I believe these are true bugs.
The problem is, when we pass an uninitialized argument into printk(), KMSAN will report an error every time this uninitialized argument is used.

I see, thanks for explanation.

Quoted 5 lines

E.g. for an uninitialized format string there'll be at least strlen(fmt) reports in format_decode(), followed by several reports in vsnprintf().
Although these reports seem to be real, printing only the first of them should be more than enough.

Isn't this a generic problem? I mean that uninitialized values can be passed and used also in many other locations.

printk() is special because this problem causes infinite loop. But it would make sense to report also any other problematic value only once.

Quoted 4 lines

In the future we'll actually want KMSAN to check every printk() argument (which will require parsing the format string to figure out the arguments' lengths), but disable reports within printk.

What is the motivation for this, please?

It looks to me that you want to do very paranoid checks of variables passed to printk()? Do you want to prevent printk() crashes? Or do you want to make sure that printk() produces correct values?

From my POV, printk() is debugging tool. It is used to show values that people are interested in. On one hand, it might make sense to warn people that a particular value was not initalized. On the other hand, printk() is not important for the kernel behavior. It just reads values and does not affect any behavior.

I would like to understand how many printk-related code is worth the effort.

Best Regards,
Petr

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

Signed-off-by Alexander Potapenko <[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

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 +31 -6

index 3d63ae320d31..3d6d4c63446e 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.

@@ -2769,6 +2789,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;

@@ -2797,6 +2818,7 @@ void *kmem_cache_alloc_trace(struct kmem_cache *s, gfp_t gfpflags, size_t size)
 	void *ret = slab_alloc(s, gfpflags, _RET_IP_);
 	trace_kmalloc(_RET_IP_, ret, size, s->size, gfpflags);
 	ret = kasan_kmalloc(s, ret, size, gfpflags);
+
 	return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_trace);

@@ -2809,7 +2831,6 @@ void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t gfpflags, int node)
 
 	trace_kmem_cache_alloc_node(_RET_IP_, ret,
 				    s->object_size, s->size, gfpflags, node);
-
 	return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_node);

@@ -2825,6 +2846,7 @@ void *kmem_cache_alloc_node_trace(struct kmem_cache *s,
 			   size, s->size, gfpflags, node);
 
 	ret = kasan_kmalloc(s, ret, size, gfpflags);
+
 	return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_node_trace);

@@ -3150,7 +3172,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);

@@ -3188,11 +3210,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);

@@ -3793,6 +3815,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;

@@ -5698,6 +5721,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;
 }
 

@@ -5847,6 +5871,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;
 }
 

Quoted 5 lines · On Fri, 2019-10-18 at 11:42 +0200, [email protected] wrote:

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.

Well, there is SLUB debug which has red zoning and poisoning checks. What's value of this patch?

Quoted 165 lines

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 +31 -6

index 3d63ae320d31..3d6d4c63446e 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.

@@ -2769,6 +2789,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;

@@ -2797,6 +2818,7 @@ void *kmem_cache_alloc_trace(struct kmem_cache *s, gfp_t gfpflags, size_t size)
      void *ret = slab_alloc(s, gfpflags, _RET_IP_);
      trace_kmalloc(_RET_IP_, ret, size, s->size, gfpflags);
      ret = kasan_kmalloc(s, ret, size, gfpflags);
+
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_trace);

@@ -2809,7 +2831,6 @@ void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t gfpflags, int node)
 
      trace_kmem_cache_alloc_node(_RET_IP_, ret,
                                  s->object_size, s->size, gfpflags, node);
-
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_node);

@@ -2825,6 +2846,7 @@ void *kmem_cache_alloc_node_trace(struct kmem_cache *s,
                         size, s->size, gfpflags, node);
 
      ret = kasan_kmalloc(s, ret, size, gfpflags);
+
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_node_trace);

@@ -3150,7 +3172,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);

@@ -3188,11 +3210,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);

@@ -3793,6 +3815,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;

@@ -5698,6 +5721,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;
 }
 

@@ -5847,6 +5871,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;
 }
 

Quoted 5 lines · Qian Cai

Quoted 3 lines · On Fri, 2019-10-18 at 11:42 +0200, [email protected] wrote:

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

Quoted 5 lines

Quoted 2 lines

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

Well, there is SLUB debug which has red zoning and poisoning checks. What's value of this patch?

Sorry, are you talking about the whole patchset or just this patch?

Note that SLUB debug is unable to detect uninitialized values with bit-to-bit precision, neither have I heard of anyone using it for detecting uses of uninitialized memory in the kernel at all. The purpose of SLUB debug is to detect corruptions of freed memory.

Quoted 165 lines

Quoted 165 lines

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 +31 -6

index 3d63ae320d31..3d6d4c63446e 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.

@@ -2769,6 +2789,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;

@@ -2797,6 +2818,7 @@ void *kmem_cache_alloc_trace(struct kmem_cache *s, gfp_t gfpflags, size_t size)
      void *ret = slab_alloc(s, gfpflags, _RET_IP_);
      trace_kmalloc(_RET_IP_, ret, size, s->size, gfpflags);
      ret = kasan_kmalloc(s, ret, size, gfpflags);
+
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_trace);

@@ -2809,7 +2831,6 @@ void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t gfpflags, int node)
 
      trace_kmem_cache_alloc_node(_RET_IP_, ret,
                                  s->object_size, s->size, gfpflags, node);
-
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_node);

@@ -2825,6 +2846,7 @@ void *kmem_cache_alloc_node_trace(struct kmem_cache *s,
                         size, s->size, gfpflags, node);
 
      ret = kasan_kmalloc(s, ret, size, gfpflags);
+
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_node_trace);

@@ -3150,7 +3172,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);

@@ -3188,11 +3210,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);

@@ -3793,6 +3815,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;

@@ -5698,6 +5721,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;
 }
 

@@ -5847,6 +5871,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;
 }
 

Quoted 6 lines · Alexander Potapenko

Quoted 5 lines · Qian Cai

Quoted 3 lines · On Fri, 2019-10-18 at 11:42 +0200, [email protected] wrote:

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

Quoted 7 lines

Quoted 5 lines

Quoted 2 lines

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

Well, there is SLUB debug which has red zoning and poisoning checks. What's value of this patch?

Sorry, are you talking about the whole patchset or just this patch?

Just this patch.

Quoted 4 lines

Note that SLUB debug is unable to detect uninitialized values with bit-to-bit precision, neither have I heard of anyone using it for detecting uses of uninitialized memory in the kernel at all. The purpose of SLUB debug is to detect corruptions of freed memory.

The point is if developers have SLUB debug enabled, all the free objects will be poisoned, so what's the point of checking uninitialized memory there?

Quoted 168 lines

Quoted 165 lines

Quoted 165 lines

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 +31 -6

index 3d63ae320d31..3d6d4c63446e 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.

@@ -2769,6 +2789,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;

@@ -2797,6 +2818,7 @@ void *kmem_cache_alloc_trace(struct kmem_cache *s, gfp_t gfpflags, size_t size)
      void *ret = slab_alloc(s, gfpflags, _RET_IP_);
      trace_kmalloc(_RET_IP_, ret, size, s->size, gfpflags);
      ret = kasan_kmalloc(s, ret, size, gfpflags);
+
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_trace);

@@ -2809,7 +2831,6 @@ void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t gfpflags, int node)
 
      trace_kmem_cache_alloc_node(_RET_IP_, ret,
                                  s->object_size, s->size, gfpflags, node);
-
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_node);

@@ -2825,6 +2846,7 @@ void *kmem_cache_alloc_node_trace(struct kmem_cache *s,
                         size, s->size, gfpflags, node);
 
      ret = kasan_kmalloc(s, ret, size, gfpflags);
+
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_node_trace);

@@ -3150,7 +3172,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);

@@ -3188,11 +3210,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);

@@ -3793,6 +3815,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;

@@ -5698,6 +5721,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;
 }
 

@@ -5847,6 +5871,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;
 }
 

Quoted 23 lines · Qian Cai

Quoted 6 lines · Alexander Potapenko

Quoted 5 lines · Qian Cai

Quoted 3 lines · On Fri, 2019-10-18 at 11:42 +0200, [email protected] wrote:

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

Quoted 7 lines

Quoted 5 lines

Quoted 2 lines

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

Well, there is SLUB debug which has red zoning and poisoning checks. What's value of this patch?

Sorry, are you talking about the whole patchset or just this patch?

Just this patch.

Quoted 4 lines

Note that SLUB debug is unable to detect uninitialized values with bit-to-bit precision, neither have I heard of anyone using it for detecting uses of uninitialized memory in the kernel at all. The purpose of SLUB debug is to detect corruptions of freed memory.

Quoted 2 lines

The point is if developers have SLUB debug enabled, all the free objects will be poisoned, so what's the point of checking uninitialized memory there?

You are right, SLUB debug has to be handled separately. If I'm understanding correctly, right now KMSAN poisons freed memory before SLUB debug wipes it, therefore the memory will count as initialized. On the other hand, newly allocated memory is still marked as uninitialized, so a lot of bugs still remain detectable.

TBH, I haven't tested KMSAN with SLUB debug good enough. Note that it's anyway a separate build that requires Clang, so right now it doesn't make much sense to combine KMSAN and SLUB debug together.

Quoted 168 lines

Quoted 168 lines

Quoted 165 lines

Quoted 165 lines

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 +31 -6

index 3d63ae320d31..3d6d4c63446e 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.

@@ -2769,6 +2789,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;

@@ -2797,6 +2818,7 @@ void *kmem_cache_alloc_trace(struct kmem_cache *s, gfp_t gfpflags, size_t size)
      void *ret = slab_alloc(s, gfpflags, _RET_IP_);
      trace_kmalloc(_RET_IP_, ret, size, s->size, gfpflags);
      ret = kasan_kmalloc(s, ret, size, gfpflags);
+
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_trace);

@@ -2809,7 +2831,6 @@ void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t gfpflags, int node)
 
      trace_kmem_cache_alloc_node(_RET_IP_, ret,
                                  s->object_size, s->size, gfpflags, node);
-
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_node);

@@ -2825,6 +2846,7 @@ void *kmem_cache_alloc_node_trace(struct kmem_cache *s,
                         size, s->size, gfpflags, node);
 
      ret = kasan_kmalloc(s, ret, size, gfpflags);
+
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_node_trace);

@@ -3150,7 +3172,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);

@@ -3188,11 +3210,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);

@@ -3793,6 +3815,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;

@@ -5698,6 +5721,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;
 }
 

@@ -5847,6 +5871,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;
 }
 

Quoted 27 lines · Alexander Potapenko

Quoted 25 lines · Qian Cai

Quoted 6 lines · Alexander Potapenko

Quoted 5 lines · Qian Cai

Quoted 3 lines · On Fri, 2019-10-18 at 11:42 +0200, [email protected] wrote:

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

Quoted 7 lines

Quoted 5 lines

Quoted 2 lines

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

Well, there is SLUB debug which has red zoning and poisoning checks. What's value of this patch?

Sorry, are you talking about the whole patchset or just this patch?

Just this patch.

Quoted 4 lines

Note that SLUB debug is unable to detect uninitialized values with bit-to-bit precision, neither have I heard of anyone using it for detecting uses of uninitialized memory in the kernel at all. The purpose of SLUB debug is to detect corruptions of freed memory.

The point is if developers have SLUB debug enabled, all the free objects will be poisoned, so what's the point of checking uninitialized memory there?

Quoted 5 lines

You are right, SLUB debug has to be handled separately. If I'm understanding correctly, right now KMSAN poisons freed memory before SLUB debug wipes it, therefore the memory will count as initialized. On the other hand, newly allocated memory is still marked as uninitialized, so a lot of bugs still remain detectable.

Yes, but newly allocated slub objects will be poisoned as well.

Quoted 3 lines

TBH, I haven't tested KMSAN with SLUB debug good enough. Note that it's anyway a separate build that requires Clang, so right now it doesn't make much sense to combine KMSAN and SLUB debug together.

Can't you just build a debug kernel with SLUB debug enabled but dropping this patch? If there is an uninitialized memory here leading to data corruption, SLUB debug should be detected as well as this patch. If not, it needs to understand why.

Quoted 172 lines

Quoted 169 lines

Quoted 169 lines

Quoted 166 lines

Quoted 166 lines

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 +31 -6

index 3d63ae320d31..3d6d4c63446e 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.

@@ -2769,6 +2789,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;

@@ -2797,6 +2818,7 @@ void *kmem_cache_alloc_trace(struct kmem_cache *s, gfp_t gfpflags, size_t size)
      void *ret = slab_alloc(s, gfpflags, _RET_IP_);
      trace_kmalloc(_RET_IP_, ret, size, s->size, gfpflags);
      ret = kasan_kmalloc(s, ret, size, gfpflags);
+
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_trace);

@@ -2809,7 +2831,6 @@ void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t gfpflags, int node)
 
      trace_kmem_cache_alloc_node(_RET_IP_, ret,
                                  s->object_size, s->size, gfpflags, node);
-
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_node);

@@ -2825,6 +2846,7 @@ void *kmem_cache_alloc_node_trace(struct kmem_cache *s,
                         size, s->size, gfpflags, node);
 
      ret = kasan_kmalloc(s, ret, size, gfpflags);
+
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_node_trace);

@@ -3150,7 +3172,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);

@@ -3188,11 +3210,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);

@@ -3793,6 +3815,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;

@@ -5698,6 +5721,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;
 }
 

@@ -5847,6 +5871,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;
 }
 

Quoted 29 lines · Qian Cai

Quoted 27 lines · Alexander Potapenko

Quoted 25 lines · Qian Cai

Quoted 6 lines · Alexander Potapenko

Quoted 5 lines · Qian Cai

Quoted 3 lines · On Fri, 2019-10-18 at 11:42 +0200, [email protected] wrote:

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

Quoted 7 lines

Quoted 5 lines

Quoted 2 lines

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

Well, there is SLUB debug which has red zoning and poisoning checks. What's value of this patch?

Sorry, are you talking about the whole patchset or just this patch?

Just this patch.

Quoted 4 lines

Note that SLUB debug is unable to detect uninitialized values with bit-to-bit precision, neither have I heard of anyone using it for detecting uses of uninitialized memory in the kernel at all. The purpose of SLUB debug is to detect corruptions of freed memory.

The point is if developers have SLUB debug enabled, all the free objects will be poisoned, so what's the point of checking uninitialized memory there?

Quoted 7 lines

Quoted 5 lines

You are right, SLUB debug has to be handled separately. If I'm understanding correctly, right now KMSAN poisons freed memory before SLUB debug wipes it, therefore the memory will count as initialized. On the other hand, newly allocated memory is still marked as uninitialized, so a lot of bugs still remain detectable.

Yes, but newly allocated slub objects will be poisoned as well.

As far as I can tell, KMSAN hook marking newly allocated objects as uninitialized is called after slub poisoning.
Therefore these allocations will be treated by KMSAN as uninitialized.

Quoted 8 lines

Quoted 3 lines

TBH, I haven't tested KMSAN with SLUB debug good enough. Note that it's anyway a separate build that requires Clang, so right now it doesn't make much sense to combine KMSAN and SLUB debug together.

Can't you just build a debug kernel with SLUB debug enabled but dropping this patch? If there is an uninitialized memory here leading to data corruption, SLUB debug should be detected as well as this patch. If not, it needs to understand why.

Sorry, there might be some misunderstanding here.
KMSAN keeps the state of heap objects separately by keeping exactly the same amount of initialized/uninitialized bits for every allocation.
A call to kmsan_slab_alloc()/kmsan_slab_free() will mark an allocation as uninitialized for KMSAN. Not doing so will result in false reports. A call to memset(object, POISON_FREE, object_size) performed by SLAB debug will actually mark this allocation as initialized from KMSAN point of view, because we're memsetting a range with initialized data. Note that use of uninitialized data doesn't necessarily lead to immediate data corruption, so there might be nothing to detect for SLUB debug.

Quoted 172 lines

Quoted 172 lines

Quoted 169 lines

Quoted 169 lines

Quoted 166 lines

Quoted 166 lines

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 +31 -6

index 3d63ae320d31..3d6d4c63446e 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.

@@ -2769,6 +2789,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;

@@ -2797,6 +2818,7 @@ void *kmem_cache_alloc_trace(struct kmem_cache *s, gfp_t gfpflags, size_t size)
      void *ret = slab_alloc(s, gfpflags, _RET_IP_);
      trace_kmalloc(_RET_IP_, ret, size, s->size, gfpflags);
      ret = kasan_kmalloc(s, ret, size, gfpflags);
+
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_trace);

@@ -2809,7 +2831,6 @@ void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t gfpflags, int node)
 
      trace_kmem_cache_alloc_node(_RET_IP_, ret,
                                  s->object_size, s->size, gfpflags, node);
-
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_node);

@@ -2825,6 +2846,7 @@ void *kmem_cache_alloc_node_trace(struct kmem_cache *s,
                         size, s->size, gfpflags, node);
 
      ret = kasan_kmalloc(s, ret, size, gfpflags);
+
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_node_trace);

@@ -3150,7 +3172,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);

@@ -3188,11 +3210,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);

@@ -3793,6 +3815,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;

@@ -5698,6 +5721,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;
 }
 

@@ -5847,6 +5871,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;
 }
 

Quoted 50 lines · Alexander Potapenko

Quoted 29 lines · Qian Cai

Quoted 27 lines · Alexander Potapenko

Quoted 25 lines · Qian Cai

Quoted 6 lines · Alexander Potapenko

Quoted 5 lines · Qian Cai

Quoted 3 lines · On Fri, 2019-10-18 at 11:42 +0200, [email protected] wrote:

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

Quoted 7 lines

Quoted 5 lines

Quoted 2 lines

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

Well, there is SLUB debug which has red zoning and poisoning checks. What's value of this patch?

Sorry, are you talking about the whole patchset or just this patch?

Just this patch.

Quoted 4 lines

Note that SLUB debug is unable to detect uninitialized values with bit-to-bit precision, neither have I heard of anyone using it for detecting uses of uninitialized memory in the kernel at all. The purpose of SLUB debug is to detect corruptions of freed memory.

The point is if developers have SLUB debug enabled, all the free objects will be poisoned, so what's the point of checking uninitialized memory there?

Quoted 7 lines

Quoted 5 lines

You are right, SLUB debug has to be handled separately. If I'm understanding correctly, right now KMSAN poisons freed memory before SLUB debug wipes it, therefore the memory will count as initialized. On the other hand, newly allocated memory is still marked as uninitialized, so a lot of bugs still remain detectable.

Yes, but newly allocated slub objects will be poisoned as well.

As far as I can tell, KMSAN hook marking newly allocated objects as uninitialized is called after slub poisoning.
Therefore these allocations will be treated by KMSAN as uninitialized.

Quoted 8 lines

Quoted 3 lines

TBH, I haven't tested KMSAN with SLUB debug good enough. Note that it's anyway a separate build that requires Clang, so right now it doesn't make much sense to combine KMSAN and SLUB debug together.

Can't you just build a debug kernel with SLUB debug enabled but dropping this patch? If there is an uninitialized memory here leading to data corruption, SLUB debug should be detected as well as this patch. If not, it needs to understand why.

Quoted 12 lines

Sorry, there might be some misunderstanding here.
KMSAN keeps the state of heap objects separately by keeping exactly the same amount of initialized/uninitialized bits for every allocation.
A call to kmsan_slab_alloc()/kmsan_slab_free() will mark an allocation as uninitialized for KMSAN. Not doing so will result in false reports. A call to memset(object, POISON_FREE, object_size) performed by SLAB debug will actually mark this allocation as initialized from KMSAN point of view, because we're memsetting a range with initialized data. Note that use of uninitialized data doesn't necessarily lead to immediate data corruption, so there might be nothing to detect for SLUB debug.

Well, SLUB debug would mark any access of uninitialized memory as data corruption with the help of poisoning and red zoning.

It is true that KMSAN might be doing a bit more than SLUB debug here, but the question is if it is worth the maintenance burden? Do you have any existing bugs to show that KMSAN would find by this patch but SLUB debug can't?

Quoted 175 lines

Quoted 172 lines

Quoted 172 lines

Quoted 169 lines

Quoted 169 lines

Quoted 166 lines

Quoted 166 lines

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 +31 -6

index 3d63ae320d31..3d6d4c63446e 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.

@@ -2769,6 +2789,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;

@@ -2797,6 +2818,7 @@ void *kmem_cache_alloc_trace(struct kmem_cache *s, gfp_t gfpflags, size_t size)
      void *ret = slab_alloc(s, gfpflags, _RET_IP_);
      trace_kmalloc(_RET_IP_, ret, size, s->size, gfpflags);
      ret = kasan_kmalloc(s, ret, size, gfpflags);
+
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_trace);

@@ -2809,7 +2831,6 @@ void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t gfpflags, int node)
 
      trace_kmem_cache_alloc_node(_RET_IP_, ret,
                                  s->object_size, s->size, gfpflags, node);
-
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_node);

@@ -2825,6 +2846,7 @@ void *kmem_cache_alloc_node_trace(struct kmem_cache *s,
                         size, s->size, gfpflags, node);
 
      ret = kasan_kmalloc(s, ret, size, gfpflags);
+
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_node_trace);

@@ -3150,7 +3172,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);

@@ -3188,11 +3210,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);

@@ -3793,6 +3815,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;

@@ -5698,6 +5721,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;
 }
 

@@ -5847,6 +5871,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;
 }
 

Quoted 65 lines · Qian Cai

Quoted 50 lines · Alexander Potapenko

Quoted 29 lines · Qian Cai

Quoted 27 lines · Alexander Potapenko

Quoted 25 lines · Qian Cai

Quoted 6 lines · Alexander Potapenko

Quoted 5 lines · Qian Cai

Quoted 3 lines · On Fri, 2019-10-18 at 11:42 +0200, [email protected] wrote:

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

Quoted 7 lines

Quoted 5 lines

Quoted 2 lines

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

Well, there is SLUB debug which has red zoning and poisoning checks. What's value of this patch?

Sorry, are you talking about the whole patchset or just this patch?

Just this patch.

Quoted 4 lines

Note that SLUB debug is unable to detect uninitialized values with bit-to-bit precision, neither have I heard of anyone using it for detecting uses of uninitialized memory in the kernel at all. The purpose of SLUB debug is to detect corruptions of freed memory.

The point is if developers have SLUB debug enabled, all the free objects will be poisoned, so what's the point of checking uninitialized memory there?

Quoted 7 lines

Quoted 5 lines

You are right, SLUB debug has to be handled separately. If I'm understanding correctly, right now KMSAN poisons freed memory before SLUB debug wipes it, therefore the memory will count as initialized. On the other hand, newly allocated memory is still marked as uninitialized, so a lot of bugs still remain detectable.

Yes, but newly allocated slub objects will be poisoned as well.

As far as I can tell, KMSAN hook marking newly allocated objects as uninitialized is called after slub poisoning.
Therefore these allocations will be treated by KMSAN as uninitialized.

Quoted 8 lines

Quoted 3 lines

TBH, I haven't tested KMSAN with SLUB debug good enough. Note that it's anyway a separate build that requires Clang, so right now it doesn't make much sense to combine KMSAN and SLUB debug together.

Can't you just build a debug kernel with SLUB debug enabled but dropping this patch? If there is an uninitialized memory here leading to data corruption, SLUB debug should be detected as well as this patch. If not, it needs to understand why.

Quoted 12 lines

Sorry, there might be some misunderstanding here.
KMSAN keeps the state of heap objects separately by keeping exactly the same amount of initialized/uninitialized bits for every allocation.
A call to kmsan_slab_alloc()/kmsan_slab_free() will mark an allocation as uninitialized for KMSAN. Not doing so will result in false reports. A call to memset(object, POISON_FREE, object_size) performed by SLAB debug will actually mark this allocation as initialized from KMSAN point of view, because we're memsetting a range with initialized data. Note that use of uninitialized data doesn't necessarily lead to immediate data corruption, so there might be nothing to detect for SLUB debug.

Quoted 6 lines

Well, SLUB debug would mark any access of uninitialized memory as data corruption with the help of poisoning and red zoning.

It is true that KMSAN might be doing a bit more than SLUB debug here, but the question is if it is worth the maintenance burden? Do you have any existing bugs to show that KMSAN would find by this patch but SLUB debug can't?

Consider the following case:

  int *a = kmalloc(sizeof(int), GFP_KERNEL);
  if (*a) {
    // do something, e.g. become root
  }

IIUC SLUB poisoning will result in the branch being taken, but as long as there are no invalid accesses the kernel won't crash and no errors will be reported.
KMSAN however will report an error, because the contents of the buffer pointed to by |a| are poisoned by kmsan_slab_alloc().
The compiler instrumentation will retrieve the shadow value for *a, compare it to zero and report a use of uninitialized memory.

Another example would be copying the contents of newly allocated buffer to the userspace.

Quoted 176 lines

Quoted 176 lines

Quoted 173 lines

Quoted 173 lines

Quoted 170 lines

Quoted 170 lines

Quoted 167 lines

Quoted 167 lines

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 +31 -6

index 3d63ae320d31..3d6d4c63446e 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.

@@ -2769,6 +2789,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;

@@ -2797,6 +2818,7 @@ void *kmem_cache_alloc_trace(struct kmem_cache *s, gfp_t gfpflags, size_t size)
      void *ret = slab_alloc(s, gfpflags, _RET_IP_);
      trace_kmalloc(_RET_IP_, ret, size, s->size, gfpflags);
      ret = kasan_kmalloc(s, ret, size, gfpflags);
+
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_trace);

@@ -2809,7 +2831,6 @@ void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t gfpflags, int node)
 
      trace_kmem_cache_alloc_node(_RET_IP_, ret,
                                  s->object_size, s->size, gfpflags, node);
-
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_node);

@@ -2825,6 +2846,7 @@ void *kmem_cache_alloc_node_trace(struct kmem_cache *s,
                         size, s->size, gfpflags, node);
 
      ret = kasan_kmalloc(s, ret, size, gfpflags);
+
      return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_node_trace);

@@ -3150,7 +3172,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);

@@ -3188,11 +3210,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);

@@ -3793,6 +3815,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;

@@ -5698,6 +5721,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;
 }
 

@@ -5847,6 +5871,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;
 }