[PATCH 1/3] rcu/kasan: record and print call_rcu() call stack

[Walter Wu]

When call_rcu() is called, we store the call_rcu() call stack into
slub alloc meta-data, so that KASAN report prints call_rcu() information.

We add new KASAN_RCU_STACK_RECORD configuration option. It will record
first and last call_rcu() call stack and KASAN report will print two
call_rcu() call stack.

This option doesn't increase the cost of memory consumption. Because
we don't enlarge struct kasan_alloc_meta size.
- add two call_rcu() call stack into kasan_alloc_meta, size is 8 bytes.
- remove free track from kasan_alloc_meta, size is 8 bytes.

[1]https://bugzilla.kernel.org/show_bug.cgi?id=198437

Signed-off-by: Walter Wu <walter...@mediatek.com>
Suggested-by: Dmitry Vyukov <dvy...@google.com>
Cc: Andrey Ryabinin <arya...@virtuozzo.com>
Cc: Dmitry Vyukov <dvy...@google.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Andrew Morton <ak...@linux-foundation.org>
Cc: Paul E. McKenney <pau...@kernel.org>
Cc: Josh Triplett <jo...@joshtriplett.org>
Cc: Mathieu Desnoyers <mathieu....@efficios.com>
Cc: Lai Jiangshan <jiangs...@gmail.com>
Cc: Joel Fernandes <jo...@joelfernandes.org>
---
include/linux/kasan.h | 7 +++++++
kernel/rcu/tree.c | 4 ++++
lib/Kconfig.kasan | 11 +++++++++++
mm/kasan/common.c | 23 +++++++++++++++++++++++
mm/kasan/kasan.h | 12 ++++++++++++
mm/kasan/report.c | 33 +++++++++++++++++++++++++++------
6 files changed, 84 insertions(+), 6 deletions(-)

diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index 31314ca7c635..5eeece6893cd 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -96,6 +96,12 @@ size_t kasan_metadata_size(struct kmem_cache *cache);
bool kasan_save_enable_multi_shot(void);
void kasan_restore_multi_shot(bool enabled);

+#ifdef CONFIG_KASAN_RCU_STACK_RECORD
+void kasan_record_callrcu(void *ptr);
+#else
+static inline void kasan_record_callrcu(void *ptr) {}
+#endif
+
#else /* CONFIG_KASAN */

