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

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@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

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@google.com>

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

Signed-off-by: Andrey Konovalov <andre...@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

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@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 <andre...@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

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@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 <andre...@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

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@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 <andre...@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

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@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 <andre...@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

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@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 <andre...@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

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@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 <andre...@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

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@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 <andre...@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

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@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 <andre...@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

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@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 <andre...@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

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@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 <andre...@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

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@google.com>

Move the addr_has_metadata() check into kasan_find_first_bad_addr().

Signed-off-by: Andrey Konovalov <andre...@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

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@google.com>

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

Signed-off-by: Andrey Konovalov <andre...@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

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@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 <andre...@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

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@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 <andre...@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

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@google.com>

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

Signed-off-by: Andrey Konovalov <andre...@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

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@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 <andre...@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

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@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 <andre...@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

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@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 <andre...@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

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@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 <andre...@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

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@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 <andre...@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

[andrey.k...@linux.dev]

From: Andrey Konovalov <andre...@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 <andre...@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

[Alexander Potapenko]

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

From: Andrey Konovalov <andre...@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 <andre...@google.com>

Reviewed-by: Alexander Potapenko <gli...@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+...@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.

[Alexander Potapenko]

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

From: Andrey Konovalov <andre...@google.com>

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

Signed-off-by: Andrey Konovalov <andre...@google.com>

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

--
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+...@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]

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

From: Andrey Konovalov <andre...@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 <andre...@google.com>

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

--
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+...@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]

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

From: Andrey Konovalov <andre...@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 <andre...@google.com>

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

--
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+...@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]

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

From: Andrey Konovalov <andre...@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 <andre...@google.com>

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

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+...@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]

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

From: Andrey Konovalov <andre...@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 <andre...@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]

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

From: Andrey Konovalov <andre...@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 <andre...@google.com>

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

 

--
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+...@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]

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

From: Andrey Konovalov <andre...@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 <andre...@google.com>

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

 

--
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+...@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]

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

From: Andrey Konovalov <andre...@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 <andre...@google.com>

Reviewed-by: Alexander Potapenko <gli...@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+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/7c066c5de26234ad2cebdd931adfe437f8a95d58.1646237226.git.andreyknvl%40google.com.

[Andrey Konovalov]

On Wed, Mar 2, 2022 at 6:38 PM Alexander Potapenko <gli...@google.com> wrote:
>
>
>
> On Wed, Mar 2, 2022 at 5:37 PM <andrey.k...@linux.dev> wrote:
>>
>> From: Andrey Konovalov <andre...@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 <andre...@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!

[Andrey Konovalov]

On Wed, Mar 2, 2022 at 6:34 PM Alexander Potapenko <gli...@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!