kcsan: Show full access type in report
authorMarco Elver <elver@google.com>
Fri, 10 Jan 2020 18:48:33 +0000 (19:48 +0100)
committerIngo Molnar <mingo@kernel.org>
Sat, 21 Mar 2020 08:40:42 +0000 (09:40 +0100)
This commit adds access-type information to KCSAN's reports as follows:
"read", "read (marked)", "write", and "write (marked)".

Suggested-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Marco Elver <elver@google.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
kernel/kcsan/core.c
kernel/kcsan/kcsan.h
kernel/kcsan/report.c

index 4d4ab5c5dc53d2c77e2fc9257d50501d2e22f4dd..87bf857c889358fcf4343b6c44bcd5fb5ad78765 100644 (file)
@@ -255,7 +255,7 @@ static inline unsigned int get_delay(void)
 
 static noinline void kcsan_found_watchpoint(const volatile void *ptr,
                                            size_t size,
-                                           bool is_write,
+                                           int type,
                                            atomic_long_t *watchpoint,
                                            long encoded_watchpoint)
 {
@@ -276,7 +276,7 @@ static noinline void kcsan_found_watchpoint(const volatile void *ptr,
        flags = user_access_save();
 
        if (consumed) {
-               kcsan_report(ptr, size, is_write, true, raw_smp_processor_id(),
+               kcsan_report(ptr, size, type, true, raw_smp_processor_id(),
                             KCSAN_REPORT_CONSUMED_WATCHPOINT);
        } else {
                /*
@@ -292,8 +292,9 @@ static noinline void kcsan_found_watchpoint(const volatile void *ptr,
 }
 
 static noinline void
-kcsan_setup_watchpoint(const volatile void *ptr, size_t size, bool is_write)
+kcsan_setup_watchpoint(const volatile void *ptr, size_t size, int type)
 {
+       const bool is_write = (type & KCSAN_ACCESS_WRITE) != 0;
        atomic_long_t *watchpoint;
        union {
                u8 _1;
@@ -415,13 +416,13 @@ kcsan_setup_watchpoint(const volatile void *ptr, size_t size, bool is_write)
                 * No need to increment 'data_races' counter, as the racing
                 * thread already did.
                 */
-               kcsan_report(ptr, size, is_write, size > 8 || value_change,
+               kcsan_report(ptr, size, type, size > 8 || value_change,
                             smp_processor_id(), KCSAN_REPORT_RACE_SIGNAL);
        } else if (value_change) {
                /* Inferring a race, since the value should not have changed. */
                kcsan_counter_inc(KCSAN_COUNTER_RACES_UNKNOWN_ORIGIN);
                if (IS_ENABLED(CONFIG_KCSAN_REPORT_RACE_UNKNOWN_ORIGIN))
-                       kcsan_report(ptr, size, is_write, true,
+                       kcsan_report(ptr, size, type, true,
                                     smp_processor_id(),
                                     KCSAN_REPORT_RACE_UNKNOWN_ORIGIN);
        }
@@ -455,10 +456,10 @@ static __always_inline void check_access(const volatile void *ptr, size_t size,
         */
 
        if (unlikely(watchpoint != NULL))
-               kcsan_found_watchpoint(ptr, size, is_write, watchpoint,
+               kcsan_found_watchpoint(ptr, size, type, watchpoint,
                                       encoded_watchpoint);
        else if (unlikely(should_watch(ptr, type)))
-               kcsan_setup_watchpoint(ptr, size, is_write);
+               kcsan_setup_watchpoint(ptr, size, type);
 }
 
 /* === Public interface ===================================================== */
index d3b9a96ac8a43bd503f1e1da11537a2e0abdece5..8492da45494bf9b61bd7dab0e7bd37fe7dac134c 100644 (file)
@@ -103,7 +103,7 @@ enum kcsan_report_type {
 /*
  * Print a race report from thread that encountered the race.
  */
-extern void kcsan_report(const volatile void *ptr, size_t size, bool is_write,
+extern void kcsan_report(const volatile void *ptr, size_t size, int access_type,
                         bool value_change, int cpu_id, enum kcsan_report_type type);
 
 #endif /* _KERNEL_KCSAN_KCSAN_H */
index 0eea05a3135b7f2f7acf86a2f4d03be13f9336a3..9f503ca2ff7a33f8b50a3cd737c7fa0d9ba0f8d9 100644 (file)
@@ -24,7 +24,7 @@
 static struct {
        const volatile void     *ptr;
        size_t                  size;
-       bool                    is_write;
+       int                     access_type;
        int                     task_pid;
        int                     cpu_id;
        unsigned long           stack_entries[NUM_STACK_ENTRIES];
@@ -41,8 +41,10 @@ static DEFINE_SPINLOCK(report_lock);
  * Special rules to skip reporting.
  */
 static bool
-skip_report(bool is_write, bool value_change, unsigned long top_frame)
+skip_report(int access_type, bool value_change, unsigned long top_frame)
 {
+       const bool is_write = (access_type & KCSAN_ACCESS_WRITE) != 0;
+
        if (IS_ENABLED(CONFIG_KCSAN_REPORT_VALUE_CHANGE_ONLY) && is_write &&
            !value_change) {
                /*
@@ -63,9 +65,20 @@ skip_report(bool is_write, bool value_change, unsigned long top_frame)
        return kcsan_skip_report_debugfs(top_frame);
 }
 
-static inline const char *get_access_type(bool is_write)
+static const char *get_access_type(int type)
 {
-       return is_write ? "write" : "read";
+       switch (type) {
+       case 0:
+               return "read";
+       case KCSAN_ACCESS_ATOMIC:
+               return "read (marked)";
+       case KCSAN_ACCESS_WRITE:
+               return "write";
+       case KCSAN_ACCESS_WRITE | KCSAN_ACCESS_ATOMIC:
+               return "write (marked)";
+       default:
+               BUG();
+       }
 }
 
 /* Return thread description: in task or interrupt. */
@@ -112,7 +125,7 @@ static int sym_strcmp(void *addr1, void *addr2)
 /*
  * Returns true if a report was generated, false otherwise.
  */
-static bool print_report(const volatile void *ptr, size_t size, bool is_write,
+static bool print_report(const volatile void *ptr, size_t size, int access_type,
                         bool value_change, int cpu_id,
                         enum kcsan_report_type type)
 {
@@ -124,7 +137,7 @@ static bool print_report(const volatile void *ptr, size_t size, bool is_write,
        /*
         * Must check report filter rules before starting to print.
         */
-       if (skip_report(is_write, true, stack_entries[skipnr]))
+       if (skip_report(access_type, true, stack_entries[skipnr]))
                return false;
 
        if (type == KCSAN_REPORT_RACE_SIGNAL) {
@@ -132,7 +145,7 @@ static bool print_report(const volatile void *ptr, size_t size, bool is_write,
                                                other_info.num_stack_entries);
 
                /* @value_change is only known for the other thread */
-               if (skip_report(other_info.is_write, value_change,
+               if (skip_report(other_info.access_type, value_change,
                                other_info.stack_entries[other_skipnr]))
                        return false;
        }
@@ -170,7 +183,7 @@ static bool print_report(const volatile void *ptr, size_t size, bool is_write,
        switch (type) {
        case KCSAN_REPORT_RACE_SIGNAL:
                pr_err("%s to 0x%px of %zu bytes by %s on cpu %i:\n",
-                      get_access_type(other_info.is_write), other_info.ptr,
+                      get_access_type(other_info.access_type), other_info.ptr,
                       other_info.size, get_thread_desc(other_info.task_pid),
                       other_info.cpu_id);
 
@@ -181,14 +194,14 @@ static bool print_report(const volatile void *ptr, size_t size, bool is_write,
 
                pr_err("\n");
                pr_err("%s to 0x%px of %zu bytes by %s on cpu %i:\n",
-                      get_access_type(is_write), ptr, size,
+                      get_access_type(access_type), ptr, size,
                       get_thread_desc(in_task() ? task_pid_nr(current) : -1),
                       cpu_id);
                break;
 
        case KCSAN_REPORT_RACE_UNKNOWN_ORIGIN:
                pr_err("race at unknown origin, with %s to 0x%px of %zu bytes by %s on cpu %i:\n",
-                      get_access_type(is_write), ptr, size,
+                      get_access_type(access_type), ptr, size,
                       get_thread_desc(in_task() ? task_pid_nr(current) : -1),
                       cpu_id);
                break;
@@ -223,7 +236,7 @@ static void release_report(unsigned long *flags, enum kcsan_report_type type)
  * required for the report type, simply acquires report_lock and returns true.
  */
 static bool prepare_report(unsigned long *flags, const volatile void *ptr,
-                          size_t size, bool is_write, int cpu_id,
+                          size_t size, int access_type, int cpu_id,
                           enum kcsan_report_type type)
 {
        if (type != KCSAN_REPORT_CONSUMED_WATCHPOINT &&
@@ -243,7 +256,7 @@ retry:
 
                other_info.ptr                  = ptr;
                other_info.size                 = size;
-               other_info.is_write             = is_write;
+               other_info.access_type          = access_type;
                other_info.task_pid             = in_task() ? task_pid_nr(current) : -1;
                other_info.cpu_id               = cpu_id;
                other_info.num_stack_entries    = stack_trace_save(other_info.stack_entries, NUM_STACK_ENTRIES, 1);
@@ -302,14 +315,14 @@ retry:
        goto retry;
 }
 
-void kcsan_report(const volatile void *ptr, size_t size, bool is_write,
+void kcsan_report(const volatile void *ptr, size_t size, int access_type,
                  bool value_change, int cpu_id, enum kcsan_report_type type)
 {
        unsigned long flags = 0;
 
        kcsan_disable_current();
-       if (prepare_report(&flags, ptr, size, is_write, cpu_id, type)) {
-               if (print_report(ptr, size, is_write, value_change, cpu_id, type) && panic_on_warn)
+       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)
                        panic("panic_on_warn set ...\n");
 
                release_report(&flags, type);