static inline void kasan_unpoison_shadow(const void *address, size_t size) {}
@@ -165,6 +171,7 @@ static inline void kasan_remove_zero_shadow(void *start,

static inline void kasan_unpoison_slab(const void *ptr) { }
static inline size_t kasan_metadata_size(struct kmem_cache *cache) { return 0; }
+static inline void kasan_record_callrcu(void *ptr) {}

#endif /* CONFIG_KASAN */

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 06548e2ebb72..145c79becf7b 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -57,6 +57,7 @@
#include <linux/slab.h>
#include <linux/sched/isolation.h>
#include <linux/sched/clock.h>
+#include <linux/kasan.h>
#include "../time/tick-internal.h"

#include "tree.h"
@@ -2694,6 +2695,9 @@ __call_rcu(struct rcu_head *head, rcu_callback_t func)
trace_rcu_callback(rcu_state.name, head,
rcu_segcblist_n_cbs(&rdp->cblist));

+ if (IS_ENABLED(CONFIG_KASAN_RCU_STACK_RECORD))
+ kasan_record_callrcu(head);
+
/* Go handle any RCU core processing required. */
if (IS_ENABLED(CONFIG_RCU_NOCB_CPU) &&
unlikely(rcu_segcblist_is_offloaded(&rdp->cblist))) {
diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index 81f5464ea9e1..022934049cc2 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -158,6 +158,17 @@ config KASAN_VMALLOC
for KASAN to detect more sorts of errors (and to support vmapped
stacks), but at the cost of higher memory usage.

+config KASAN_RCU_STACK_RECORD
+ bool "Record and print call_rcu() call stack"
+ depends on KASAN_GENERIC
+ help
+ By default, the KASAN report doesn't print call_rcu() call stack.
+ It is very difficult to analyze memory issues(e.g., use-after-free).
+
+ Enabling this option will print first and last call_rcu() call stack.
+ It doesn't enlarge slub alloc meta-data size, so it doesn't increase
+ the cost of memory consumption.
+
config TEST_KASAN
tristate "Module for testing KASAN for bug detection"
depends on m && KASAN
diff --git a/mm/kasan/common.c b/mm/kasan/common.c
index 2906358e42f0..32d422bdf127 100644
--- a/mm/kasan/common.c
+++ b/mm/kasan/common.c
@@ -299,6 +299,29 @@ struct kasan_free_meta *get_free_info(struct kmem_cache *cache,
return (void *)object + cache->kasan_info.free_meta_offset;
}

+#ifdef CONFIG_KASAN_RCU_STACK_RECORD
+void kasan_record_callrcu(void *addr)
+{
+ struct page *page = kasan_addr_to_page(addr);
+ struct kmem_cache *cache;
+ struct kasan_alloc_meta *alloc_info;
+ void *object;
+
+ if (!(page && PageSlab(page)))
+ return;
+
+ cache = page->slab_cache;
+ object = nearest_obj(cache, page, addr);
+ alloc_info = get_alloc_info(cache, object);
+
+ if (!alloc_info->rcu_free_stack[0])
+ /* record first call_rcu() call stack */
+ alloc_info->rcu_free_stack[0] = save_stack(GFP_NOWAIT);
+ else
+ /* record last call_rcu() call stack */
+ alloc_info->rcu_free_stack[1] = save_stack(GFP_NOWAIT);
+}
+#endif

static void kasan_set_free_info(struct kmem_cache *cache,
void *object, u8 tag)
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index e8f37199d885..adc105b9cd07 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -96,15 +96,27 @@ struct kasan_track {
depot_stack_handle_t stack;
};

+#ifdef CONFIG_KASAN_RCU_STACK_RECORD
+#define BYTES_PER_WORD 4
+#define KASAN_NR_RCU_FREE_STACKS 2
+#else /* CONFIG_KASAN_RCU_STACK_RECORD */
#ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY
#define KASAN_NR_FREE_STACKS 5
#else
#define KASAN_NR_FREE_STACKS 1
#endif
+#endif /* CONFIG_KASAN_RCU_STACK_RECORD */

struct kasan_alloc_meta {
struct kasan_track alloc_track;
+#ifdef CONFIG_KASAN_RCU_STACK_RECORD
+ /* call_rcu() call stack is stored into kasan_alloc_meta.
+ * free stack is stored into freed object.
+ */
+ depot_stack_handle_t rcu_free_stack[KASAN_NR_RCU_FREE_STACKS];
+#else
struct kasan_track free_track[KASAN_NR_FREE_STACKS];
+#endif
#ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY
u8 free_pointer_tag[KASAN_NR_FREE_STACKS];
u8 free_track_idx;
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 80f23c9da6b0..7aaccc70b65b 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -105,9 +105,13 @@ static void end_report(unsigned long *flags)
kasan_enable_current();
}

-static void print_track(struct kasan_track *track, const char *prefix)
+static void print_track(struct kasan_track *track, const char *prefix,
+ bool is_callrcu)
{
- pr_err("%s by task %u:\n", prefix, track->pid);
+ if (is_callrcu)
+ pr_err("%s:\n", prefix);
+ else
+ pr_err("%s by task %u:\n", prefix, track->pid);
if (track->stack) {
unsigned long *entries;
unsigned int nr_entries;
@@ -159,8 +163,22 @@ static void describe_object_addr(struct kmem_cache *cache, void *object,
(void *)(object_addr + cache->object_size));
}

+#ifdef CONFIG_KASAN_RCU_STACK_RECORD
+static void kasan_print_rcu_free_stack(struct kasan_alloc_meta *alloc_info)
+{
+ struct kasan_track free_track;
+
+ free_track.stack = alloc_info->rcu_free_stack[0];
+ print_track(&free_track, "First call_rcu() call stack", true);
+ pr_err("\n");
+ free_track.stack = alloc_info->rcu_free_stack[1];
+ print_track(&free_track, "Last call_rcu() call stack", true);
+ pr_err("\n");
+}
+#endif
+
static struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,
- void *object, u8 tag)
+ void *object, u8 tag, const void *addr)
{
struct kasan_alloc_meta *alloc_meta;
int i = 0;
@@ -187,11 +205,14 @@ static void describe_object(struct kmem_cache *cache, void *object,
if (cache->flags & SLAB_KASAN) {
struct kasan_track *free_track;

- print_track(&alloc_info->alloc_track, "Allocated");
+ print_track(&alloc_info->alloc_track, "Allocated", false);
pr_err("\n");
- free_track = kasan_get_free_track(cache, object, tag);
- print_track(free_track, "Freed");
+ free_track = kasan_get_free_track(cache, object, tag, addr);
+ print_track(free_track, "Freed", false);
pr_err("\n");
+#ifdef CONFIG_KASAN_RCU_STACK_RECORD
+ kasan_print_rcu_free_stack(alloc_info);
+#endif
}

describe_object_addr(cache, object, addr);
--
2.18.0

[Dmitry Vyukov]

The issue also mentions workqueue and timer stacks.
Have you considered supporting them as well? What was your motivation
for doing only rcu?

Looking at the first report for "workqueue use-after-free":
https://syzkaller.appspot.com/bug?extid=9cba1e478f91aad39876
This is exactly the same situation as for call_rcu, just a workqueue
is used to invoke a callback that frees the object.

If you don't want to do all at the same time, I would at least
name/branch everything inside of KASAN more generally (I think in the
issue I called it "aux" (auxiliary), or maybe something like
"additional"). But then call this kasan_record_aux_stack() only from
rcu for now. But then later we can separately decide and extend to
other callers.
It just feels wrong to have KASAN over-specialized for rcu only in this way.
And I think if the UAF is really caused by call_rcu callback, then it
sill will be recorded as last stack most of the time because rcu
callbacks are invoked relatively fast and there should not be much
else happening with the object since it's near end of life already.

The if is not necessary, this function is no-op when not enabled.

> --
> 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/20200506052046.14451-1-walter-zh.wu%40mediatek.com.

[Walter Wu]

I will try to implement them when I have free time, maybe we can do it
step by step, finish the printing call_rcu() first.
I remember that I saw the following link, recording call_rcu seems like
be needed.

https://groups.google.com/forum/#!searchin/kasan-dev/better$20stack
$20traces$20for$20rcu%7Csort:date/kasan-dev/KQsjT_88hDE/7rNUZprRBgAJ

> Looking at the first report for "workqueue use-after-free":
> https://syzkaller.appspot.com/bug?extid=9cba1e478f91aad39876
> This is exactly the same situation as for call_rcu, just a workqueue
> is used to invoke a callback that frees the object.
>
> If you don't want to do all at the same time, I would at least
> name/branch everything inside of KASAN more generally (I think in the
> issue I called it "aux" (auxiliary), or maybe something like
> "additional"). But then call this kasan_record_aux_stack() only from
> rcu for now. But then later we can separately decide and extend to
> other callers.
> It just feels wrong to have KASAN over-specialized for rcu only in this way.
> And I think if the UAF is really caused by call_rcu callback, then it
> sill will be recorded as last stack most of the time because rcu
> callbacks are invoked relatively fast and there should not be much
> else happening with the object since it's near end of life already.
>

Yes, I agree with you. I will send v2 patchset as you saying.

[Walter Wu]

This feature will record first and last call_rcu() call stack and
print two call_rcu() call stack in KASAN report.

When call_rcu() is called, we store the call_rcu() call stack into

slub alloc meta-data, so that KASAN report can print rcu stack.

It doesn't increase the cost of memory consumption. Because we don't

enlarge struct kasan_alloc_meta size.
- add two call_rcu() call stack into kasan_alloc_meta, size is 8 bytes.
- remove free track from kasan_alloc_meta, size is 8 bytes.

[1]https://bugzilla.kernel.org/show_bug.cgi?id=198437

[2]https://groups.google.com/forum/#!searchin/kasan-dev/better$20stack$20traces$20for$20rcu%7Csort:date/kasan-dev/KQsjT_88hDE/7rNUZprRBgAJ

Signed-off-by: Walter Wu <walter...@mediatek.com>
Suggested-by: Dmitry Vyukov <dvy...@google.com>
Cc: Andrey Ryabinin <arya...@virtuozzo.com>
Cc: Dmitry Vyukov <dvy...@google.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Andrew Morton <ak...@linux-foundation.org>
Cc: Paul E. McKenney <pau...@kernel.org>
Cc: Josh Triplett <jo...@joshtriplett.org>
Cc: Mathieu Desnoyers <mathieu....@efficios.com>
Cc: Lai Jiangshan <jiangs...@gmail.com>
Cc: Joel Fernandes <jo...@joelfernandes.org>
---

include/linux/kasan.h | 2 ++
kernel/rcu/tree.c | 3 +++
lib/Kconfig.kasan | 2 ++
mm/kasan/common.c | 4 ++--
mm/kasan/generic.c | 29 +++++++++++++++++++++++++++++
mm/kasan/kasan.h | 19 +++++++++++++++++++
mm/kasan/report.c | 21 +++++++++++++++++----
7 files changed, 74 insertions(+), 6 deletions(-)

diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index 31314ca7c635..23b7ee00572d 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -174,11 +174,13 @@ static inline size_t kasan_metadata_size(struct kmem_cache *cache) { return 0; }

void kasan_cache_shrink(struct kmem_cache *cache);
void kasan_cache_shutdown(struct kmem_cache *cache);
+void kasan_record_aux_stack(void *ptr);

#else /* CONFIG_KASAN_GENERIC */

static inline void kasan_cache_shrink(struct kmem_cache *cache) {}
static inline void kasan_cache_shutdown(struct kmem_cache *cache) {}
+static inline void kasan_record_aux_stack(void *ptr) {}

#endif /* CONFIG_KASAN_GENERIC */

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 06548e2ebb72..de872b6cc261 100644

--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -57,6 +57,7 @@
#include <linux/slab.h>
#include <linux/sched/isolation.h>
#include <linux/sched/clock.h>
+#include <linux/kasan.h>
#include "../time/tick-internal.h"

#include "tree.h"

@@ -2694,6 +2695,8 @@ __call_rcu(struct rcu_head *head, rcu_callback_t func)
trace_rcu_callback(rcu_state.name, head,
rcu_segcblist_n_cbs(&rdp->cblist));

+ kasan_record_aux_stack(head);

+
/* Go handle any RCU core processing required. */
if (IS_ENABLED(CONFIG_RCU_NOCB_CPU) &&
unlikely(rcu_segcblist_is_offloaded(&rdp->cblist))) {
diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan

index 81f5464ea9e1..56a89291f1cc 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -58,6 +58,8 @@ config KASAN_GENERIC
For better error detection enable CONFIG_STACKTRACE.
Currently CONFIG_KASAN_GENERIC doesn't work with CONFIG_DEBUG_SLAB
(the resulting kernel does not boot).
+ Currently CONFIG_KASAN_GENERIC will print first and last call_rcu()
+ call stack. It doesn't increase the cost of memory consumption.

config KASAN_SW_TAGS
bool "Software tag-based mode"
diff --git a/mm/kasan/common.c b/mm/kasan/common.c
index 2906358e42f0..8bc618289bb1 100644
--- a/mm/kasan/common.c
+++ b/mm/kasan/common.c
@@ -41,7 +41,7 @@
#include "kasan.h"
#include "../slab.h"

-static inline depot_stack_handle_t save_stack(gfp_t flags)
+depot_stack_handle_t kasan_save_stack(gfp_t flags)
{
unsigned long entries[KASAN_STACK_DEPTH];
unsigned int nr_entries;
@@ -54,7 +54,7 @@ static inline depot_stack_handle_t save_stack(gfp_t flags)
static inline void set_track(struct kasan_track *track, gfp_t flags)
{
track->pid = current->pid;
- track->stack = save_stack(flags);
+ track->stack = kasan_save_stack(flags);
}

void kasan_enable_current(void)
diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c
index 56ff8885fe2e..b86880c338e2 100644
--- a/mm/kasan/generic.c
+++ b/mm/kasan/generic.c
@@ -325,3 +325,32 @@ DEFINE_ASAN_SET_SHADOW(f2);
DEFINE_ASAN_SET_SHADOW(f3);
DEFINE_ASAN_SET_SHADOW(f5);
DEFINE_ASAN_SET_SHADOW(f8);
+
+void kasan_record_aux_stack(void *addr)

+{
+ struct page *page = kasan_addr_to_page(addr);
+ struct kmem_cache *cache;
+ struct kasan_alloc_meta *alloc_info;
+ void *object;
+
+ if (!(page && PageSlab(page)))
+ return;
+
+ cache = page->slab_cache;
+ object = nearest_obj(cache, page, addr);
+ alloc_info = get_alloc_info(cache, object);
+

+ if (!alloc_info->rcu_stack[0])

+ /* record first call_rcu() call stack */

+ alloc_info->rcu_stack[0] = kasan_save_stack(GFP_NOWAIT);

+ else
+ /* record last call_rcu() call stack */

+ alloc_info->rcu_stack[1] = kasan_save_stack(GFP_NOWAIT);
+}
+
+struct kasan_track *kasan_get_aux_stack(struct kasan_alloc_meta *alloc_info,
+ u8 idx)
+{
+ return container_of(&alloc_info->rcu_stack[idx],
+ struct kasan_track, stack);
+}
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index e8f37199d885..1cc1fb7b0de3 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -96,15 +96,28 @@ struct kasan_track {
depot_stack_handle_t stack;
};

+#ifdef CONFIG_KASAN_GENERIC
+#define SIZEOF_PTR sizeof(void *)
+#define KASAN_NR_RCU_CALL_STACKS 2
+#else /* CONFIG_KASAN_GENERIC */

#ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY
#define KASAN_NR_FREE_STACKS 5
#else
#define KASAN_NR_FREE_STACKS 1
#endif

+#endif /* CONFIG_KASAN_GENERIC */

struct kasan_alloc_meta {
struct kasan_track alloc_track;

+#ifdef CONFIG_KASAN_GENERIC
+ /*
+ * call_rcu() call stack is stored into struct kasan_alloc_meta.
+ * The free stack is stored into freed object.
+ */
+ depot_stack_handle_t rcu_stack[KASAN_NR_RCU_CALL_STACKS];

+#else
struct kasan_track free_track[KASAN_NR_FREE_STACKS];
+#endif
#ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY
u8 free_pointer_tag[KASAN_NR_FREE_STACKS];
u8 free_track_idx;

@@ -159,16 +172,22 @@ void kasan_report_invalid_free(void *object, unsigned long ip);

struct page *kasan_addr_to_page(const void *addr);

+depot_stack_handle_t kasan_save_stack(gfp_t flags);
+
#if defined(CONFIG_KASAN_GENERIC) && \
(defined(CONFIG_SLAB) || defined(CONFIG_SLUB))
void quarantine_put(struct kasan_free_meta *info, struct kmem_cache *cache);
void quarantine_reduce(void);
void quarantine_remove_cache(struct kmem_cache *cache);
+struct kasan_track *kasan_get_aux_stack(struct kasan_alloc_meta *alloc_info,
+ u8 idx);
#else
static inline void quarantine_put(struct kasan_free_meta *info,
struct kmem_cache *cache) { }
static inline void quarantine_reduce(void) { }
static inline void quarantine_remove_cache(struct kmem_cache *cache) { }
+static inline struct kasan_track *kasan_get_aux_stack(
+ struct kasan_alloc_meta *alloc_info, u8 idx) { return NULL; }
#endif

#ifdef CONFIG_KASAN_SW_TAGS
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 80f23c9da6b0..f16a1a210815 100644

--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -105,9 +105,13 @@ static void end_report(unsigned long *flags)
kasan_enable_current();
}

-static void print_track(struct kasan_track *track, const char *prefix)
+static void print_track(struct kasan_track *track, const char *prefix,
+ bool is_callrcu)
{
- pr_err("%s by task %u:\n", prefix, track->pid);
+ if (is_callrcu)
+ pr_err("%s:\n", prefix);
+ else
+ pr_err("%s by task %u:\n", prefix, track->pid);
if (track->stack) {
unsigned long *entries;
unsigned int nr_entries;

@@ -187,11 +191,20 @@ static void describe_object(struct kmem_cache *cache, void *object,

if (cache->flags & SLAB_KASAN) {
struct kasan_track *free_track;

- print_track(&alloc_info->alloc_track, "Allocated");
+ print_track(&alloc_info->alloc_track, "Allocated", false);
pr_err("\n");

free_track = kasan_get_free_track(cache, object, tag);
- print_track(free_track, "Freed");

+ print_track(free_track, "Freed", false);
pr_err("\n");
+

+ if (IS_ENABLED(CONFIG_KASAN_GENERIC)) {
+ free_track = kasan_get_aux_stack(alloc_info, 0);
+ print_track(free_track, "First call_rcu() call stack", true);
+ pr_err("\n");
+ free_track = kasan_get_aux_stack(alloc_info, 1);
+ print_track(free_track, "Last call_rcu() call stack", true);

+ pr_err("\n");
+ }
}

[Dmitry Vyukov]

This happens after we queued the object onto some queue and after some
return statements. I think this is wrong.
We need to do this somewhere at the very beginning of the function.

This is what I meant by "hard to review". This is completely invisible
in the diff.

> --
> 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/20200511023111.15310-1-walter-zh.wu%40mediatek.com.

[Dmitry Vyukov]

On Mon, May 11, 2020 at 4:31 AM Walter Wu <walter...@mediatek.com> wrote:
>

We don't plan to change this and this is not a bug, right? So I think
using "Currently" is confusing. What's changing in future?

s/will print/prints/
Simple present tense is the default for documentation, we are just
stating facts.

The remark about not increasing memory consumption is both false and
not useful (we don't give an option to change this).

I would just say:

"In generic mode KASAN prints first and last call_rcu() call stacks in reports."

[Dmitry Vyukov]

On Mon, May 11, 2020 at 4:31 AM Walter Wu <walter...@mediatek.com> wrote:
>

This is not type safe, there is no kasan_track object. And we create a
pointer to kasan_track just to carefully not treat it as valid
kasan_track in print_track.

This adds an unnecessary if to print_track. And does not seem to be
useful/nice to print:

First call_rcu() call stack:
(stack is not available)

Last call_rcu() call stack:
(stack is not available)

when no rcu stacks are memorized.
Your intention seems to be to reuse 2 lines of code from print_track.
I would factor them out into a function:

static void print_stack(depot_stack_handle_t stack)

{
unsigned long *entries;
unsigned int nr_entries;

nr_entries = stack_depot_fetch(stack, &entries);
stack_trace_print(entries, nr_entries, 0);
}

And then this can expressed as:

if (IS_ENABLED(CONFIG_KASAN_GENERIC)) {
stack = alloc_info->rcu_stack[0];
if (stack) {
pr_err("First call_rcu() call stack:\n");
print_stack(stack);
pr_err("\n");
}
stack = alloc_info->rcu_stack[1];
if (stack) {
pr_err("Last call_rcu() call stack:\n");
print_stack(stack);
pr_err("\n");
}
}


Or with another helper function it becomes:

if (IS_ENABLED(CONFIG_KASAN_GENERIC)) {
print_aux_stack(alloc_info->rcu_stack[0], "First");
print_aux_stack(alloc_info->rcu_stack[1], "Last");

[Walter Wu]

Yes, we should make sure that all recording are fully and correctly.

> This is what I meant by "hard to review". This is completely invisible
> in the diff.
>

Thanks.

[Dmitry Vyukov]

On Mon, May 11, 2020 at 4:31 AM Walter Wu <walter...@mediatek.com> wrote:
>

Please move this to generic.c closer to kasan_set_free_info.
Unnecessary in the header.

> +#define KASAN_NR_RCU_CALL_STACKS 2

Since KASAN_NR_RCU_CALL_STACKS is only used once below, you could as
well use 2 instead of it.
Reduces level of indirection and cognitive load.

[Dmitry Vyukov]

Some higher level comments.

1. I think we need to put the free track into kasan_free_meta as it
was before. It looks like exactly the place for it. We have logic to
properly place it and to do the casts.

2. We need to zero aux stacks when we reallocate the object. Otherwise
we print confusing garbage.

3. __kasan_slab_free now contains a window of inconsistency when it
marked the object as KASAN_KMALLOC_FREE, but did not store the free
track yet. If another thread prints a report now, it will print random
garbage.

4. We need some tests. At least (2) should be visible on tests.

[Walter Wu]

Good catch.

rcu_stack doesn't exist at report.c, it need at generic.c,
otherwise it will have build error, unless add the #ifdef GENERIC_KASAN

Maybe we can make kasan_get_aux_stack() return NULL? then print_stack()
determine whether it will print aux stack.

[Dmitry Vyukov]

I would put all this under #ifdef CONFIG_KASAN_GENERIC now.

> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/1589201640.21284.8.camel%40mtksdccf07.

[Walter Wu]

If the free track put kasan_free_meta, then it increase slab meta size?
Our original goal does not enlarge it.

2. We need to zero aux stacks when we reallocate the object. Otherwise
we print confusing garbage.

My local has an UT about use-after-free and rcu, but it is hard to test the printing confusing garbage, because we may need to get the same object(old pointer and new pointer). In generic KASAN is not easy to get it.

3. __kasan_slab_free now contains a window of inconsistency when it
marked the object as KASAN_KMALLOC_FREE, but did not store the free
track yet. If another thread prints a report now, it will print random
garbage.

It is possible, but the window is so tiny. It sets free track immediately after write the KASAN_KMALLOC_FREE.

4. We need some tests. At least (2) should be visible on tests.

Ok.

[Dmitry Vyukov]

Are you sure it will increase object size?
I think we overlap kasan_free_meta with the object as well. The only
case we don't overlap kasan_free_meta with the object are
SLAB_TYPESAFE_BY_RCU || cache->ctor. But these are rare and it should
only affect small objects with small redzones.
And I think now we simply have a bug for these objects, we check
KASAN_KMALLOC_FREE and then assume object contains free stack, but for
objects with ctor, they still contain live object data, we don't store
free stack in them.
Such objects can be both free and still contain user data.

> 2. We need to zero aux stacks when we reallocate the object. Otherwise
> we print confusing garbage.
>
> My local has an UT about use-after-free and rcu, but it is hard to test the printing confusing garbage, because we may need to get the same object(old pointer and new pointer). In generic KASAN is not easy to get it.
>
> 3. __kasan_slab_free now contains a window of inconsistency when it
> marked the object as KASAN_KMALLOC_FREE, but did not store the free
> track yet. If another thread prints a report now, it will print random
> garbage.
>
>
> It is possible, but the window is so tiny. It sets free track immediately after write the KASAN_KMALLOC_FREE.

It is small. But (1) why do we want to allow it at all, (2) there is
actually a more serious problem. If we mark an object as
KASAN_KMALLOC_FREE, but don't do kasan_set_free_info (because object
has ctor), now we will treat live object data as free track. We need
to fix it anyway.

[Paul E. McKenney]

On Mon, May 11, 2020 at 10:31:11AM +0800, Walter Wu wrote:
> This feature will record first and last call_rcu() call stack and
> print two call_rcu() call stack in KASAN report.

Suppose that a given rcu_head structure is passed to call_rcu(), then
the grace period elapses, the callback is invoked, and the enclosing
data structure is freed. But then that same region of memory is
immediately reallocated as the same type of structure and again
passed to call_rcu(), and that this cycle repeats several times.

Would the first call stack forever be associated with the first
call_rcu() in this series? If so, wouldn't the last two usually
be the most useful? Or am I unclear on the use case?

Just for the record, at this point we have not yet queued the callback.
We have also not yet disabled interrupts. Which might be OK, but I
figured I should call out the possibility of moving this down a few
lines to follow the local_irq_save().

If someone incorrectly invokes concurrently invokes call_rcu() on this
same region of memory, possibly from an interrupt handler, we are OK
corrupting the stack traces, right?

But what happens if a given structure has more than one rcu_head
structure? In that case, RCU would be just fine with it being
concurrently passed to different call_rcu() invocations as long as the
two invocations didn't both use the same rcu_head structure. (In that
case, they had best not be both freeing the object, and if even one of
them is freeing the object, coordination is necessary.)

If this is a problem, one approach would be to move the
kasan_record_aux_stack(head) call to kfree_rcu(). After all, it is
definitely illegal to pass the same memory to a pair of kfree_rcu()
invocations! ;-)

Thanx, Paul

I> 2.18.0

[Walter Wu]

On Mon, 2020-05-11 at 11:05 -0700, Paul E. McKenney wrote:
> On Mon, May 11, 2020 at 10:31:11AM +0800, Walter Wu wrote:
> > This feature will record first and last call_rcu() call stack and
> > print two call_rcu() call stack in KASAN report.
>
> Suppose that a given rcu_head structure is passed to call_rcu(), then
> the grace period elapses, the callback is invoked, and the enclosing
> data structure is freed. But then that same region of memory is
> immediately reallocated as the same type of structure and again
> passed to call_rcu(), and that this cycle repeats several times.
>
> Would the first call stack forever be associated with the first
> call_rcu() in this series? If so, wouldn't the last two usually
> be the most useful? Or am I unclear on the use case?
>

The first call stack doesn't forever associate with first call_rcu(),
if someone object freed and reallocated, then the first call stack will
replace with new object.

We will intend to do it.

> If someone incorrectly invokes concurrently invokes call_rcu() on this
> same region of memory, possibly from an interrupt handler, we are OK
> corrupting the stack traces, right?
>

Yes, and the wrong invoking call_rcu should be recorded.

> But what happens if a given structure has more than one rcu_head
> structure? In that case, RCU would be just fine with it being
> concurrently passed to different call_rcu() invocations as long as the
> two invocations didn't both use the same rcu_head structure. (In that
> case, they had best not be both freeing the object, and if even one of
> them is freeing the object, coordination is necessary.)
>
> If this is a problem, one approach would be to move the
> kasan_record_aux_stack(head) call to kfree_rcu(). After all, it is
> definitely illegal to pass the same memory to a pair of kfree_rcu()
> invocations! ;-)
>

The function of kasan_record_aux_stack(head) is simple, it is only to
record call stack by the 'head' object.


Thanks.

[Walter Wu]

Overlay kasan_free_meta. I see. but overlay it only when the object was
freed. kasan_free_meta will be used until free object.
1). When put object into quarantine, it need kasan_free_meta.
2). When the object exit from quarantine, it need kasan_free_meta

If we choose to overlay kasan_free_meta, then the free stack will be
stored very late. It may has no free stack in report.

>
> > 2. We need to zero aux stacks when we reallocate the object. Otherwise
> > we print confusing garbage.
> >
> > My local has an UT about use-after-free and rcu, but it is hard to test the printing confusing garbage, because we may need to get the same object(old pointer and new pointer). In generic KASAN is not easy to get it.
> >
> > 3. __kasan_slab_free now contains a window of inconsistency when it
> > marked the object as KASAN_KMALLOC_FREE, but did not store the free
> > track yet. If another thread prints a report now, it will print random
> > garbage.
> >
> >
> > It is possible, but the window is so tiny. It sets free track immediately after write the KASAN_KMALLOC_FREE.
>
> It is small. But (1) why do we want to allow it at all, (2) there is
> actually a more serious problem. If we mark an object as
> KASAN_KMALLOC_FREE, but don't do kasan_set_free_info (because object
> has ctor), now we will treat live object data as free track. We need
> to fix it anyway.
>

I see.

[Dmitry Vyukov]

On Tue, May 12, 2020 at 4:36 AM Walter Wu <walter...@mediatek.com> wrote:
>
> On Mon, 2020-05-11 at 11:05 -0700, Paul E. McKenney wrote:
> > On Mon, May 11, 2020 at 10:31:11AM +0800, Walter Wu wrote:
> > > This feature will record first and last call_rcu() call stack and
> > > print two call_rcu() call stack in KASAN report.
> >
> > Suppose that a given rcu_head structure is passed to call_rcu(), then
> > the grace period elapses, the callback is invoked, and the enclosing
> > data structure is freed. But then that same region of memory is
> > immediately reallocated as the same type of structure and again
> > passed to call_rcu(), and that this cycle repeats several times.
> >
> > Would the first call stack forever be associated with the first
> > call_rcu() in this series? If so, wouldn't the last two usually
> > be the most useful? Or am I unclear on the use case?

2 points here:

1. With KASAN the object won't be immediately reallocated. KASAN has
'quarantine' to delay reuse of heap objects. It is assumed that the
object is still in quarantine when we detect a use-after-free. In such
a case we will have proper call_rcu stacks as well.
It is possible that the object is not in quarantine already and was
reused several times (quarantine is not infinite), but then KASAN will
report non-sense stacks for allocation/free as well. So wrong call_rcu
stacks are less of a problem in such cases.

2. We would like to memorize 2 last call_rcu stacks regardless, but we
just don't have a good place for the index (bit which of the 2 is the
one to overwrite). Probably could shove it into some existing field,
but then will require atomic operations, etc.

Nobody knows how well/bad it will work. I think we need to get the
first version in, deploy on syzbot, accumulate some base of example
reports and iterate from there.

I will sleep better if we move it up :)
It qualifies a "debug check", which are generally done on entrance to
the function. Or are these all debug checks up to this point?
But if the callback did not leak anywhere up to this point and we will
maintain it that way, then formally it is fine.

