[PATCH v2 1/2] printk: introduce dump_stack_lvl()

[PATCH v2 1/2] printk: introduce dump_stack_lvl()

[Alexander Potapenko]

dump_stack() is used for many different cases, which may require a log
level consistent with other kernel messages surrounding the dump_stack()
call.
Without that, certain systems that are configured to ignore the default
level messages will miss stack traces in critical error reports.

This patch introduces dump_stack_lvl() that behaves similarly to
dump_stack(), but accepts a custom log level.
The old dump_stack() becomes equal to dump_stack_lvl(KERN_DEFAULT).

A somewhat similar patch has been proposed in 2012:
https://lore.kernel.org/lkml/1332493269.2359.9.camel@hebo/
, but wasn't merged.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: he, bo <bo.he@intel.com>
Cc: Yanmin Zhang <yanmin_zhang@linux.intel.com>
Cc: Prasad Sodagudi <psodagud@quicinc.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Alexander Potapenko <glider@google.com>

---

v2:
 -- export dump_stack_lvl() (spotted by Marco Elver)
---
 include/linux/printk.h |  1 +
 lib/dump_stack.c       | 20 +++++++++++++-------
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index fe7eb2351610..abe274305d79 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -206,6 +206,7 @@ void __init setup_log_buf(int early);
 __printf(1, 2) void dump_stack_set_arch_desc(const char *fmt, ...);
 void dump_stack_print_info(const char *log_lvl);
 void show_regs_print_info(const char *log_lvl);
+extern asmlinkage void dump_stack_lvl(const char *log_lvl) __cold;
 extern asmlinkage void dump_stack(void) __cold;
 extern void printk_safe_flush(void);
 extern void printk_safe_flush_on_panic(void);
diff --git a/lib/dump_stack.c b/lib/dump_stack.c
index f5a33b6f773f..586e3f2c6a15 100644
--- a/lib/dump_stack.c
+++ b/lib/dump_stack.c
@@ -73,10 +73,10 @@ void show_regs_print_info(const char *log_lvl)
 	dump_stack_print_info(log_lvl);
 }
 
-static void __dump_stack(void)
+static void __dump_stack(const char *log_lvl)
 {
-	dump_stack_print_info(KERN_DEFAULT);
-	show_stack(NULL, NULL, KERN_DEFAULT);
+	dump_stack_print_info(log_lvl);
+	show_stack(NULL, NULL, log_lvl);
 }
 
 /**
@@ -87,7 +87,7 @@ static void __dump_stack(void)
 #ifdef CONFIG_SMP
 static atomic_t dump_lock = ATOMIC_INIT(-1);
 
-asmlinkage __visible void dump_stack(void)
+asmlinkage __visible void dump_stack_lvl(const char *log_lvl)
 {
 	unsigned long flags;
 	int was_locked;
@@ -117,7 +117,7 @@ asmlinkage __visible void dump_stack(void)
 		goto retry;
 	}
 
-	__dump_stack();
+	__dump_stack(log_lvl);
 
 	if (!was_locked)
 		atomic_set(&dump_lock, -1);
@@ -125,9 +125,15 @@ asmlinkage __visible void dump_stack(void)
 	local_irq_restore(flags);
 }
 #else
-asmlinkage __visible void dump_stack(void)
+asmlinkage __visible void dump_stack_lvl(const char *log_lvl)
 {
-	__dump_stack();
+	__dump_stack(log_lvl);
 }
 #endif
+EXPORT_SYMBOL(dump_stack_lvl);
+
+asmlinkage __visible void dump_stack(void)
+{
+	dump_stack_lvl(KERN_DEFAULT);
+}
 EXPORT_SYMBOL(dump_stack);
-- 
2.31.1.527.g47e6f16901-goog

[PATCH v2 2/2] kasan: use dump_stack_lvl(KERN_ERR) to print stacks

[Alexander Potapenko]

Most of the contents of KASAN reports are printed with pr_err(), so use
a consistent logging level to print the memory access stacks.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com>
Cc: Prasad Sodagudi <psodagud@quicinc.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Alexander Potapenko <glider@google.com>
---
 mm/kasan/report.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 14bd51ea2348..8fff1825b22c 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -230,7 +230,7 @@ static void print_address_description(void *addr, u8 tag)
 {
 	struct page *page = kasan_addr_to_page(addr);
 
-	dump_stack();
+	dump_stack_lvl(KERN_ERR);
 	pr_err("\n");
 
 	if (page && PageSlab(page)) {
@@ -375,7 +375,7 @@ void kasan_report_async(void)
 	pr_err("BUG: KASAN: invalid-access\n");
 	pr_err("Asynchronous mode enabled: no access details available\n");
 	pr_err("\n");
-	dump_stack();
+	dump_stack_lvl(KERN_ERR);
 	end_report(&flags, 0);
 }
 #endif /* CONFIG_KASAN_HW_TAGS */
