linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm64: Use larger stacks when KASAN is selected
@ 2017-10-03 17:25 Mark Rutland
  2017-10-04 11:31 ` Catalin Marinas
  0 siblings, 1 reply; 4+ messages in thread
From: Mark Rutland @ 2017-10-03 17:25 UTC (permalink / raw)
  To: linux-arm-kernel

AddressSanitizer instrumentation can significantly bloat the stack, and
with GCC 7 this can result in stack overflows at boot time in some
configurations.

We can avoid this by doubling our stack size when KASAN is in use, as is
already done on x86 (and has been since KASAN was introduced).
Regardless of other patches to decrease KASAN's stack utilization,
kernels built with KASAN will always require more stack space than those
built without, and we should take this into account.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 arch/arm64/include/asm/memory.h | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

In local testing (with KASAN and other options enabled), I've seen a stack
depth of 23928 bytes while booting to a prompt with an Ubuntu 14.04 filesystem
mounted over NFS. Without KASAN (but with other debug options enabled), this
was 7992 bytes.

I wonder if we want to go for 4x rather than 2x...

Thanks,
Mark.

diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 3585a5e..f7c4d21 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -95,16 +95,19 @@
 #define KERNEL_END        _end
 
 /*
- * The size of the KASAN shadow region. This should be 1/8th of the
- * size of the entire kernel virtual address space.
+ * KASAN requires 1/8th of the kernel virtual address space for the shadow
+ * region. KASAN can bloat the stack significantly, so double the (minimum)
+ * stack size when KASAN is in use.
  */
 #ifdef CONFIG_KASAN
 #define KASAN_SHADOW_SIZE	(UL(1) << (VA_BITS - 3))
+#define KASAN_THREAD_SHIFT	1
 #else
 #define KASAN_SHADOW_SIZE	(0)
+#define KASAN_THREAD_SHIFT	0
 #endif
 
-#define MIN_THREAD_SHIFT	14
+#define MIN_THREAD_SHIFT	(14 + KASAN_THREAD_SHIFT)
 
 /*
  * VMAP'd stacks are allocated@page granularity, so we must ensure that such
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH] arm64: Use larger stacks when KASAN is selected
  2017-10-03 17:25 [PATCH] arm64: Use larger stacks when KASAN is selected Mark Rutland
@ 2017-10-04 11:31 ` Catalin Marinas
  2017-10-04 11:39   ` Mark Rutland
  0 siblings, 1 reply; 4+ messages in thread
From: Catalin Marinas @ 2017-10-04 11:31 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Oct 03, 2017 at 06:25:46PM +0100, Mark Rutland wrote:
> AddressSanitizer instrumentation can significantly bloat the stack, and
> with GCC 7 this can result in stack overflows at boot time in some
> configurations.
> 
> We can avoid this by doubling our stack size when KASAN is in use, as is
> already done on x86 (and has been since KASAN was introduced).
> Regardless of other patches to decrease KASAN's stack utilization,
> kernels built with KASAN will always require more stack space than those
> built without, and we should take this into account.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  arch/arm64/include/asm/memory.h | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> In local testing (with KASAN and other options enabled), I've seen a stack
> depth of 23928 bytes while booting to a prompt with an Ubuntu 14.04 filesystem
> mounted over NFS. Without KASAN (but with other debug options enabled), this
> was 7992 bytes.
> 
> I wonder if we want to go for 4x rather than 2x...

Let's hope it's fine for now with 32K stacks. We may run into allocation
issues with 64K stacks (and without VMAP_STACKS).

Do you want this patch for 4.14?

-- 
Catalin

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] arm64: Use larger stacks when KASAN is selected
  2017-10-04 11:31 ` Catalin Marinas
@ 2017-10-04 11:39   ` Mark Rutland
  2017-10-05 15:11     ` Catalin Marinas
  0 siblings, 1 reply; 4+ messages in thread
