2022-11-28 09:56:26

by Alexander Potapenko

[permalink] [raw]
Subject: [PATCH 1/2] lockdep: allow instrumenting lockdep.c with KMSAN

Lockdep and KMSAN used to play badly together, causing deadlocks when
KMSAN instrumentation of lockdep.c called lockdep functions recursively.

Looks like this is no more the case, and a kernel can run (yet slower)
with both KMSAN and lockdep enabled.
This patch should fix false positives on wq_head->lock->dep_map, which
KMSAN used to consider uninitialized because of lockdep.c not being
instrumented.

Link: https://lore.kernel.org/lkml/[email protected]/
Reported-by: Eric Biggers <[email protected]>
Signed-off-by: Alexander Potapenko <[email protected]>
---
kernel/locking/Makefile | 1 -
1 file changed, 1 deletion(-)

diff --git a/kernel/locking/Makefile b/kernel/locking/Makefile
index ea925731fa40f..0db4093d17b8a 100644
--- a/kernel/locking/Makefile
+++ b/kernel/locking/Makefile
@@ -7,7 +7,6 @@ obj-y += mutex.o semaphore.o rwsem.o percpu-rwsem.o

# Avoid recursion lockdep -> sanitizer -> ... -> lockdep.
KCSAN_SANITIZE_lockdep.o := n
-KMSAN_SANITIZE_lockdep.o := n

ifdef CONFIG_FUNCTION_TRACER
CFLAGS_REMOVE_lockdep.o = $(CC_FLAGS_FTRACE)
--
2.38.1.584.g0f3c55d4c2-goog


2022-11-28 10:05:18

by Alexander Potapenko

[permalink] [raw]
Subject: [PATCH 2/2] kmsan: allow using __msan_instrument_asm_store() inside runtime

In certain cases (e.g. when handling a softirq)
__msan_instrument_asm_store(&var, sizeof(var)) may be called with
from within KMSAN runtime, but later the value of @var is used
with !kmsan_in_runtime(), leading to false positives.

Because kmsan_internal_unpoison_memory() doesn't take locks, it should
be fine to call it without kmsan_in_runtime() checks, which fixes the
mentioned false positives.

Signed-off-by: Alexander Potapenko <[email protected]>
---
mm/kmsan/instrumentation.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/mm/kmsan/instrumentation.c b/mm/kmsan/instrumentation.c
index 271f135f97a16..770fe02904f36 100644
--- a/mm/kmsan/instrumentation.c
+++ b/mm/kmsan/instrumentation.c
@@ -81,12 +81,16 @@ DECLARE_METADATA_PTR_GETTER(8);
* Handle a memory store performed by inline assembly. KMSAN conservatively
* attempts to unpoison the outputs of asm() directives to prevent false
* positives caused by missed stores.
+ *
+ * __msan_instrument_asm_store() may be called for inline assembly code when
+ * entering or leaving IRQ. We omit the check for kmsan_in_runtime() to ensure
+ * the memory written to in these cases is also marked as initialized.
*/
void __msan_instrument_asm_store(void *addr, uintptr_t size)
{
unsigned long ua_flags;

- if (!kmsan_enabled || kmsan_in_runtime())
+ if (!kmsan_enabled)
return;

ua_flags = user_access_save();
@@ -103,10 +107,8 @@ void __msan_instrument_asm_store(void *addr, uintptr_t size)
user_access_restore(ua_flags);
return;
}
- kmsan_enter_runtime();
/* Unpoisoning the memory on best effort. */
kmsan_internal_unpoison_memory(addr, size, /*checked*/ false);
- kmsan_leave_runtime();
user_access_restore(ua_flags);
}
EXPORT_SYMBOL(__msan_instrument_asm_store);
--
2.38.1.584.g0f3c55d4c2-goog