> > If someone incorrectly invokes concurrently invokes call_rcu() on this
> > same region of memory, possibly from an interrupt handler, we are OK
> > corrupting the stack traces, right?
> >
>
> Yes, and the wrong invoking call_rcu should be recorded.
>
> > But what happens if a given structure has more than one rcu_head
> > structure? In that case, RCU would be just fine with it being
> > concurrently passed to different call_rcu() invocations as long as the
> > two invocations didn't both use the same rcu_head structure. (In that
> > case, they had best not be both freeing the object, and if even one of
> > them is freeing the object, coordination is necessary.)
> >
> > If this is a problem, one approach would be to move the
> > kasan_record_aux_stack(head) call to kfree_rcu(). After all, it is
> > definitely illegal to pass the same memory to a pair of kfree_rcu()
> > invocations! ;-)
> >
>
> The function of kasan_record_aux_stack(head) is simple, it is only to
> record call stack by the 'head' object.

I would say "corrupting" stacks on some races is fine-ish. In the end
we are just storing an u32 stack id.
On syzbot we generally have multiple samples of the same crash, so
even if one is "corrupted" there may be others that are not corrupted.
Just protecting from this looks too complex and expensive. And in the
end there is not much we can do anyway.

Recording all call_rcu stacks (not just kfree_rcu) is intentional. I
think it may be useful to even extend to recording workqueue and timer
stacks as well.

