From: Andrey Konovalov <[email protected]>
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
From: Andrey Konovalov <[email protected]>
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 <[email protected]>
---
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
From: Andrey Konovalov <[email protected]>
Restructure kasan_report() to make reviewing the subsequent patches
easier.
Signed-off-by: Andrey Konovalov <[email protected]>
---
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
From: Andrey Konovalov <[email protected]>
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 <[email protected]>
---
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
From: Andrey Konovalov <[email protected]>
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 <[email protected]>
---
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
From: Andrey Konovalov <[email protected]>
- 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 <[email protected]>
---
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
From: Andrey Konovalov <[email protected]>
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 <[email protected]>
---
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
From: Andrey Konovalov <[email protected]>
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 <[email protected]>
---
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
From: Andrey Konovalov <[email protected]>
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 <[email protected]>
---
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
From: Andrey Konovalov <[email protected]>
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 <[email protected]>
---
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
From: Andrey Konovalov <[email protected]>
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 <[email protected]>
---
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
From: Andrey Konovalov <[email protected]>
Merge __kasan_report() into kasan_report(). The code is simple enough
to be readable without the __kasan_report() helper.
Signed-off-by: Andrey Konovalov <[email protected]>
---
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
From: Andrey Konovalov <[email protected]>
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 <[email protected]>
---
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
From: Andrey Konovalov <[email protected]>
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 <[email protected]>
---
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
From: Andrey Konovalov <[email protected]>
Place kasan_report_async() next to the other main reporting routines.
Also simplify printed information.
Signed-off-by: Andrey Konovalov <[email protected]>
---
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
From: Andrey Konovalov <[email protected]>
- 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 <[email protected]>
---
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
From: Andrey Konovalov <[email protected]>
Rename kasan_access_info to kasan_report_info, as the latter name better
reflects the struct's purpose.
Signed-off-by: Andrey Konovalov <[email protected]>
---
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
From: Andrey Konovalov <[email protected]>
Move the addr_has_metadata() check into kasan_find_first_bad_addr().
Signed-off-by: Andrey Konovalov <[email protected]>
---
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
From: Andrey Konovalov <[email protected]>
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 <[email protected]>
---
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
From: Andrey Konovalov <[email protected]>
Add a line break after each part that describes the buggy address.
Improves readability of reports.
Signed-off-by: Andrey Konovalov <[email protected]>
---
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
On Wed, Mar 2, 2022 at 6:38 PM Alexander Potapenko <[email protected]> wrote:
>
>
>
> On Wed, Mar 2, 2022 at 5:37 PM <[email protected]> wrote:
>>
>> From: Andrey Konovalov <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> 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!
On Wed, Mar 2, 2022 at 6:34 PM Alexander Potapenko <[email protected]> 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!