kcsan: Make KCSAN compatible with lockdep
authorMarco Elver <elver@google.com>
Wed, 15 Jan 2020 16:25:12 +0000 (17:25 +0100)
committerIngo Molnar <mingo@kernel.org>
Sat, 21 Mar 2020 08:41:16 +0000 (09:41 +0100)
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: Qian Cai <cai@lca.pw>
Signed-off-by: Marco Elver <elver@google.com>
Acked-by: Alexander Potapenko <glider@google.com>
Tested-by: Qian Cai <cai@lca.pw>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
kernel/kcsan/core.c
kernel/kcsan/report.c
kernel/locking/Makefile

index 87bf857c889358fcf4343b6c44bcd5fb5ad78765..64b30f7716a12a4f45aeb40e572d2c8a4166886c 100644 (file)
@@ -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
         *      detection point of view) to simply disable preemptions to ensure
         *      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);
        if (watchpoint == NULL) {
@@ -429,7 +431,7 @@ kcsan_setup_watchpoint(const volatile void *ptr, size_t size, int type)
 
        kcsan_counter_dec(KCSAN_COUNTER_USED_WATCHPOINTS);
 out_unlock:
-       local_irq_restore(irq_flags);
+       raw_local_irq_restore(irq_flags);
 out:
        user_access_restore(ua_flags);
 }
index b5b4feea49de5ba837da76d1580c52291533759b..33bdf8b229b531934f81ce82d18736a3e4cd4cda 100644 (file)
@@ -2,6 +2,7 @@
 
 #include <linux/jiffies.h>
 #include <linux/kernel.h>
+#include <linux/lockdep.h>
 #include <linux/preempt.h>
 #include <linux/printk.h>
 #include <linux/sched.h>
@@ -410,6 +411,14 @@ void kcsan_report(const volatile void *ptr, size_t size, int access_type,
 {
        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();
        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)
@@ -418,4 +427,6 @@ void kcsan_report(const volatile void *ptr, size_t size, int access_type,
                release_report(&flags, type);
        }
        kcsan_enable_current();
+
+       lockdep_on();
 }
index 45452facff3b43f15ca0b41b31a7303d656bc605..6d11cfb9b41f28ebab4cc0b980a7a42b7c3b358c 100644 (file)
@@ -5,6 +5,9 @@ KCOV_INSTRUMENT         := n
 
 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
 CFLAGS_REMOVE_lockdep.o = $(CC_FLAGS_FTRACE)
 CFLAGS_REMOVE_lockdep_proc.o = $(CC_FLAGS_FTRACE)