> --
> 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/1589250993.19238.22.camel%40mtksdccf07.

[Dmitry Vyukov]

On Tue, May 12, 2020 at 5:38 AM Walter Wu <walter...@mediatek.com> wrote:
> > Are you sure it will increase object size?
> > I think we overlap kasan_free_meta with the object as well. The only
> > case we don't overlap kasan_free_meta with the object are
> > SLAB_TYPESAFE_BY_RCU || cache->ctor. But these are rare and it should
> > only affect small objects with small redzones.
> > And I think now we simply have a bug for these objects, we check
> > KASAN_KMALLOC_FREE and then assume object contains free stack, but for
> > objects with ctor, they still contain live object data, we don't store
> > free stack in them.
> > Such objects can be both free and still contain user data.
> >
>
> Overlay kasan_free_meta. I see. but overlay it only when the object was
> freed. kasan_free_meta will be used until free object.
> 1). When put object into quarantine, it need kasan_free_meta.
> 2). When the object exit from quarantine, it need kasan_free_meta
>
> If we choose to overlay kasan_free_meta, then the free stack will be
> stored very late. It may has no free stack in report.

Sorry, I don't understand what you mean.

Why will it be stored too late?
In __kasan_slab_free() putting into quarantine and recording free
stack are literally adjacent lines of code:

