Re: [PATCH v9 14/44] kasan: decode stack frame only with KASAN_STACK_ENABLE

From: Alexander Potapenko
Date: Wed Nov 11 2020 - 09:25:45 EST

Next message: Rob Herring: "Re: [PATCH v3 1/3] dt-bindings: Convert graph bindings to json-schema"
Previous message: Alexander Potapenko: "Re: [PATCH v9 13/44] kasan: hide invalid free check implementation"
In reply to: Andrey Konovalov: "[PATCH v9 14/44] kasan: decode stack frame only with KASAN_STACK_ENABLE"
Next in thread: Andrey Konovalov: "[PATCH v9 17/44] kasan, arm64: move initialization message"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Tue, Nov 10, 2020 at 11:11 PM Andrey Konovalov <andreyknvl@xxxxxxxxxx> wrote:
>
> Decoding routines aren't needed when CONFIG_KASAN_STACK_ENABLE is not
> enabled. Currently only generic KASAN mode implements stack error
> reporting.
>
> No functional changes for software modes.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@xxxxxxxxxx>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@xxxxxxx>
> Reviewed-by: Marco Elver <elver@xxxxxxxxxx>
Reviewed-by: Alexander Potapenko <glider@xxxxxxxxxx>

> ---
> Change-Id: I084e3214f2b40dc0bef7c5a9fafdc6f5c42b06a2
> ---
> mm/kasan/kasan.h | 6 ++
> mm/kasan/report.c | 162 --------------------------------------
> mm/kasan/report_generic.c | 162 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 168 insertions(+), 162 deletions(-)
>
> diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
> index 3eff57e71ff5..d0cf61d4d70d 100644
> --- a/mm/kasan/kasan.h
> +++ b/mm/kasan/kasan.h
> @@ -169,6 +169,12 @@ bool check_invalid_free(void *addr);
> void *find_first_bad_addr(void *addr, size_t size);
> const char *get_bug_type(struct kasan_access_info *info);
>
> +#if defined(CONFIG_KASAN_GENERIC) && CONFIG_KASAN_STACK
> +void print_address_stack_frame(const void *addr);
> +#else
> +static inline void print_address_stack_frame(const void *addr) { }
> +#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 fff0c7befbfe..b18d193f7f58 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -211,168 +211,6 @@ static inline bool init_task_stack_addr(const void *addr)
> sizeof(init_thread_union.stack));
> }
>
> -static bool __must_check tokenize_frame_descr(const char **frame_descr,
> - char *token, size_t max_tok_len,
> - unsigned long *value)
> -{
> - const char *sep = strchr(*frame_descr, ' ');
> -
> - if (sep == NULL)
> - sep = *frame_descr + strlen(*frame_descr);
> -
> - if (token != NULL) {
> - const size_t tok_len = sep - *frame_descr;
> -
> - if (tok_len + 1 > max_tok_len) {
> - pr_err("KASAN internal error: frame description too long: %s\n",
> - *frame_descr);
> - return false;
> - }
> -
> - /* Copy token (+ 1 byte for '\0'). */
> - strlcpy(token, *frame_descr, tok_len + 1);
> - }
> -
> - /* Advance frame_descr past separator. */
> - *frame_descr = sep + 1;
> -
> - if (value != NULL && kstrtoul(token, 10, value)) {
> - pr_err("KASAN internal error: not a valid number: %s\n", token);
> - return false;
> - }
> -
> - return true;
> -}
> -
> -static void print_decoded_frame_descr(const char *frame_descr)
> -{
> - /*
> - * We need to parse the following string:
> - * "n alloc_1 alloc_2 ... alloc_n"
> - * where alloc_i looks like
> - * "offset size len name"
> - * or "offset size len name:line".
> - */
> -
> - char token[64];
> - unsigned long num_objects;
> -
> - if (!tokenize_frame_descr(&frame_descr, token, sizeof(token),
> - &num_objects))
> - return;
> -
> - pr_err("\n");
> - pr_err("this frame has %lu %s:\n", num_objects,
> - num_objects == 1 ? "object" : "objects");
> -
> - while (num_objects--) {
> - unsigned long offset;
> - unsigned long size;
> -
> - /* access offset */
> - if (!tokenize_frame_descr(&frame_descr, token, sizeof(token),
> - &offset))
> - return;
> - /* access size */
> - if (!tokenize_frame_descr(&frame_descr, token, sizeof(token),
> - &size))
> - return;
> - /* name length (unused) */
> - if (!tokenize_frame_descr(&frame_descr, NULL, 0, NULL))
> - return;
> - /* object name */
> - if (!tokenize_frame_descr(&frame_descr, token, sizeof(token),
> - NULL))
> - return;
> -
> - /* Strip line number; without filename it's not very helpful. */
> - strreplace(token, ':', '\0');
> -
> - /* Finally, print object information. */
> - pr_err(" [%lu, %lu) '%s'", offset, offset + size, token);
> - }
> -}
> -
> -static bool __must_check get_address_stack_frame_info(const void *addr,
> - unsigned long *offset,
> - const char **frame_descr,
> - const void **frame_pc)
> -{
> - unsigned long aligned_addr;
> - unsigned long mem_ptr;
> - const u8 *shadow_bottom;
> - const u8 *shadow_ptr;
> - const unsigned long *frame;
> -
> - 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);
> - shadow_bottom = kasan_mem_to_shadow(end_of_stack(current));
> -
> - while (shadow_ptr >= shadow_bottom && *shadow_ptr != KASAN_STACK_LEFT) {
> - shadow_ptr--;
> - mem_ptr -= KASAN_GRANULE_SIZE;
> - }
> -
> - while (shadow_ptr >= shadow_bottom && *shadow_ptr == KASAN_STACK_LEFT) {
> - shadow_ptr--;
> - mem_ptr -= KASAN_GRANULE_SIZE;
> - }
> -
> - if (shadow_ptr < shadow_bottom)
> - return false;
> -
> - frame = (const unsigned long *)(mem_ptr + KASAN_GRANULE_SIZE);
> - if (frame[0] != KASAN_CURRENT_STACK_FRAME_MAGIC) {
> - pr_err("KASAN internal error: frame info validation failed; invalid marker: %lu\n",
> - frame[0]);
> - return false;
> - }
> -
> - *offset = (unsigned long)addr - (unsigned long)frame;
> - *frame_descr = (const char *)frame[1];
> - *frame_pc = (void *)frame[2];
> -
> - return true;
> -}
> -
> -static void print_address_stack_frame(const void *addr)
> -{
> - unsigned long offset;
> - const char *frame_descr;
> - const void *frame_pc;
> -
> - if (IS_ENABLED(CONFIG_KASAN_SW_TAGS))
> - 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);
> - pr_err(" %pS\n", frame_pc);
> -
> - if (!frame_descr)
> - return;
> -
> - print_decoded_frame_descr(frame_descr);
> -}
> -
> static void print_address_description(void *addr, u8 tag)
> {
> struct page *page = kasan_addr_to_page(addr);
> diff --git a/mm/kasan/report_generic.c b/mm/kasan/report_generic.c
> index 7d5b9e5c7cfe..b543a1ed6078 100644
> --- a/mm/kasan/report_generic.c
> +++ b/mm/kasan/report_generic.c
> @@ -16,6 +16,7 @@
> #include <linux/mm.h>
> #include <linux/printk.h>
> #include <linux/sched.h>
> +#include <linux/sched/task_stack.h>
> #include <linux/slab.h>
> #include <linux/stackdepot.h>
> #include <linux/stacktrace.h>
> @@ -122,6 +123,167 @@ const char *get_bug_type(struct kasan_access_info *info)
> return get_wild_bug_type(info);
> }
>
> +#if CONFIG_KASAN_STACK
> +static bool __must_check tokenize_frame_descr(const char **frame_descr,
> + char *token, size_t max_tok_len,
> + unsigned long *value)
> +{
> + const char *sep = strchr(*frame_descr, ' ');
> +
> + if (sep == NULL)
> + sep = *frame_descr + strlen(*frame_descr);
> +
> + if (token != NULL) {
> + const size_t tok_len = sep - *frame_descr;
> +
> + if (tok_len + 1 > max_tok_len) {
> + pr_err("KASAN internal error: frame description too long: %s\n",
> + *frame_descr);
> + return false;
> + }
> +
> + /* Copy token (+ 1 byte for '\0'). */
> + strlcpy(token, *frame_descr, tok_len + 1);
> + }
> +
> + /* Advance frame_descr past separator. */
> + *frame_descr = sep + 1;
> +
> + if (value != NULL && kstrtoul(token, 10, value)) {
> + pr_err("KASAN internal error: not a valid number: %s\n", token);
> + return false;
> + }
> +
> + return true;
> +}
> +
> +static void print_decoded_frame_descr(const char *frame_descr)
> +{
> + /*
> + * We need to parse the following string:
> + * "n alloc_1 alloc_2 ... alloc_n"
> + * where alloc_i looks like
> + * "offset size len name"
> + * or "offset size len name:line".
> + */
> +
> + char token[64];
> + unsigned long num_objects;
> +
> + if (!tokenize_frame_descr(&frame_descr, token, sizeof(token),
> + &num_objects))
> + return;
> +
> + pr_err("\n");
> + pr_err("this frame has %lu %s:\n", num_objects,
> + num_objects == 1 ? "object" : "objects");
> +
> + while (num_objects--) {
> + unsigned long offset;
> + unsigned long size;
> +
> + /* access offset */
> + if (!tokenize_frame_descr(&frame_descr, token, sizeof(token),
> + &offset))
> + return;
> + /* access size */
> + if (!tokenize_frame_descr(&frame_descr, token, sizeof(token),
> + &size))
> + return;
> + /* name length (unused) */
> + if (!tokenize_frame_descr(&frame_descr, NULL, 0, NULL))
> + return;
> + /* object name */
> + if (!tokenize_frame_descr(&frame_descr, token, sizeof(token),
> + NULL))
> + return;
> +
> + /* Strip line number; without filename it's not very helpful. */
> + strreplace(token, ':', '\0');
> +
> + /* Finally, print object information. */
> + pr_err(" [%lu, %lu) '%s'", offset, offset + size, token);
> + }
> +}
> +
> +static bool __must_check get_address_stack_frame_info(const void *addr,
> + unsigned long *offset,
> + const char **frame_descr,
> + const void **frame_pc)
> +{
> + unsigned long aligned_addr;
> + unsigned long mem_ptr;
> + const u8 *shadow_bottom;
> + const u8 *shadow_ptr;
> + const unsigned long *frame;
> +
> + 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);
> + shadow_bottom = kasan_mem_to_shadow(end_of_stack(current));
> +
> + while (shadow_ptr >= shadow_bottom && *shadow_ptr != KASAN_STACK_LEFT) {
> + shadow_ptr--;
> + mem_ptr -= KASAN_GRANULE_SIZE;
> + }
> +
> + while (shadow_ptr >= shadow_bottom && *shadow_ptr == KASAN_STACK_LEFT) {
> + shadow_ptr--;
> + mem_ptr -= KASAN_GRANULE_SIZE;
> + }
> +
> + if (shadow_ptr < shadow_bottom)
> + return false;
> +
> + frame = (const unsigned long *)(mem_ptr + KASAN_GRANULE_SIZE);
> + if (frame[0] != KASAN_CURRENT_STACK_FRAME_MAGIC) {
> + pr_err("KASAN internal error: frame info validation failed; invalid marker: %lu\n",
> + frame[0]);
> + return false;
> + }
> +
> + *offset = (unsigned long)addr - (unsigned long)frame;
> + *frame_descr = (const char *)frame[1];
> + *frame_pc = (void *)frame[2];
> +
> + return true;
> +}
> +
> +void print_address_stack_frame(const void *addr)
> +{
> + unsigned long offset;
> + const char *frame_descr;
> + const void *frame_pc;
> +
> + 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);
> + pr_err(" %pS\n", frame_pc);
> +
> + if (!frame_descr)
> + return;
> +
> + print_decoded_frame_descr(frame_descr);
> +}
> +#endif /* CONFIG_KASAN_STACK */
> +
> #define DEFINE_ASAN_REPORT_LOAD(size) \
> void __asan_report_load##size##_noabort(unsigned long addr) \
> { \
> --
> 2.29.2.222.g5d2a92d10f8-goog
>

--
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Next message: Rob Herring: "Re: [PATCH v3 1/3] dt-bindings: Convert graph bindings to json-schema"
Previous message: Alexander Potapenko: "Re: [PATCH v9 13/44] kasan: hide invalid free check implementation"
In reply to: Andrey Konovalov: "[PATCH v9 14/44] kasan: decode stack frame only with KASAN_STACK_ENABLE"
Next in thread: Andrey Konovalov: "[PATCH v9 17/44] kasan, arm64: move initialization message"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]