Skip to contentSkip to conversation

arm64: avoid KASAN stack overflows

PATCH New Mark Rutland 3 participants 6 messages Jun 7, 2017 arm64

lore
Share & tools
lore

Mark Rutland

author reviewer
#0 lore raw

cover arm64: avoid KASAN stack overflows

Hi,

I recently tried building the kernel with a GCC 7.1.0 toolchain, and encountered a number of new and surprising failures on kernels buitl with KASAN.

It looks like this is due to stack instrumentation, which my prior toolchain didn't support. KASAN's stack instrumentation significantly bloats the stack significantly, leading to stack overflows and subsequent failures as a result of the data corruption they cause.

These patches avoid the issue by doubling the stack size for kernels built with KASAN, as is already done for x86.

I've built and booted 4K and 64K kernels with these patches applied (and with KASAN enabled), and so far I haven't seen stack overflows when testing with Syzkaller.

Thanks,
Mark.

Mark Rutland (2):
arm64: avoid open-coding THREAD_SIZE{,_ORDER}
arm64: use larger stacks for KASAN

 arch/arm64/include/asm/thread_info.h | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)
Signature · 1 line
1.9.1
author reviewer
#1 lore raw

PATCH 1/2 arm64: avoid open-coding THREAD_SIZE{,_ORDER}

Currently we define THREAD_SIZE_ORDER dependent on which arm64-specific page size kconfig symbol was selected. This is unfortunate, as it hides the relationship between THREAD_SIZE_ORDER and THREAD_SIZE, and makes it painful more painful than necessary to modify the thread size as we will need to do for some debug configurations.

This patch follows arch/metag's approach of consistently defining THREAD_SIZE in terms of THREAD_SIZE_ORDER. This avoids having ifdefs for particular page size configurations, and allows us to change a single definition to change the thread size.

Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
---
 arch/arm64/include/asm/thread_info.h | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
arch/arm64/include/asm/thread_info.h +8 -4
index 46c3b93..4f42275 100644
@@ -23,13 +23,17 @@
 
 #include <linux/compiler.h>
 
-#ifdef CONFIG_ARM64_4K_PAGES
-#define THREAD_SIZE_ORDER	2
-#elif defined(CONFIG_ARM64_16K_PAGES)
+#include <asm/page.h>
+
+#define THREAD_SHIFT		14
+
+#if THREAD_SHIFT >= PAGE_SHIFT
+#define THREAD_SIZE_ORDER	(THREAD_SHIFT - PAGE_SHIFT)
+#else
 #define THREAD_SIZE_ORDER	0
 #endif
 
-#define THREAD_SIZE		16384
+#define THREAD_SIZE		(PAGE_SIZE << THREAD_SIZE_ORDER)
 #define THREAD_START_SP		(THREAD_SIZE - 16)
 
 #ifndef __ASSEMBLY__
Signature · 1 line
1.9.1
author reviewer
#2 lore raw

PATCH 2/2 arm64: use larger stacks for KASAN

In reply to Mark Rutland

With recent toolchains (e.g. GCC 7.1.0), KASAN will instrument stack accesses, significantly increasing stack pressure. This has been observed to result in a boot-time stack overflow on Juno R1, and various failures under Syzkaller-directed Qemu VMs.

On x86, this was addressed in commit:

c420f167db8c799d ("kasan: enable stack instrumentation")

... by doubling the stack size to 32K when the kernel is compiled with KASAN.

This patch does the same for arm64, enabling us to make use of KASAN's stack instrumentation. Kernels without KASAN are not affected and will retain the current 16K stack size.

Cc Alexander Potapenko <[email protected]>
Cc Andrey Ryabinin <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Dmitry Vyukov <[email protected]>
Cc Will Deacon <[email protected]>
---
 arch/arm64/include/asm/thread_info.h | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
arch/arm64/include/asm/thread_info.h +11 -1
index 4f42275..1dec96e8 100644
@@ -25,7 +25,17 @@
 
 #include <asm/page.h>
 
-#define THREAD_SHIFT		14
+/*
+ * AddressSanitizer's stack instrumentation significantly increases stack
+ * usage. We double the stack size when KASAN is selected to avoid overflows.
+ */
+#ifdef CONFIG_KASAN
+#define KASAN_THREAD_SHIFT	1
+#else
+#define KASAN_THREAD_SHIFT	0
+#endif
+
+#define THREAD_SHIFT		(14 + KASAN_THREAD_SHIFT)
 
 #if THREAD_SHIFT >= PAGE_SHIFT
 #define THREAD_SIZE_ORDER	(THREAD_SHIFT - PAGE_SHIFT)