static bool __kasan_slab_free(struct kmem_cache *cache, void *object,
unsigned long ip, bool quarantine)
{
...
kasan_set_free_info(cache, object, tag);
quarantine_put(get_free_info(cache, object), cache);


Just to make sure, what I meant is that we add free_track to kasan_free_meta:

struct kasan_free_meta {
struct qlist_node quarantine_link;
+ struct kasan_track free_track;
};

And I think its life-time and everything should be exactly what we need.

Also it should help to fix the problem with ctors: kasan_free_meta is
already allocated on the side for such objects, and that's exactly
what we need for objects with ctor's.

[Paul E. McKenney]

If I understood the stack-index point below, why not just move the
previous stackm index to clobber the previous-to-previous stack index,
then put the current stack index into the spot thus opened up?

There are debug checks, then initialization of presumed private
structures, disabling of interrupts, more check that are now safe given
that we are pinned to a specific CPU, and so on.

I am OK with it being at the beginning of the function.

Given the u32 nature of the stack ID, I agree that there is no point
in excluding call_rcu(). At least until such time as we start getting
false positives due to multiple rcu_head structures in the same structure.

[Dmitry Vyukov]

We don't have any index in this change (don't have memory for such index).
The pseudo code is"

u32 aux_stacks[2]; // = {0,0}

if (aux_stacks[0] != 0)
aux_stacks[0] = stack;
else
aux_stacks[1] = stack;

[Paul E. McKenney]

I was thinking in terms of something like this:

u32 aux_stacks[2]; // = {0,0}

if (aux_stacks[0] != 0) {
aux_stacks[0] = stack;

} else {
if (aux_stacks[1])
aux_stacks[0] = aux_stacks[1];
aux_stacks[1] = stack;
}

Whether this actually makes sense in real life, I have no idea.
The theory is that you want the last two stacks. However, if these
elements get cleared at kfree() time, then I could easily believe that
the approach you already have (first and last) is the way to go.

Just asking the question, not arguing for a change!

Thanx, Paul

[Dmitry Vyukov]

Oh, this is so obvious... in hindsight! :)

Walter, what do you think?

I would do this. I think latter stacks are generally more interesting
wrt shedding light on a bug. The first stack may even be "statically
known" (e.g. if object is always queued into a workqueue for some lazy
initialization during construction).

[Walter Wu]

When I see above struct kasan_free_meta, I know why you don't understand
my meaning, because I thought you were going to overlay the
quarantine_link by free_track, but it seems like to add free_track to
kasan_free_meta. Does it enlarge meta-data size?

> And I think its life-time and everything should be exactly what we need.
>
> Also it should help to fix the problem with ctors: kasan_free_meta is
> already allocated on the side for such objects, and that's exactly
> what we need for objects with ctor's.

I see.

[Walter Wu]

Hmm...why I think it will always cover aux_stacks[0] after aux_stacks[0]
has stack, it should not record last two stacks?

How about this:

u32 aux_stacks[2]; // = {0,0}

if (aux_stacks[1])
aux_stacks[0] = aux_stacks[1];
aux_stacks[1] = stack;

> I would do this. I think latter stacks are generally more interesting
> wrt shedding light on a bug. The first stack may even be "statically
> known" (e.g. if object is always queued into a workqueue for some lazy
> initialization during construction).

I think it make more sense to record latter stack, too.

Thanks for your and Paul's suggestion.

[Paul E. McKenney]

Even better! ;-)

Thanx, Paul

[Dmitry Vyukov]

I would assume it should not increase meta-data size. In both cases we
store exactly the same information inside of the object: quarantine
link and free track.
I see it more as a question of code organization. We already have a
concept of "this data is placed inside of the freed object", we
already have a name for it (kasan_free_meta), we already have code to
choose where to place it, we already have helper functions to access
it. And your change effectively duplicates all of this to place the
free track.

[Walter Wu]

I want to make a summary. Which of the following is the approach we
want? or if I have some misunderstandings, please help me to correct.
Thanks.

1) For different object, then it will has two ways.
1.a) When object are LAB_TYPESAFE_BY_RCU || cache->ctor, then store free
stack into free track of struct kasan_free_meta.
2.b) Except 1.a), store free stack into freed object.

or

2) We always store free stack into free track of struct kasan_free_meta

[Dmitry Vyukov]

I meant 2): We always store free stack into free track of struct
kasan_free_meta.
I think it will do the same as other options but just with less code
(and simpler code).

Maybe I am missing something here?

[Walter Wu]

You are right, I only make a final confirmation with you. Now there
should be no problems, I will try to implement it.

Thank you for your good suggestion.

[Walter Wu]

This feature will record the last two call_rcu() call stack and
prints up to 2 call_rcu() call stacks in KASAN report.

When call_rcu() is called, we store the call_rcu() call stack into

slub alloc meta-data, so that the KASAN report can print rcu stack.

[1]https://bugzilla.kernel.org/show_bug.cgi?id=198437
[2]https://groups.google.com/forum/#!searchin/kasan-dev/better$20stack$20traces$20for$20rcu%7Csort:date/kasan-dev/KQsjT_88hDE/7rNUZprRBgAJ

Signed-off-by: Walter Wu <walter...@mediatek.com>
Suggested-by: Dmitry Vyukov <dvy...@google.com>
Cc: Andrey Ryabinin <arya...@virtuozzo.com>
Cc: Dmitry Vyukov <dvy...@google.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Andrew Morton <ak...@linux-foundation.org>
Cc: Paul E. McKenney <pau...@kernel.org>
Cc: Josh Triplett <jo...@joshtriplett.org>
Cc: Mathieu Desnoyers <mathieu....@efficios.com>
Cc: Lai Jiangshan <jiangs...@gmail.com>
Cc: Joel Fernandes <jo...@joelfernandes.org>
---
include/linux/kasan.h | 2 ++

kernel/rcu/tree.c | 2 ++

lib/Kconfig.kasan | 2 ++
mm/kasan/common.c | 4 ++--

mm/kasan/generic.c | 20 ++++++++++++++++++++
mm/kasan/kasan.h | 10 ++++++++++
mm/kasan/report.c | 24 ++++++++++++++++++++++++
7 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index 31314ca7c635..23b7ee00572d 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -174,11 +174,13 @@ static inline size_t kasan_metadata_size(struct kmem_cache *cache) { return 0; }

void kasan_cache_shrink(struct kmem_cache *cache);
void kasan_cache_shutdown(struct kmem_cache *cache);
+void kasan_record_aux_stack(void *ptr);

#else /* CONFIG_KASAN_GENERIC */

static inline void kasan_cache_shrink(struct kmem_cache *cache) {}
static inline void kasan_cache_shutdown(struct kmem_cache *cache) {}
+static inline void kasan_record_aux_stack(void *ptr) {}

#endif /* CONFIG_KASAN_GENERIC */

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c

index 06548e2ebb72..36a4ff7f320b 100644

--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -57,6 +57,7 @@
#include <linux/slab.h>
#include <linux/sched/isolation.h>
#include <linux/sched/clock.h>
+#include <linux/kasan.h>
#include "../time/tick-internal.h"

#include "tree.h"

