[PATCH] mm/kasan: Print frame description for stack bugs

20 views
Skip to first unread message

Marco Elver

unread,
May 17, 2019, 9:12:38 AM5/17/19
to arya...@virtuozzo.com, dvy...@google.com, gli...@google.com, andre...@google.com, ak...@linux-foundation.org, linux-...@vger.kernel.org, linu...@kvack.org, kasa...@googlegroups.com, Marco Elver
This adds support for printing stack frame description on invalid stack
accesses. The frame description is embedded by the compiler, which is
parsed and then pretty-printed.

Currently, we can only print the stack frame info for accesses to the
task's own stack, but not accesses to other tasks' stacks.

Example of what it looks like:

[ 17.924050] page dumped because: kasan: bad access detected
[ 17.924908]
[ 17.925153] addr ffff8880673ef98a is located in stack of task insmod/2008 at offset 106 in frame:
[ 17.926542] kasan_stack_oob+0x0/0xf5 [test_kasan]
[ 17.927932]
[ 17.928206] this frame has 2 objects:
[ 17.928783] [32, 36) 'i'
[ 17.928784] [96, 106) 'stack_array'
[ 17.929216]
[ 17.930031] Memory state around the buggy address:

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=198435
Signed-off-by: Marco Elver <el...@google.com>
---
Change-Id: I4836cde103052991ac8871796a45b4c977c9e2e7
---
mm/kasan/kasan.h | 5 ++
mm/kasan/report.c | 160 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 165 insertions(+)

diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 3ce956efa0cb..1979db4763e2 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -43,6 +43,11 @@

#define KASAN_ALLOCA_REDZONE_SIZE 32

+/*
+ * Stack frame marker (compiler ABI).
+ */
+#define KASAN_CURRENT_STACK_FRAME_MAGIC 0x41B58AB3
+
/* Don't break randconfig/all*config builds */
#ifndef KASAN_ABI_VERSION
#define KASAN_ABI_VERSION 1
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 03a443579386..c6ad8462c0dc 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -28,6 +28,7 @@
#include <linux/types.h>
#include <linux/kasan.h>
#include <linux/module.h>
+#include <linux/sched/task_stack.h>

#include <asm/sections.h>