@@ -420,7 +420,7 @@ static void __kasan_report(unsigned long addr, size_t size, bool is_write,
 		pr_err("\n");
 		print_memory_metadata(info.first_bad_addr);
 	} else {
-		dump_stack();
+		dump_stack_lvl(KERN_ERR);
 	}
 
 	end_report(&flags, addr);
-- 
2.31.1.527.g47e6f16901-goog

Re: [PATCH v2 1/2] printk: introduce dump_stack_lvl()

[Marco Elver]

On Thu, 6 May 2021 at 12:54, Alexander Potapenko <glider@google.com> wrote:
>
> dump_stack() is used for many different cases, which may require a log
> level consistent with other kernel messages surrounding the dump_stack()
> call.
> Without that, certain systems that are configured to ignore the default
> level messages will miss stack traces in critical error reports.
>
> This patch introduces dump_stack_lvl() that behaves similarly to
> dump_stack(), but accepts a custom log level.
> The old dump_stack() becomes equal to dump_stack_lvl(KERN_DEFAULT).
>
> A somewhat similar patch has been proposed in 2012:
> https://lore.kernel.org/lkml/1332493269.2359.9.camel@hebo/
> , but wasn't merged.
>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Petr Mladek <pmladek@suse.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: he, bo <bo.he@intel.com>
> Cc: Yanmin Zhang <yanmin_zhang@linux.intel.com>
> Cc: Prasad Sodagudi <psodagud@quicinc.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: Marco Elver <elver@google.com>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Alexander Potapenko <glider@google.com>

Reviewed-by: Marco Elver <elver@google.com>


> ---
>
> v2:
>  -- export dump_stack_lvl() (spotted by Marco Elver)
> ---
>  include/linux/printk.h |  1 +
>  lib/dump_stack.c       | 20 +++++++++++++-------
>  2 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/printk.h b/include/linux/printk.h
> index fe7eb2351610..abe274305d79 100644
> --- a/include/linux/printk.h
> +++ b/include/linux/printk.h
> @@ -206,6 +206,7 @@ void __init setup_log_buf(int early);
>  __printf(1, 2) void dump_stack_set_arch_desc(const char *fmt, ...);
>  void dump_stack_print_info(const char *log_lvl);
>  void show_regs_print_info(const char *log_lvl);
> +extern asmlinkage void dump_stack_lvl(const char *log_lvl) __cold;
>  extern asmlinkage void dump_stack(void) __cold;
>  extern void printk_safe_flush(void);
>  extern void printk_safe_flush_on_panic(void);
> diff --git a/lib/dump_stack.c b/lib/dump_stack.c
> index f5a33b6f773f..586e3f2c6a15 100644
> --- a/lib/dump_stack.c
> +++ b/lib/dump_stack.c
> @@ -73,10 +73,10 @@ void show_regs_print_info(const char *log_lvl)
>         dump_stack_print_info(log_lvl);
>  }
>
> -static void __dump_stack(void)
> +static void __dump_stack(const char *log_lvl)
>  {
> -       dump_stack_print_info(KERN_DEFAULT);
> -       show_stack(NULL, NULL, KERN_DEFAULT);
> +       dump_stack_print_info(log_lvl);
> +       show_stack(NULL, NULL, log_lvl);
>  }
>
>  /**
> @@ -87,7 +87,7 @@ static void __dump_stack(void)
>  #ifdef CONFIG_SMP
>  static atomic_t dump_lock = ATOMIC_INIT(-1);
>
> -asmlinkage __visible void dump_stack(void)
> +asmlinkage __visible void dump_stack_lvl(const char *log_lvl)
>  {
>         unsigned long flags;
>         int was_locked;
> @@ -117,7 +117,7 @@ asmlinkage __visible void dump_stack(void)
>                 goto retry;
>         }
>
> -       __dump_stack();
> +       __dump_stack(log_lvl);
>
>         if (!was_locked)
>                 atomic_set(&dump_lock, -1);
> @@ -125,9 +125,15 @@ asmlinkage __visible void dump_stack(void)
>         local_irq_restore(flags);
>  }
>  #else
> -asmlinkage __visible void dump_stack(void)
> +asmlinkage __visible void dump_stack_lvl(const char *log_lvl)
>  {
> -       __dump_stack();
> +       __dump_stack(log_lvl);
>  }
>  #endif
> +EXPORT_SYMBOL(dump_stack_lvl);
> +
> +asmlinkage __visible void dump_stack(void)
> +{
> +       dump_stack_lvl(KERN_DEFAULT);
> +}
>  EXPORT_SYMBOL(dump_stack);
> --
> 2.31.1.527.g47e6f16901-goog
>

Re: [PATCH v2 2/2] kasan: use dump_stack_lvl(KERN_ERR) to print stacks

[Marco Elver]

On Thu, 6 May 2021 at 12:54, Alexander Potapenko <glider@google.com> wrote:
>
> Most of the contents of KASAN reports are printed with pr_err(), so use
> a consistent logging level to print the memory access stacks.
>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com>
> Cc: Prasad Sodagudi <psodagud@quicinc.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: Marco Elver <elver@google.com>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Alexander Potapenko <glider@google.com>

Reviewed-by: Marco Elver <elver@google.com>


> ---
>  mm/kasan/report.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index 14bd51ea2348..8fff1825b22c 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -230,7 +230,7 @@ static void print_address_description(void *addr, u8 tag)
>  {
>         struct page *page = kasan_addr_to_page(addr);
>
> -       dump_stack();
> +       dump_stack_lvl(KERN_ERR);
>         pr_err("\n");
>
>         if (page && PageSlab(page)) {
> @@ -375,7 +375,7 @@ void kasan_report_async(void)
>         pr_err("BUG: KASAN: invalid-access\n");
>         pr_err("Asynchronous mode enabled: no access details available\n");
>         pr_err("\n");
> -       dump_stack();
> +       dump_stack_lvl(KERN_ERR);
>         end_report(&flags, 0);
>  }
>  #endif /* CONFIG_KASAN_HW_TAGS */
> @@ -420,7 +420,7 @@ static void __kasan_report(unsigned long addr, size_t size, bool is_write,
>                 pr_err("\n");
>                 print_memory_metadata(info.first_bad_addr);
>         } else {
> -               dump_stack();
> +               dump_stack_lvl(KERN_ERR);
>         }
>
>         end_report(&flags, addr);
> --
> 2.31.1.527.g47e6f16901-goog
>

Re: [PATCH v2 1/2] printk: introduce dump_stack_lvl()

[Andrew Morton]

On Thu,  6 May 2021 12:54:04 +0200 Alexander Potapenko <glider@google.com> wrote:

> dump_stack() is used for many different cases, which may require a log
> level consistent with other kernel messages surrounding the dump_stack()
> call.
> Without that, certain systems that are configured to ignore the default
> level messages will miss stack traces in critical error reports.
> 
> This patch introduces dump_stack_lvl() that behaves similarly to
> dump_stack(), but accepts a custom log level.
> The old dump_stack() becomes equal to dump_stack_lvl(KERN_DEFAULT).
> 
> A somewhat similar patch has been proposed in 2012:
> https://lore.kernel.org/lkml/1332493269.2359.9.camel@hebo/
> , but wasn't merged.

Um yeah, I dropped the ball on that one, didn't I?  I suspect pretty
much all dump_stack() callsites should be using this.