[PATCH 0/2] arm64: avoid KASAN stack overflows

[PATCH 0/2] arm64: avoid KASAN stack overflows

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

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

-- 
1.9.1

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

[Mark Rutland]

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.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/thread_info.h | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 46c3b93..4f42275 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -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__
-- 
1.9.1

[PATCH 2/2] arm64: use larger stacks for KASAN

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

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Alexander Potapenko <glider@google.com>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/thread_info.h | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 4f42275..1dec96e8 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -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)
-- 
1.9.1

[PATCH 0/2] arm64: avoid KASAN stack overflows

[Andrey Ryabinin]

On 06/07/2017 06:35 PM, Mark Rutland wrote:
> 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.  


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

[PATCH 0/2] arm64: avoid KASAN stack overflows

[Mark Rutland]

On Wed, Jun 07, 2017 at 07:12:30PM +0300, Andrey Ryabinin wrote:
> On 06/07/2017 06:35 PM, Mark Rutland wrote:
> > 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.

Ah. Sorry for the bogus attribution, then.

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

[PATCH 0/2] arm64: avoid KASAN stack overflows

[Arnd Bergmann]

On Wed, Jun 7, 2017 at 6:18 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Jun 07, 2017 at 07:12:30PM +0300, Andrey Ryabinin wrote:
>> On 06/07/2017 06:35 PM, Mark Rutland wrote:
>> > 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.
>
> Ah. Sorry for the bogus attribution, then.
>
>> 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.

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