diff mbox series

[mm,03/22] kasan: rearrange stack frame info in reports

Message ID 1ee113a4c111df97d168c820b527cda77a3cac40.1646237226.git.andreyknvl@google.com (mailing list archive)
State New
Headers show
Series kasan: report clean-ups and improvements | expand

Commit Message

andrey.konovalov@linux.dev March 2, 2022, 4:36 p.m. UTC
From: Andrey Konovalov <andreyknvl@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 <andreyknvl@google.com>
---
 mm/kasan/report.c         | 12 +++++++++---
 mm/kasan/report_generic.c | 15 ++++-----------
 2 files changed, 13 insertions(+), 14 deletions(-)

Comments

Alexander Potapenko March 2, 2022, 5:29 p.m. UTC | #1
On Wed, Mar 2, 2022 at 5:36 PM <andrey.konovalov@linux.dev> wrote:

> From: Andrey Konovalov <andreyknvl@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 <andreyknvl@google.com>
>
Reviewed-by: Alexander Potapenko <glider@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
>
> --
> 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+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/kasan-dev/1ee113a4c111df97d168c820b527cda77a3cac40.1646237226.git.andreyknvl%40google.com
> .
>
diff mbox series

Patch

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);