Commit f1bc9621 authored by Marco Elver's avatar Marco Elver Committed by Ingo Molnar

kcsan: Make KCSAN compatible with lockdep

We must avoid any recursion into lockdep if KCSAN is enabled on utilities
used by lockdep. One manifestation of this is corruption of lockdep's
IRQ trace state (if TRACE_IRQFLAGS), resulting in spurious warnings
(see below).  This commit fixes this by:

1. Using raw_local_irq{save,restore} in kcsan_setup_watchpoint().
2. Disabling lockdep in kcsan_report().

Tested with:

  CONFIG_LOCKDEP=y
  CONFIG_DEBUG_LOCKDEP=y
  CONFIG_TRACE_IRQFLAGS=y

This fix eliminates spurious warnings such as the following one:

    WARNING: CPU: 0 PID: 2 at kernel/locking/lockdep.c:4406 check_flags.part.0+0x101/0x220
    Modules linked in:
    CPU: 0 PID: 2 Comm: kthreadd Not tainted 5.5.0-rc1+ #11
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
    RIP: 0010:check_flags.part.0+0x101/0x220
    <snip>
    Call Trace:
     lock_is_held_type+0x69/0x150
     freezer_fork+0x20b/0x370
     cgroup_post_fork+0x2c9/0x5c0
     copy_process+0x2675/0x3b40
     _do_fork+0xbe/0xa30
     ? _raw_spin_unlock_irqrestore+0x40/0x50
     ? match_held_lock+0x56/0x250
     ? kthread_park+0xf0/0xf0
     kernel_thread+0xa6/0xd0
     ? kthread_park+0xf0/0xf0
     kthreadd+0x321/0x3d0
     ? kthread_create_on_cpu+0x130/0x130
     ret_from_fork+0x3a/0x50
    irq event stamp: 64
    hardirqs last  enabled at (63): [<ffffffff9a7995d0>] _raw_spin_unlock_irqrestore+0x40/0x50
    hardirqs last disabled at (64): [<ffffffff992a96d2>] kcsan_setup_watchpoint+0x92/0x460
    softirqs last  enabled at (32): [<ffffffff990489b8>] fpu__copy+0xe8/0x470
    softirqs last disabled at (30): [<ffffffff99048939>] fpu__copy+0x69/0x470
Reported-by: default avatarQian Cai <cai@lca.pw>
Signed-off-by: default avatarMarco Elver <elver@google.com>
Acked-by: default avatarAlexander Potapenko <glider@google.com>
Tested-by: default avatarQian Cai <cai@lca.pw>
Signed-off-by: default avatarPaul E. McKenney <paulmck@kernel.org>
Signed-off-by: default avatarIngo Molnar <mingo@kernel.org>
parent 05f9a406
...@@ -336,8 +336,10 @@ kcsan_setup_watchpoint(const volatile void *ptr, size_t size, int type) ...@@ -336,8 +336,10 @@ kcsan_setup_watchpoint(const volatile void *ptr, size_t size, int type)
* CPU-local data accesses), it makes more sense (from a data race * CPU-local data accesses), it makes more sense (from a data race
* detection point of view) to simply disable preemptions to ensure * detection point of view) to simply disable preemptions to ensure
* as many tasks as possible run on other CPUs. * as many tasks as possible run on other CPUs.
*
* Use raw versions, to avoid lockdep recursion via IRQ flags tracing.
*/ */
local_irq_save(irq_flags); raw_local_irq_save(irq_flags);
watchpoint = insert_watchpoint((unsigned long)ptr, size, is_write); watchpoint = insert_watchpoint((unsigned long)ptr, size, is_write);
if (watchpoint == NULL) { if (watchpoint == NULL) {
...@@ -429,7 +431,7 @@ kcsan_setup_watchpoint(const volatile void *ptr, size_t size, int type) ...@@ -429,7 +431,7 @@ kcsan_setup_watchpoint(const volatile void *ptr, size_t size, int type)
kcsan_counter_dec(KCSAN_COUNTER_USED_WATCHPOINTS); kcsan_counter_dec(KCSAN_COUNTER_USED_WATCHPOINTS);
out_unlock: out_unlock:
local_irq_restore(irq_flags); raw_local_irq_restore(irq_flags);
out: out:
user_access_restore(ua_flags); user_access_restore(ua_flags);
} }
......
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
#include <linux/jiffies.h> #include <linux/jiffies.h>
#include <linux/kernel.h> #include <linux/kernel.h>
#include <linux/lockdep.h>
#include <linux/preempt.h> #include <linux/preempt.h>
#include <linux/printk.h> #include <linux/printk.h>
#include <linux/sched.h> #include <linux/sched.h>
...@@ -410,6 +411,14 @@ void kcsan_report(const volatile void *ptr, size_t size, int access_type, ...@@ -410,6 +411,14 @@ void kcsan_report(const volatile void *ptr, size_t size, int access_type,
{ {
unsigned long flags = 0; unsigned long flags = 0;
/*
* With TRACE_IRQFLAGS, lockdep's IRQ trace state becomes corrupted if
* we do not turn off lockdep here; this could happen due to recursion
* into lockdep via KCSAN if we detect a data race in utilities used by
* lockdep.
*/
lockdep_off();
kcsan_disable_current(); kcsan_disable_current();
if (prepare_report(&flags, ptr, size, access_type, cpu_id, type)) { if (prepare_report(&flags, ptr, size, access_type, cpu_id, type)) {
if (print_report(ptr, size, access_type, value_change, cpu_id, type) && panic_on_warn) if (print_report(ptr, size, access_type, value_change, cpu_id, type) && panic_on_warn)
...@@ -418,4 +427,6 @@ void kcsan_report(const volatile void *ptr, size_t size, int access_type, ...@@ -418,4 +427,6 @@ void kcsan_report(const volatile void *ptr, size_t size, int access_type,
release_report(&flags, type); release_report(&flags, type);
} }
kcsan_enable_current(); kcsan_enable_current();
lockdep_on();
} }
...@@ -5,6 +5,9 @@ KCOV_INSTRUMENT := n ...@@ -5,6 +5,9 @@ KCOV_INSTRUMENT := n
obj-y += mutex.o semaphore.o rwsem.o percpu-rwsem.o obj-y += mutex.o semaphore.o rwsem.o percpu-rwsem.o
# Avoid recursion lockdep -> KCSAN -> ... -> lockdep.
KCSAN_SANITIZE_lockdep.o := n
ifdef CONFIG_FUNCTION_TRACER ifdef CONFIG_FUNCTION_TRACER
CFLAGS_REMOVE_lockdep.o = $(CC_FLAGS_FTRACE) CFLAGS_REMOVE_lockdep.o = $(CC_FLAGS_FTRACE)
CFLAGS_REMOVE_lockdep_proc.o = $(CC_FLAGS_FTRACE) CFLAGS_REMOVE_lockdep_proc.o = $(CC_FLAGS_FTRACE)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment