]> git.proxmox.com Git - mirror_ubuntu-hirsute-kernel.git/commitdiff
kcsan: Add current->state to implicitly atomic accesses
authorMarco Elver <elver@google.com>
Tue, 25 Feb 2020 14:32:58 +0000 (15:32 +0100)
committerPaul E. McKenney <paulmck@kernel.org>
Wed, 25 Mar 2020 16:56:00 +0000 (09:56 -0700)
Add volatile current->state to list of implicitly atomic accesses. This
is in preparation to eventually enable KCSAN on kernel/sched (which
currently still has KCSAN_SANITIZE := n).

Since accesses that match the special check in atomic.h are rare, it
makes more sense to move this check to the slow-path, avoiding the
additional compare in the fast-path. With the microbenchmark, a speedup
of ~6% is measured.

Signed-off-by: Marco Elver <elver@google.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
kernel/kcsan/atomic.h
kernel/kcsan/core.c
kernel/kcsan/debugfs.c

index a9c1930534914d3fbadfeb08518554aba27f7868..be9e625227f3b543dc94111d856ebf8d7a92c317 100644 (file)
@@ -4,24 +4,17 @@
 #define _KERNEL_KCSAN_ATOMIC_H
 
 #include <linux/jiffies.h>
+#include <linux/sched.h>
 
 /*
- * Helper that returns true if access to @ptr should be considered an atomic
- * access, even though it is not explicitly atomic.
- *
- * List all volatile globals that have been observed in races, to suppress
- * data race reports between accesses to these variables.
- *
- * For now, we assume that volatile accesses of globals are as strong as atomic
- * accesses (READ_ONCE, WRITE_ONCE cast to volatile). The situation is still not
- * entirely clear, as on some architectures (Alpha) READ_ONCE/WRITE_ONCE do more
- * than cast to volatile. Eventually, we hope to be able to remove this
- * function.
+ * Special rules for certain memory where concurrent conflicting accesses are
+ * common, however, the current convention is to not mark them; returns true if
+ * access to @ptr should be considered atomic. Called from slow-path.
  */
-static __always_inline bool kcsan_is_atomic(const volatile void *ptr)
+static bool kcsan_is_atomic_special(const volatile void *ptr)
 {
-       /* only jiffies for now */
-       return ptr == &jiffies;
+       /* volatile globals that have been observed in data races. */
+       return ptr == &jiffies || ptr == &current->state;
 }
 
 #endif /* _KERNEL_KCSAN_ATOMIC_H */
index 065615df88eaa9b4a1a3a7d1efa083aa4339be83..eb30ecdc8c0092a11b3d25c614ee072f4cc2219a 100644 (file)
@@ -188,12 +188,13 @@ static __always_inline struct kcsan_ctx *get_ctx(void)
        return in_task() ? &current->kcsan_ctx : raw_cpu_ptr(&kcsan_cpu_ctx);
 }
 
+/* Rules for generic atomic accesses. Called from fast-path. */
 static __always_inline bool
 is_atomic(const volatile void *ptr, size_t size, int type)
 {
        struct kcsan_ctx *ctx;
 
-       if ((type & KCSAN_ACCESS_ATOMIC) != 0)
+       if (type & KCSAN_ACCESS_ATOMIC)
                return true;
 
        /*
@@ -201,16 +202,16 @@ is_atomic(const volatile void *ptr, size_t size, int type)
         * as atomic. This allows using them also in atomic regions, such as
         * seqlocks, without implicitly changing their semantics.
         */
-       if ((type & KCSAN_ACCESS_ASSERT) != 0)
+       if (type & KCSAN_ACCESS_ASSERT)
                return false;
 
        if (IS_ENABLED(CONFIG_KCSAN_ASSUME_PLAIN_WRITES_ATOMIC) &&
-           (type & KCSAN_ACCESS_WRITE) != 0 && size <= sizeof(long) &&
+           (type & KCSAN_ACCESS_WRITE) && size <= sizeof(long) &&
            IS_ALIGNED((unsigned long)ptr, size))
                return true; /* Assume aligned writes up to word size are atomic. */
 
        ctx = get_ctx();
-       if (unlikely(ctx->atomic_next > 0)) {
+       if (ctx->atomic_next > 0) {
                /*
                 * Because we do not have separate contexts for nested
                 * interrupts, in case atomic_next is set, we simply assume that
@@ -224,10 +225,8 @@ is_atomic(const volatile void *ptr, size_t size, int type)
                        --ctx->atomic_next; /* in task, or outer interrupt */
                return true;
        }
-       if (unlikely(ctx->atomic_nest_count > 0 || ctx->in_flat_atomic))
-               return true;
 
-       return kcsan_is_atomic(ptr);
+       return ctx->atomic_nest_count > 0 || ctx->in_flat_atomic;
 }
 
 static __always_inline bool
@@ -367,6 +366,15 @@ kcsan_setup_watchpoint(const volatile void *ptr, size_t size, int type)
        if (!kcsan_is_enabled())
                goto out;
 
+       /*
+        * Special atomic rules: unlikely to be true, so we check them here in
+        * the slow-path, and not in the fast-path in is_atomic(). Call after
+        * kcsan_is_enabled(), as we may access memory that is not yet
+        * initialized during early boot.
+        */
+       if (!is_assert && kcsan_is_atomic_special(ptr))
+               goto out;
+
        if (!check_encodable((unsigned long)ptr, size)) {
                kcsan_counter_inc(KCSAN_COUNTER_UNENCODABLE_ACCESSES);
                goto out;
index 2ff1961239778008e06b27d3d30ac585ff3d72b8..72ee188ebc54ac96126ff8bc01cd1135e5852ab3 100644 (file)
@@ -74,25 +74,34 @@ void kcsan_counter_dec(enum kcsan_counter_id id)
  */
 static noinline void microbenchmark(unsigned long iters)
 {
+       const struct kcsan_ctx ctx_save = current->kcsan_ctx;
+       const bool was_enabled = READ_ONCE(kcsan_enabled);
        cycles_t cycles;
 
+       /* We may have been called from an atomic region; reset context. */
+       memset(&current->kcsan_ctx, 0, sizeof(current->kcsan_ctx));
+       /*
+        * Disable to benchmark fast-path for all accesses, and (expected
+        * negligible) call into slow-path, but never set up watchpoints.
+        */
+       WRITE_ONCE(kcsan_enabled, false);
+
        pr_info("KCSAN: %s begin | iters: %lu\n", __func__, iters);
 
        cycles = get_cycles();
        while (iters--) {
-               /*
-                * We can run this benchmark from multiple tasks; this address
-                * calculation increases likelyhood of some accesses
-                * overlapping. Make the access type an atomic read, to never
-                * set up watchpoints and test the fast-path only.
-                */
-               unsigned long addr =
-                       iters % (CONFIG_KCSAN_NUM_WATCHPOINTS * PAGE_SIZE);
-               __kcsan_check_access((void *)addr, sizeof(long), KCSAN_ACCESS_ATOMIC);
+               unsigned long addr = iters & ((PAGE_SIZE << 8) - 1);
+               int type = !(iters & 0x7f) ? KCSAN_ACCESS_ATOMIC :
+                               (!(iters & 0xf) ? KCSAN_ACCESS_WRITE : 0);
+               __kcsan_check_access((void *)addr, sizeof(long), type);
        }
        cycles = get_cycles() - cycles;
 
        pr_info("KCSAN: %s end   | cycles: %llu\n", __func__, cycles);
+
+       WRITE_ONCE(kcsan_enabled, was_enabled);
+       /* restore context */
+       current->kcsan_ctx = ctx_save;
 }
 
 /*