From: Mark Rutland @ 2017-10-04 11:39 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Oct 04, 2017 at 12:31:31PM +0100, Catalin Marinas wrote:
> On Tue, Oct 03, 2017 at 06:25:46PM +0100, Mark Rutland wrote:
> > AddressSanitizer instrumentation can significantly bloat the stack, and
> > with GCC 7 this can result in stack overflows at boot time in some
> > configurations.
> > 
> > We can avoid this by doubling our stack size when KASAN is in use, as is
> > already done on x86 (and has been since KASAN was introduced).
> > Regardless of other patches to decrease KASAN's stack utilization,
> > kernels built with KASAN will always require more stack space than those
> > built without, and we should take this into account.
> > 
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> > ---
> >  arch/arm64/include/asm/memory.h | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > In local testing (with KASAN and other options enabled), I've seen a stack
> > depth of 23928 bytes while booting to a prompt with an Ubuntu 14.04 filesystem
> > mounted over NFS. Without KASAN (but with other debug options enabled), this
> > was 7992 bytes.
> > 
> > I wonder if we want to go for 4x rather than 2x...
> 
> Let's hope it's fine for now with 32K stacks. We may run into allocation
> issues with 64K stacks (and without VMAP_STACKS).
> 
> Do you want this patch for 4.14?

If you're happy to take it, that would be great, but otherwise it can
wait for v4.15.

Arguably this fixes the introduction of KASAN to arm64, but I'm not sure
how much we care about backporting this to stable.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] arm64: Use larger stacks when KASAN is selected
  2017-10-04 11:39   ` Mark Rutland
@ 2017-10-05 15:11     ` Catalin Marinas
  0 siblings, 0 replies; 4+ messages in thread
From: Catalin Marinas @ 2017-10-05 15:11 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Oct 04, 2017 at 12:39:09PM +0100, Mark Rutland wrote:
> On Wed, Oct 04, 2017 at 12:31:31PM +0100, Catalin Marinas wrote:
> > On Tue, Oct 03, 2017 at 06:25:46PM +0100, Mark Rutland wrote:
> > > AddressSanitizer instrumentation can significantly bloat the stack, and
> > > with GCC 7 this can result in stack overflows at boot time in some
> > > configurations.
> > > 
> > > We can avoid this by doubling our stack size when KASAN is in use, as is
> > > already done on x86 (and has been since KASAN was introduced).
> > > Regardless of other patches to decrease KASAN's stack utilization,
> > > kernels built with KASAN will always require more stack space than those
> > > built without, and we should take this into account.
> > > 
> > > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > Cc: Will Deacon <will.deacon@arm.com>
> > > Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> > > ---
> > >  arch/arm64/include/asm/memory.h | 9 ++++++---
> > >  1 file changed, 6 insertions(+), 3 deletions(-)
> > > 
> > > In local testing (with KASAN and other options enabled), I've seen a stack
> > > depth of 23928 bytes while booting to a prompt with an Ubuntu 14.04 filesystem
> > > mounted over NFS. Without KASAN (but with other debug options enabled), this
> > > was 7992 bytes.
> > > 
> > > I wonder if we want to go for 4x rather than 2x...
> > 
> > Let's hope it's fine for now with 32K stacks. We may run into allocation
> > issues with 64K stacks (and without VMAP_STACKS).
> > 
> > Do you want this patch for 4.14?
> 
> If you're happy to take it, that would be great, but otherwise it can
> wait for v4.15.
> 
> Arguably this fixes the introduction of KASAN to arm64, but I'm not sure
> how much we care about backporting this to stable.

I queued it for 4.14. If anyone thinks it's really important for older
kernels, we can send it to stable later (I don't think it is though).

-- 
Catalin

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-10-05 15:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-03 17:25 [PATCH] arm64: Use larger stacks when KASAN is selected Mark Rutland
2017-10-04 11:31 ` Catalin Marinas
2017-10-04 11:39   ` Mark Rutland
2017-10-05 15:11     ` Catalin Marinas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).