diff mbox series

[06/32] kasan: introduce kasan_print_aux_stacks

Message ID 11a7bfb5ed5de141b50db8c08e9c6ad37ef3febc.1655150842.git.andreyknvl@google.com (mailing list archive)
State New
Headers show
Series kasan: switch tag-based modes to stack ring from per-object metadata | expand

Commit Message

andrey.konovalov@linux.dev June 13, 2022, 8:13 p.m. UTC 
From: Andrey Konovalov <andreyknvl@google.com>

Add a kasan_print_aux_stacks() helper that prints the auxiliary stack
traces for the Generic mode.

This change hides references to alloc_meta from the common reporting code.
This is desired as only the Generic mode will be using per-object metadata
after this series.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/kasan/kasan.h          |  6 ++++++
 mm/kasan/report.c         | 15 +--------------
 mm/kasan/report_generic.c | 20 ++++++++++++++++++++
 3 files changed, 27 insertions(+), 14 deletions(-)

Comments

Marco Elver June 17, 2022, 11:34 a.m. UTC | #1 
On Mon, 13 Jun 2022 at 22:16, <andrey.konovalov@linux.dev> wrote:
>
> From: Andrey Konovalov <andreyknvl@google.com>
>
> Add a kasan_print_aux_stacks() helper that prints the auxiliary stack
> traces for the Generic mode.
>
> This change hides references to alloc_meta from the common reporting code.
> This is desired as only the Generic mode will be using per-object metadata
> after this series.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> ---
>  mm/kasan/kasan.h          |  6 ++++++
>  mm/kasan/report.c         | 15 +--------------
>  mm/kasan/report_generic.c | 20 ++++++++++++++++++++
>  3 files changed, 27 insertions(+), 14 deletions(-)
>
> diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
> index aa6b43936f8d..bcea5ed15631 100644
> --- a/mm/kasan/kasan.h
> +++ b/mm/kasan/kasan.h
> @@ -265,6 +265,12 @@ void kasan_print_address_stack_frame(const void *addr);
>  static inline void kasan_print_address_stack_frame(const void *addr) { }
>  #endif
>
> +#ifdef CONFIG_KASAN_GENERIC
> +void kasan_print_aux_stacks(struct kmem_cache *cache, const void *object);
> +#else
> +static inline void kasan_print_aux_stacks(struct kmem_cache *cache, const void *object) { }
> +#endif

Why not put this into one of the existing "#ifdef
CONFIG_KASAN_GENERIC" blocks? There are several; probably the one 10
lines down might be ok?
Andrey Konovalov July 18, 2022, 10:41 p.m. UTC | #2 
On Fri, Jun 17, 2022 at 1:35 PM Marco Elver <elver@google.com> wrote:
>
> > diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
> > index aa6b43936f8d..bcea5ed15631 100644
> > --- a/mm/kasan/kasan.h
> > +++ b/mm/kasan/kasan.h
> > @@ -265,6 +265,12 @@ void kasan_print_address_stack_frame(const void *addr);
> >  static inline void kasan_print_address_stack_frame(const void *addr) { }
> >  #endif
> >
> > +#ifdef CONFIG_KASAN_GENERIC
> > +void kasan_print_aux_stacks(struct kmem_cache *cache, const void *object);
> > +#else
> > +static inline void kasan_print_aux_stacks(struct kmem_cache *cache, const void *object) { }
> > +#endif
>
> Why not put this into one of the existing "#ifdef
> CONFIG_KASAN_GENERIC" blocks? There are several; probably the one 10
> lines down might be ok?

The idea was to group functions based on their purpose, not on which
mode uses them. Here, kasan_print_aux_stacks() is related to printing
reports, so it goes next to other such functions. We could rework the
order of functions in this file, but I'd rather keep it as is in this
change. Thanks!
diff mbox series

Patch

diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index aa6b43936f8d..bcea5ed15631 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -265,6 +265,12 @@  void kasan_print_address_stack_frame(const void *addr);
 static inline void kasan_print_address_stack_frame(const void *addr) { }
 #endif
 
+#ifdef CONFIG_KASAN_GENERIC
+void kasan_print_aux_stacks(struct kmem_cache *cache, const void *object);
+#else
+static inline void kasan_print_aux_stacks(struct kmem_cache *cache, const void *object) { }
+#endif
+
 bool kasan_report(unsigned long addr, size_t size,
 		bool is_write, unsigned long ip);
 void kasan_report_invalid_free(void *object, unsigned long ip);
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index b341a191651d..35dd8aeb115c 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -266,20 +266,7 @@  static void describe_object_stacks(struct kmem_cache *cache, void *object,
 		pr_err("\n");
 	}
 
-#ifdef CONFIG_KASAN_GENERIC
-	if (!alloc_meta)
-		return;
-	if (alloc_meta->aux_stack[0]) {
-		pr_err("Last potentially related work creation:\n");
-		stack_depot_print(alloc_meta->aux_stack[0]);
-		pr_err("\n");
-	}
-	if (alloc_meta->aux_stack[1]) {
-		pr_err("Second to last potentially related work creation:\n");
-		stack_depot_print(alloc_meta->aux_stack[1]);
-		pr_err("\n");
-	}
-#endif
+	kasan_print_aux_stacks(cache, object);
 }
 
 static void describe_object(struct kmem_cache *cache, void *object,
diff --git a/mm/kasan/report_generic.c b/mm/kasan/report_generic.c
index 6689fb9a919b..348dc207d462 100644
--- a/mm/kasan/report_generic.c
+++ b/mm/kasan/report_generic.c
@@ -132,6 +132,26 @@  void kasan_metadata_fetch_row(char *buffer, void *row)
 	memcpy(buffer, kasan_mem_to_shadow(row), META_BYTES_PER_ROW);
 }
 
+void kasan_print_aux_stacks(struct kmem_cache *cache, const void *object)
+{
+	struct kasan_alloc_meta *alloc_meta;
+
+	alloc_meta = kasan_get_alloc_meta(cache, object);
+	if (!alloc_meta)
+		return;
+
+	if (alloc_meta->aux_stack[0]) {
+		pr_err("Last potentially related work creation:\n");
+		stack_depot_print(alloc_meta->aux_stack[0]);
+		pr_err("\n");
+	}
+	if (alloc_meta->aux_stack[1]) {
+		pr_err("Second to last potentially related work creation:\n");
+		stack_depot_print(alloc_meta->aux_stack[1]);
+		pr_err("\n");
+	}
+}
+
 #ifdef CONFIG_KASAN_STACK
 static bool __must_check tokenize_frame_descr(const char **frame_descr,
 					      char *token, size_t max_tok_len,