@@ -2668,6 +2669,7 @@ __call_rcu(struct rcu_head *head, rcu_callback_t func)
head->func = func;
head->next = NULL;
local_irq_save(flags);
+ kasan_record_aux_stack(head);
rdp = this_cpu_ptr(&rcu_data);

/* Add the callback to our list. */
diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index 81f5464ea9e1..4e83cf6e3caa 100644

--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -58,6 +58,8 @@ config KASAN_GENERIC
For better error detection enable CONFIG_STACKTRACE.
Currently CONFIG_KASAN_GENERIC doesn't work with CONFIG_DEBUG_SLAB
(the resulting kernel does not boot).

+ In generic mode KASAN prints the last two call_rcu() call stacks in
+ reports.

config KASAN_SW_TAGS
bool "Software tag-based mode"
diff --git a/mm/kasan/common.c b/mm/kasan/common.c
index 2906358e42f0..8bc618289bb1 100644
--- a/mm/kasan/common.c
+++ b/mm/kasan/common.c
@@ -41,7 +41,7 @@
#include "kasan.h"
#include "../slab.h"

-static inline depot_stack_handle_t save_stack(gfp_t flags)
+depot_stack_handle_t kasan_save_stack(gfp_t flags)
{
unsigned long entries[KASAN_STACK_DEPTH];
unsigned int nr_entries;
@@ -54,7 +54,7 @@ static inline depot_stack_handle_t save_stack(gfp_t flags)
static inline void set_track(struct kasan_track *track, gfp_t flags)
{
track->pid = current->pid;
- track->stack = save_stack(flags);
+ track->stack = kasan_save_stack(flags);
}

void kasan_enable_current(void)
diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c

index 56ff8885fe2e..78d8e0a75a8a 100644
--- a/mm/kasan/generic.c
+++ b/mm/kasan/generic.c
@@ -325,3 +325,23 @@ DEFINE_ASAN_SET_SHADOW(f2);

DEFINE_ASAN_SET_SHADOW(f3);
DEFINE_ASAN_SET_SHADOW(f5);
DEFINE_ASAN_SET_SHADOW(f8);
+
+void kasan_record_aux_stack(void *addr)
+{
+ struct page *page = kasan_addr_to_page(addr);
+ struct kmem_cache *cache;
+ struct kasan_alloc_meta *alloc_info;
+ void *object;
+
+ if (!(page && PageSlab(page)))
+ return;
+
+ cache = page->slab_cache;
+ object = nearest_obj(cache, page, addr);
+ alloc_info = get_alloc_info(cache, object);
+

+ /* record last two call_rcu() call stacks */
+ if (alloc_info->rcu_stack[0])
+ alloc_info->rcu_stack[1] = alloc_info->rcu_stack[0];

+ alloc_info->rcu_stack[0] = kasan_save_stack(GFP_NOWAIT);
+}

diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index e8f37199d885..870c5dd07756 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -104,7 +104,15 @@ struct kasan_track {

struct kasan_alloc_meta {
struct kasan_track alloc_track;
+#ifdef CONFIG_KASAN_GENERIC
+ /*
+ * call_rcu() call stack is stored into struct kasan_alloc_meta.

+ * The free stack is stored into struct kasan_free_meta.
+ */
+ depot_stack_handle_t rcu_stack[2];

+#else
struct kasan_track free_track[KASAN_NR_FREE_STACKS];
+#endif
#ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY
u8 free_pointer_tag[KASAN_NR_FREE_STACKS];
u8 free_track_idx;

@@ -159,6 +167,8 @@ void kasan_report_invalid_free(void *object, unsigned long ip);

struct page *kasan_addr_to_page(const void *addr);

+depot_stack_handle_t kasan_save_stack(gfp_t flags);
+
#if defined(CONFIG_KASAN_GENERIC) && \
(defined(CONFIG_SLAB) || defined(CONFIG_SLUB))
void quarantine_put(struct kasan_free_meta *info, struct kmem_cache *cache);

diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 80f23c9da6b0..5ee66cf7e27c 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -179,6 +179,17 @@ static struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,
return &alloc_meta->free_track[i];
}

+#ifdef CONFIG_KASAN_GENERIC
+static void print_stack(depot_stack_handle_t stack)
+{
+ unsigned long *entries;
+ unsigned int nr_entries;
+
+ nr_entries = stack_depot_fetch(stack, &entries);
+ stack_trace_print(entries, nr_entries, 0);
+}
+#endif
+

static void describe_object(struct kmem_cache *cache, void *object,

const void *addr, u8 tag)
{
@@ -192,6 +203,19 @@ static void describe_object(struct kmem_cache *cache, void *object,

free_track = kasan_get_free_track(cache, object, tag);

print_track(free_track, "Freed");
pr_err("\n");
+
+#ifdef CONFIG_KASAN_GENERIC
+ if (alloc_info->rcu_stack[0]) {
+ pr_err("Last one call_rcu() call stack:\n");
+ print_stack(alloc_info->rcu_stack[0]);

+ pr_err("\n");
+ }

+ if (alloc_info->rcu_stack[1]) {
+ pr_err("Second to last call_rcu() call stack:\n");
+ print_stack(alloc_info->rcu_stack[1]);

+ pr_err("\n");
+ }

+#endif

[Dmitry Vyukov]

Do we need this if?

If we do "alloc_info->rcu_stack[1] = alloc_info->rcu_stack[0]"
unconditionally, then we will just move 0 from [0] to [1], which
should be 0 at this point anyway.

I think it will be more reasonable to rename rcu_stack to aux_stack,
the function that stores the stacks is kasan_record_aux_stack.

> --
> 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/20200518062603.4570-1-walter-zh.wu%40mediatek.com.

[Walter Wu]

Yes, this if is redundant.

> I think it will be more reasonable to rename rcu_stack to aux_stack,
> the function that stores the stacks is kasan_record_aux_stack.
>

ok, we will change it's name.

[Walter Wu]

This feature will record the last two call_rcu() call stacks and

prints up to 2 call_rcu() call stacks in KASAN report.

When call_rcu() is called, we store the call_rcu() call stack into
slub alloc meta-data, so that the KASAN report can print rcu stack.

[1]https://bugzilla.kernel.org/show_bug.cgi?id=198437
[2]https://groups.google.com/forum/#!searchin/kasan-dev/better$20stack$20traces$20for$20rcu%7Csort:date/kasan-dev/KQsjT_88hDE/7rNUZprRBgAJ

Signed-off-by: Walter Wu <walter...@mediatek.com>
Suggested-by: Dmitry Vyukov <dvy...@google.com>
Cc: Andrey Ryabinin <arya...@virtuozzo.com>
Cc: Dmitry Vyukov <dvy...@google.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Andrew Morton <ak...@linux-foundation.org>
Cc: Paul E. McKenney <pau...@kernel.org>
Cc: Josh Triplett <jo...@joshtriplett.org>
Cc: Mathieu Desnoyers <mathieu....@efficios.com>
Cc: Lai Jiangshan <jiangs...@gmail.com>
Cc: Joel Fernandes <jo...@joelfernandes.org>
---
include/linux/kasan.h | 2 ++
kernel/rcu/tree.c | 2 ++
lib/Kconfig.kasan | 2 ++
mm/kasan/common.c | 4 ++--

mm/kasan/generic.c | 19 +++++++++++++++++++

mm/kasan/kasan.h | 10 ++++++++++
mm/kasan/report.c | 24 ++++++++++++++++++++++++

7 files changed, 61 insertions(+), 2 deletions(-)

index 56ff8885fe2e..3372bdcaf92a 100644
--- a/mm/kasan/generic.c
+++ b/mm/kasan/generic.c
@@ -325,3 +325,22 @@ DEFINE_ASAN_SET_SHADOW(f2);

DEFINE_ASAN_SET_SHADOW(f3);
DEFINE_ASAN_SET_SHADOW(f5);
DEFINE_ASAN_SET_SHADOW(f8);
+
+void kasan_record_aux_stack(void *addr)
+{
+ struct page *page = kasan_addr_to_page(addr);
+ struct kmem_cache *cache;
+ struct kasan_alloc_meta *alloc_info;
+ void *object;
+
+ if (!(page && PageSlab(page)))
+ return;
+
+ cache = page->slab_cache;
+ object = nearest_obj(cache, page, addr);
+ alloc_info = get_alloc_info(cache, object);
+

+ /* record the last two call_rcu() call stacks */
+ alloc_info->aux_stack[1] = alloc_info->aux_stack[0];
+ alloc_info->aux_stack[0] = kasan_save_stack(GFP_NOWAIT);

+}
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h

index e8f37199d885..a7391bc83070 100644

--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -104,7 +104,15 @@ struct kasan_track {

struct kasan_alloc_meta {
struct kasan_track alloc_track;
+#ifdef CONFIG_KASAN_GENERIC
+ /*
+ * call_rcu() call stack is stored into struct kasan_alloc_meta.
+ * The free stack is stored into struct kasan_free_meta.
+ */

+ depot_stack_handle_t aux_stack[2];

+#else
struct kasan_track free_track[KASAN_NR_FREE_STACKS];
+#endif
#ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY
u8 free_pointer_tag[KASAN_NR_FREE_STACKS];
u8 free_track_idx;
@@ -159,6 +167,8 @@ void kasan_report_invalid_free(void *object, unsigned long ip);

struct page *kasan_addr_to_page(const void *addr);

+depot_stack_handle_t kasan_save_stack(gfp_t flags);
+
#if defined(CONFIG_KASAN_GENERIC) && \
(defined(CONFIG_SLAB) || defined(CONFIG_SLUB))
void quarantine_put(struct kasan_free_meta *info, struct kmem_cache *cache);
diff --git a/mm/kasan/report.c b/mm/kasan/report.c

index 80f23c9da6b0..6f8f2bf8f53b 100644

--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -179,6 +179,17 @@ static struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,
return &alloc_meta->free_track[i];
}

+#ifdef CONFIG_KASAN_GENERIC
+static void print_stack(depot_stack_handle_t stack)
+{
+ unsigned long *entries;
+ unsigned int nr_entries;
+
+ nr_entries = stack_depot_fetch(stack, &entries);
+ stack_trace_print(entries, nr_entries, 0);
+}
+#endif
+
static void describe_object(struct kmem_cache *cache, void *object,
const void *addr, u8 tag)
{
@@ -192,6 +203,19 @@ static void describe_object(struct kmem_cache *cache, void *object,
free_track = kasan_get_free_track(cache, object, tag);
print_track(free_track, "Freed");
pr_err("\n");
+
+#ifdef CONFIG_KASAN_GENERIC

+ if (alloc_info->aux_stack[0]) {

+ pr_err("Last one call_rcu() call stack:\n");

+ print_stack(alloc_info->aux_stack[0]);

+ pr_err("\n");
+ }

+ if (alloc_info->aux_stack[1]) {

+ pr_err("Second to last call_rcu() call stack:\n");

+ print_stack(alloc_info->aux_stack[1]);

[Andrey Konovalov]

I don't think we need this here, mentioning this in the documentation is fine.

Let's put this function just before print_track() and reuse there.

> +{
> + unsigned long *entries;
> + unsigned int nr_entries;
> +
> + nr_entries = stack_depot_fetch(stack, &entries);
> + stack_trace_print(entries, nr_entries, 0);
> +}
> +#endif
> +
> static void describe_object(struct kmem_cache *cache, void *object,
> const void *addr, u8 tag)
> {
> @@ -192,6 +203,19 @@ static void describe_object(struct kmem_cache *cache, void *object,
> free_track = kasan_get_free_track(cache, object, tag);
> print_track(free_track, "Freed");
> pr_err("\n");
> +
> +#ifdef CONFIG_KASAN_GENERIC
> + if (alloc_info->aux_stack[0]) {
> + pr_err("Last one call_rcu() call stack:\n");

Could you change this to "Last call_rcu():\n",

> + print_stack(alloc_info->aux_stack[0]);
> + pr_err("\n");
> + }
> + if (alloc_info->aux_stack[1]) {
> + pr_err("Second to last call_rcu() call stack:\n");

and this to "Second to last call_rcu():\n"?

It's shorter, but provides the same info.

> + print_stack(alloc_info->aux_stack[1]);
> + pr_err("\n");
> + }
> +#endif
> }
>
> describe_object_addr(cache, object, addr);
> --
> 2.18.0
>

> --
> 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/20200519022359.24115-1-walter-zh.wu%40mediatek.com.

[Paul E. McKenney]

On Tue, May 19, 2020 at 10:23:59AM +0800, Walter Wu wrote:
> This feature will record the last two call_rcu() call stacks and
> prints up to 2 call_rcu() call stacks in KASAN report.
>
> When call_rcu() is called, we store the call_rcu() call stack into
> slub alloc meta-data, so that the KASAN report can print rcu stack.
>
> [1]https://bugzilla.kernel.org/show_bug.cgi?id=198437
> [2]https://groups.google.com/forum/#!searchin/kasan-dev/better$20stack$20traces$20for$20rcu%7Csort:date/kasan-dev/KQsjT_88hDE/7rNUZprRBgAJ
>
> Signed-off-by: Walter Wu <walter...@mediatek.com>
> Suggested-by: Dmitry Vyukov <dvy...@google.com>
> Cc: Andrey Ryabinin <arya...@virtuozzo.com>
> Cc: Dmitry Vyukov <dvy...@google.com>
> Cc: Alexander Potapenko <gli...@google.com>
> Cc: Andrew Morton <ak...@linux-foundation.org>
> Cc: Paul E. McKenney <pau...@kernel.org>
> Cc: Josh Triplett <jo...@joshtriplett.org>
> Cc: Mathieu Desnoyers <mathieu....@efficios.com>
> Cc: Lai Jiangshan <jiangs...@gmail.com>
> Cc: Joel Fernandes <jo...@joelfernandes.org>

From an RCU perspective:

Acked-by: Paul E. McKenney <pau...@kernel.org>

[Walter Wu]

Hi Andrey,

On Tue, 2020-05-19 at 16:52 +0200, 'Andrey Konovalov' via kasan-dev
wrote:

Ok, we will remove it. Only mention it in the documentation.

Ok.

We will change their name.

Thank you for your review.

[Walter Wu]

On Tue, 2020-05-19 at 08:48 -0700, Paul E. McKenney wrote:
> On Tue, May 19, 2020 at 10:23:59AM +0800, Walter Wu wrote:
> > This feature will record the last two call_rcu() call stacks and
> > prints up to 2 call_rcu() call stacks in KASAN report.
> >
> > When call_rcu() is called, we store the call_rcu() call stack into
> > slub alloc meta-data, so that the KASAN report can print rcu stack.
> >
> > [1]https://bugzilla.kernel.org/show_bug.cgi?id=198437
> > [2]https://groups.google.com/forum/#!searchin/kasan-dev/better$20stack$20traces$20for$20rcu%7Csort:date/kasan-dev/KQsjT_88hDE/7rNUZprRBgAJ
> >
> > Signed-off-by: Walter Wu <walter...@mediatek.com>
> > Suggested-by: Dmitry Vyukov <dvy...@google.com>
> > Cc: Andrey Ryabinin <arya...@virtuozzo.com>
> > Cc: Dmitry Vyukov <dvy...@google.com>
> > Cc: Alexander Potapenko <gli...@google.com>
> > Cc: Andrew Morton <ak...@linux-foundation.org>
> > Cc: Paul E. McKenney <pau...@kernel.org>
> > Cc: Josh Triplett <jo...@joshtriplett.org>
> > Cc: Mathieu Desnoyers <mathieu....@efficios.com>
> > Cc: Lai Jiangshan <jiangs...@gmail.com>
> > Cc: Joel Fernandes <jo...@joelfernandes.org>
>
> From an RCU perspective:
>
> Acked-by: Paul E. McKenney <pau...@kernel.org>
>

Hi Paul,

Thank you for your suggestion and review.


Walter

[Walter Wu]

This feature will record the last two call_rcu() call stacks and
prints up to 2 call_rcu() call stacks in KASAN report.

When call_rcu() is called, we store the call_rcu() call stack into
slub alloc meta-data, so that the KASAN report can print rcu stack.

[1]https://bugzilla.kernel.org/show_bug.cgi?id=198437
[2]https://groups.google.com/forum/#!searchin/kasan-dev/better$20stack$20traces$20for$20rcu%7Csort:date/kasan-dev/KQsjT_88hDE/7rNUZprRBgAJ

Signed-off-by: Walter Wu <walter...@mediatek.com>
Suggested-by: Dmitry Vyukov <dvy...@google.com>

Acked-by: Paul E. McKenney <pau...@kernel.org>

Cc: Andrey Ryabinin <arya...@virtuozzo.com>
Cc: Dmitry Vyukov <dvy...@google.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Andrew Morton <ak...@linux-foundation.org>

Cc: Josh Triplett <jo...@joshtriplett.org>
Cc: Mathieu Desnoyers <mathieu....@efficios.com>
Cc: Lai Jiangshan <jiangs...@gmail.com>
Cc: Joel Fernandes <jo...@joelfernandes.org>

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

---
include/linux/kasan.h | 2 ++
kernel/rcu/tree.c | 2 ++

mm/kasan/common.c | 4 ++--
mm/kasan/generic.c | 21 +++++++++++++++++++++

mm/kasan/kasan.h | 10 ++++++++++
mm/kasan/report.c | 24 ++++++++++++++++++++++++

6 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/mm/kasan/common.c b/mm/kasan/common.c
index 2906358e42f0..8bc618289bb1 100644
--- a/mm/kasan/common.c
+++ b/mm/kasan/common.c
@@ -41,7 +41,7 @@
#include "kasan.h"
#include "../slab.h"

-static inline depot_stack_handle_t save_stack(gfp_t flags)
+depot_stack_handle_t kasan_save_stack(gfp_t flags)
{
unsigned long entries[KASAN_STACK_DEPTH];
unsigned int nr_entries;
@@ -54,7 +54,7 @@ static inline depot_stack_handle_t save_stack(gfp_t flags)
static inline void set_track(struct kasan_track *track, gfp_t flags)
{
track->pid = current->pid;
- track->stack = save_stack(flags);
+ track->stack = kasan_save_stack(flags);
}

void kasan_enable_current(void)
diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c

index 56ff8885fe2e..8acf48882ba2 100644
--- a/mm/kasan/generic.c
+++ b/mm/kasan/generic.c
@@ -325,3 +325,24 @@ DEFINE_ASAN_SET_SHADOW(f2);

DEFINE_ASAN_SET_SHADOW(f3);
DEFINE_ASAN_SET_SHADOW(f5);
DEFINE_ASAN_SET_SHADOW(f8);
+
+void kasan_record_aux_stack(void *addr)
+{
+ struct page *page = kasan_addr_to_page(addr);
+ struct kmem_cache *cache;
+ struct kasan_alloc_meta *alloc_info;
+ void *object;
+
+ if (!(page && PageSlab(page)))
+ return;
+
+ cache = page->slab_cache;
+ object = nearest_obj(cache, page, addr);
+ alloc_info = get_alloc_info(cache, object);
+
+ /*

+ * record the last two call_rcu() call stacks.
+ */

index 80f23c9da6b0..29a801d5cd74 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -105,6 +105,17 @@ static void end_report(unsigned long *flags)
kasan_enable_current();
}

+#ifdef CONFIG_KASAN_GENERIC
+static void print_stack(depot_stack_handle_t stack)
+{
+ unsigned long *entries;
+ unsigned int nr_entries;
+
+ nr_entries = stack_depot_fetch(stack, &entries);
+ stack_trace_print(entries, nr_entries, 0);
+}
+#endif
+

static void print_track(struct kasan_track *track, const char *prefix)

{

pr_err("%s by task %u:\n", prefix, track->pid);

@@ -192,6 +203,19 @@ static void describe_object(struct kmem_cache *cache, void *object,
free_track = kasan_get_free_track(cache, object, tag);
print_track(free_track, "Freed");
pr_err("\n");
+
+#ifdef CONFIG_KASAN_GENERIC
+ if (alloc_info->aux_stack[0]) {

+ pr_err("Last call_rcu():\n");

+ print_stack(alloc_info->aux_stack[0]);
+ pr_err("\n");
+ }
+ if (alloc_info->aux_stack[1]) {

+ pr_err("Second to last call_rcu():\n");

[Andrey Konovalov]

The idea of moving it here was to reuse print_stack() in print_track() :)

> +
> static void print_track(struct kasan_track *track, const char *prefix)
> {
> pr_err("%s by task %u:\n", prefix, track->pid);
> @@ -192,6 +203,19 @@ static void describe_object(struct kmem_cache *cache, void *object,
> free_track = kasan_get_free_track(cache, object, tag);
> print_track(free_track, "Freed");
> pr_err("\n");
> +
> +#ifdef CONFIG_KASAN_GENERIC
> + if (alloc_info->aux_stack[0]) {
> + pr_err("Last call_rcu():\n");
> + print_stack(alloc_info->aux_stack[0]);
> + pr_err("\n");
> + }
> + if (alloc_info->aux_stack[1]) {
> + pr_err("Second to last call_rcu():\n");
> + print_stack(alloc_info->aux_stack[1]);
> + pr_err("\n");
> + }
> +#endif
> }
>
> describe_object_addr(cache, object, addr);
> --
> 2.18.0
>

> --
> 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/20200520123434.3888-1-walter-zh.wu%40mediatek.com.

[Walter Wu]

Ok. I see. Next patch will fix it.

[Walter Wu]

This feature will record the last two call_rcu() call stacks and
prints up to 2 call_rcu() call stacks in KASAN report.

When call_rcu() is called, we store the call_rcu() call stack into
slub alloc meta-data, so that the KASAN report can print rcu stack.

[1]https://bugzilla.kernel.org/show_bug.cgi?id=198437
[2]https://groups.google.com/forum/#!searchin/kasan-dev/better$20stack$20traces$20for$20rcu%7Csort:date/kasan-dev/KQsjT_88hDE/7rNUZprRBgAJ

Signed-off-by: Walter Wu <walter...@mediatek.com>
Suggested-by: Dmitry Vyukov <dvy...@google.com>
Acked-by: Paul E. McKenney <pau...@kernel.org>
Cc: Andrey Ryabinin <arya...@virtuozzo.com>
Cc: Dmitry Vyukov <dvy...@google.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Andrew Morton <ak...@linux-foundation.org>
Cc: Josh Triplett <jo...@joshtriplett.org>
Cc: Mathieu Desnoyers <mathieu....@efficios.com>
Cc: Lai Jiangshan <jiangs...@gmail.com>
Cc: Joel Fernandes <jo...@joelfernandes.org>
Cc: Andrey Konovalov <andre...@google.com>
---
include/linux/kasan.h | 2 ++
kernel/rcu/tree.c | 2 ++
mm/kasan/common.c | 4 ++--
mm/kasan/generic.c | 21 +++++++++++++++++++++
mm/kasan/kasan.h | 10 ++++++++++

mm/kasan/report.c | 28 +++++++++++++++++++++++-----
6 files changed, 60 insertions(+), 7 deletions(-)

index 80f23c9da6b0..2421a4bd9227 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -105,15 +105,20 @@ static void end_report(unsigned long *flags)
kasan_enable_current();
}

+static void print_stack(depot_stack_handle_t stack)
+{
+ unsigned long *entries;
+ unsigned int nr_entries;
+
+ nr_entries = stack_depot_fetch(stack, &entries);
+ stack_trace_print(entries, nr_entries, 0);
+}

+
static void print_track(struct kasan_track *track, const char *prefix)
{
pr_err("%s by task %u:\n", prefix, track->pid);

if (track->stack) {
- unsigned long *entries;
- unsigned int nr_entries;
-
- nr_entries = stack_depot_fetch(track->stack, &entries);
- stack_trace_print(entries, nr_entries, 0);
+ print_stack(track->stack);
} else {
pr_err("(stack is not available)\n");
}
@@ -192,6 +197,19 @@ static void describe_object(struct kmem_cache *cache, void *object,

[Dmitry Vyukov]

On Fri, May 22, 2020 at 4:01 AM Walter Wu <walter...@mediatek.com> wrote:
>
> This feature will record the last two call_rcu() call stacks and
> prints up to 2 call_rcu() call stacks in KASAN report.
>
> When call_rcu() is called, we store the call_rcu() call stack into
> slub alloc meta-data, so that the KASAN report can print rcu stack.
>
> [1]https://bugzilla.kernel.org/show_bug.cgi?id=198437
> [2]https://groups.google.com/forum/#!searchin/kasan-dev/better$20stack$20traces$20for$20rcu%7Csort:date/kasan-dev/KQsjT_88hDE/7rNUZprRBgAJ

Hi Walter,

The series look good to me. Thanks for bearing with me. I am eager to
see this in syzbot reports.

Reviewed-and-tested-by: Dmitry Vyukov <dvy...@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/20200522020059.22332-1-walter-zh.wu%40mediatek.com.

[Walter Wu]

On Mon, 2020-05-25 at 11:56 +0200, Dmitry Vyukov wrote:
> On Fri, May 22, 2020 at 4:01 AM Walter Wu <walter...@mediatek.com> wrote:
> >
> > This feature will record the last two call_rcu() call stacks and
> > prints up to 2 call_rcu() call stacks in KASAN report.
> >
> > When call_rcu() is called, we store the call_rcu() call stack into
> > slub alloc meta-data, so that the KASAN report can print rcu stack.
> >
> > [1]https://bugzilla.kernel.org/show_bug.cgi?id=198437
> > [2]https://groups.google.com/forum/#!searchin/kasan-dev/better$20stack$20traces$20for$20rcu%7Csort:date/kasan-dev/KQsjT_88hDE/7rNUZprRBgAJ
>
> Hi Walter,
>
> The series look good to me. Thanks for bearing with me. I am eager to
> see this in syzbot reports.
>
> Reviewed-and-tested-by: Dmitry Vyukov <dvy...@google.com>
>

Hi Dmitry,

I appreciate for your response. This patches make KASAN report more
better and let me learn a lot. Thank you for good suggestion and
detailed explanation.

Walter