[PATCH mm 00/22] kasan: report clean-ups and improvements

[PATCH mm 00/22] kasan: report clean-ups and improvements

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

A number of clean-up patches for KASAN reporting code.

Most are non-functional and only improve readability.

The patches go on top of mm.

Andrey Konovalov (22):
  kasan: drop addr check from describe_object_addr
  kasan: more line breaks in reports
  kasan: rearrange stack frame info in reports
  kasan: improve stack frame info in reports
  kasan: print basic stack frame info for SW_TAGS
  kasan: simplify async check in end_report
  kasan: simplify kasan_update_kunit_status and call sites
  kasan: check CONFIG_KASAN_KUNIT_TEST instead of CONFIG_KUNIT
  kasan: move update_kunit_status to start_report
  kasan: move disable_trace_on_warning to start_report
  kasan: split out print_report from __kasan_report
  kasan: simplify kasan_find_first_bad_addr call sites
  kasan: restructure kasan_report
  kasan: merge __kasan_report into kasan_report
  kasan: call print_report from kasan_report_invalid_free
  kasan: move and simplify kasan_report_async
  kasan: rename kasan_access_info to kasan_report_info
  kasan: add comment about UACCESS regions to kasan_report
  kasan: respect KASAN_BIT_REPORTED in all reporting routines
  kasan: reorder reporting functions
  kasan: move and hide kasan_save_enable/restore_multi_shot
  kasan: disable LOCKDEP when printing reports

 include/linux/kasan.h     |   4 -
 mm/kasan/kasan.h          |  44 ++++--
 mm/kasan/report.c         | 312 ++++++++++++++++++++++----------------
 mm/kasan/report_generic.c |  34 ++---
 mm/kasan/report_hw_tags.c |   1 +
 mm/kasan/report_sw_tags.c |  15 ++
 mm/kasan/report_tags.c    |   2 +-
 7 files changed, 241 insertions(+), 171 deletions(-)

-- 
2.25.1

[PATCH mm 01/22] kasan: drop addr check from describe_object_addr

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

describe_object_addr() used to be called with NULL addr in the early
days of KASAN. This no longer happens, so drop the check.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/kasan/report.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index f64352008bb8..607a8c2e4674 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -162,9 +162,6 @@ static void describe_object_addr(struct kmem_cache *cache, void *object,
 	       " which belongs to the cache %s of size %d\n",
 		object, cache->name, cache->object_size);
 
-	if (!addr)
-		return;
-
 	if (access_addr < object_addr) {
 		rel_type = "to the left";
 		rel_bytes = object_addr - access_addr;
-- 
2.25.1

[PATCH mm 02/22] kasan: more line breaks in reports

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

Add a line break after each part that describes the buggy address.
Improves readability of reports.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/kasan/report.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 607a8c2e4674..ded648c0a0e4 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -250,11 +250,13 @@ static void print_address_description(void *addr, u8 tag)
 		void *object = nearest_obj(cache, slab,	addr);
 
 		describe_object(cache, object, addr, tag);
+		pr_err("\n");
 	}
 
 	if (kernel_or_module_addr(addr) && !init_task_stack_addr(addr)) {
 		pr_err("The buggy address belongs to the variable:\n");
 		pr_err(" %pS\n", addr);
+		pr_err("\n");
 	}
 
 	if (is_vmalloc_addr(addr)) {
@@ -265,6 +267,7 @@ static void print_address_description(void *addr, u8 tag)
 			       " [%px, %px) created by:\n"
 			       " %pS\n",
 			       va->addr, va->addr + va->size, va->caller);
+			pr_err("\n");
 
 			page = vmalloc_to_page(page);
 		}
@@ -273,9 +276,11 @@ static void print_address_description(void *addr, u8 tag)
 	if (page) {
 		pr_err("The buggy address belongs to the physical page:\n");
 		dump_page(page, "kasan: bad access detected");
+		pr_err("\n");
 	}
 
 	kasan_print_address_stack_frame(addr);
+	pr_err("\n");
 }
 
 static bool meta_row_is_guilty(const void *row, const void *addr)
@@ -382,7 +387,6 @@ void kasan_report_invalid_free(void *object, unsigned long ip)
 	kasan_print_tags(tag, object);
 	pr_err("\n");
 	print_address_description(object, tag);
-	pr_err("\n");
 	print_memory_metadata(object);
 	end_report(&flags, (unsigned long)object);
 }
@@ -443,7 +447,6 @@ static void __kasan_report(unsigned long addr, size_t size, bool is_write,
 
 	if (addr_has_metadata(untagged_addr)) {
 		print_address_description(untagged_addr, get_tag(tagged_addr));
-		pr_err("\n");
 		print_memory_metadata(info.first_bad_addr);
 	} else {
 		dump_stack_lvl(KERN_ERR);
-- 
2.25.1

[PATCH mm 03/22] kasan: rearrange stack frame info in reports

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

- Move printing stack frame info before printing page info.

- Add object_is_on_stack() check to print_address_description()
  and add a corresponding WARNING to kasan_print_address_stack_frame().
  This looks more in line with the rest of the checks in this function
  and also allows to avoid complicating code logic wrt line breaks.

- Clean up comments related to get_address_stack_frame_info().

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/kasan/report.c         | 12 +++++++++---
 mm/kasan/report_generic.c | 15 ++++-----------
 2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index ded648c0a0e4..d60ee8b81e2b 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -259,6 +259,15 @@ static void print_address_description(void *addr, u8 tag)
 		pr_err("\n");
 	}
 
+	if (object_is_on_stack(addr)) {
+		/*
+		 * Currently, KASAN supports printing frame information only
+		 * for accesses to the task's own stack.
+		 */
+		kasan_print_address_stack_frame(addr);
+		pr_err("\n");
+	}
+
 	if (is_vmalloc_addr(addr)) {
 		struct vm_struct *va = find_vm_area(addr);
 
@@ -278,9 +287,6 @@ static void print_address_description(void *addr, u8 tag)
 		dump_page(page, "kasan: bad access detected");
 		pr_err("\n");
 	}
-
-	kasan_print_address_stack_frame(addr);
-	pr_err("\n");
 }
 
 static bool meta_row_is_guilty(const void *row, const void *addr)
diff --git a/mm/kasan/report_generic.c b/mm/kasan/report_generic.c
index 139615ef326b..3751391ff11a 100644
--- a/mm/kasan/report_generic.c
+++ b/mm/kasan/report_generic.c
@@ -211,6 +211,7 @@ static void print_decoded_frame_descr(const char *frame_descr)
 	}
 }
 