Signature · 1 line
1.9.1
#3 lore raw

cover arm64: avoid KASAN stack overflows

In reply to Mark Rutland
Quoted 11 lines · Mark Rutland

Hi,

I recently tried building the kernel with a GCC 7.1.0 toolchain, and encountered a number of new and surprising failures on kernels buitl with KASAN.

It looks like this is due to stack instrumentation, which my prior toolchain didn't support. KASAN's stack instrumentation significantly bloats the stack significantly, leading to stack overflows and subsequent failures as a result of the data corruption they cause.

This is caused by -fsanitize-address-use-after-scope which is added in gcc 7. Arnd reported that sometimes it causes enormously huge stack growth.

Given that we haven't found any single use-after-scope bug so far, I wouldn't object removing it completely.

Quoted 17 lines

These patches avoid the issue by doubling the stack size for kernels built with KASAN, as is already done for x86.

I've built and booted 4K and 64K kernels with these patches applied (and with KASAN enabled), and so far I haven't seen stack overflows when testing with Syzkaller.

Thanks,
Mark.

Mark Rutland (2):
arm64: avoid open-coding THREAD_SIZE{,_ORDER}
arm64: use larger stacks for KASAN

 arch/arm64/include/asm/thread_info.h | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)
author reviewer
#4 lore raw

cover arm64: avoid KASAN stack overflows

Quoted 5 lines · Andrey Ryabinin
Quoted 4 lines · Mark Rutland

I recently tried building the kernel with a GCC 7.1.0 toolchain, and encountered a number of new and surprising failures on kernels buitl with KASAN.

Quoted 7 lines
Quoted 4 lines

It looks like this is due to stack instrumentation, which my prior toolchain didn't support. KASAN's stack instrumentation significantly bloats the stack significantly, leading to stack overflows and subsequent failures as a result of the data corruption they cause.

This is caused by -fsanitize-address-use-after-scope which is added in gcc 7. Arnd reported that sometimes it causes enormously huge stack growth.

Ah. Sorry for the bogus attribution, then.

Quoted 2 lines

Given that we haven't found any single use-after-scope bug so far, I wouldn't object removing it completely.

FWIW, I saw a single use-after-scope splat when testing with syzkaller (prior to these patches), but that may have been a result of things going wrong after a stack overflow. Unfortuantely I threw away all of the results of that run.

I'll see if anything triggers overnight with this patch.

Otherwise, I'm also happy for use-after-scope checks to be disabled.

Thanks,
Mark.

#5 lore raw

cover arm64: avoid KASAN stack overflows

Quoted 19 lines · Mark Rutland
Quoted 5 lines · Andrey Ryabinin
Quoted 4 lines · Mark Rutland

I recently tried building the kernel with a GCC 7.1.0 toolchain, and encountered a number of new and surprising failures on kernels buitl with KASAN.

Quoted 7 lines
Quoted 4 lines

It looks like this is due to stack instrumentation, which my prior toolchain didn't support. KASAN's stack instrumentation significantly bloats the stack significantly, leading to stack overflows and subsequent failures as a result of the data corruption they cause.

This is caused by -fsanitize-address-use-after-scope which is added in gcc 7. Arnd reported that sometimes it causes enormously huge stack growth.

Ah. Sorry for the bogus attribution, then.

Quoted 2 lines

Given that we haven't found any single use-after-scope bug so far, I wouldn't object removing it completely.

Quoted 8 lines

FWIW, I saw a single use-after-scope splat when testing with syzkaller (prior to these patches), but that may have been a result of things going wrong after a stack overflow. Unfortuantely I threw away all of the results of that run.

I'll see if anything triggers overnight with this patch.

Otherwise, I'm also happy for use-after-scope checks to be disabled.

I've been trying to get my patch series updated for a while, sorry for taking so long with it.

My latest state still has use-after-scope enabled with a separate CONFIG_KASAN_EXTRA option that is muturally exclusive with CONFIG_KMEMCHECK (the combination of the two is particularly bad), and it increases the default stack warning size limit to 3072 bytes. With the other patches I have for reducing the stack frame sizes, the default 64-bit warning limit can get lowered to 1280 (this took only a few simple patches to catch all the warnings, surprisingly), and with the regular CONFIG_KASAN the limit gets increased a little to 1536.

Arnd