@@ -181,6 +182,163 @@ 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, ' ');
+ const ptrdiff_t tok_len = sep - *frame_descr;
+
+ if (sep == NULL)
+ sep = *frame_descr + strlen(*frame_descr);
+
+ if (token != NULL) {
+ 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 %zu %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, if it exists. */
+ strreplace(token, ':', '\0');
+
+ /* Finally, print object information. */
+ pr_err(" [%zu, %zu) '%s'", offset, offset + size, token);
+ }
+}
+
+static bool __must_check get_address_stack_frame_info(const void *addr,
+ size_t *offset,
+ const char **frame_descr,
+ const void **frame_pc)
+{
+ size_t aligned_addr;
+ size_t mem_ptr;
+ const u8 *shadow_bottom;
+ const u8 *shadow_ptr;
+ const size_t *frame;
+
+ /*
+ * 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((size_t)addr, sizeof(long));
+ mem_ptr = round_down(aligned_addr, KASAN_SHADOW_SCALE_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_SHADOW_SCALE_SIZE;
+ }
+
+ while (shadow_ptr >= shadow_bottom && *shadow_ptr == KASAN_STACK_LEFT) {
+ shadow_ptr--;
+ mem_ptr -= KASAN_SHADOW_SCALE_SIZE;
+ }
+
+ if (shadow_ptr < shadow_bottom)
+ return false;
+
+ frame = (const size_t *)(mem_ptr + KASAN_SHADOW_SCALE_SIZE);
+ if (frame[0] != KASAN_CURRENT_STACK_FRAME_MAGIC) {
+ pr_err("KASAN internal error: frame info validation failed; invalid marker: %zu\n",
+ frame[0]);
+ return false;
+ }
+
+ *offset = (size_t)addr - (size_t)frame;
+ *frame_descr = (const char *)frame[1];
+ *frame_pc = (void *)frame[2];
+
+ return true;
+}
+
+static void print_address_stack_frame(const void *addr)
+{
+ size_t 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 %zu 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)
{
struct page *page = addr_to_page(addr);
@@ -204,6 +362,8 @@ static void print_address_description(void *addr)
pr_err("The buggy address belongs to the page:\n");
dump_page(page, "kasan: bad access detected");
}
+
+ print_address_stack_frame(addr);
}

static bool row_is_guilty(const void *row, const void *guilty)
--
2.21.0.1020.gf2820cf01a-goog

kbuild test robot

unread,
May 18, 2019, 4:49:25 PM5/18/19
to Marco Elver, kbuil...@01.org, arya...@virtuozzo.com, dvy...@google.com, gli...@google.com, andre...@google.com, ak...@linux-foundation.org, linux-...@vger.kernel.org, linu...@kvack.org, kasa...@googlegroups.com, Marco Elver
Hi Marco,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.1 next-20190517]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Marco-Elver/mm-kasan-Print-frame-description-for-stack-bugs/20190519-040214
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.1.0 make.cross ARCH=xtensa

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <l...@intel.com>

All warnings (new ones prefixed by >>):

In file included from include/linux/printk.h:7,
from include/linux/kernel.h:15,
from include/linux/kallsyms.h:10,
from include/linux/ftrace.h:11,
from mm/kasan/report.c:18:
mm/kasan/report.c: In function 'print_decoded_frame_descr':
>> include/linux/kern_levels.h:5:18: warning: format '%zu' expects argument of type 'size_t', but argument 2 has type 'long unsigned int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^~~~~~
include/linux/kern_levels.h:11:18: note: in expansion of macro 'KERN_SOH'
#define KERN_ERR KERN_SOH "3" /* error conditions */
^~~~~~~~
include/linux/printk.h:304:9: note: in expansion of macro 'KERN_ERR'
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~
mm/kasan/report.c:233:2: note: in expansion of macro 'pr_err'
pr_err("this frame has %zu %s:\n", num_objects,
^~~~~~
mm/kasan/report.c:233:27: note: format string is defined here
pr_err("this frame has %zu %s:\n", num_objects,
~~^
%lu
In file included from include/linux/printk.h:7,
from include/linux/kernel.h:15,
from include/linux/kallsyms.h:10,
from include/linux/ftrace.h:11,
from mm/kasan/report.c:18:
>> include/linux/kern_levels.h:5:18: warning: format '%zu' expects argument of type 'size_t', but argument 2 has type 'long unsigned int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^~~~~~
include/linux/kern_levels.h:11:18: note: in expansion of macro 'KERN_SOH'
#define KERN_ERR KERN_SOH "3" /* error conditions */
^~~~~~~~
include/linux/printk.h:304:9: note: in expansion of macro 'KERN_ERR'
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~
mm/kasan/report.c:260:3: note: in expansion of macro 'pr_err'
pr_err(" [%zu, %zu) '%s'", offset, offset + size, token);
^~~~~~
mm/kasan/report.c:260:15: note: format string is defined here
pr_err(" [%zu, %zu) '%s'", offset, offset + size, token);
~~^
%lu
In file included from include/linux/printk.h:7,
from include/linux/kernel.h:15,
from include/linux/kallsyms.h:10,
from include/linux/ftrace.h:11,
from mm/kasan/report.c:18:
include/linux/kern_levels.h:5:18: warning: format '%zu' expects argument of type 'size_t', but argument 3 has type 'long unsigned int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^~~~~~
include/linux/kern_levels.h:11:18: note: in expansion of macro 'KERN_SOH'
#define KERN_ERR KERN_SOH "3" /* error conditions */
^~~~~~~~
include/linux/printk.h:304:9: note: in expansion of macro 'KERN_ERR'
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~
mm/kasan/report.c:260:3: note: in expansion of macro 'pr_err'
pr_err(" [%zu, %zu) '%s'", offset, offset + size, token);
^~~~~~
mm/kasan/report.c:260:20: note: format string is defined here
pr_err(" [%zu, %zu) '%s'", offset, offset + size, token);
~~^
%lu
--
In file included from include/linux/printk.h:7,
from include/linux/kernel.h:15,
from include/linux/kallsyms.h:10,
from include/linux/ftrace.h:11,
from mm//kasan/report.c:18:
mm//kasan/report.c: In function 'print_decoded_frame_descr':
>> include/linux/kern_levels.h:5:18: warning: format '%zu' expects argument of type 'size_t', but argument 2 has type 'long unsigned int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^~~~~~
include/linux/kern_levels.h:11:18: note: in expansion of macro 'KERN_SOH'
#define KERN_ERR KERN_SOH "3" /* error conditions */
^~~~~~~~
include/linux/printk.h:304:9: note: in expansion of macro 'KERN_ERR'
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~
mm//kasan/report.c:233:2: note: in expansion of macro 'pr_err'
pr_err("this frame has %zu %s:\n", num_objects,
^~~~~~
mm//kasan/report.c:233:27: note: format string is defined here
pr_err("this frame has %zu %s:\n", num_objects,
~~^
%lu
In file included from include/linux/printk.h:7,
from include/linux/kernel.h:15,
from include/linux/kallsyms.h:10,
from include/linux/ftrace.h:11,
from mm//kasan/report.c:18:
>> include/linux/kern_levels.h:5:18: warning: format '%zu' expects argument of type 'size_t', but argument 2 has type 'long unsigned int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^~~~~~
include/linux/kern_levels.h:11:18: note: in expansion of macro 'KERN_SOH'
#define KERN_ERR KERN_SOH "3" /* error conditions */
^~~~~~~~
include/linux/printk.h:304:9: note: in expansion of macro 'KERN_ERR'
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~
mm//kasan/report.c:260:3: note: in expansion of macro 'pr_err'
pr_err(" [%zu, %zu) '%s'", offset, offset + size, token);
^~~~~~
mm//kasan/report.c:260:15: note: format string is defined here
pr_err(" [%zu, %zu) '%s'", offset, offset + size, token);
~~^
%lu
In file included from include/linux/printk.h:7,
from include/linux/kernel.h:15,
from include/linux/kallsyms.h:10,
from include/linux/ftrace.h:11,
from mm//kasan/report.c:18:
include/linux/kern_levels.h:5:18: warning: format '%zu' expects argument of type 'size_t', but argument 3 has type 'long unsigned int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^~~~~~
include/linux/kern_levels.h:11:18: note: in expansion of macro 'KERN_SOH'
#define KERN_ERR KERN_SOH "3" /* error conditions */
^~~~~~~~
include/linux/printk.h:304:9: note: in expansion of macro 'KERN_ERR'
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~
mm//kasan/report.c:260:3: note: in expansion of macro 'pr_err'
pr_err(" [%zu, %zu) '%s'", offset, offset + size, token);
^~~~~~
mm//kasan/report.c:260:20: note: format string is defined here
pr_err(" [%zu, %zu) '%s'", offset, offset + size, token);
~~^
%lu

vim +5 include/linux/kern_levels.h

314ba352 Joe Perches 2012-07-30 4
04d2c8c8 Joe Perches 2012-07-30 @5 #define KERN_SOH "\001" /* ASCII Start Of Header */
04d2c8c8 Joe Perches 2012-07-30 6 #define KERN_SOH_ASCII '\001'
04d2c8c8 Joe Perches 2012-07-30 7

:::::: The code at line 5 was first introduced by commit
:::::: 04d2c8c83d0e3ac5f78aeede51babb3236200112 printk: convert the format for KERN_<LEVEL> to a 2 byte pattern

:::::: TO: Joe Perches <j...@perches.com>
:::::: CC: Linus Torvalds <torv...@linux-foundation.org>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz

Marco Elver

unread,
May 20, 2019, 11:49:25 AM5/20/19
to arya...@virtuozzo.com, dvy...@google.com, gli...@google.com, andre...@google.com, ak...@linux-foundation.org, linux-...@vger.kernel.org, linu...@kvack.org, kasa...@googlegroups.com, Marco Elver
This adds support for printing stack frame description on invalid stack
accesses. The frame description is embedded by the compiler, which is
parsed and then pretty-printed.

Currently, we can only print the stack frame info for accesses to the
task's own stack, but not accesses to other tasks' stacks.

Example of what it looks like:

[ 17.924050] page dumped because: kasan: bad access detected
[ 17.924908]
[ 17.925153] addr ffff8880673ef98a is located in stack of task insmod/2008 at offset 106 in frame:
[ 17.926542] kasan_stack_oob+0x0/0xf5 [test_kasan]
[ 17.927932]
[ 17.928206] this frame has 2 objects:
[ 17.928783] [32, 36) 'i'
[ 17.928784] [96, 106) 'stack_array'
[ 17.929216]
[ 17.930031] Memory state around the buggy address:

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=198435
Signed-off-by: Marco Elver <el...@google.com>
---

Changes since V1:
- Fix types in printf (%zu -> %lu).
- Prefer 'unsigned long', to ensure offsets/addrs are pointer sized, as
emitted by ASAN instrumentation.

Change-Id: I4836cde103052991ac8871796a45b4c977c9e2e7
---
mm/kasan/kasan.h | 5 ++
mm/kasan/report.c | 163 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 168 insertions(+)

diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 3ce956efa0cb..1979db4763e2 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -43,6 +43,11 @@

#define KASAN_ALLOCA_REDZONE_SIZE 32

+/*
+ * Stack frame marker (compiler ABI).
+ */
+#define KASAN_CURRENT_STACK_FRAME_MAGIC 0x41B58AB3
+
/* Don't break randconfig/all*config builds */
#ifndef KASAN_ABI_VERSION
#define KASAN_ABI_VERSION 1
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 03a443579386..36e55956acaf 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -28,6 +28,7 @@
#include <linux/types.h>
#include <linux/kasan.h>
#include <linux/module.h>
+#include <linux/sched/task_stack.h>

#include <asm/sections.h>

@@ -181,6 +182,166 @@ 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;
+
+ pr_err("this frame has %lu %s:\n", num_objects,
+ 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;
+
+ /*
+ * 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_SHADOW_SCALE_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_SHADOW_SCALE_SIZE;
+ }
+
+ while (shadow_ptr >= shadow_bottom && *shadow_ptr == KASAN_STACK_LEFT) {
+ shadow_ptr--;
+ mem_ptr -= KASAN_SHADOW_SCALE_SIZE;
+ }
+
+ if (shadow_ptr < shadow_bottom)
+ return false;
+
+ frame = (const unsigned long *)(mem_ptr + KASAN_SHADOW_SCALE_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)
{
struct page *page = addr_to_page(addr);
@@ -204,6 +365,8 @@ static void print_address_description(void *addr)

Andrey Ryabinin

unread,
May 21, 2019, 11:43:40 AM5/21/19
to Marco Elver, dvy...@google.com, gli...@google.com, andre...@google.com, ak...@linux-foundation.org, linux-...@vger.kernel.org, linu...@kvack.org, kasa...@googlegroups.com
Why?

> + strreplace(token, ':', '\0');
> +

...

> +
> + aligned_addr = round_down((unsigned long)addr, sizeof(long));
> + mem_ptr = round_down(aligned_addr, KASAN_SHADOW_SCALE_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_SHADOW_SCALE_SIZE;
> + }
> +
> + while (shadow_ptr >= shadow_bottom && *shadow_ptr == KASAN_STACK_LEFT) {
> + shadow_ptr--;
> + mem_ptr -= KASAN_SHADOW_SCALE_SIZE;
> + }
> +

I suppose this won't work if stack grows up, which is fine because it grows up only on parisc arch.
But "BUILD_BUG_ON(IS_ENABLED(CONFIG_STACK_GROUWSUP))" somewhere wouldn't hurt.


Alexander Potapenko

unread,
May 21, 2019, 11:53:08 AM5/21/19
to Andrey Ryabinin, Marco Elver, Dmitriy Vyukov, Andrey Konovalov, Andrew Morton, LKML, Linux Memory Management List, kasan-dev
Note that KASAN was broken on parisc from day 1 because of other
assumptions on the stack growth direction hardcoded into KASAN
(e.g. __kasan_unpoison_stack() and __asan_allocas_unpoison()).
So maybe this BUILD_BUG_ON can be added in a separate patch as it's
not specific to what Marco is doing here?
>
> --
> 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 post to this group, send email to kasa...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/ebec4325-f91b-b392-55ed-95dbd36bbb8e%40virtuozzo.com.
> For more options, visit https://groups.google.com/d/optout.



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

Marco Elver

unread,
May 21, 2019, 12:07:50 PM5/21/19
to Alexander Potapenko, Andrey Ryabinin, Dmitriy Vyukov, Andrey Konovalov, Andrew Morton, LKML, Linux Memory Management List, kasan-dev
The filename is not included, and I don't think it adds much in terms
of ability to debug; nor is the line number included with all
descriptions. I think, the added complexity of separating the line
number and parsing is not worthwhile here. Alternatively, I could not
pay attention to the line number at all, and leave it as is -- in that
case, some variable names will display as "foo:123".
Happy to send a follow-up patch, or add here. Let me know what you prefer.

Thanks,
-- Marco

Andrey Ryabinin

unread,
May 21, 2019, 2:07:29 PM5/21/19
to Marco Elver, Alexander Potapenko, Dmitriy Vyukov, Andrey Konovalov, Andrew Morton, LKML, Linux Memory Management List, kasan-dev
Either way is fine by me. But explain why in comment if you decide
to keep current code. Something like
/* Strip line number cause it's not very helpful. */


>>>
>>>> + strreplace(token, ':', '\0');
>>>> +
>>>
>>> ...
>>>
>>>> +
>>>> + aligned_addr = round_down((unsigned long)addr, sizeof(long));
>>>> + mem_ptr = round_down(aligned_addr, KASAN_SHADOW_SCALE_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_SHADOW_SCALE_SIZE;
>>>> + }
>>>> +
>>>> + while (shadow_ptr >= shadow_bottom && *shadow_ptr == KASAN_STACK_LEFT) {
>>>> + shadow_ptr--;
>>>> + mem_ptr -= KASAN_SHADOW_SCALE_SIZE;
>>>> + }
>>>> +
>>>
>>> I suppose this won't work if stack grows up, which is fine because it grows up only on parisc arch.
>>> But "BUILD_BUG_ON(IS_ENABLED(CONFIG_STACK_GROUWSUP))" somewhere wouldn't hurt.
>> Note that KASAN was broken on parisc from day 1 because of other
>> assumptions on the stack growth direction hardcoded into KASAN
>> (e.g. __kasan_unpoison_stack() and __asan_allocas_unpoison()).

It's not broken, it doesn't exist.

>> So maybe this BUILD_BUG_ON can be added in a separate patch as it's
>> not specific to what Marco is doing here?
>

I think it's fine to add it in this patch because BUILD_BUG_ON() is just a hint for developers
that this particular function depends on growing down stack. So it's more a property of the function
rather than KASAN in general.

Other functions you mentioned can be marked with BUILD_BUG_ON()s as well, but not in this patch indeed.

> Happy to send a follow-up patch, or add here. Let me know what you prefer.
>

Send v3 please.

Andrew Morton

unread,
May 21, 2019, 10:10:53 PM5/21/19
to kbuild test robot, Marco Elver, kbuil...@01.org, arya...@virtuozzo.com, dvy...@google.com, gli...@google.com, andre...@google.com, linux-...@vger.kernel.org, linu...@kvack.org, kasa...@googlegroups.com
On Sun, 19 May 2019 04:48:21 +0800 kbuild test robot <l...@intel.com> wrote:

> Hi Marco,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on linus/master]
> [also build test WARNING on v5.1 next-20190517]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Marco-Elver/mm-kasan-Print-frame-description-for-stack-bugs/20190519-040214
> config: xtensa-allyesconfig (attached as .config)
> compiler: xtensa-linux-gcc (GCC) 8.1.0
> reproduce:
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> GCC_VERSION=8.1.0 make.cross ARCH=xtensa
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <l...@intel.com>
>

This, I assume?

--- a/mm/kasan/report.c~mm-kasan-print-frame-description-for-stack-bugs-fix
+++ a/mm/kasan/report.c
@@ -230,7 +230,7 @@ static void print_decoded_frame_descr(co
return;

pr_err("\n");
- pr_err("this frame has %zu %s:\n", num_objects,
+ pr_err("this frame has %lu %s:\n", num_objects,
num_objects == 1 ? "object" : "objects");

while (num_objects--) {
@@ -257,7 +257,7 @@ static void print_decoded_frame_descr(co
strreplace(token, ':', '\0');

/* Finally, print object information. */
- pr_err(" [%zu, %zu) '%s'", offset, offset + size, token);
+ pr_err(" [%lu, %lu) '%s'", offset, offset + size, token);
}
}

_

Marco Elver

unread,
May 22, 2019, 6:02:00 AM5/22/19
to arya...@virtuozzo.com, dvy...@google.com, gli...@google.com, andre...@google.com, ak...@linux-foundation.org, linux-...@vger.kernel.org, linu...@kvack.org, kasa...@googlegroups.com, Marco Elver
This adds support for printing stack frame description on invalid stack
accesses. The frame description is embedded by the compiler, which is
parsed and then pretty-printed.

Currently, we can only print the stack frame info for accesses to the
task's own stack, but not accesses to other tasks' stacks.

Example of what it looks like:

[ 17.924050] page dumped because: kasan: bad access detected
[ 17.924908]
[ 17.925153] addr ffff8880673ef98a is located in stack of task insmod/2008 at offset 106 in frame:
[ 17.926542] kasan_stack_oob+0x0/0xf5 [test_kasan]
[ 17.927932]
[ 17.928206] this frame has 2 objects:
[ 17.928783] [32, 36) 'i'
[ 17.928784] [96, 106) 'stack_array'
[ 17.929216]
[ 17.930031] Memory state around the buggy address:

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=198435
Signed-off-by: Marco Elver <el...@google.com>
---

Changes since v2:
- Comment about why line number is stripped.
- Add BUILD_BUG_ON(CONFIG_STACK_GROWSUP).

Changes since v1:
- Fix types in printf (%zu -> %lu).
- Prefer 'unsigned long', to ensure offset/points are pointer sized, as
emitted by ASAN instrumentation.

Change-Id: I4836cde103052991ac8871796a45b4c977c9e2e7
---
mm/kasan/kasan.h | 5 ++
mm/kasan/report.c | 165 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 170 insertions(+)

diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 3ce956efa0cb..1979db4763e2 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -43,6 +43,11 @@

#define KASAN_ALLOCA_REDZONE_SIZE 32

+/*
+ * Stack frame marker (compiler ABI).
+ */
+#define KASAN_CURRENT_STACK_FRAME_MAGIC 0x41B58AB3
+
/* Don't break randconfig/all*config builds */
#ifndef KASAN_ABI_VERSION
#define KASAN_ABI_VERSION 1
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 03a443579386..0e5f965f1882 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -28,6 +28,7 @@
#include <linux/types.h>
#include <linux/kasan.h>
#include <linux/module.h>
+#include <linux/sched/task_stack.h>

#include <asm/sections.h>

@@ -181,6 +182,168 @@ static inline bool init_task_stack_addr(const void *addr)
+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_SHADOW_SCALE_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_SHADOW_SCALE_SIZE;
+ }
+
+ while (shadow_ptr >= shadow_bottom && *shadow_ptr == KASAN_STACK_LEFT) {
+ shadow_ptr--;
+ mem_ptr -= KASAN_SHADOW_SCALE_SIZE;
+ }
+
+ if (shadow_ptr < shadow_bottom)
+ return false;
+
+ frame = (const unsigned long *)(mem_ptr + KASAN_SHADOW_SCALE_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)
{
struct page *page = addr_to_page(addr);
@@ -204,6 +367,8 @@ static void print_address_description(void *addr)

Marco Elver

unread,
May 22, 2019, 6:02:59 AM5/22/19
to Andrew Morton, kbuild test robot, kbuil...@01.org, Andrey Ryabinin, Dmitry Vyukov, Alexander Potapenko, Andrey Konovalov, LKML, Linux Memory Management List, kasan-dev
I've sent v3. If possible, please replace current version with v3,
which also includes the fix.

Many thanks,
-- Marco
> --
> 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 post to this group, send email to kasa...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20190521191050.b8ddb9bb660d13330896529e%40linux-foundation.org.

Andrey Ryabinin

unread,
May 22, 2019, 8:19:14 AM5/22/19
to Marco Elver, dvy...@google.com, gli...@google.com, andre...@google.com, ak...@linux-foundation.org, linux-...@vger.kernel.org, linu...@kvack.org, kasa...@googlegroups.com
On 5/22/19 1:00 PM, Marco Elver wrote:
> This adds support for printing stack frame description on invalid stack
> accesses. The frame description is embedded by the compiler, which is
> parsed and then pretty-printed.
>
> Currently, we can only print the stack frame info for accesses to the
> task's own stack, but not accesses to other tasks' stacks.
>
> Example of what it looks like:
>
> [ 17.924050] page dumped because: kasan: bad access detected
> [ 17.924908]
> [ 17.925153] addr ffff8880673ef98a is located in stack of task insmod/2008 at offset 106 in frame:
> [ 17.926542] kasan_stack_oob+0x0/0xf5 [test_kasan]
> [ 17.927932]
> [ 17.928206] this frame has 2 objects:
> [ 17.928783] [32, 36) 'i'
> [ 17.928784] [96, 106) 'stack_array'
> [ 17.929216]
> [ 17.930031] Memory state around the buggy address:
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=198435
> Signed-off-by: Marco Elver <el...@google.com>

Reviewed-by: Andrey Ryabinin <arya...@virtuozzo.com>

kbuild test robot

unread,
May 23, 2019, 5:51:24 AM5/23/19
to Marco Elver, kbuil...@01.org, arya...@virtuozzo.com, dvy...@google.com, gli...@google.com, andre...@google.com, ak...@linux-foundation.org, linux-...@vger.kernel.org, linu...@kvack.org, kasa...@googlegroups.com, Marco Elver
Hi Marco,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.2-rc1 next-20190522]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Marco-Elver/mm-kasan-Print-frame-description-for-stack-bugs/20190519-040214
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=xtensa

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <l...@intel.com>

All warnings (new ones prefixed by >>):

In file included from include/linux/printk.h:7:0,
from include/linux/kernel.h:15,
from include/linux/kallsyms.h:10,
from include/linux/ftrace.h:11,
from mm/kasan/report.c:18:
mm/kasan/report.c: In function 'print_decoded_frame_descr':
include/linux/kern_levels.h:5:18: warning: format '%zu' expects argument of type 'size_t', but argument 2 has type 'long unsigned int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
include/linux/kern_levels.h:11:18: note: in expansion of macro 'KERN_SOH'
#define KERN_ERR KERN_SOH "3" /* error conditions */
^~~~~~~~
include/linux/printk.h:304:9: note: in expansion of macro 'KERN_ERR'
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~
>> mm/kasan/report.c:233:2: note: in expansion of macro 'pr_err'
pr_err("this frame has %zu %s:\n", num_objects,
^~~~~~
mm/kasan/report.c:233:27: note: format string is defined here
pr_err("this frame has %zu %s:\n", num_objects,
~~^
%lu
In file included from include/linux/printk.h:7:0,
from include/linux/kernel.h:15,
from include/linux/kallsyms.h:10,
from include/linux/ftrace.h:11,
from mm/kasan/report.c:18:
include/linux/kern_levels.h:5:18: warning: format '%zu' expects argument of type 'size_t', but argument 2 has type 'long unsigned int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
include/linux/kern_levels.h:11:18: note: in expansion of macro 'KERN_SOH'
#define KERN_ERR KERN_SOH "3" /* error conditions */
^~~~~~~~
include/linux/printk.h:304:9: note: in expansion of macro 'KERN_ERR'
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~
mm/kasan/report.c:260:3: note: in expansion of macro 'pr_err'
pr_err(" [%zu, %zu) '%s'", offset, offset + size, token);
^~~~~~
mm/kasan/report.c:260:15: note: format string is defined here
pr_err(" [%zu, %zu) '%s'", offset, offset + size, token);
~~^
%lu
In file included from include/linux/printk.h:7:0,
from include/linux/kernel.h:15,
from include/linux/kallsyms.h:10,
from include/linux/ftrace.h:11,
from mm/kasan/report.c:18:
include/linux/kern_levels.h:5:18: warning: format '%zu' expects argument of type 'size_t', but argument 3 has type 'long unsigned int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
include/linux/kern_levels.h:11:18: note: in expansion of macro 'KERN_SOH'
#define KERN_ERR KERN_SOH "3" /* error conditions */
^~~~~~~~
include/linux/printk.h:304:9: note: in expansion of macro 'KERN_ERR'
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~
mm/kasan/report.c:260:3: note: in expansion of macro 'pr_err'
pr_err(" [%zu, %zu) '%s'", offset, offset + size, token);
^~~~~~
mm/kasan/report.c:260:20: note: format string is defined here
pr_err(" [%zu, %zu) '%s'", offset, offset + size, token);
~~^
%lu

vim +/pr_err +233 mm/kasan/report.c

214
215 static void print_decoded_frame_descr(const char *frame_descr)
216 {
217 /*
218 * We need to parse the following string:
219 * "n alloc_1 alloc_2 ... alloc_n"
220 * where alloc_i looks like
221 * "offset size len name"
222 * or "offset size len name:line".
223 */
224
225 char token[64];
226 unsigned long num_objects;
227
228 if (!tokenize_frame_descr(&frame_descr, token, sizeof(token),
229 &num_objects))
230 return;
231
232 pr_err("\n");
> 233 pr_err("this frame has %zu %s:\n", num_objects,
234 num_objects == 1 ? "object" : "objects");
235
236 while (num_objects--) {
237 unsigned long offset;
238 unsigned long size;
239
240 /* access offset */
241 if (!tokenize_frame_descr(&frame_descr, token, sizeof(token),
242 &offset))
243 return;
244 /* access size */
245 if (!tokenize_frame_descr(&frame_descr, token, sizeof(token),
246 &size))
247 return;
248 /* name length (unused) */
249 if (!tokenize_frame_descr(&frame_descr, NULL, 0, NULL))
250 return;
251 /* object name */
252 if (!tokenize_frame_descr(&frame_descr, token, sizeof(token),
253 NULL))
254 return;
255
256 /* Strip line number, if it exists. */
257 strreplace(token, ':', '\0');
258
259 /* Finally, print object information. */
260 pr_err(" [%zu, %zu) '%s'", offset, offset + size, token);
261 }
262 }
263
.config.gz
Reply all
Reply to author
Forward
0 new messages