+/* Returns true only if the address is on the current task's stack. */
 static bool __must_check get_address_stack_frame_info(const void *addr,
 						      unsigned long *offset,
 						      const char **frame_descr,
@@ -224,13 +225,6 @@ static bool __must_check get_address_stack_frame_info(const void *addr,
 
 	BUILD_BUG_ON(IS_ENABLED(CONFIG_STACK_GROWSUP));
 
-	/*
-	 * NOTE: We currently only support printing frame information for
-	 * accesses to the task's own stack.
-	 */
-	if (!object_is_on_stack(addr))
-		return false;
-
 	aligned_addr = round_down((unsigned long)addr, sizeof(long));
 	mem_ptr = round_down(aligned_addr, KASAN_GRANULE_SIZE);
 	shadow_ptr = kasan_mem_to_shadow((void *)aligned_addr);
@@ -269,14 +263,13 @@ void kasan_print_address_stack_frame(const void *addr)
 	const char *frame_descr;
 	const void *frame_pc;
 
+	if (WARN_ON(!object_is_on_stack(addr)))
+		return;
+
 	if (!get_address_stack_frame_info(addr, &offset, &frame_descr,
 					  &frame_pc))
 		return;
 
-	/*
-	 * get_address_stack_frame_info only returns true if the given addr is
-	 * on the current task's stack.
-	 */
 	pr_err("\n");
 	pr_err("addr %px is located in stack of task %s/%d at offset %lu in frame:\n",
 	       addr, current->comm, task_pid_nr(current), offset);
-- 
2.25.1

[PATCH mm 04/22] kasan: improve stack frame info in reports

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

- Print at least task name and id for reports affecting allocas
  (get_address_stack_frame_info() does not support them).

- Capitalize first letter of each sentence.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/kasan/report_generic.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/mm/kasan/report_generic.c b/mm/kasan/report_generic.c
index 3751391ff11a..7e03cca569a7 100644
--- a/mm/kasan/report_generic.c
+++ b/mm/kasan/report_generic.c
@@ -180,7 +180,7 @@ static void print_decoded_frame_descr(const char *frame_descr)
 		return;
 
 	pr_err("\n");
-	pr_err("this frame has %lu %s:\n", num_objects,
+	pr_err("This frame has %lu %s:\n", num_objects,
 	       num_objects == 1 ? "object" : "objects");
 
 	while (num_objects--) {
@@ -266,13 +266,14 @@ void kasan_print_address_stack_frame(const void *addr)
 	if (WARN_ON(!object_is_on_stack(addr)))
 		return;
 
+	pr_err("The buggy address belongs to stack of task %s/%d\n",
+	       current->comm, task_pid_nr(current));
+
 	if (!get_address_stack_frame_info(addr, &offset, &frame_descr,
 					  &frame_pc))
 		return;
 
-	pr_err("\n");
-	pr_err("addr %px is located in stack of task %s/%d at offset %lu in frame:\n",
-	       addr, current->comm, task_pid_nr(current), offset);
+	pr_err(" and is located at offset %lu in frame:\n", offset);
 	pr_err(" %pS\n", frame_pc);
 
 	if (!frame_descr)
-- 
2.25.1

[PATCH mm 05/22] kasan: print basic stack frame info for SW_TAGS

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

Software Tag-Based mode tags stack allocations when CONFIG_KASAN_STACK
is enabled. Print task name and id in reports for stack-related bugs.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/kasan/kasan.h          |  2 +-
 mm/kasan/report_sw_tags.c | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index d1e111b7d5d8..4447df0d7343 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -274,7 +274,7 @@ void *kasan_find_first_bad_addr(void *addr, size_t size);
 const char *kasan_get_bug_type(struct kasan_access_info *info);
 void kasan_metadata_fetch_row(char *buffer, void *row);
 
-#if defined(CONFIG_KASAN_GENERIC) && defined(CONFIG_KASAN_STACK)
+#if defined(CONFIG_KASAN_STACK)
 void kasan_print_address_stack_frame(const void *addr);
 #else
 static inline void kasan_print_address_stack_frame(const void *addr) { }
diff --git a/mm/kasan/report_sw_tags.c b/mm/kasan/report_sw_tags.c
index d2298c357834..44577b8d47a7 100644
--- a/mm/kasan/report_sw_tags.c
+++ b/mm/kasan/report_sw_tags.c
@@ -51,3 +51,14 @@ void kasan_print_tags(u8 addr_tag, const void *addr)
 
 	pr_err("Pointer tag: [%02x], memory tag: [%02x]\n", addr_tag, *shadow);
 }
+
+#ifdef CONFIG_KASAN_STACK
+void kasan_print_address_stack_frame(const void *addr)
+{
+	if (WARN_ON(!object_is_on_stack(addr)))
+		return;
+
+	pr_err("The buggy address belongs to stack of task %s/%d\n",
+	       current->comm, task_pid_nr(current));
+}
+#endif
-- 
2.25.1

[PATCH mm 06/22] kasan: simplify async check in end_report

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

Currently, end_report() does not call trace_error_report_end() for bugs
detected in either async or asymm mode (when kasan_async_fault_possible()
returns true), as the address of the bad access might be unknown.

However, for asymm mode, the address is known for faults triggered by
read operations.

Instead of using kasan_async_fault_possible(), simply check that
the addr is not NULL when calling trace_error_report_end().

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/kasan/report.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index d60ee8b81e2b..2d892ec050be 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -112,7 +112,7 @@ static void start_report(unsigned long *flags)
 
 static void end_report(unsigned long *flags, unsigned long addr)
 {
-	if (!kasan_async_fault_possible())
+	if (addr)
 		trace_error_report_end(ERROR_DETECTOR_KASAN, addr);
 	pr_err("==================================================================\n");
 	add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE);
-- 
2.25.1

[PATCH mm 07/22] kasan: simplify kasan_update_kunit_status and call sites

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

- Rename kasan_update_kunit_status() to update_kunit_status()
  (the function is static).

- Move the IS_ENABLED(CONFIG_KUNIT) to the function's
  definition instead of duplicating it at call sites.

- Obtain and check current->kunit_test within the function.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/kasan/report.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 2d892ec050be..59db81211b8a 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -357,24 +357,31 @@ static bool report_enabled(void)
 }
 
 #if IS_ENABLED(CONFIG_KUNIT)
-static void kasan_update_kunit_status(struct kunit *cur_test, bool sync)
+static void update_kunit_status(bool sync)
 {
+	struct kunit *test;
 	struct kunit_resource *resource;
 	struct kunit_kasan_status *status;
 
-	resource = kunit_find_named_resource(cur_test, "kasan_status");
+	test = current->kunit_test;
+	if (!test)
+		return;
 
+	resource = kunit_find_named_resource(test, "kasan_status");
 	if (!resource) {
-		kunit_set_failure(cur_test);
+		kunit_set_failure(test);
 		return;
 	}
 
 	status = (struct kunit_kasan_status *)resource->data;
 	WRITE_ONCE(status->report_found, true);
 	WRITE_ONCE(status->sync_fault, sync);
+
 	kunit_put_resource(resource);
 }
-#endif /* IS_ENABLED(CONFIG_KUNIT) */
+#else
+static void update_kunit_status(bool sync) { }
+#endif
 
 void kasan_report_invalid_free(void *object, unsigned long ip)
 {
@@ -383,10 +390,7 @@ void kasan_report_invalid_free(void *object, unsigned long ip)
 
 	object = kasan_reset_tag(object);
 
-#if IS_ENABLED(CONFIG_KUNIT)
-	if (current->kunit_test)
-		kasan_update_kunit_status(current->kunit_test, true);
-#endif /* IS_ENABLED(CONFIG_KUNIT) */
+	update_kunit_status(true);
 
 	start_report(&flags);
 	pr_err("BUG: KASAN: double-free or invalid-free in %pS\n", (void *)ip);
@@ -402,10 +406,7 @@ void kasan_report_async(void)
 {
 	unsigned long flags;
 
-#if IS_ENABLED(CONFIG_KUNIT)
-	if (current->kunit_test)
-		kasan_update_kunit_status(current->kunit_test, false);
-#endif /* IS_ENABLED(CONFIG_KUNIT) */
+	update_kunit_status(false);
 
 	start_report(&flags);
 	pr_err("BUG: KASAN: invalid-access\n");
@@ -424,10 +425,7 @@ static void __kasan_report(unsigned long addr, size_t size, bool is_write,
 	void *untagged_addr;
 	unsigned long flags;
 
-#if IS_ENABLED(CONFIG_KUNIT)
-	if (current->kunit_test)
-		kasan_update_kunit_status(current->kunit_test, true);
-#endif /* IS_ENABLED(CONFIG_KUNIT) */
+	update_kunit_status(true);
 
 	disable_trace_on_warning();
 
-- 
2.25.1

[PATCH mm 08/22] kasan: check CONFIG_KASAN_KUNIT_TEST instead of CONFIG_KUNIT

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

Check the more specific CONFIG_KASAN_KUNIT_TEST config option when
defining things related to KUnit-compatible KASAN tests instead of
CONFIG_KUNIT.

Also put the kunit_kasan_status definition next to the definitons of
other KASAN-related structs.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/kasan/kasan.h  | 18 ++++++++----------
 mm/kasan/report.c |  2 +-
 2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 4447df0d7343..cc7162a9f304 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -7,16 +7,6 @@
 #include <linux/kfence.h>
 #include <linux/stackdepot.h>
 
-#if IS_ENABLED(CONFIG_KUNIT)
-
-/* Used in KUnit-compatible KASAN tests. */
-struct kunit_kasan_status {
-	bool report_found;
-	bool sync_fault;
-};
-
-#endif
-
 #ifdef CONFIG_KASAN_HW_TAGS
 
 #include <linux/static_key.h>
@@ -224,6 +214,14 @@ struct kasan_free_meta {
 #endif
 };
 
+#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
+/* Used in KUnit-compatible KASAN tests. */
+struct kunit_kasan_status {
+	bool report_found;
+	bool sync_fault;
+};
+#endif
+
 struct kasan_alloc_meta *kasan_get_alloc_meta(struct kmem_cache *cache,
 						const void *object);
 #ifdef CONFIG_KASAN_GENERIC
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 59db81211b8a..93543157d3e1 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -356,7 +356,7 @@ static bool report_enabled(void)
 	return !test_and_set_bit(KASAN_BIT_REPORTED, &kasan_flags);
 }
 
-#if IS_ENABLED(CONFIG_KUNIT)
+#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
 static void update_kunit_status(bool sync)
 {
 	struct kunit *test;
-- 
2.25.1

[PATCH mm 09/22] kasan: move update_kunit_status to start_report

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

Instead of duplicating calls to update_kunit_status() in every error
report routine, call it once in start_report(). Pass the sync flag
as an additional argument to start_report().

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/kasan/report.c | 75 +++++++++++++++++++++--------------------------
 1 file changed, 34 insertions(+), 41 deletions(-)

diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 93543157d3e1..0b6c8a14f0ea 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -98,13 +98,40 @@ static void print_error_description(struct kasan_access_info *info)
 			info->access_addr, current->comm, task_pid_nr(current));
 }
 
+#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
+static void update_kunit_status(bool sync)
+{
+	struct kunit *test;
+	struct kunit_resource *resource;
+	struct kunit_kasan_status *status;
+
+	test = current->kunit_test;
+	if (!test)
+		return;
+
+	resource = kunit_find_named_resource(test, "kasan_status");
+	if (!resource) {
+		kunit_set_failure(test);
+		return;
+	}
+
+	status = (struct kunit_kasan_status *)resource->data;
+	WRITE_ONCE(status->report_found, true);
+	WRITE_ONCE(status->sync_fault, sync);
+
+	kunit_put_resource(resource);
+}
+#else
+static void update_kunit_status(bool sync) { }
+#endif
+
 static DEFINE_SPINLOCK(report_lock);
 
-static void start_report(unsigned long *flags)
+static void start_report(unsigned long *flags, bool sync)
 {
-	/*
-	 * Make sure we don't end up in loop.
-	 */
+	/* Update status of the currently running KASAN test. */
+	update_kunit_status(sync);
+	/* Make sure we don't end up in loop. */
 	kasan_disable_current();
 	spin_lock_irqsave(&report_lock, *flags);
 	pr_err("==================================================================\n");
@@ -356,33 +383,6 @@ static bool report_enabled(void)
 	return !test_and_set_bit(KASAN_BIT_REPORTED, &kasan_flags);
 }
 
-#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
-static void update_kunit_status(bool sync)
-{
-	struct kunit *test;
-	struct kunit_resource *resource;
-	struct kunit_kasan_status *status;
-
-	test = current->kunit_test;
-	if (!test)
-		return;
-
-	resource = kunit_find_named_resource(test, "kasan_status");
-	if (!resource) {
-		kunit_set_failure(test);
-		return;
-	}
-
-	status = (struct kunit_kasan_status *)resource->data;
-	WRITE_ONCE(status->report_found, true);
-	WRITE_ONCE(status->sync_fault, sync);
-
-	kunit_put_resource(resource);
-}
-#else
-static void update_kunit_status(bool sync) { }
-#endif
-
 void kasan_report_invalid_free(void *object, unsigned long ip)
 {
 	unsigned long flags;
@@ -390,9 +390,7 @@ void kasan_report_invalid_free(void *object, unsigned long ip)
 
 	object = kasan_reset_tag(object);
 
-	update_kunit_status(true);
-
-	start_report(&flags);
+	start_report(&flags, true);
 	pr_err("BUG: KASAN: double-free or invalid-free in %pS\n", (void *)ip);
 	kasan_print_tags(tag, object);
 	pr_err("\n");
@@ -406,9 +404,7 @@ void kasan_report_async(void)
 {
 	unsigned long flags;
 
-	update_kunit_status(false);
-
-	start_report(&flags);
+	start_report(&flags, false);
 	pr_err("BUG: KASAN: invalid-access\n");
 	pr_err("Asynchronous mode enabled: no access details available\n");
 	pr_err("\n");
@@ -425,9 +421,8 @@ static void __kasan_report(unsigned long addr, size_t size, bool is_write,
 	void *untagged_addr;
 	unsigned long flags;
 
-	update_kunit_status(true);
-
 	disable_trace_on_warning();
+	start_report(&flags, true);
 
 	tagged_addr = (void *)addr;
 	untagged_addr = kasan_reset_tag(tagged_addr);
@@ -442,8 +437,6 @@ static void __kasan_report(unsigned long addr, size_t size, bool is_write,
 	info.is_write = is_write;
 	info.ip = ip;
 
-	start_report(&flags);
-
 	print_error_description(&info);
 	if (addr_has_metadata(untagged_addr))
 		kasan_print_tags(get_tag(tagged_addr), info.first_bad_addr);
-- 
2.25.1

[PATCH mm 10/22] kasan: move disable_trace_on_warning to start_report

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

Move the disable_trace_on_warning() call, which enables the
/proc/sys/kernel/traceoff_on_warning interface for KASAN bugs,
to start_report(), so that it functions for all types of KASAN reports.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/kasan/report.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 0b6c8a14f0ea..9286ff6ae1a7 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -129,6 +129,8 @@ static DEFINE_SPINLOCK(report_lock);
 
 static void start_report(unsigned long *flags, bool sync)
 {
+	/* Respect the /proc/sys/kernel/traceoff_on_warning interface. */
+	disable_trace_on_warning();
 	/* Update status of the currently running KASAN test. */
 	update_kunit_status(sync);
 	/* Make sure we don't end up in loop. */
@@ -421,7 +423,6 @@ static void __kasan_report(unsigned long addr, size_t size, bool is_write,
 	void *untagged_addr;
 	unsigned long flags;
 
-	disable_trace_on_warning();
 	start_report(&flags, true);
 
 	tagged_addr = (void *)addr;
-- 
2.25.1

[PATCH mm 11/22] kasan: split out print_report from __kasan_report

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

Split out the part of __kasan_report() that prints things into
print_report(). One of the subsequent patches makes another error
handler use print_report() as well.

Includes lower-level changes:

- Allow addr_has_metadata() accepting a tagged address.
- Drop the const qualifier from the fields of kasan_access_info to avoid
  excessive type casts.
- Change the type of the address argument of __kasan_report() and
  end_report() to void * to reduce the number of type casts.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/kasan/kasan.h  |  7 +++---
 mm/kasan/report.c | 58 +++++++++++++++++++++++++----------------------
 2 files changed, 35 insertions(+), 30 deletions(-)

diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index cc7162a9f304..40b863e289ec 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -128,8 +128,8 @@ static inline bool kasan_sync_fault_possible(void)
 #define META_ROWS_AROUND_ADDR 2
 
 struct kasan_access_info {
-	const void *access_addr;
-	const void *first_bad_addr;
+	void *access_addr;
+	void *first_bad_addr;
 	size_t access_size;
 	bool is_write;
 	unsigned long ip;
@@ -239,7 +239,8 @@ static inline const void *kasan_shadow_to_mem(const void *shadow_addr)
 
 static inline bool addr_has_metadata(const void *addr)
 {
-	return (addr >= kasan_shadow_to_mem((void *)KASAN_SHADOW_START));
+	return (kasan_reset_tag(addr) >=
+		kasan_shadow_to_mem((void *)KASAN_SHADOW_START));
 }
 
 /**
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 9286ff6ae1a7..bb4c29b439b1 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -139,10 +139,11 @@ static void start_report(unsigned long *flags, bool sync)
 	pr_err("==================================================================\n");
 }
 
-static void end_report(unsigned long *flags, unsigned long addr)
+static void end_report(unsigned long *flags, void *addr)
 {
 	if (addr)
-		trace_error_report_end(ERROR_DETECTOR_KASAN, addr);
+		trace_error_report_end(ERROR_DETECTOR_KASAN,
+				       (unsigned long)addr);
 	pr_err("==================================================================\n");
 	add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE);
 	spin_unlock_irqrestore(&report_lock, *flags);
@@ -398,7 +399,7 @@ void kasan_report_invalid_free(void *object, unsigned long ip)
 	pr_err("\n");
 	print_address_description(object, tag);
 	print_memory_metadata(object);
-	end_report(&flags, (unsigned long)object);
+	end_report(&flags, object);
 }
 
 #ifdef CONFIG_KASAN_HW_TAGS
@@ -411,44 +412,47 @@ void kasan_report_async(void)
 	pr_err("Asynchronous mode enabled: no access details available\n");
 	pr_err("\n");
 	dump_stack_lvl(KERN_ERR);
-	end_report(&flags, 0);
+	end_report(&flags, NULL);
 }
 #endif /* CONFIG_KASAN_HW_TAGS */
 
-static void __kasan_report(unsigned long addr, size_t size, bool is_write,
+static void print_report(struct kasan_access_info *info)
+{
+	void *tagged_addr = info->access_addr;
+	void *untagged_addr = kasan_reset_tag(tagged_addr);
+	u8 tag = get_tag(tagged_addr);
+
+	print_error_description(info);
+	if (addr_has_metadata(untagged_addr))
+		kasan_print_tags(tag, info->first_bad_addr);
+	pr_err("\n");
+
+	if (addr_has_metadata(untagged_addr)) {
+		print_address_description(untagged_addr, tag);
+		print_memory_metadata(info->first_bad_addr);
+	} else {
+		dump_stack_lvl(KERN_ERR);
+	}
+}
+
+static void __kasan_report(void *addr, size_t size, bool is_write,
 				unsigned long ip)
 {
 	struct kasan_access_info info;
-	void *tagged_addr;
-	void *untagged_addr;
 	unsigned long flags;
 
 	start_report(&flags, true);
 
-	tagged_addr = (void *)addr;
-	untagged_addr = kasan_reset_tag(tagged_addr);
-
-	info.access_addr = tagged_addr;
-	if (addr_has_metadata(untagged_addr))
-		info.first_bad_addr =
-			kasan_find_first_bad_addr(tagged_addr, size);
+	info.access_addr = addr;
+	if (addr_has_metadata(addr))
+		info.first_bad_addr = kasan_find_first_bad_addr(addr, size);
 	else
-		info.first_bad_addr = untagged_addr;
+		info.first_bad_addr = addr;
 	info.access_size = size;
 	info.is_write = is_write;
 	info.ip = ip;
 
-	print_error_description(&info);
-	if (addr_has_metadata(untagged_addr))
-		kasan_print_tags(get_tag(tagged_addr), info.first_bad_addr);
-	pr_err("\n");
-
-	if (addr_has_metadata(untagged_addr)) {
-		print_address_description(untagged_addr, get_tag(tagged_addr));
-		print_memory_metadata(info.first_bad_addr);
-	} else {
-		dump_stack_lvl(KERN_ERR);
-	}
+	print_report(&info);
 
 	end_report(&flags, addr);
 }
@@ -460,7 +464,7 @@ bool kasan_report(unsigned long addr, size_t size, bool is_write,
 	bool ret = false;
 
 	if (likely(report_enabled())) {
-		__kasan_report(addr, size, is_write, ip);
+		__kasan_report((void *)addr, size, is_write, ip);
 		ret = true;
 	}
 
-- 
2.25.1

[PATCH mm 12/22] kasan: simplify kasan_find_first_bad_addr call sites

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

Move the addr_has_metadata() check into kasan_find_first_bad_addr().

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/kasan/report.c         | 5 +----
 mm/kasan/report_generic.c | 4 ++++
 mm/kasan/report_hw_tags.c | 1 +
 mm/kasan/report_sw_tags.c | 4 ++++
 4 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index bb4c29b439b1..a0d4a9d3f933 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -444,10 +444,7 @@ static void __kasan_report(void *addr, size_t size, bool is_write,
 	start_report(&flags, true);
 
 	info.access_addr = addr;
-	if (addr_has_metadata(addr))
-		info.first_bad_addr = kasan_find_first_bad_addr(addr, size);
-	else
-		info.first_bad_addr = addr;
+	info.first_bad_addr = kasan_find_first_bad_addr(addr, size);
 	info.access_size = size;
 	info.is_write = is_write;
 	info.ip = ip;
diff --git a/mm/kasan/report_generic.c b/mm/kasan/report_generic.c
index 7e03cca569a7..182239ca184c 100644
--- a/mm/kasan/report_generic.c
+++ b/mm/kasan/report_generic.c
@@ -34,8 +34,12 @@ void *kasan_find_first_bad_addr(void *addr, size_t size)
 {
 	void *p = addr;
 
+	if (!addr_has_metadata(p))
+		return p;
+
 	while (p < addr + size && !(*(u8 *)kasan_mem_to_shadow(p)))
 		p += KASAN_GRANULE_SIZE;
+
 	return p;
 }
 
diff --git a/mm/kasan/report_hw_tags.c b/mm/kasan/report_hw_tags.c
index 5dbbbb930e7a..f3d3be614e4b 100644
--- a/mm/kasan/report_hw_tags.c
+++ b/mm/kasan/report_hw_tags.c
@@ -17,6 +17,7 @@
 
 void *kasan_find_first_bad_addr(void *addr, size_t size)
 {
+	/* Return the same value regardless of whether addr_has_metadata(). */
 	return kasan_reset_tag(addr);
 }
 
diff --git a/mm/kasan/report_sw_tags.c b/mm/kasan/report_sw_tags.c
index 44577b8d47a7..68724ba3d814 100644
--- a/mm/kasan/report_sw_tags.c
+++ b/mm/kasan/report_sw_tags.c
@@ -35,8 +35,12 @@ void *kasan_find_first_bad_addr(void *addr, size_t size)
 	void *p = kasan_reset_tag(addr);
 	void *end = p + size;
 
+	if (!addr_has_metadata(p))
+		return p;
+
 	while (p < end && tag == *(u8 *)kasan_mem_to_shadow(p))
 		p += KASAN_GRANULE_SIZE;
+
 	return p;
 }
 
-- 
2.25.1

[PATCH mm 13/22] kasan: restructure kasan_report

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

Restructure kasan_report() to make reviewing the subsequent patches
easier.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/kasan/report.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index a0d4a9d3f933..41c7966451e3 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -457,15 +457,18 @@ static void __kasan_report(void *addr, size_t size, bool is_write,
 bool kasan_report(unsigned long addr, size_t size, bool is_write,
 			unsigned long ip)
 {
-	unsigned long flags = user_access_save();
-	bool ret = false;
+	unsigned long ua_flags = user_access_save();
+	bool ret = true;
 
-	if (likely(report_enabled())) {
-		__kasan_report((void *)addr, size, is_write, ip);
-		ret = true;
+	if (unlikely(!report_enabled())) {
+		ret = false;
+		goto out;
 	}
 
-	user_access_restore(flags);
+	__kasan_report((void *)addr, size, is_write, ip);
+
+out:
+	user_access_restore(ua_flags);
 
 	return ret;
 }
-- 
2.25.1

[PATCH mm 14/22] kasan: merge __kasan_report into kasan_report

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

Merge __kasan_report() into kasan_report(). The code is simple enough
to be readable without the __kasan_report() helper.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/kasan/report.c | 36 +++++++++++++++---------------------
 1 file changed, 15 insertions(+), 21 deletions(-)

diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 41c7966451e3..56d5ba235542 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -435,37 +435,31 @@ static void print_report(struct kasan_access_info *info)
 	}
 }
 
-static void __kasan_report(void *addr, size_t size, bool is_write,
-				unsigned long ip)
-{
-	struct kasan_access_info info;
-	unsigned long flags;
-
-	start_report(&flags, true);
-
-	info.access_addr = addr;
-	info.first_bad_addr = kasan_find_first_bad_addr(addr, size);
-	info.access_size = size;
-	info.is_write = is_write;
-	info.ip = ip;
-
-	print_report(&info);
-
-	end_report(&flags, addr);
-}
-
 bool kasan_report(unsigned long addr, size_t size, bool is_write,
 			unsigned long ip)
 {
-	unsigned long ua_flags = user_access_save();
 	bool ret = true;
+	void *ptr = (void *)addr;
+	unsigned long ua_flags = user_access_save();
+	unsigned long irq_flags;
+	struct kasan_access_info info;
 
 	if (unlikely(!report_enabled())) {
 		ret = false;
 		goto out;
 	}
 
-	__kasan_report((void *)addr, size, is_write, ip);
+	start_report(&irq_flags, true);
+
+	info.access_addr = ptr;
+	info.first_bad_addr = kasan_find_first_bad_addr(ptr, size);
+	info.access_size = size;
+	info.is_write = is_write;
+	info.ip = ip;
+
+	print_report(&info);
+
+	end_report(&irq_flags, ptr);
 
 out:
 	user_access_restore(ua_flags);
-- 
2.25.1

[PATCH mm 15/22] kasan: call print_report from kasan_report_invalid_free

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

Call print_report() in kasan_report_invalid_free() instead of calling
printing functions directly. Compared to the existing implementation
of kasan_report_invalid_free(), print_report() makes sure that the
buggy address has metadata before printing it.

The change requires adding a report type field into kasan_access_info
and using it accordingly.

kasan_report_async() is left as is, as using print_report() will only
complicate the code.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/kasan/kasan.h  |  6 ++++++
 mm/kasan/report.c | 42 ++++++++++++++++++++++++++----------------
 2 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 40b863e289ec..8c9a855152c2 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -127,7 +127,13 @@ static inline bool kasan_sync_fault_possible(void)
 #define META_MEM_BYTES_PER_ROW (META_BYTES_PER_ROW * KASAN_GRANULE_SIZE)
 #define META_ROWS_AROUND_ADDR 2
 
+enum kasan_report_type {
+	KASAN_REPORT_ACCESS,
+	KASAN_REPORT_INVALID_FREE,
+};
+
 struct kasan_access_info {
+	enum kasan_report_type type;
 	void *access_addr;
 	void *first_bad_addr;
 	size_t access_size;
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 56d5ba235542..73348f83b813 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -86,6 +86,12 @@ __setup("kasan_multi_shot", kasan_set_multi_shot);
 
 static void print_error_description(struct kasan_access_info *info)
 {
+	if (info->type == KASAN_REPORT_INVALID_FREE) {
+		pr_err("BUG: KASAN: double-free or invalid-free in %pS\n",
+		       (void *)info->ip);
+		return;
+	}
+
 	pr_err("BUG: KASAN: %s in %pS\n",
 		kasan_get_bug_type(info), (void *)info->ip);
 	if (info->access_size)
@@ -386,22 +392,6 @@ static bool report_enabled(void)
 	return !test_and_set_bit(KASAN_BIT_REPORTED, &kasan_flags);
 }
 
-void kasan_report_invalid_free(void *object, unsigned long ip)
-{
-	unsigned long flags;
-	u8 tag = get_tag(object);
-
-	object = kasan_reset_tag(object);
-
-	start_report(&flags, true);
-	pr_err("BUG: KASAN: double-free or invalid-free in %pS\n", (void *)ip);
-	kasan_print_tags(tag, object);
-	pr_err("\n");
-	print_address_description(object, tag);
-	print_memory_metadata(object);
-	end_report(&flags, object);
-}
-
 #ifdef CONFIG_KASAN_HW_TAGS
 void kasan_report_async(void)
 {
@@ -435,6 +425,25 @@ static void print_report(struct kasan_access_info *info)
 	}
 }
 
+void kasan_report_invalid_free(void *ptr, unsigned long ip)
+{
+	unsigned long flags;
+	struct kasan_access_info info;
+
+	start_report(&flags, true);
+
+	info.type = KASAN_REPORT_INVALID_FREE;
+	info.access_addr = ptr;
+	info.first_bad_addr = kasan_reset_tag(ptr);
+	info.access_size = 0;
+	info.is_write = false;
+	info.ip = ip;
+
+	print_report(&info);
+
+	end_report(&flags, ptr);
+}
+
 bool kasan_report(unsigned long addr, size_t size, bool is_write,
 			unsigned long ip)
 {
@@ -451,6 +460,7 @@ bool kasan_report(unsigned long addr, size_t size, bool is_write,
 
 	start_report(&irq_flags, true);
 
+	info.type = KASAN_REPORT_ACCESS;
 	info.access_addr = ptr;
 	info.first_bad_addr = kasan_find_first_bad_addr(ptr, size);
 	info.access_size = size;
-- 
2.25.1

[PATCH mm 16/22] kasan: move and simplify kasan_report_async

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

Place kasan_report_async() next to the other main reporting routines.
Also simplify printed information.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/kasan/report.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 73348f83b813..162fd2d6209e 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -392,20 +392,6 @@ static bool report_enabled(void)
 	return !test_and_set_bit(KASAN_BIT_REPORTED, &kasan_flags);
 }
 
-#ifdef CONFIG_KASAN_HW_TAGS
-void kasan_report_async(void)
-{
-	unsigned long flags;
-
-	start_report(&flags, false);
-	pr_err("BUG: KASAN: invalid-access\n");
-	pr_err("Asynchronous mode enabled: no access details available\n");
-	pr_err("\n");
-	dump_stack_lvl(KERN_ERR);
-	end_report(&flags, NULL);
-}
-#endif /* CONFIG_KASAN_HW_TAGS */
-
 static void print_report(struct kasan_access_info *info)
 {
 	void *tagged_addr = info->access_addr;
@@ -477,6 +463,20 @@ bool kasan_report(unsigned long addr, size_t size, bool is_write,
 	return ret;
 }
 
+#ifdef CONFIG_KASAN_HW_TAGS
+void kasan_report_async(void)
+{
+	unsigned long flags;
+
+	start_report(&flags, false);
+	pr_err("BUG: KASAN: invalid-access\n");
+	pr_err("Asynchronous fault: no details available\n");
+	pr_err("\n");
+	dump_stack_lvl(KERN_ERR);
+	end_report(&flags, NULL);
+}
+#endif /* CONFIG_KASAN_HW_TAGS */
+
 #ifdef CONFIG_KASAN_INLINE
 /*
  * With CONFIG_KASAN_INLINE, accesses to bogus pointers (outside the high
-- 
2.25.1

[PATCH mm 17/22] kasan: rename kasan_access_info to kasan_report_info

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

Rename kasan_access_info to kasan_report_info, as the latter name better
reflects the struct's purpose.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/kasan/kasan.h          | 4 ++--
 mm/kasan/report.c         | 8 ++++----
 mm/kasan/report_generic.c | 6 +++---
 mm/kasan/report_tags.c    | 2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 8c9a855152c2..9d2e128eb623 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -132,7 +132,7 @@ enum kasan_report_type {
 	KASAN_REPORT_INVALID_FREE,
 };
 
-struct kasan_access_info {
+struct kasan_report_info {
 	enum kasan_report_type type;
 	void *access_addr;
 	void *first_bad_addr;
@@ -276,7 +276,7 @@ static inline void kasan_print_tags(u8 addr_tag, const void *addr) { }
 #endif
 
 void *kasan_find_first_bad_addr(void *addr, size_t size);
-const char *kasan_get_bug_type(struct kasan_access_info *info);
+const char *kasan_get_bug_type(struct kasan_report_info *info);
 void kasan_metadata_fetch_row(char *buffer, void *row);
 
 #if defined(CONFIG_KASAN_STACK)
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 162fd2d6209e..7915af810815 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -84,7 +84,7 @@ static int __init kasan_set_multi_shot(char *str)
 }
 __setup("kasan_multi_shot", kasan_set_multi_shot);
 
-static void print_error_description(struct kasan_access_info *info)
+static void print_error_description(struct kasan_report_info *info)
 {
 	if (info->type == KASAN_REPORT_INVALID_FREE) {
 		pr_err("BUG: KASAN: double-free or invalid-free in %pS\n",
@@ -392,7 +392,7 @@ static bool report_enabled(void)
 	return !test_and_set_bit(KASAN_BIT_REPORTED, &kasan_flags);
 }
 
-static void print_report(struct kasan_access_info *info)
+static void print_report(struct kasan_report_info *info)
 {
 	void *tagged_addr = info->access_addr;
 	void *untagged_addr = kasan_reset_tag(tagged_addr);
@@ -414,7 +414,7 @@ static void print_report(struct kasan_access_info *info)
 void kasan_report_invalid_free(void *ptr, unsigned long ip)
 {
 	unsigned long flags;
-	struct kasan_access_info info;
+	struct kasan_report_info info;
 
 	start_report(&flags, true);
 
@@ -437,7 +437,7 @@ bool kasan_report(unsigned long addr, size_t size, bool is_write,
 	void *ptr = (void *)addr;
 	unsigned long ua_flags = user_access_save();
 	unsigned long irq_flags;
-	struct kasan_access_info info;
+	struct kasan_report_info info;
 
 	if (unlikely(!report_enabled())) {
 		ret = false;
diff --git a/mm/kasan/report_generic.c b/mm/kasan/report_generic.c
index 182239ca184c..efc5e79a103f 100644
--- a/mm/kasan/report_generic.c
+++ b/mm/kasan/report_generic.c
@@ -43,7 +43,7 @@ void *kasan_find_first_bad_addr(void *addr, size_t size)
 	return p;
 }
 
-static const char *get_shadow_bug_type(struct kasan_access_info *info)
+static const char *get_shadow_bug_type(struct kasan_report_info *info)
 {
 	const char *bug_type = "unknown-crash";
 	u8 *shadow_addr;
@@ -95,7 +95,7 @@ static const char *get_shadow_bug_type(struct kasan_access_info *info)
 	return bug_type;
 }
 
-static const char *get_wild_bug_type(struct kasan_access_info *info)
+static const char *get_wild_bug_type(struct kasan_report_info *info)
 {
 	const char *bug_type = "unknown-crash";
 
@@ -109,7 +109,7 @@ static const char *get_wild_bug_type(struct kasan_access_info *info)
 	return bug_type;
 }
 
-const char *kasan_get_bug_type(struct kasan_access_info *info)
+const char *kasan_get_bug_type(struct kasan_report_info *info)
 {
 	/*
 	 * If access_size is a negative number, then it has reason to be
diff --git a/mm/kasan/report_tags.c b/mm/kasan/report_tags.c
index 1b41de88c53e..e25d2166e813 100644
--- a/mm/kasan/report_tags.c
+++ b/mm/kasan/report_tags.c
@@ -7,7 +7,7 @@
 #include "kasan.h"
 #include "../slab.h"
 
-const char *kasan_get_bug_type(struct kasan_access_info *info)
+const char *kasan_get_bug_type(struct kasan_report_info *info)
 {
 #ifdef CONFIG_KASAN_TAGS_IDENTIFY
 	struct kasan_alloc_meta *alloc_meta;
-- 
2.25.1

[PATCH mm 18/22] kasan: add comment about UACCESS regions to kasan_report

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

Add a comment explaining why kasan_report() is the only reporting
function that uses user_access_save/restore().

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/kasan/report.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 7915af810815..08631d873204 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -430,6 +430,11 @@ void kasan_report_invalid_free(void *ptr, unsigned long ip)
 	end_report(&flags, ptr);
 }
 
+/*
+ * kasan_report() is the only reporting function that uses
+ * user_access_save/restore(): kasan_report_invalid_free() cannot be called
+ * from a UACCESS region, and kasan_report_async() is not used on x86.
+ */
 bool kasan_report(unsigned long addr, size_t size, bool is_write,
 			unsigned long ip)
 {
-- 
2.25.1

[PATCH mm 19/22] kasan: respect KASAN_BIT_REPORTED in all reporting routines

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

Currently, only kasan_report() checks the KASAN_BIT_REPORTED and
KASAN_BIT_MULTI_SHOT flags.

Make other reporting routines check these flags as well.

Also add explanatory comments.

Note that the current->kasan_depth check is split out into
report_suppressed() and only called for kasan_report().

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/kasan/report.c | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 08631d873204..ef649f5cee29 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -381,12 +381,26 @@ static void print_memory_metadata(const void *addr)
 	}
 }
 
-static bool report_enabled(void)
+/*
+ * Used to suppress reports within kasan_disable/enable_current() critical
+ * sections, which are used for marking accesses to slab metadata.
+ */
+static bool report_suppressed(void)
 {
 #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS)
 	if (current->kasan_depth)
-		return false;
+		return true;
 #endif
+	return false;
+}
+
+/*
+ * Used to avoid reporting more than one KASAN bug unless kasan_multi_shot
+ * is enabled. Note that KASAN tests effectively enable kasan_multi_shot
+ * for their duration.
+ */
+static bool report_enabled(void)
+{
 	if (test_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags))
 		return true;
 	return !test_and_set_bit(KASAN_BIT_REPORTED, &kasan_flags);
@@ -416,6 +430,14 @@ void kasan_report_invalid_free(void *ptr, unsigned long ip)
 	unsigned long flags;
 	struct kasan_report_info info;
 
+	/*
+	 * Do not check report_suppressed(), as an invalid-free cannot be
+	 * caused by accessing slab metadata and thus should not be
+	 * suppressed by kasan_disable/enable_current() critical sections.
+	 */
+	if (unlikely(!report_enabled()))
+		return;
+
 	start_report(&flags, true);
 
 	info.type = KASAN_REPORT_INVALID_FREE;
@@ -444,7 +466,7 @@ bool kasan_report(unsigned long addr, size_t size, bool is_write,
 	unsigned long irq_flags;
 	struct kasan_report_info info;
 
-	if (unlikely(!report_enabled())) {
+	if (unlikely(report_suppressed()) || unlikely(!report_enabled())) {
 		ret = false;
 		goto out;
 	}
@@ -473,6 +495,13 @@ void kasan_report_async(void)
 {
 	unsigned long flags;
 
+	/*
+	 * Do not check report_suppressed(), as kasan_disable/enable_current()
+	 * critical sections do not affect Hardware Tag-Based KASAN.
+	 */
+	if (unlikely(!report_enabled()))
+		return;
+
 	start_report(&flags, false);
 	pr_err("BUG: KASAN: invalid-access\n");
 	pr_err("Asynchronous fault: no details available\n");
-- 
2.25.1

[PATCH mm 20/22] kasan: reorder reporting functions

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

Move print_error_description()'s, report_suppressed()'s, and
report_enabled()'s definitions to improve the logical order of
function definitions in report.c.

No functional changes.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/kasan/report.c | 82 +++++++++++++++++++++++------------------------
 1 file changed, 41 insertions(+), 41 deletions(-)

diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index ef649f5cee29..7ef3b0455603 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -84,24 +84,29 @@ static int __init kasan_set_multi_shot(char *str)
 }
 __setup("kasan_multi_shot", kasan_set_multi_shot);
 
-static void print_error_description(struct kasan_report_info *info)
+/*
+ * Used to suppress reports within kasan_disable/enable_current() critical
+ * sections, which are used for marking accesses to slab metadata.
+ */
+static bool report_suppressed(void)
 {
-	if (info->type == KASAN_REPORT_INVALID_FREE) {
-		pr_err("BUG: KASAN: double-free or invalid-free in %pS\n",
-		       (void *)info->ip);
-		return;
-	}
+#if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS)
+	if (current->kasan_depth)
+		return true;
+#endif
+	return false;
+}
 
-	pr_err("BUG: KASAN: %s in %pS\n",
-		kasan_get_bug_type(info), (void *)info->ip);
-	if (info->access_size)
-		pr_err("%s of size %zu at addr %px by task %s/%d\n",
-			info->is_write ? "Write" : "Read", info->access_size,
-			info->access_addr, current->comm, task_pid_nr(current));
-	else
-		pr_err("%s at addr %px by task %s/%d\n",
-			info->is_write ? "Write" : "Read",
-			info->access_addr, current->comm, task_pid_nr(current));
+/*
+ * Used to avoid reporting more than one KASAN bug unless kasan_multi_shot
+ * is enabled. Note that KASAN tests effectively enable kasan_multi_shot
+ * for their duration.
+ */
+static bool report_enabled(void)
+{
+	if (test_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags))
+		return true;
+	return !test_and_set_bit(KASAN_BIT_REPORTED, &kasan_flags);
 }
 
 #if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
@@ -160,6 +165,26 @@ static void end_report(unsigned long *flags, void *addr)
 	kasan_enable_current();
 }
 
+static void print_error_description(struct kasan_report_info *info)
+{
+	if (info->type == KASAN_REPORT_INVALID_FREE) {
+		pr_err("BUG: KASAN: double-free or invalid-free in %pS\n",
+		       (void *)info->ip);
+		return;
+	}
+
+	pr_err("BUG: KASAN: %s in %pS\n",
+		kasan_get_bug_type(info), (void *)info->ip);
+	if (info->access_size)
+		pr_err("%s of size %zu at addr %px by task %s/%d\n",
+			info->is_write ? "Write" : "Read", info->access_size,
+			info->access_addr, current->comm, task_pid_nr(current));
+	else
+		pr_err("%s at addr %px by task %s/%d\n",
+			info->is_write ? "Write" : "Read",
+			info->access_addr, current->comm, task_pid_nr(current));
+}
+
 static void print_track(struct kasan_track *track, const char *prefix)
 {
 	pr_err("%s by task %u:\n", prefix, track->pid);
@@ -381,31 +406,6 @@ static void print_memory_metadata(const void *addr)
 	}
 }
 
-/*
- * Used to suppress reports within kasan_disable/enable_current() critical
- * sections, which are used for marking accesses to slab metadata.
- */
-static bool report_suppressed(void)
-{
-#if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS)
-	if (current->kasan_depth)
-		return true;
-#endif
-	return false;
-}
-
-/*
- * Used to avoid reporting more than one KASAN bug unless kasan_multi_shot
- * is enabled. Note that KASAN tests effectively enable kasan_multi_shot
- * for their duration.
- */
-static bool report_enabled(void)
-{
-	if (test_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags))
-		return true;
-	return !test_and_set_bit(KASAN_BIT_REPORTED, &kasan_flags);
-}
-
 static void print_report(struct kasan_report_info *info)
 {
 	void *tagged_addr = info->access_addr;
-- 
2.25.1

[PATCH mm 21/22] kasan: move and hide kasan_save_enable/restore_multi_shot

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

- Move kasan_save_enable/restore_multi_shot() declarations to
  mm/kasan/kasan.h, as there is no need for them to be visible outside
  of KASAN implementation.

- Only define and export these functions when KASAN tests are enabled.

- Move their definitions closer to other test-related code in report.c.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 include/linux/kasan.h |  4 ----
 mm/kasan/kasan.h      |  7 +++++++
 mm/kasan/report.c     | 30 +++++++++++++++++-------------
 3 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index fe36215807f7..ceebcb9de7bf 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -267,10 +267,6 @@ static __always_inline bool kasan_check_byte(const void *addr)
 	return true;
 }
 
-
-bool kasan_save_enable_multi_shot(void);
-void kasan_restore_multi_shot(bool enabled);
-
 #else /* CONFIG_KASAN */
 
 static inline slab_flags_t kasan_never_merge(void)
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 9d2e128eb623..d79b83d673b1 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -492,6 +492,13 @@ static inline bool kasan_arch_is_ready(void)	{ return true; }
 #error kasan_arch_is_ready only works in KASAN generic outline mode!
 #endif
 
+#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST) || IS_ENABLED(CONFIG_KASAN_MODULE_TEST)
+
+bool kasan_save_enable_multi_shot(void);
+void kasan_restore_multi_shot(bool enabled);
+
+#endif
+
 /*
  * Exported functions for interfaces called from assembly or from generated
  * code. Declarations here to avoid warning about missing declarations.
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 7ef3b0455603..c9bfffe931b4 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -64,19 +64,6 @@ static int __init early_kasan_fault(char *arg)
 }
 early_param("kasan.fault", early_kasan_fault);
 
-bool kasan_save_enable_multi_shot(void)
-{
-	return test_and_set_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags);
-}
-EXPORT_SYMBOL_GPL(kasan_save_enable_multi_shot);
-
-void kasan_restore_multi_shot(bool enabled)
-{
-	if (!enabled)
-		clear_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags);
-}
-EXPORT_SYMBOL_GPL(kasan_restore_multi_shot);
-
 static int __init kasan_set_multi_shot(char *str)
 {
 	set_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags);
@@ -109,6 +96,23 @@ static bool report_enabled(void)
 	return !test_and_set_bit(KASAN_BIT_REPORTED, &kasan_flags);
 }
 
+#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST) || IS_ENABLED(CONFIG_KASAN_MODULE_TEST)
+
+bool kasan_save_enable_multi_shot(void)
+{
+	return test_and_set_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags);
+}
+EXPORT_SYMBOL_GPL(kasan_save_enable_multi_shot);
+
+void kasan_restore_multi_shot(bool enabled)
+{
+	if (!enabled)
+		clear_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags);
+}
+EXPORT_SYMBOL_GPL(kasan_restore_multi_shot);
+
+#endif
+
 #if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
 static void update_kunit_status(bool sync)
 {
-- 
2.25.1

[PATCH mm 22/22] kasan: disable LOCKDEP when printing reports

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

If LOCKDEP detects a bug while KASAN is printing a report and if
panic_on_warn is set, KASAN will not be able to finish.
Disable LOCKDEP while KASAN is printing a report.

See https://bugzilla.kernel.org/show_bug.cgi?id=202115 for an example
of the issue.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/kasan/report.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index c9bfffe931b4..199d77cce21a 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -13,6 +13,7 @@
 #include <linux/ftrace.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
+#include <linux/lockdep.h>
 #include <linux/mm.h>
 #include <linux/printk.h>
 #include <linux/sched.h>
@@ -148,6 +149,8 @@ static void start_report(unsigned long *flags, bool sync)
 	disable_trace_on_warning();
 	/* Update status of the currently running KASAN test. */
 	update_kunit_status(sync);
+	/* Do not allow LOCKDEP mangling KASAN reports. */
+	lockdep_off();
 	/* Make sure we don't end up in loop. */
 	kasan_disable_current();
 	spin_lock_irqsave(&report_lock, *flags);
@@ -160,12 +163,13 @@ static void end_report(unsigned long *flags, void *addr)
 		trace_error_report_end(ERROR_DETECTOR_KASAN,
 				       (unsigned long)addr);
 	pr_err("==================================================================\n");
-	add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE);
 	spin_unlock_irqrestore(&report_lock, *flags);
 	if (panic_on_warn && !test_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags))
 		panic("panic_on_warn set ...\n");
 	if (kasan_arg_fault == KASAN_ARG_FAULT_PANIC)
 		panic("kasan.fault=panic set ...\n");
+	add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE);
+	lockdep_on();
 	kasan_enable_current();
 }
 
-- 
2.25.1

Re: [PATCH mm 01/22] kasan: drop addr check from describe_object_addr

[Alexander Potapenko]

[-- Attachment #1: Type: text/plain, Size: 2218 bytes --]

On Wed, Mar 2, 2022 at 5:36 PM <andrey.konovalov@linux.dev> wrote:

> From: Andrey Konovalov <andreyknvl@google.com>
>
> describe_object_addr() used to be called with NULL addr in the early
> days of KASAN. This no longer happens, so drop the check.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
>
Reviewed-by: Alexander Potapenko <glider@google.com>

> ---
>  mm/kasan/report.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index f64352008bb8..607a8c2e4674 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -162,9 +162,6 @@ static void describe_object_addr(struct kmem_cache
> *cache, void *object,
>                " which belongs to the cache %s of size %d\n",
>                 object, cache->name, cache->object_size);
>
> -       if (!addr)
> -               return;
> -
>         if (access_addr < object_addr) {
>                 rel_type = "to the left";
>                 rel_bytes = object_addr - access_addr;
> --
> 2.25.1
>
> --
> You received this message because you are subscribed to the Google Groups
> "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to kasan-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/kasan-dev/761f8e5a6ee040d665934d916a90afe9f322f745.1646237226.git.andreyknvl%40google.com
> .
>


-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Liana Sebastian
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich. Falls Sie diese fälschlicherweise erhalten
haben sollten, leiten Sie diese bitte nicht an jemand anderes weiter,
löschen Sie alle Kopien und Anhänge davon und lassen Sie mich bitte wissen,
dass die E-Mail an die falsche Person gesendet wurde.



This e-mail is confidential. If you received this communication by mistake,
please don't forward it to anyone else, please erase all copies and
attachments, and please let me know that it has gone to the wrong person.

[-- Attachment #2: Type: text/html, Size: 3363 bytes --]

Re: [PATCH mm 02/22] kasan: more line breaks in reports

[Alexander Potapenko]

[-- Attachment #1: Type: text/plain, Size: 3919 bytes --]

On Wed, Mar 2, 2022 at 5:36 PM <andrey.konovalov@linux.dev> wrote:

> From: Andrey Konovalov <andreyknvl@google.com>
>
> Add a line break after each part that describes the buggy address.
> Improves readability of reports.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
>
Reviewed-by: Alexander Potapenko <glider@google.com>

> ---
>  mm/kasan/report.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index 607a8c2e4674..ded648c0a0e4 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -250,11 +250,13 @@ static void print_address_description(void *addr, u8
> tag)
>                 void *object = nearest_obj(cache, slab, addr);
>
>                 describe_object(cache, object, addr, tag);
> +               pr_err("\n");
>         }
>
>         if (kernel_or_module_addr(addr) && !init_task_stack_addr(addr)) {
>                 pr_err("The buggy address belongs to the variable:\n");
>                 pr_err(" %pS\n", addr);
> +               pr_err("\n");
>         }
>
>         if (is_vmalloc_addr(addr)) {
> @@ -265,6 +267,7 @@ static void print_address_description(void *addr, u8
> tag)
>                                " [%px, %px) created by:\n"
>                                " %pS\n",
>                                va->addr, va->addr + va->size, va->caller);
> +                       pr_err("\n");
>
>                         page = vmalloc_to_page(page);
>                 }
> @@ -273,9 +276,11 @@ static void print_address_description(void *addr, u8
> tag)
>         if (page) {
>                 pr_err("The buggy address belongs to the physical
> page:\n");
>                 dump_page(page, "kasan: bad access detected");
> +               pr_err("\n");
>         }
>
>         kasan_print_address_stack_frame(addr);
> +       pr_err("\n");
>  }
>
>  static bool meta_row_is_guilty(const void *row, const void *addr)
> @@ -382,7 +387,6 @@ void kasan_report_invalid_free(void *object, unsigned
> long ip)
>         kasan_print_tags(tag, object);
>         pr_err("\n");
>         print_address_description(object, tag);
> -       pr_err("\n");
>         print_memory_metadata(object);
>         end_report(&flags, (unsigned long)object);
>  }
> @@ -443,7 +447,6 @@ static void __kasan_report(unsigned long addr, size_t
> size, bool is_write,
>
>         if (addr_has_metadata(untagged_addr)) {
>                 print_address_description(untagged_addr,
> get_tag(tagged_addr));
> -               pr_err("\n");
>                 print_memory_metadata(info.first_bad_addr);
>         } else {
>                 dump_stack_lvl(KERN_ERR);
> --
> 2.25.1
>
> --
> You received this message because you are subscribed to the Google Groups
> "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to kasan-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/kasan-dev/8682c4558e533cd0f99bdb964ce2fe741f2a9212.1646237226.git.andreyknvl%40google.com
> .
>


-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Liana Sebastian
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich. Falls Sie diese fälschlicherweise erhalten
haben sollten, leiten Sie diese bitte nicht an jemand anderes weiter,
löschen Sie alle Kopien und Anhänge davon und lassen Sie mich bitte wissen,
dass die E-Mail an die falsche Person gesendet wurde.



This e-mail is confidential. If you received this communication by mistake,
please don't forward it to anyone else, please erase all copies and
attachments, and please let me know that it has gone to the wrong person.

[-- Attachment #2: Type: text/html, Size: 5470 bytes --]

Re: [PATCH mm 03/22] kasan: rearrange stack frame info in reports

[Alexander Potapenko]

[-- Attachment #1: Type: text/plain, Size: 5064 bytes --]

On Wed, Mar 2, 2022 at 5:36 PM <andrey.konovalov@linux.dev> wrote:

> From: Andrey Konovalov <andreyknvl@google.com>
>
> - Move printing stack frame info before printing page info.
>
> - Add object_is_on_stack() check to print_address_description()
>   and add a corresponding WARNING to kasan_print_address_stack_frame().
>   This looks more in line with the rest of the checks in this function
>   and also allows to avoid complicating code logic wrt line breaks.
>
> - Clean up comments related to get_address_stack_frame_info().
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
>
Reviewed-by: Alexander Potapenko <glider@google.com>

> ---
>  mm/kasan/report.c         | 12 +++++++++---
>  mm/kasan/report_generic.c | 15 ++++-----------
>  2 files changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index ded648c0a0e4..d60ee8b81e2b 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -259,6 +259,15 @@ static void print_address_description(void *addr, u8
> tag)
>                 pr_err("\n");
>         }
>
> +       if (object_is_on_stack(addr)) {
> +               /*
> +                * Currently, KASAN supports printing frame information
> only
> +                * for accesses to the task's own stack.
> +                */
> +               kasan_print_address_stack_frame(addr);
> +               pr_err("\n");
> +       }
> +
>         if (is_vmalloc_addr(addr)) {
>                 struct vm_struct *va = find_vm_area(addr);
>
> @@ -278,9 +287,6 @@ static void print_address_description(void *addr, u8
> tag)
>                 dump_page(page, "kasan: bad access detected");
>                 pr_err("\n");
>         }
> -
> -       kasan_print_address_stack_frame(addr);
> -       pr_err("\n");
>  }
>
>  static bool meta_row_is_guilty(const void *row, const void *addr)
> diff --git a/mm/kasan/report_generic.c b/mm/kasan/report_generic.c
> index 139615ef326b..3751391ff11a 100644
> --- a/mm/kasan/report_generic.c
> +++ b/mm/kasan/report_generic.c
> @@ -211,6 +211,7 @@ static void print_decoded_frame_descr(const char
> *frame_descr)
>         }
>  }
>
> +/* Returns true only if the address is on the current task's stack. */
>  static bool __must_check get_address_stack_frame_info(const void *addr,
>                                                       unsigned long
> *offset,
>                                                       const char
> **frame_descr,
> @@ -224,13 +225,6 @@ static bool __must_check
> get_address_stack_frame_info(const void *addr,
>
>         BUILD_BUG_ON(IS_ENABLED(CONFIG_STACK_GROWSUP));
>
> -       /*
> -        * NOTE: We currently only support printing frame information for
> -        * accesses to the task's own stack.
> -        */
> -       if (!object_is_on_stack(addr))
> -               return false;
> -
>         aligned_addr = round_down((unsigned long)addr, sizeof(long));
>         mem_ptr = round_down(aligned_addr, KASAN_GRANULE_SIZE);
>         shadow_ptr = kasan_mem_to_shadow((void *)aligned_addr);
> @@ -269,14 +263,13 @@ void kasan_print_address_stack_frame(const void
> *addr)
>         const char *frame_descr;
>         const void *frame_pc;
>
> +       if (WARN_ON(!object_is_on_stack(addr)))
> +               return;
> +
>         if (!get_address_stack_frame_info(addr, &offset, &frame_descr,
>                                           &frame_pc))
>                 return;
>
> -       /*
> -        * get_address_stack_frame_info only returns true if the given
> addr is
> -        * on the current task's stack.
> -        */
>         pr_err("\n");
>         pr_err("addr %px is located in stack of task %s/%d at offset %lu
> in frame:\n",
>                addr, current->comm, task_pid_nr(current), offset);
> --
> 2.25.1
>
> --
> You received this message because you are subscribed to the Google Groups
> "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to kasan-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/kasan-dev/1ee113a4c111df97d168c820b527cda77a3cac40.1646237226.git.andreyknvl%40google.com
> .
>


-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Liana Sebastian
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich. Falls Sie diese fälschlicherweise erhalten
haben sollten, leiten Sie diese bitte nicht an jemand anderes weiter,
löschen Sie alle Kopien und Anhänge davon und lassen Sie mich bitte wissen,
dass die E-Mail an die falsche Person gesendet wurde.



This e-mail is confidential. If you received this communication by mistake,
please don't forward it to anyone else, please erase all copies and
attachments, and please let me know that it has gone to the wrong person.

[-- Attachment #2: Type: text/html, Size: 6657 bytes --]

Re: [PATCH mm 04/22] kasan: improve stack frame info in reports

[Alexander Potapenko]

[-- Attachment #1: Type: text/plain, Size: 3046 bytes --]

On Wed, Mar 2, 2022 at 5:36 PM <andrey.konovalov@linux.dev> wrote:

> From: Andrey Konovalov <andreyknvl@google.com>
>
> - Print at least task name and id for reports affecting allocas
>   (get_address_stack_frame_info() does not support them).
>
> - Capitalize first letter of each sentence.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
>
Reviewed-by: Alexander Potapenko <glider@google.com>

> ---
>  mm/kasan/report_generic.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/mm/kasan/report_generic.c b/mm/kasan/report_generic.c
> index 3751391ff11a..7e03cca569a7 100644
> --- a/mm/kasan/report_generic.c
> +++ b/mm/kasan/report_generic.c
> @@ -180,7 +180,7 @@ static void print_decoded_frame_descr(const char
> *frame_descr)
>                 return;
>
>         pr_err("\n");
> -       pr_err("this frame has %lu %s:\n", num_objects,
> +       pr_err("This frame has %lu %s:\n", num_objects,
>                num_objects == 1 ? "object" : "objects");
>
>         while (num_objects--) {
> @@ -266,13 +266,14 @@ void kasan_print_address_stack_frame(const void
> *addr)
>         if (WARN_ON(!object_is_on_stack(addr)))
>                 return;
>
> +       pr_err("The buggy address belongs to stack of task %s/%d\n",
> +              current->comm, task_pid_nr(current));
> +
>         if (!get_address_stack_frame_info(addr, &offset, &frame_descr,
>                                           &frame_pc))
>                 return;
>
> -       pr_err("\n");
> -       pr_err("addr %px is located in stack of task %s/%d at offset %lu
> in frame:\n",
> -              addr, current->comm, task_pid_nr(current), offset);
> +       pr_err(" and is located at offset %lu in frame:\n", offset);
>         pr_err(" %pS\n", frame_pc);
>
>         if (!frame_descr)
> --
> 2.25.1
>
> --
> You received this message because you are subscribed to the Google Groups
> "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to kasan-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/kasan-dev/aa613f097c12f7b75efb17f2618ae00480fb4bc3.1646237226.git.andreyknvl%40google.com
> .
>


-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Liana Sebastian
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich. Falls Sie diese fälschlicherweise erhalten
haben sollten, leiten Sie diese bitte nicht an jemand anderes weiter,
löschen Sie alle Kopien und Anhänge davon und lassen Sie mich bitte wissen,
dass die E-Mail an die falsche Person gesendet wurde.



This e-mail is confidential. If you received this communication by mistake,
please don't forward it to anyone else, please erase all copies and
attachments, and please let me know that it has gone to the wrong person.

[-- Attachment #2: Type: text/html, Size: 4385 bytes --]

Re: [PATCH mm 05/22] kasan: print basic stack frame info for SW_TAGS

[Alexander Potapenko]

[-- Attachment #1: Type: text/plain, Size: 3147 bytes --]

On Wed, Mar 2, 2022 at 5:36 PM <andrey.konovalov@linux.dev> wrote:

> From: Andrey Konovalov <andreyknvl@google.com>
>
> Software Tag-Based mode tags stack allocations when CONFIG_KASAN_STACK
> is enabled. Print task name and id in reports for stack-related bugs.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
>
Reviewed-by: Alexander Potapenko <glider@google.com>

> ---
>  mm/kasan/kasan.h          |  2 +-
>  mm/kasan/report_sw_tags.c | 11 +++++++++++
>  2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
> index d1e111b7d5d8..4447df0d7343 100644
> --- a/mm/kasan/kasan.h
> +++ b/mm/kasan/kasan.h
> @@ -274,7 +274,7 @@ void *kasan_find_first_bad_addr(void *addr, size_t
> size);
>  const char *kasan_get_bug_type(struct kasan_access_info *info);
>  void kasan_metadata_fetch_row(char *buffer, void *row);
>
> -#if defined(CONFIG_KASAN_GENERIC) && defined(CONFIG_KASAN_STACK)
> +#if defined(CONFIG_KASAN_STACK)
>  void kasan_print_address_stack_frame(const void *addr);
>  #else
>  static inline void kasan_print_address_stack_frame(const void *addr) { }
> diff --git a/mm/kasan/report_sw_tags.c b/mm/kasan/report_sw_tags.c
> index d2298c357834..44577b8d47a7 100644
> --- a/mm/kasan/report_sw_tags.c
> +++ b/mm/kasan/report_sw_tags.c
> @@ -51,3 +51,14 @@ void kasan_print_tags(u8 addr_tag, const void *addr)
>
>         pr_err("Pointer tag: [%02x], memory tag: [%02x]\n", addr_tag,
> *shadow);
>  }
> +
> +#ifdef CONFIG_KASAN_STACK
> +void kasan_print_address_stack_frame(const void *addr)
> +{
> +       if (WARN_ON(!object_is_on_stack(addr)))
> +               return;
> +
> +       pr_err("The buggy address belongs to stack of task %s/%d\n",
> +              current->comm, task_pid_nr(current));
>
This comm/pid pattern starts to appear often, maybe we could replace it
with an inline function performing pr_cont()?


> +}
> +#endif
> --
> 2.25.1
>
> --
> You received this message because you are subscribed to the Google Groups
> "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to kasan-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/kasan-dev/029aaa87ceadde0702f3312a34697c9139c9fb53.1646237226.git.andreyknvl%40google.com
> .
>


-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Liana Sebastian
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich. Falls Sie diese fälschlicherweise erhalten
haben sollten, leiten Sie diese bitte nicht an jemand anderes weiter,
löschen Sie alle Kopien und Anhänge davon und lassen Sie mich bitte wissen,
dass die E-Mail an die falsche Person gesendet wurde.



This e-mail is confidential. If you received this communication by mistake,
please don't forward it to anyone else, please erase all copies and
attachments, and please let me know that it has gone to the wrong person.

[-- Attachment #2: Type: text/html, Size: 4470 bytes --]

Re: [PATCH mm 06/22] kasan: simplify async check in end_report

[Alexander Potapenko]

[-- Attachment #1: Type: text/plain, Size: 2020 bytes --]

On Wed, Mar 2, 2022 at 5:37 PM <andrey.konovalov@linux.dev> wrote:

> From: Andrey Konovalov <andreyknvl@google.com>
>
> Currently, end_report() does not call trace_error_report_end() for bugs
> detected in either async or asymm mode (when kasan_async_fault_possible()
> returns true), as the address of the bad access might be unknown.
>
> However, for asymm mode, the address is known for faults triggered by
> read operations.
>
> Instead of using kasan_async_fault_possible(), simply check that
> the addr is not NULL when calling trace_error_report_end().
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> ---
>  mm/kasan/report.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index d60ee8b81e2b..2d892ec050be 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -112,7 +112,7 @@ static void start_report(unsigned long *flags)
>
>  static void end_report(unsigned long *flags, unsigned long addr)
>  {
> -       if (!kasan_async_fault_possible())
> +       if (addr)
>                 trace_error_report_end(ERROR_DETECTOR_KASAN, addr);
>

What happens in the case of a NULL dereference? Don't we want to trigger
the tracepoint as well?


-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Liana Sebastian
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich. Falls Sie diese fälschlicherweise erhalten
haben sollten, leiten Sie diese bitte nicht an jemand anderes weiter,
löschen Sie alle Kopien und Anhänge davon und lassen Sie mich bitte wissen,
dass die E-Mail an die falsche Person gesendet wurde.



This e-mail is confidential. If you received this communication by mistake,
please don't forward it to anyone else, please erase all copies and
attachments, and please let me know that it has gone to the wrong person.

[-- Attachment #2: Type: text/html, Size: 2694 bytes --]

Re: [PATCH mm 07/22] kasan: simplify kasan_update_kunit_status and call sites

[Alexander Potapenko]

[-- Attachment #1: Type: text/plain, Size: 4474 bytes --]

On Wed, Mar 2, 2022 at 5:37 PM <andrey.konovalov@linux.dev> wrote:

> From: Andrey Konovalov <andreyknvl@google.com>
>
> - Rename kasan_update_kunit_status() to update_kunit_status()
>   (the function is static).
>
> - Move the IS_ENABLED(CONFIG_KUNIT) to the function's
>   definition instead of duplicating it at call sites.
>
> - Obtain and check current->kunit_test within the function.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
>

Reviewed-by: Alexander Potapenko <glider@google.com>


> ---
>  mm/kasan/report.c | 30 ++++++++++++++----------------
>  1 file changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index 2d892ec050be..59db81211b8a 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -357,24 +357,31 @@ static bool report_enabled(void)
>  }
>
>  #if IS_ENABLED(CONFIG_KUNIT)
> -static void kasan_update_kunit_status(struct kunit *cur_test, bool sync)
> +static void update_kunit_status(bool sync)
>  {
> +       struct kunit *test;
>         struct kunit_resource *resource;
>         struct kunit_kasan_status *status;
>
> -       resource = kunit_find_named_resource(cur_test, "kasan_status");
> +       test = current->kunit_test;
> +       if (!test)
> +               return;
>
> +       resource = kunit_find_named_resource(test, "kasan_status");
>         if (!resource) {
> -               kunit_set_failure(cur_test);
> +               kunit_set_failure(test);
>                 return;
>         }
>
>         status = (struct kunit_kasan_status *)resource->data;
>         WRITE_ONCE(status->report_found, true);
>         WRITE_ONCE(status->sync_fault, sync);
> +
>         kunit_put_resource(resource);
>  }
> -#endif /* IS_ENABLED(CONFIG_KUNIT) */
> +#else
> +static void update_kunit_status(bool sync) { }
> +#endif
>
>  void kasan_report_invalid_free(void *object, unsigned long ip)
>  {
> @@ -383,10 +390,7 @@ void kasan_report_invalid_free(void *object, unsigned
> long ip)
>
>         object = kasan_reset_tag(object);
>
> -#if IS_ENABLED(CONFIG_KUNIT)
> -       if (current->kunit_test)
> -               kasan_update_kunit_status(current->kunit_test, true);
> -#endif /* IS_ENABLED(CONFIG_KUNIT) */
> +       update_kunit_status(true);
>
>         start_report(&flags);
>         pr_err("BUG: KASAN: double-free or invalid-free in %pS\n", (void
> *)ip);
> @@ -402,10 +406,7 @@ void kasan_report_async(void)
>  {
>         unsigned long flags;
>
> -#if IS_ENABLED(CONFIG_KUNIT)
> -       if (current->kunit_test)
> -               kasan_update_kunit_status(current->kunit_test, false);
> -#endif /* IS_ENABLED(CONFIG_KUNIT) */
> +       update_kunit_status(false);
>
>         start_report(&flags);
>         pr_err("BUG: KASAN: invalid-access\n");
> @@ -424,10 +425,7 @@ static void __kasan_report(unsigned long addr, size_t
> size, bool is_write,
>         void *untagged_addr;
>         unsigned long flags;
>
> -#if IS_ENABLED(CONFIG_KUNIT)
> -       if (current->kunit_test)
> -               kasan_update_kunit_status(current->kunit_test, true);
> -#endif /* IS_ENABLED(CONFIG_KUNIT) */
> +       update_kunit_status(true);
>
>         disable_trace_on_warning();
>
> --
> 2.25.1
>
> --
> You received this message because you are subscribed to the Google Groups
> "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to kasan-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/kasan-dev/dac26d811ae31856c3d7666de0b108a3735d962d.1646237226.git.andreyknvl%40google.com
> .
>


-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Liana Sebastian
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich. Falls Sie diese fälschlicherweise erhalten
haben sollten, leiten Sie diese bitte nicht an jemand anderes weiter,
löschen Sie alle Kopien und Anhänge davon und lassen Sie mich bitte wissen,
dass die E-Mail an die falsche Person gesendet wurde.



This e-mail is confidential. If you received this communication by mistake,
please don't forward it to anyone else, please erase all copies and
attachments, and please let me know that it has gone to the wrong person.

[-- Attachment #2: Type: text/html, Size: 5968 bytes --]

Re: [PATCH mm 08/22] kasan: check CONFIG_KASAN_KUNIT_TEST instead of CONFIG_KUNIT

[Alexander Potapenko]

[-- Attachment #1: Type: text/plain, Size: 3246 bytes --]

On Wed, Mar 2, 2022 at 5:37 PM <andrey.konovalov@linux.dev> wrote:

> From: Andrey Konovalov <andreyknvl@google.com>
>
> Check the more specific CONFIG_KASAN_KUNIT_TEST config option when
> defining things related to KUnit-compatible KASAN tests instead of
> CONFIG_KUNIT.
>
> Also put the kunit_kasan_status definition next to the definitons of
> other KASAN-related structs.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
>
Reviewed-by: Alexander Potapenko <glider@google.com>


> ---
>  mm/kasan/kasan.h  | 18 ++++++++----------
>  mm/kasan/report.c |  2 +-
>  2 files changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
> index 4447df0d7343..cc7162a9f304 100644
> --- a/mm/kasan/kasan.h
> +++ b/mm/kasan/kasan.h
> @@ -7,16 +7,6 @@
>  #include <linux/kfence.h>
>  #include <linux/stackdepot.h>
>
> -#if IS_ENABLED(CONFIG_KUNIT)
> -
> -/* Used in KUnit-compatible KASAN tests. */
> -struct kunit_kasan_status {
> -       bool report_found;
> -       bool sync_fault;
> -};
> -
> -#endif
> -
>  #ifdef CONFIG_KASAN_HW_TAGS
>
>  #include <linux/static_key.h>
> @@ -224,6 +214,14 @@ struct kasan_free_meta {
>  #endif
>  };
>
> +#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
> +/* Used in KUnit-compatible KASAN tests. */
> +struct kunit_kasan_status {
> +       bool report_found;
> +       bool sync_fault;
> +};
> +#endif
> +
>  struct kasan_alloc_meta *kasan_get_alloc_meta(struct kmem_cache *cache,
>                                                 const void *object);
>  #ifdef CONFIG_KASAN_GENERIC
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index 59db81211b8a..93543157d3e1 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -356,7 +356,7 @@ static bool report_enabled(void)
>         return !test_and_set_bit(KASAN_BIT_REPORTED, &kasan_flags);
>  }
>
> -#if IS_ENABLED(CONFIG_KUNIT)
> +#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
>  static void update_kunit_status(bool sync)
>  {
>         struct kunit *test;
> --
> 2.25.1
>
> --
> You received this message because you are subscribed to the Google Groups
> "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to kasan-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/kasan-dev/223592d38d2a601a160a3b2b3d5a9f9090350e62.1646237226.git.andreyknvl%40google.com
> .
>


-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Liana Sebastian
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich. Falls Sie diese fälschlicherweise erhalten
haben sollten, leiten Sie diese bitte nicht an jemand anderes weiter,
löschen Sie alle Kopien und Anhänge davon und lassen Sie mich bitte wissen,
dass die E-Mail an die falsche Person gesendet wurde.



This e-mail is confidential. If you received this communication by mistake,
please don't forward it to anyone else, please erase all copies and
attachments, and please let me know that it has gone to the wrong person.

[-- Attachment #2: Type: text/html, Size: 4494 bytes --]

Re: [PATCH mm 10/22] kasan: move disable_trace_on_warning to start_report

[Alexander Potapenko]

[-- Attachment #1: Type: text/plain, Size: 2571 bytes --]

On Wed, Mar 2, 2022 at 5:37 PM <andrey.konovalov@linux.dev> wrote:

> From: Andrey Konovalov <andreyknvl@google.com>
>
> Move the disable_trace_on_warning() call, which enables the
> /proc/sys/kernel/traceoff_on_warning interface for KASAN bugs,
> to start_report(), so that it functions for all types of KASAN reports.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
>
Reviewed-by: Alexander Potapenko <glider@google.com>


---

>  mm/kasan/report.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index 0b6c8a14f0ea..9286ff6ae1a7 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -129,6 +129,8 @@ static DEFINE_SPINLOCK(report_lock);
>
>  static void start_report(unsigned long *flags, bool sync)
>  {
> +       /* Respect the /proc/sys/kernel/traceoff_on_warning interface. */
> +       disable_trace_on_warning();
>         /* Update status of the currently running KASAN test. */
>         update_kunit_status(sync);
>         /* Make sure we don't end up in loop. */
> @@ -421,7 +423,6 @@ static void __kasan_report(unsigned long addr, size_t
> size, bool is_write,
>         void *untagged_addr;
>         unsigned long flags;
>
> -       disable_trace_on_warning();
>         start_report(&flags, true);
>
>         tagged_addr = (void *)addr;
> --
> 2.25.1
>
> --
> You received this message because you are subscribed to the Google Groups
> "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to kasan-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/kasan-dev/7c066c5de26234ad2cebdd931adfe437f8a95d58.1646237226.git.andreyknvl%40google.com
> .
>


-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Liana Sebastian
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich. Falls Sie diese fälschlicherweise erhalten
haben sollten, leiten Sie diese bitte nicht an jemand anderes weiter,
löschen Sie alle Kopien und Anhänge davon und lassen Sie mich bitte wissen,
dass die E-Mail an die falsche Person gesendet wurde.



This e-mail is confidential. If you received this communication by mistake,
please don't forward it to anyone else, please erase all copies and
attachments, and please let me know that it has gone to the wrong person.

[-- Attachment #2: Type: text/html, Size: 4697 bytes --]

Re: [PATCH mm 06/22] kasan: simplify async check in end_report

[Andrey Konovalov]

On Wed, Mar 2, 2022 at 6:38 PM Alexander Potapenko <glider@google.com> wrote:
>
>
>
> On Wed, Mar 2, 2022 at 5:37 PM <andrey.konovalov@linux.dev> wrote:
>>
>> From: Andrey Konovalov <andreyknvl@google.com>
>>
>> Currently, end_report() does not call trace_error_report_end() for bugs
>> detected in either async or asymm mode (when kasan_async_fault_possible()
>> returns true), as the address of the bad access might be unknown.
>>
>> However, for asymm mode, the address is known for faults triggered by
>> read operations.
>>
>> Instead of using kasan_async_fault_possible(), simply check that
>> the addr is not NULL when calling trace_error_report_end().
>>
>> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
>> ---
>>  mm/kasan/report.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
>> index d60ee8b81e2b..2d892ec050be 100644
>> --- a/mm/kasan/report.c
>> +++ b/mm/kasan/report.c
>> @@ -112,7 +112,7 @@ static void start_report(unsigned long *flags)
>>
>>  static void end_report(unsigned long *flags, unsigned long addr)
>>  {
>> -       if (!kasan_async_fault_possible())
>> +       if (addr)
>>                 trace_error_report_end(ERROR_DETECTOR_KASAN, addr);
>
>
> What happens in the case of a NULL dereference? Don't we want to trigger the tracepoint as well?

A NULL pointer dereference is never reported through KASAN: for
software modes, it triggers a GPF when accessing shadow, and for
HW_TAGS, it takes precedence over a tag mismatch.

Thanks!

Re: [PATCH mm 05/22] kasan: print basic stack frame info for SW_TAGS

[Andrey Konovalov]

On Wed, Mar 2, 2022 at 6:34 PM Alexander Potapenko <glider@google.com> wrote:
>
>> diff --git a/mm/kasan/report_sw_tags.c b/mm/kasan/report_sw_tags.c
>> index d2298c357834..44577b8d47a7 100644
>> --- a/mm/kasan/report_sw_tags.c
>> +++ b/mm/kasan/report_sw_tags.c
>> @@ -51,3 +51,14 @@ void kasan_print_tags(u8 addr_tag, const void *addr)
>>
>>         pr_err("Pointer tag: [%02x], memory tag: [%02x]\n", addr_tag, *shadow);
>>  }
>> +
>> +#ifdef CONFIG_KASAN_STACK
>> +void kasan_print_address_stack_frame(const void *addr)
>> +{
>> +       if (WARN_ON(!object_is_on_stack(addr)))
>> +               return;
>> +
>> +       pr_err("The buggy address belongs to stack of task %s/%d\n",
>> +              current->comm, task_pid_nr(current));
>
> This comm/pid pattern starts to appear often, maybe we could replace it with an inline function performing pr_cont()?

Sounds good, will do if/when posting a v2. Thanks!