All of lore.kernel.org
 help / color / mirror / Atom feed
From: paulmck@kernel.org
To: linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com,
	kernel-team@fb.com, mingo@kernel.org
Cc: elver@google.com, andreyknvl@google.com, glider@google.com,
	dvyukov@google.com, cai@lca.pw, boqun.feng@gmail.com,
	"Paul E . McKenney" <paulmck@kernel.org>
Subject: [PATCH kcsan 02/32] kcsan: Show full access type in report
Date: Mon,  9 Mar 2020 12:03:50 -0700	[thread overview]
Message-ID: <20200309190420.6100-2-paulmck@kernel.org> (raw)
In-Reply-To: <20200309190359.GA5822@paulmck-ThinkPad-P72>

From: Marco Elver <elver@google.com>

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>
---
 kernel/kcsan/core.c   | 15 ++++++++-------
 kernel/kcsan/kcsan.h  |  2 +-
 kernel/kcsan/report.c | 43 ++++++++++++++++++++++++++++---------------
 3 files changed, 37 insertions(+), 23 deletions(-)

diff --git a/kernel/kcsan/core.c b/kernel/kcsan/core.c
index 4d4ab5c..87bf857 100644
--- a/kernel/kcsan/core.c
+++ b/kernel/kcsan/core.c
@@ -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 ===================================================== */
diff --git a/kernel/kcsan/kcsan.h b/kernel/kcsan/kcsan.h
index d3b9a96..8492da4 100644
--- a/kernel/kcsan/kcsan.h
+++ b/kernel/kcsan/kcsan.h
@@ -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 */
diff --git a/kernel/kcsan/report.c b/kernel/kcsan/report.c
index 0eea05a..9f503ca 100644
--- a/kernel/kcsan/report.c
+++ b/kernel/kcsan/report.c
@@ -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 @@ static bool prepare_report(unsigned long *flags, const volatile void *ptr,
 
 		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 @@ static bool prepare_report(unsigned long *flags, const volatile void *ptr,
 	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);
-- 
2.9.5


  parent reply	other threads:[~2020-03-09 19:06 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-09 19:03 [PATCH kcsan 0/32] KCSAN commits for v5.7 Paul E. McKenney
2020-03-09 19:03 ` [PATCH kcsan 01/32] kcsan: Prefer __always_inline for fast-path paulmck
2020-03-09 19:03 ` paulmck [this message]
2020-03-09 19:03 ` [PATCH kcsan 03/32] kcsan: Rate-limit reporting per data races paulmck
2020-03-09 19:03 ` [PATCH kcsan 04/32] kcsan: Make KCSAN compatible with lockdep paulmck
2020-03-09 19:03 ` [PATCH kcsan 05/32] kcsan: Address missing case with KCSAN_REPORT_VALUE_CHANGE_ONLY paulmck
2020-03-09 19:03 ` [PATCH kcsan 06/32] include/linux: Add instrumented.h infrastructure paulmck
2020-03-09 19:03 ` [PATCH kcsan 07/32] asm-generic, atomic-instrumented: Use generic instrumented.h paulmck
2020-03-09 19:03 ` [PATCH kcsan 08/32] asm-generic, kcsan: Add KCSAN instrumentation for bitops paulmck
2020-03-09 19:03 ` [PATCH kcsan 09/32] iov_iter: Use generic instrumented.h paulmck
2020-03-09 19:03 ` [PATCH kcsan 10/32] copy_to_user, copy_from_user: " paulmck
2020-03-09 19:03 ` [PATCH kcsan 11/32] kcsan: Add docbook header for data_race() paulmck
2020-03-09 19:04 ` [PATCH kcsan 12/32] kcsan: Add option to assume plain aligned writes up to word size are atomic paulmck
2020-03-09 19:04 ` [PATCH kcsan 13/32] kcsan: Clarify Kconfig option KCSAN_IGNORE_ATOMICS paulmck
2020-03-09 19:04 ` [PATCH kcsan 14/32] kcsan: Cleanup of main KCSAN Kconfig option paulmck
2020-03-09 19:04 ` [PATCH kcsan 15/32] kcsan: Fix 0-sized checks paulmck
2020-03-09 19:04 ` [PATCH kcsan 16/32] kcsan: Introduce KCSAN_ACCESS_ASSERT access type paulmck
2020-03-09 19:04 ` [PATCH kcsan 17/32] kcsan: Introduce ASSERT_EXCLUSIVE_* macros paulmck
2020-03-13  8:52   ` Boqun Feng
2020-03-13 16:15     ` Marco Elver
2020-03-14  2:22       ` Boqun Feng
2020-03-17 11:12         ` Marco Elver
2020-03-19  3:23           ` Boqun Feng
2020-03-20 14:49             ` Marco Elver
2020-03-09 19:04 ` [PATCH kcsan 18/32] kcsan: Add test to generate conflicts via debugfs paulmck
2020-03-09 19:04 ` [PATCH kcsan 19/32] kcsan: Expose core configuration parameters as module params paulmck
2020-03-09 19:04 ` [PATCH kcsan 20/32] kcsan: Fix misreporting if concurrent races on same address paulmck
2020-03-09 19:04 ` [PATCH kcsan 21/32] kcsan: Move interfaces that affects checks to kcsan-checks.h paulmck
2020-03-09 19:04 ` [PATCH kcsan 22/32] compiler.h, seqlock.h: Remove unnecessary kcsan.h includes paulmck
2020-03-09 19:04 ` [PATCH kcsan 23/32] kcsan: Introduce kcsan_value_change type paulmck
2020-03-09 19:04 ` [PATCH kcsan 24/32] kcsan: Add kcsan_set_access_mask() support paulmck
2020-03-09 19:04 ` [PATCH kcsan 25/32] kcsan: Introduce ASSERT_EXCLUSIVE_BITS(var, mask) paulmck
2020-03-09 19:04 ` [PATCH kcsan 26/32] kcsan, trace: Make KCSAN compatible with tracing paulmck
2020-03-09 19:57   ` Steven Rostedt
2020-03-09 20:27     ` Paul E. McKenney
2020-03-09 19:04 ` [PATCH kcsan 27/32] kcsan: Add option to allow watcher interruptions paulmck
2020-03-12 18:03   ` Paul E. McKenney
2020-03-12 18:04     ` Paul E. McKenney
2020-03-13 15:28       ` Marco Elver
2020-03-16 13:56         ` Marco Elver
2020-03-16 15:45           ` Paul E. McKenney
2020-03-16 16:22             ` Marco Elver
2020-03-17 17:13               ` Paul E. McKenney
2020-03-17 17:44                 ` Marco Elver
2020-03-18 17:42           ` Marco Elver
2020-03-09 19:04 ` [PATCH kcsan 28/32] kcsan: Add option for verbose reporting paulmck
2020-03-09 19:04 ` [PATCH kcsan 29/32] kcsan: Add current->state to implicitly atomic accesses paulmck
2020-03-09 19:04 ` [PATCH kcsan 30/32] kcsan: Fix a typo in a comment paulmck
2020-03-09 19:04 ` [PATCH kcsan 31/32] kcsan: Update Documentation/dev-tools/kcsan.rst paulmck
2020-03-09 19:04 ` [PATCH kcsan 32/32] kcsan: Update API documentation in kcsan-checks.h paulmck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200309190420.6100-2-paulmck@kernel.org \
    --to=paulmck@kernel.org \
    --cc=andreyknvl@google.com \
    --cc=boqun.feng@gmail.com \
    --cc=cai@lca.pw \
    --cc=dvyukov@google.com \
    --cc=elver@google.com \
    --cc=glider@google.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.