Introduce the STACKLEAK feature and a test for it

This is the 14th version of the patch series introducing STACKLEAK to the mainline kernel for x86. This version comes with style changes according to the feedback from Ingo Molnar and includes Laura Abbott's modifications of scripts/Makefile.gcc-plugins.

Quoted 5 lines · Alexander Popov

This is the 14th version of the patch series introducing STACKLEAK to the mainline kernel for x86. This version comes with style changes according to the feedback from Ingo Molnar and includes Laura Abbott's modifications of scripts/Makefile.gcc-plugins.

As before, I will repeat that I think 1% on a kernel compile (which has almost no actual kernel footprint) is a huge cost, and other - more kernel-centric - loads will likely see much worse performance.

And I still think that this is stupid, and peoples time would be *much* better spent on plugins that have much lower costs and get pretty much the same advanytages (ie the whole "just initialize all automatics to zero" thing or similar).

But whatever. I'll just continue to tell people not to use silly config options that have a high cost-to-point ratio. If people *want* to waste their CPU, that's their problem.

Quoted 11 lines · Linus Torvalds

Quoted 5 lines · Alexander Popov

This is the 14th version of the patch series introducing STACKLEAK to the mainline kernel for x86. This version comes with style changes according to the feedback from Ingo Molnar and includes Laura Abbott's modifications of scripts/Makefile.gcc-plugins.

As before, I will repeat that I think 1% on a kernel compile (which has almost no actual kernel footprint) is a huge cost, and other - more kernel-centric - loads will likely see much worse performance.

Quoted 8 lines

And I still think that this is stupid, and peoples time would be *much* better spent on plugins that have much lower costs and get pretty much the same advanytages (ie the whole "just initialize all automatics to zero" thing or similar).

But whatever. I'll just continue to tell people not to use silly config options that have a high cost-to-point ratio. If people *want* to waste their CPU, that's their problem.

So I agree that it's pretty stupid. Since 95%+ of our users just use what the distro configured - so at minimum I'd like to see a runtime patching based way for root to disable it, so that people (who care or who are hurt by it) can turn the check off - which should be the most expensive part causing much of the 1% overhead.

Shouldn't be too hard to implement, this is just a checking method called for every system call that does nothing substantial in 99.99999% of the cases.

Quoted 6 lines

So I agree that it's pretty stupid. Since 95%+ of our users just use what the distro configured - so at minimum I'd like to see a runtime patching based way for root to disable it, so that people (who care or who are hurt by it) can turn the check off - which should be the most expensive part causing much of the 1% overhead.

Quoted 2 lines

Shouldn't be too hard to implement, this is just a checking method called for every system call that does nothing substantial in 99.99999% of the cases.

I'd like to not have this be required for initial inclusion.

[...] Most people interested in this feature build their own kernels.

[...] Improving on this to make it available to all distros will be excellent, and I think we can do it with static keys or alternatives (?). I'd prefer this feature request not block the merge for 4.19, though. Would that be reasonable?

Let's make sure informed users have an easy runtime way out from the worst of the overhead that doesn't involve "recompile your distro kernel". Also, make it easier to measure and fingerpoint the overhead...

Quoted 3 lines · Ingo Molnar

Let's make sure informed users have an easy runtime way out from the worst of the overhead that doesn't involve "recompile your distro kernel". Also, make it easier to measure and fingerpoint the overhead...

Would you like the following solution?

I'll create the CONFIG_STACKLEAK_RUNTIME_DISABLE config option, which would be similar to CONFIG_SECURITY_SELINUX_DISABLE. That option will provide a sysctl switch disabling stackleak_erase(), which provides the most of performance penalty.

Quoted 10 lines · Alexander Popov

Quoted 3 lines · Ingo Molnar

Let's make sure informed users have an easy runtime way out from the worst of the overhead that doesn't involve "recompile your distro kernel". Also, make it easier to measure and fingerpoint the overhead...

Would you like the following solution?

I'll create the CONFIG_STACKLEAK_RUNTIME_DISABLE config option, which would be similar to CONFIG_SECURITY_SELINUX_DISABLE. That option will provide a sysctl switch disabling stackleak_erase(), which provides the most of performance penalty.

I don't think CONFIG/sysctl is the way to go. I'd recommend making it a boot arg and using a static key, similar to what's happening to hardened_usercopy:

https://patchwork.kernel.org/patch/10505059/

Quoted 7 lines · Alexander Popov

Quoted 3 lines · Ingo Molnar

Let's make sure informed users have an easy runtime way out from the worst of the overhead that doesn't involve "recompile your distro kernel". Also, make it easier to measure and fingerpoint the overhead...

Would you like the following solution?

Quoted 3 lines

I'll create the CONFIG_STACKLEAK_RUNTIME_DISABLE config option, which would be similar to CONFIG_SECURITY_SELINUX_DISABLE. That option will provide a sysctl switch disabling stackleak_erase(), which provides the most of performance penalty.

I don't think CONFIG/sysctl is the way to go. I'd recommend making it a boot arg and using a static key, similar to what's happening to hardened_usercopy:

Quoted 12 lines · Kees Cook

Quoted 10 lines · Alexander Popov

Quoted 3 lines · Ingo Molnar

Let's make sure informed users have an easy runtime way out from the worst of the overhead that doesn't involve "recompile your distro kernel". Also, make it easier to measure and fingerpoint the overhead...

Would you like the following solution?

I'll create the CONFIG_STACKLEAK_RUNTIME_DISABLE config option, which would be similar to CONFIG_SECURITY_SELINUX_DISABLE. That option will provide a sysctl switch disabling stackleak_erase(), which provides the most of performance penalty.

Quoted 3 lines

I don't think CONFIG/sysctl is the way to go. I'd recommend making it a boot arg and using a static key, similar to what's happening to hardened_usercopy:

Why no sysctl? It's a PITA to reboot systems just to turn a stupid knob off.

Also, it's _much_ easier to measure performance impact when there's a sysctl.

Quoted 21 lines · Ingo Molnar

Quoted 8 lines · Kees Cook

Quoted 7 lines · Alexander Popov

Quoted 3 lines · Ingo Molnar

Let's make sure informed users have an easy runtime way out from the worst of the overhead that doesn't involve "recompile your distro kernel". Also, make it easier to measure and fingerpoint the overhead...

Would you like the following solution?

Quoted 7 lines

Quoted 3 lines

I'll create the CONFIG_STACKLEAK_RUNTIME_DISABLE config option, which would be similar to CONFIG_SECURITY_SELINUX_DISABLE. That option will provide a sysctl switch disabling stackleak_erase(), which provides the most of performance penalty.

I don't think CONFIG/sysctl is the way to go. I'd recommend making it a boot arg and using a static key, similar to what's happening to hardened_usercopy:

Why no sysctl? It's a PITA to reboot systems just to turn a stupid knob off.

Quoted 1 line

Also, it's _much_ easier to measure performance impact when there's a sysctl.

Yes, you are right.

But I looked carefully and now see the troubles which sysctl would bring us. Each 'task_struct' has 'lowest_stack', which must be initialized before enabling STACKLEAK. So runtime enabling via sysctl is not plain and may bring race conditions.

On the other hand, a boot param + static key that can only disable STACKLEAK during __init are much more robust. I'm preparing the patch and performance measurements.

Quoted 26 lines · Alexander Popov

Quoted 15 lines · Ingo Molnar

Quoted 12 lines · Kees Cook

Quoted 10 lines · Alexander Popov

Quoted 3 lines · Ingo Molnar

Let's make sure informed users have an easy runtime way out from the worst of the overhead that doesn't involve "recompile your distro kernel". Also, make it easier to measure and fingerpoint the overhead...

Would you like the following solution?

I'll create the CONFIG_STACKLEAK_RUNTIME_DISABLE config option, which would be similar to CONFIG_SECURITY_SELINUX_DISABLE. That option will provide a sysctl switch disabling stackleak_erase(), which provides the most of performance penalty.

Quoted 7 lines

Quoted 3 lines

I don't think CONFIG/sysctl is the way to go. I'd recommend making it a boot arg and using a static key, similar to what's happening to hardened_usercopy:

Why no sysctl? It's a PITA to reboot systems just to turn a stupid knob off.

Also, it's _much_ easier to measure performance impact when there's a sysctl.

Yes, you are right.

Quoted 4 lines

But I looked carefully and now see the troubles which sysctl would bring us. Each 'task_struct' has 'lowest_stack', which must be initialized before enabling STACKLEAK. So runtime enabling via sysctl is not plain and may bring race conditions.

Firstly, a sysctl could still allow it to be *disabled*, once - which is the most important usecase of the sysctl anyway.

Secondly, in the first iteration this could be kept included unconditionally:

	current->lowest_stack = current_top_of_stack() - THREAD_SIZE/64;

... which would keep it initialized and wouldn't be racy, right?

I.e. only the most expensive part of the function, the scanning, would be turned off via the sysctl. I submit that this will avoid all measurable aspects of the 1% kbuild performance overhead.

A more involved approach can be done in the future if warranted, and the feature could be disabled/enabled more thoroughly - but the runtime sysctl would be acceptable for me for now, as a first iteration.

Quoted 1 line

... which would keep it initialized and wouldn't be racy, right?

No, this will break the poison search logic.

Quoted 3 lines

I.e. only the most expensive part of the function, the scanning, would be turned off via the sysctl. I submit that this will avoid all measurable aspects of the 1% kbuild performance overhead.

Yes, I've made an experiment - skipping stackleak_erase() cuts almost all performance penalty of the feature.

Quoted 3 lines

A more involved approach can be done in the future if warranted, and the feature could be disabled/enabled more thoroughly - but the runtime sysctl would be acceptable for me for now, as a first iteration.

Thanks, I see your point.
I'll return with an additional patch introducing sysctl knob + static key for one-time disabling of stack erasing.

Quoted 3 lines · Alexander Popov

Thanks, I see your point.
I'll return with an additional patch introducing sysctl knob + static key for one-time disabling of stack erasing.

Thank you!

I'll have a final review of the patch but I guess it will be fine, so if that's the only change then you can also include my:

Acked-by Ingo Molnar <[email protected]>

... to not hold the patch up for v4.19, and this can go upstream via Kees's tree?

Quoted 11 lines · Ingo Molnar

Quoted 3 lines · Alexander Popov

Thanks, I see your point.
I'll return with an additional patch introducing sysctl knob + static key for one-time disabling of stack erasing.

Thank you!

I'll have a final review of the patch but I guess it will be fine, so if that's the only change then you can also include my:

Quoted 3 lines

Acked-by Ingo Molnar <[email protected]>

... to not hold the patch up for v4.19, and this can go upstream via Kees's tree?

Thanks for your reviews, Ingo! Is this Ack for the entire series or a specific subset?

Documentation/sysctl/kernel.txt detect a hard lockup condition. · line 974

Quoted 12 lines · Alexander Popov

Introduce CONFIG_STACKLEAK_RUNTIME_DISABLE option, which provides 'stack_erasing_bypass' sysctl. It can be used in runtime to disable kernel stack erasing for kernels built with CONFIG_GCC_PLUGIN_STACKLEAK. Stack erasing will then remain disabled and STACKLEAK_METRICS will not be updated until the next boot.

Signed-off-by Alexander Popov <[email protected]>

[...]

Documentation/sysctl/kernel.txt +4 -0

+That erasing reduces the information which kernel stack leak bugs
+can reveal and blocks some uninitialized stack variable attacks.
+The tradeoff is the performance impact: on a single CPU system kernel
+compilation sees a 1% slowdown, other systems and workloads may vary.

I continue to have a hard time measuring even the 1% impact. Clearly I need some better workloads. :)

kernel/stackleak.c asmlinkage void stackleak_erase(void) · line 58

Quoted 12 lines

[...]
asmlinkage void stackleak_erase(void)

 {
        /* It would be nice not to have 'kstack_ptr' and 'boundary' on stack */

kernel/stackleak.c +4 -0

@@ -22,6 +52,11 @@ asmlinkage void stackleak_erase(void)
        unsigned int poison_count = 0;
        const unsigned int depth = STACKLEAK_SEARCH_DEPTH / sizeof(unsigned long);
 
+#ifdef CONFIG_STACKLEAK_RUNTIME_DISABLE
+       if (static_branch_unlikely(&stack_erasing_bypass))
+               return;
+#endif

I collapsed this into a macro (and took your other fix) and will push this to my -next tree:

Quoted 8 lines

kernel/stackleak.c +1 -0

+
        /* Search for the poison value in the kernel stack */
        while (kstack_ptr > boundary && poison_count <= depth) {
                if (*(unsigned long *)kstack_ptr == STACKLEAK_POISON)

@@ -78,6 +113,11 @@ void __used stackleak_track_stack(void)
         */
        unsigned long sp = (unsigned long)&sp;
 

kernel/stackleak.c asmlinkage void stackleak_erase(void) · line 58

Quoted 4 lines

kernel/stackleak.c +4 -0

+#ifdef CONFIG_STACKLEAK_RUNTIME_DISABLE
+       if (static_branch_unlikely(&stack_erasing_bypass))
+               return;
+#endif

I would expect stackleak_erase() to be the expensive part, not the tracking part? Shouldn't timings be unchanged by leaving this in unconditionally, which would mean the sysctl could be re-enabled?

Quoted 33 lines · Kees Cook

Documentation/sysctl/kernel.txt detect a hard lockup condition. · line 974

Quoted 12 lines · Alexander Popov

Introduce CONFIG_STACKLEAK_RUNTIME_DISABLE option, which provides 'stack_erasing_bypass' sysctl. It can be used in runtime to disable kernel stack erasing for kernels built with CONFIG_GCC_PLUGIN_STACKLEAK. Stack erasing will then remain disabled and STACKLEAK_METRICS will not be updated until the next boot.

Signed-off-by Alexander Popov <[email protected]>

[...]

Documentation/sysctl/kernel.txt +4 -0

+That erasing reduces the information which kernel stack leak bugs
+can reveal and blocks some uninitialized stack variable attacks.
+The tradeoff is the performance impact: on a single CPU system kernel
+compilation sees a 1% slowdown, other systems and workloads may vary.

I continue to have a hard time measuring even the 1% impact. Clearly I need some better workloads. :)

kernel/stackleak.c asmlinkage void stackleak_erase(void) · line 58

Quoted 12 lines

[...]
asmlinkage void stackleak_erase(void)

 {
        /* It would be nice not to have 'kstack_ptr' and 'boundary' on stack */

kernel/stackleak.c +4 -0

@@ -22,6 +52,11 @@ asmlinkage void stackleak_erase(void)
        unsigned int poison_count = 0;
        const unsigned int depth = STACKLEAK_SEARCH_DEPTH / sizeof(unsigned long);
 
+#ifdef CONFIG_STACKLEAK_RUNTIME_DISABLE
+       if (static_branch_unlikely(&stack_erasing_bypass))
+               return;
+#endif

I collapsed this into a macro (and took your other fix) and will push this to my -next tree:

Review

Quoted 8 lines

(quoted diff) +7 -0

+#define skip_erasing() static_branch_unlikely(&stack_erasing_bypass)
+#else
+#define skip_erasing() false
+#endif /* CONFIG_STACKLEAK_RUNTIME_DISABLE */
...
+       if (skip_erasing())
+               return;
+

That's nice! Thank you, I'll test it tomorrow.

Quoted 8 lines

Quoted 8 lines

kernel/stackleak.c +1 -0

+
        /* Search for the poison value in the kernel stack */
        while (kstack_ptr > boundary && poison_count <= depth) {
                if (*(unsigned long *)kstack_ptr == STACKLEAK_POISON)

@@ -78,6 +113,11 @@ void __used stackleak_track_stack(void)
         */
        unsigned long sp = (unsigned long)&sp;
 

Quoted 8 lines

kernel/stackleak.c asmlinkage void stackleak_erase(void) · line 58

Quoted 4 lines

kernel/stackleak.c +4 -0

+#ifdef CONFIG_STACKLEAK_RUNTIME_DISABLE
+       if (static_branch_unlikely(&stack_erasing_bypass))
+               return;
+#endif

I would expect stackleak_erase() to be the expensive part, not the tracking part? Shouldn't timings be unchanged by leaving this in unconditionally, which would mean the sysctl could be re-enabled?

Dropping the bypass in stackleak_track_stack() will not help against the troubles with re-enabling stack erasing (tracking and erasing depend on each

other). Moreover, it will also make the STACKLEAK_METRICS show insane values. So I think we should have the bypass in both functions.

Quoted 15 lines · Alexander Popov

Quoted 5 lines · Kees Cook

Quoted 4 lines · Alexander Popov

kernel/stackleak.c +0 -0

@@ -78,6 +113,11 @@ void __used stackleak_track_stack(void)
         */
        unsigned long sp = (unsigned long)&sp;
 

Quoted 8 lines

kernel/stackleak.c asmlinkage void stackleak_erase(void) · line 58

Quoted 4 lines

kernel/stackleak.c +4 -0

+#ifdef CONFIG_STACKLEAK_RUNTIME_DISABLE
+       if (static_branch_unlikely(&stack_erasing_bypass))
+               return;
+#endif

I would expect stackleak_erase() to be the expensive part, not the tracking part? Shouldn't timings be unchanged by leaving this in unconditionally, which would mean the sysctl could be re-enabled?

Quoted 2 lines

Dropping the bypass in stackleak_track_stack() will not help against the troubles with re-enabling stack erasing (tracking and erasing depend on each

Isn't the tracking checking "sp < current->lowest_stack", so if erasure was off, lowest_stack would only ever get further into the stack? And when erasure was turned back on, it would start getting reset correctly again. Or is the concern the poison searching could break? It seems like it would still work right? I must be missing something. :)

Introduce CONFIG_STACKLEAK_RUNTIME_DISABLE option, which provides 'stack_erasing' sysctl. It can be used in runtime to control kernel stack erasing for kernels built with CONFIG_GCC_PLUGIN_STACKLEAK.

Suggested-by Ingo Molnar <[email protected]>

Signed-off-by Alexander Popov <[email protected]>

---
 Documentation/sysctl/kernel.txt | 18 ++++++++++++++++++
 include/linux/stackleak.h       |  6 ++++++
 kernel/stackleak.c              | 38 ++++++++++++++++++++++++++++++++++++++
 kernel/sysctl.c                 | 15 ++++++++++++++-
 scripts/gcc-plugins/Kconfig     |  8 ++++++++
 5 files changed, 84 insertions(+), 1 deletion(-)

Documentation/sysctl/kernel.txt +0 -0

index eded671d..1feae79 100644

Documentation/sysctl/kernel.txt +17 -0

 - stop-a                      [ SPARC only ]
 - sysrq                       ==> Documentation/admin-guide/sysrq.rst
 - sysctl_writes_strict

@@ -962,6 +963,23 @@ detect a hard lockup condition.
 
 ==============================================================
 
+stack_erasing
+
+This parameter can be used to control kernel stack erasing at the end
+of syscalls for kernels built with CONFIG_GCC_PLUGIN_STACKLEAK.
+
+That erasing reduces the information which kernel stack leak bugs
+can reveal and blocks some uninitialized stack variable attacks.
+The tradeoff is the performance impact: on a single CPU system kernel
+compilation sees a 1% slowdown, other systems and workloads may vary.
+
+  0: kernel stack erasing is disabled, STACKLEAK_METRICS are not updated.
+
+  1: kernel stack erasing is enabled (default), it is performed before
+     returning to the userspace at the end of syscalls.
+
+==============================================================
+
 tainted:
 
 Non-zero if the kernel has been tainted. Numeric values, which can be

include/linux/stackleak.h +6 -0

index b911b97..3d5c327 100644
@@ -22,6 +22,12 @@ static inline void stackleak_task_init(struct task_struct *t)
        t->prev_lowest_stack = t->lowest_stack;
 # endif
 }
+
+#ifdef CONFIG_STACKLEAK_RUNTIME_DISABLE
+int stack_erasing_sysctl(struct ctl_table *table, int write,
+                       void __user *buffer, size_t *lenp, loff_t *ppos);
+#endif
+
 #else /* !CONFIG_GCC_PLUGIN_STACKLEAK */
 static inline void stackleak_task_init(struct task_struct *t) { }
 #endif

kernel/stackleak.c +17 -0

index f5c4111..2d21372 100644
@@ -14,6 +14,41 @@
 
 #include <linux/stackleak.h>
 
+#ifdef CONFIG_STACKLEAK_RUNTIME_DISABLE
+#include <linux/jump_label.h>
+#include <linux/sysctl.h>
+
+static DEFINE_STATIC_KEY_FALSE(stack_erasing_bypass);
+
+int stack_erasing_sysctl(struct ctl_table *table, int write,
+                       void __user *buffer, size_t *lenp, loff_t *ppos)
+{
+       int ret = 0;
+       int state = !static_branch_unlikely(&stack_erasing_bypass);
+       int prev_state = state;
+
+       table->data = &state;
+       table->maxlen = sizeof(int);
+       ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
+       state = !!state;

kernel/stackleak.c +8 -0

+       return ret;
+}
+
+#define skip_erasing() static_branch_unlikely(&stack_erasing_bypass)
+#else
+#define skip_erasing() false
+#endif /* CONFIG_STACKLEAK_RUNTIME_DISABLE */
+
 asmlinkage void stackleak_erase(void)
 {
        /* It would be nice not to have 'kstack_ptr' and 'boundary' on stack */

In preparation for enabling the stackleak plugin on arm64, we need a way to get the bounds of the current stack. Introduce a new primitive current_stack_type which is similar to x86's get_stack_info. Utilize that to rework on_accessible_stack slightly as well.

Signed-off-by Laura Abbott <[email protected]>

---
So this did end up looking quite a bit like get_stack_info but I didn't really see the need to integrate this more than this. I do think actually enumerating the types makes things a bit cleaner.

---
 arch/arm64/include/asm/sdei.h       |  8 ++-
 arch/arm64/include/asm/stacktrace.h | 94 ++++++++++++++++++++++++-----
 arch/arm64/kernel/ptrace.c          |  2 +-
 arch/arm64/kernel/sdei.c            | 21 ++++++-
 4 files changed, 103 insertions(+), 22 deletions(-)

arch/arm64/include/asm/sdei.h +5 -3

index e073e6886685..34f7b203845b 100644
@@ -40,15 +40,17 @@ asmlinkage unsigned long __sdei_handler(struct pt_regs *regs,
 unsigned long sdei_arch_get_entry_point(int conduit);
 #define sdei_arch_get_entry_point(x)	sdei_arch_get_entry_point(x)
 
-bool _on_sdei_stack(unsigned long sp);
-static inline bool on_sdei_stack(unsigned long sp)
+bool _on_sdei_stack(unsigned long sp, unsigned long *, unsigned long *);
+static inline bool on_sdei_stack(unsigned long sp,
+				unsigned long *stack_low,
+				unsigned long *stack_high)
 {
 	if (!IS_ENABLED(CONFIG_VMAP_STACK))
 		return false;
 	if (!IS_ENABLED(CONFIG_ARM_SDE_INTERFACE))
 		return false;
 	if (in_nmi())
-		return _on_sdei_stack(sp);
+		return _on_sdei_stack(sp, stack_low, stack_high);
 
 	return false;
 }

arch/arm64/include/asm/stacktrace.h +3 -2

index 902f9edacbea..9855a0425e64 100644
@@ -39,7 +39,9 @@ extern void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk);
 
 DECLARE_PER_CPU(unsigned long *, irq_stack_ptr);
 
-static inline bool on_irq_stack(unsigned long sp)
+static inline bool on_irq_stack(unsigned long sp,
+				unsigned long *stack_low,
+				unsigned long *stack_high)
 {
 	unsigned long low = (unsigned long)raw_cpu_read(irq_stack_ptr);
 	unsigned long high = low + IRQ_STACK_SIZE;

@@ -47,47 +49,109 @@ static inline bool on_irq_stack(unsigned long sp)
 	if (!low)
 		return false;
 
-	return (low <= sp && sp < high);

arch/arm64/include/asm/stacktrace.h +26 -5

-static inline bool on_task_stack(struct task_struct *tsk, unsigned long sp)
+static inline bool on_task_stack(struct task_struct *tsk, unsigned long sp,
+				unsigned long *stack_low,
+				unsigned long *stack_high)
 {
 	unsigned long low = (unsigned long)task_stack_page(tsk);
 	unsigned long high = low + THREAD_SIZE;
 
-	return (low <= sp && sp < high);
+	if (sp < low || sp >= high)
+		return false;
+
+	if (stack_low && stack_high) {
+		*stack_low = low;
+		*stack_high = high;
+	}
+
+	return true;
 }
 
 #ifdef CONFIG_VMAP_STACK
 DECLARE_PER_CPU(unsigned long [OVERFLOW_STACK_SIZE/sizeof(long)], overflow_stack);
 
-static inline bool on_overflow_stack(unsigned long sp)
+static inline bool on_overflow_stack(unsigned long sp,
+				unsigned long *stack_low,
+				unsigned long *stack_high)
 {
 	unsigned long low = (unsigned long)raw_cpu_ptr(overflow_stack);
 	unsigned long high = low + OVERFLOW_STACK_SIZE;
 
-	return (low <= sp && sp < high);
+	if (sp < low || sp >= high)
+		return false;
+
+	if (stack_low && stack_high) {
+		*stack_low = low;
+		*stack_high = high;
+	}
+
+	return true;
 }
 #else
-static inline bool on_overflow_stack(unsigned long sp) { return false; }
+static inline bool on_overflow_stack(unsigned long sp,
+			unsigned long *stack_low,

arch/arm64/include/asm/stacktrace.h +32 -2

+
+static inline enum stack_type current_stack_type(struct task_struct *tsk,
+						unsigned long sp,
+						unsigned long *stack_low,
+						unsigned long *stack_high)
+{
+	if (on_task_stack(tsk, sp, stack_low, stack_high))
+		return STACK_TYPE_TASK;
+	if (on_irq_stack(sp, stack_low, stack_high))
+		return STACK_TYPE_IRQ;
+	if (on_overflow_stack(sp, stack_low, stack_high))
+		return STACK_TYPE_OVERFLOW;
+	if (on_sdei_stack(sp, stack_low, stack_high))
+		return STACK_TYPE_SDEI;
+	return STACK_TYPE_UNKNOWN;
+}
+
 /*
  * We can only safely access per-cpu stacks from current in a non-preemptible
  * context.
  */
 static inline bool on_accessible_stack(struct task_struct *tsk, unsigned long sp)
 {
-	if (on_task_stack(tsk, sp))
+	enum stack_type type;
+	unsigned long low, high;
+
+	type = current_stack_type(tsk, sp, &low, &high);
+
+	switch (type) {
+	case STACK_TYPE_TASK:
 		return true;
-	if (tsk != current || preemptible())
+	case STACK_TYPE_IRQ:
+	case STACK_TYPE_OVERFLOW:
+	case STACK_TYPE_SDEI:
+		if (tsk != current || preemptible())
+			return false;
+		else
+			return true;
+	case STACK_TYPE_UNKNOWN:
 		return false;

arch/arm64/kernel/ptrace.c +1 -1

index 5c338ce5a7fa..a6b3a2be7561 100644
@@ -132,7 +132,7 @@ static bool regs_within_kernel_stack(struct pt_regs *regs, unsigned long addr)
 {
 	return ((addr & ~(THREAD_SIZE - 1))  ==
 		(kernel_stack_pointer(regs) & ~(THREAD_SIZE - 1))) ||
-		on_irq_stack(addr);
+		on_irq_stack(addr, NULL, NULL);
 }
 
 /**

arch/arm64/kernel/sdei.c +9 -3

index 6b8d90d5ceae..8e18913a53fd 100644
@@ -88,7 +88,9 @@ static int init_sdei_stacks(void)
 	return err;
 }
 
-bool _on_sdei_stack(unsigned long sp)
+bool _on_sdei_stack(unsigned long sp,
+		unsigned long *stack_low,
+		unsigned long *stack_high)
 {
 	unsigned long low, high;
 

@@ -98,13 +100,26 @@ bool _on_sdei_stack(unsigned long sp)
 	low = (unsigned long)raw_cpu_read(sdei_stack_critical_ptr);
 	high = low + SDEI_STACK_SIZE;
 
-	if (low <= sp && sp < high)
+	if (low <= sp && sp < high) {
+		if (stack_low && stack_high) {
+			*stack_low = low;
+			*stack_high = high;
+		}
 		return true;
+	}
 
 	low = (unsigned long)raw_cpu_read(sdei_stack_normal_ptr);
 	high = low + SDEI_STACK_SIZE;
 
-	return (low <= sp && sp < high);

Implementation of stackleak based heavily on the x86 version

Signed-off-by Laura Abbott <[email protected]>

---
Since last time: Minor style cleanups. Re-wrote check_alloca to correctly handle all stack types. While doing that, I also realized current_top_of_stack was incorrect so I fixed that as well.

---
 arch/arm64/Kconfig                    |  1 +
 arch/arm64/include/asm/processor.h    | 17 ++++++++++++++
 arch/arm64/kernel/entry.S             |  7 ++++++
 arch/arm64/kernel/process.c           | 32 +++++++++++++++++++++++++++
 arch/arm64/kvm/hyp/Makefile           |  3 ++-
 drivers/firmware/efi/libstub/Makefile |  3 ++-
 include/linux/stackleak.h             |  1 +
 7 files changed, 62 insertions(+), 2 deletions(-)

arch/arm64/Kconfig +1 -0

index 42c090cf0292..216d36a49ab5 100644
@@ -96,6 +96,7 @@ config ARM64
        select HAVE_ARCH_MMAP_RND_BITS
        select HAVE_ARCH_MMAP_RND_COMPAT_BITS if COMPAT
        select HAVE_ARCH_SECCOMP_FILTER
+       select HAVE_ARCH_STACKLEAK
        select HAVE_ARCH_THREAD_STRUCT_WHITELIST
        select HAVE_ARCH_TRACEHOOK
        select HAVE_ARCH_TRANSPARENT_HUGEPAGE

arch/arm64/include/asm/processor.h +17 -0

index a73ae1e49200..4f3062ee22c6 100644
@@ -266,5 +266,22 @@ extern void __init minsigstksz_setup(void);
 #define SVE_SET_VL(arg)        sve_set_current_vl(arg)
 #define SVE_GET_VL()   sve_get_current_vl()
 
+/*
+ * For CONFIG_STACKLEAK
+ *
+ * These need to be macros because otherwise we get stuck in a nightmare
+ * of header definitions for the use of task_stack_page.
+ */
+
+#define current_top_of_stack()         \
+({                                     \
+       unsigned long _low = 0;         \
+       unsigned long _high = 0;        \
+                                       \
+       current_stack_type(current, current_stack_pointer, &_low, &_high); \
+       _high;  \
+})
+#define on_thread_stack()      (on_task_stack(current, current_stack_pointer, NULL, NULL))
+
 #endif /* __ASSEMBLY__ */
 #endif /* __ASM_PROCESSOR_H */

arch/arm64/kernel/entry.S +7 -0

index 28ad8799406f..67d12016063d 100644
@@ -431,6 +431,11 @@ tsk        .req    x28             // current thread_info
 
        .text
 
+       .macro  stackleak_erase
+#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
+       bl      stackleak_erase
+#endif
+       .endm
 /*
  * Exception vectors.
  */

@@ -910,6 +915,7 @@ ret_fast_syscall:
        and     x2, x1, #_TIF_WORK_MASK
        cbnz    x2, work_pending
        enable_step_tsk x1, x2
+       stackleak_erase
        kernel_exit 0
 ret_fast_syscall_trace:
        enable_daif

@@ -936,6 +942,7 @@ ret_to_user:
        cbnz    x2, work_pending
 finish_ret_to_user:
        enable_step_tsk x1, x2
+       stackleak_erase
        kernel_exit 0
 ENDPROC(ret_to_user)
 

arch/arm64/kernel/process.c +32 -0

index e10bc363f533..904defa36689 100644
@@ -493,3 +493,35 @@ void arch_setup_new_exec(void)
 {
        current->mm->context.flags = is_compat_task() ? MMCF_AARCH32 : 0;
 }
+
+#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
+void __used stackleak_check_alloca(unsigned long size)
+{
+       unsigned long stack_left;
+       enum stack_type type;
+       unsigned long current_sp = current_stack_pointer;
+       unsigned long low, high;
+
+       type = current_stack_type(current, current_sp, &low, &high);
+       BUG_ON(type == STACK_TYPE_UNKNOWN);
+
+       stack_left = current_sp - low;
+
+       if (size >= stack_left) {
+               /*
+                * Kernel stack depth overflow is detected, let's report that.
+                * If CONFIG_VMAP_STACK is enabled, we can safely use BUG().
+                * If CONFIG_VMAP_STACK is disabled, BUG() handling can corrupt
+                * the neighbour memory. CONFIG_SCHED_STACK_END_CHECK calls
+                * panic() in a similar situation, so let's do the same if that
+                * option is on. Otherwise just use BUG() and hope for the best.
+                */
+#if !defined(CONFIG_VMAP_STACK) && defined(CONFIG_SCHED_STACK_END_CHECK)
+               panic("alloca() over the kernel stack boundary\n");
+#else
+               BUG();
+#endif
+       }
+}
+EXPORT_SYMBOL(stackleak_check_alloca);
+#endif

arch/arm64/kvm/hyp/Makefile +2 -1

index 4313f7475333..2fabc2dc1966 100644
@@ -3,7 +3,8 @@
 # Makefile for Kernel-based Virtual Machine module, HYP part
 #
 
-ccflags-y += -fno-stack-protector -DDISABLE_BRANCH_PROFILING
+ccflags-y += -fno-stack-protector -DDISABLE_BRANCH_PROFILING \
+               $(DISABLE_STACKLEAK_PLUGIN)
 
 KVM=../../../../virt/kvm
 

drivers/firmware/efi/libstub/Makefile +2 -1

index a34e9290a699..25dd2a14560d 100644
@@ -20,7 +20,8 @@ cflags-$(CONFIG_EFI_ARMSTUB)  += -I$(srctree)/scripts/dtc/libfdt
 KBUILD_CFLAGS                  := $(cflags-y) -DDISABLE_BRANCH_PROFILING \
                                   -D__NO_FORTIFY \
                                   $(call cc-option,-ffreestanding) \
-                                  $(call cc-option,-fno-stack-protector)
+                                  $(call cc-option,-fno-stack-protector) \
+                                  $(DISABLE_STACKLEAK_PLUGIN)
 
 GCOV_PROFILE                   := n
 KASAN_SANITIZE                 := n

include/linux/stackleak.h +1 -1

index b911b973d328..08420ec6b7c3 100644
@@ -5,6 +5,7 @@
 #include <linux/sched.h>
 #include <linux/sched/task_stack.h>
 
+#include <asm/stacktrace.h>
 /*
  * Check that the poison value points to the unused hole in the
  * virtual memory map for your platform.
--

2.17.1

Implementation of stackleak based heavily on the x86 version

Signed-off-by Laura Abbott <[email protected]>

---
Since last time: Minor style cleanups. Re-wrote check_alloca to correctly handle all stack types. While doing that, I also realized current_top_of_stack was incorrect so I fixed that as well.

---
 arch/arm64/Kconfig                    |  1 +
 arch/arm64/include/asm/processor.h    | 17 ++++++++++++++
 arch/arm64/kernel/entry.S             |  7 ++++++
 arch/arm64/kernel/process.c           | 32 +++++++++++++++++++++++++++
 arch/arm64/kvm/hyp/Makefile           |  3 ++-
 drivers/firmware/efi/libstub/Makefile |  3 ++-
 include/linux/stackleak.h             |  1 +
 7 files changed, 62 insertions(+), 2 deletions(-)

arch/arm64/Kconfig +1 -0

index 42c090cf0292..216d36a49ab5 100644
@@ -96,6 +96,7 @@ config ARM64
 	select HAVE_ARCH_MMAP_RND_BITS
 	select HAVE_ARCH_MMAP_RND_COMPAT_BITS if COMPAT
 	select HAVE_ARCH_SECCOMP_FILTER
+	select HAVE_ARCH_STACKLEAK
 	select HAVE_ARCH_THREAD_STRUCT_WHITELIST
 	select HAVE_ARCH_TRACEHOOK
 	select HAVE_ARCH_TRANSPARENT_HUGEPAGE

arch/arm64/include/asm/processor.h +0 -0

index a73ae1e49200..4f3062ee22c6 100644

arch/arm64/include/asm/processor.h +2 -0

+#define on_thread_stack()	(on_task_stack(current, current_stack_pointer, NULL, NULL))
+
 #endif /* __ASSEMBLY__ */
 #endif /* __ASM_PROCESSOR_H */

arch/arm64/kernel/entry.S +7 -0

index 28ad8799406f..67d12016063d 100644
@@ -431,6 +431,11 @@ tsk	.req	x28		// current thread_info
 
 	.text
 
+	.macro	stackleak_erase
+#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
+	bl	stackleak_erase
+#endif
+	.endm
 /*
  * Exception vectors.
  */

@@ -910,6 +915,7 @@ ret_fast_syscall:
 	and	x2, x1, #_TIF_WORK_MASK
 	cbnz	x2, work_pending
 	enable_step_tsk x1, x2
+	stackleak_erase
 	kernel_exit 0
 ret_fast_syscall_trace:
 	enable_daif

@@ -936,6 +942,7 @@ ret_to_user:
 	cbnz	x2, work_pending
 finish_ret_to_user:
 	enable_step_tsk x1, x2
+	stackleak_erase
 	kernel_exit 0
 ENDPROC(ret_to_user)
 

arch/arm64/kernel/process.c +18 -0

index e10bc363f533..904defa36689 100644
@@ -493,3 +493,35 @@ void arch_setup_new_exec(void)
 {
 	current->mm->context.flags = is_compat_task() ? MMCF_AARCH32 : 0;
 }
+
+#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
+void __used stackleak_check_alloca(unsigned long size)
+{
+	unsigned long stack_left;
+	enum stack_type type;
+	unsigned long current_sp = current_stack_pointer;
+	unsigned long low, high;
+
+	type = current_stack_type(current, current_sp, &low, &high);
+	BUG_ON(type == STACK_TYPE_UNKNOWN);
+
+	stack_left = current_sp - low;
+
+	if (size >= stack_left) {
+		/*
+		 * Kernel stack depth overflow is detected, let's report that.
+		 * If CONFIG_VMAP_STACK is enabled, we can safely use BUG().

arch/arm64/kernel/process.c +4 -0

+	}
+}
+EXPORT_SYMBOL(stackleak_check_alloca);
+#endif

arch/arm64/kvm/hyp/Makefile +2 -1

index 4313f7475333..2fabc2dc1966 100644
@@ -3,7 +3,8 @@
 # Makefile for Kernel-based Virtual Machine module, HYP part
 #
 
-ccflags-y += -fno-stack-protector -DDISABLE_BRANCH_PROFILING
+ccflags-y += -fno-stack-protector -DDISABLE_BRANCH_PROFILING \
+		$(DISABLE_STACKLEAK_PLUGIN)
 
 KVM=../../../../virt/kvm
 

drivers/firmware/efi/libstub/Makefile +2 -1

index a34e9290a699..25dd2a14560d 100644
@@ -20,7 +20,8 @@ cflags-$(CONFIG_EFI_ARMSTUB)	+= -I$(srctree)/scripts/dtc/libfdt
 KBUILD_CFLAGS			:= $(cflags-y) -DDISABLE_BRANCH_PROFILING \
 				   -D__NO_FORTIFY \
 				   $(call cc-option,-ffreestanding) \
-				   $(call cc-option,-fno-stack-protector)
+				   $(call cc-option,-fno-stack-protector) \
+				   $(DISABLE_STACKLEAK_PLUGIN)
 
 GCOV_PROFILE			:= n
 KASAN_SANITIZE			:= n

include/linux/stackleak.h +1 -0

index b911b973d328..08420ec6b7c3 100644
@@ -5,6 +5,7 @@
 #include <linux/sched.h>
 #include <linux/sched/task_stack.h>
 
+#include <asm/stacktrace.h>
 /*
  * Check that the poison value points to the unused hole in the
  * virtual memory map for your platform.

Implementation of stackleak based heavily on the x86 version

Signed-off-by Laura Abbott <[email protected]>

---
Since last time: Minor style cleanups. Re-wrote check_alloca to correctly handle all stack types. While doing that, I also realized current_top_of_stack was incorrect so I fixed that as well.

---
 arch/arm64/Kconfig                    |  1 +
 arch/arm64/include/asm/processor.h    | 17 ++++++++++++++
 arch/arm64/kernel/entry.S             |  7 ++++++
 arch/arm64/kernel/process.c           | 32 +++++++++++++++++++++++++++
 arch/arm64/kvm/hyp/Makefile           |  3 ++-
 drivers/firmware/efi/libstub/Makefile |  3 ++-
 include/linux/stackleak.h             |  1 +
 7 files changed, 62 insertions(+), 2 deletions(-)

arch/arm64/Kconfig +1 -0

index 42c090cf0292..216d36a49ab5 100644
@@ -96,6 +96,7 @@ config ARM64
 	select HAVE_ARCH_MMAP_RND_BITS
 	select HAVE_ARCH_MMAP_RND_COMPAT_BITS if COMPAT
 	select HAVE_ARCH_SECCOMP_FILTER
+	select HAVE_ARCH_STACKLEAK
 	select HAVE_ARCH_THREAD_STRUCT_WHITELIST
 	select HAVE_ARCH_TRACEHOOK
 	select HAVE_ARCH_TRANSPARENT_HUGEPAGE

arch/arm64/include/asm/processor.h +5 -0

index a73ae1e49200..4f3062ee22c6 100644
@@ -266,5 +266,22 @@ extern void __init minsigstksz_setup(void);
 #define SVE_SET_VL(arg)	sve_set_current_vl(arg)
 #define SVE_GET_VL()	sve_get_current_vl()
 
+/*
+ * For CONFIG_STACKLEAK
+ *
+ * These need to be macros because otherwise we get stuck in a nightmare
+ * of header definitions for the use of task_stack_page.

arch/arm64/include/asm/processor.h +1 -0

+
 #endif /* __ASSEMBLY__ */
 #endif /* __ASM_PROCESSOR_H */

arch/arm64/kernel/entry.S +7 -0

index 28ad8799406f..67d12016063d 100644
@@ -431,6 +431,11 @@ tsk	.req	x28		// current thread_info
 
 	.text
 
+	.macro	stackleak_erase
+#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
+	bl	stackleak_erase
+#endif
+	.endm
 /*
  * Exception vectors.
  */

@@ -910,6 +915,7 @@ ret_fast_syscall:
 	and	x2, x1, #_TIF_WORK_MASK
 	cbnz	x2, work_pending
 	enable_step_tsk x1, x2
+	stackleak_erase
 	kernel_exit 0
 ret_fast_syscall_trace:
 	enable_daif

@@ -936,6 +942,7 @@ ret_to_user:
 	cbnz	x2, work_pending
 finish_ret_to_user:
 	enable_step_tsk x1, x2
+	stackleak_erase
 	kernel_exit 0
 ENDPROC(ret_to_user)
 

arch/arm64/kernel/process.c +3 -0

index e10bc363f533..904defa36689 100644
@@ -493,3 +493,35 @@ void arch_setup_new_exec(void)
 {
 	current->mm->context.flags = is_compat_task() ? MMCF_AARCH32 : 0;
 }
+
+#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
+void __used stackleak_check_alloca(unsigned long size)

arch/arm64/kernel/process.c +19 -0

+
+	if (size >= stack_left) {
+		/*
+		 * Kernel stack depth overflow is detected, let's report that.
+		 * If CONFIG_VMAP_STACK is enabled, we can safely use BUG().
+		 * If CONFIG_VMAP_STACK is disabled, BUG() handling can corrupt
+		 * the neighbour memory. CONFIG_SCHED_STACK_END_CHECK calls
+		 * panic() in a similar situation, so let's do the same if that
+		 * option is on. Otherwise just use BUG() and hope for the best.
+		 */
+#if !defined(CONFIG_VMAP_STACK) && defined(CONFIG_SCHED_STACK_END_CHECK)
+		panic("alloca() over the kernel stack boundary\n");
+#else
+		BUG();
+#endif
+	}
+}
+EXPORT_SYMBOL(stackleak_check_alloca);
+#endif

arch/arm64/kvm/hyp/Makefile +2 -1

index 4313f7475333..2fabc2dc1966 100644
@@ -3,7 +3,8 @@
 # Makefile for Kernel-based Virtual Machine module, HYP part
 #
 
-ccflags-y += -fno-stack-protector -DDISABLE_BRANCH_PROFILING
+ccflags-y += -fno-stack-protector -DDISABLE_BRANCH_PROFILING \
+		$(DISABLE_STACKLEAK_PLUGIN)
 
 KVM=../../../../virt/kvm
 

drivers/firmware/efi/libstub/Makefile +2 -1

index a34e9290a699..25dd2a14560d 100644
@@ -20,7 +20,8 @@ cflags-$(CONFIG_EFI_ARMSTUB)	+= -I$(srctree)/scripts/dtc/libfdt
 KBUILD_CFLAGS			:= $(cflags-y) -DDISABLE_BRANCH_PROFILING \
 				   -D__NO_FORTIFY \
 				   $(call cc-option,-ffreestanding) \
-				   $(call cc-option,-fno-stack-protector)
+				   $(call cc-option,-fno-stack-protector) \
+				   $(DISABLE_STACKLEAK_PLUGIN)
 
 GCOV_PROFILE			:= n
 KASAN_SANITIZE			:= n

include/linux/stackleak.h +1 -0

index b911b973d328..08420ec6b7c3 100644
@@ -5,6 +5,7 @@
 #include <linux/sched.h>
 #include <linux/sched/task_stack.h>
 
+#include <asm/stacktrace.h>
 /*
  * Check that the poison value points to the unused hole in the
  * virtual memory map for your platform.

Signature · 1 line

2.17.1

In preparation for enabling the stackleak plugin on arm64, we need a way to get the bounds of the current stack. Extend on_accessible_stack to get this information.

Signed-off-by Laura Abbott <[email protected]>

---
v2: Switched to using struct stack_info for argument passing. on_accessible_stack is now the primary API. Split STACK_TYPE_SDEI into STACK_TYPE_SDEI_NORMAL and STACK_TYPE_SDEI_CRITICAL.

arch/arm64/include/asm/stacktrace.h +12 -2

-static inline bool on_irq_stack(unsigned long sp)
+static inline bool on_irq_stack(unsigned long sp,
+				struct stack_info *info)
 {
 	unsigned long low = (unsigned long)raw_cpu_read(irq_stack_ptr);
 	unsigned long high = low + IRQ_STACK_SIZE;

@@ -47,46 +63,79 @@ static inline bool on_irq_stack(unsigned long sp)
 	if (!low)
 		return false;
 
-	return (low <= sp && sp < high);
+	if (sp < low || sp >= high)
+		return false;
+
+	if (info) {
+		info->low = low;
+		info->high = high;
+		info->type = STACK_TYPE_IRQ;
+	}
+
+	return true;
 }

arch/arm64/kernel/sdei.c +25 -1

-bool _on_sdei_stack(unsigned long sp)
+bool on_sdei_normal_stack(unsigned long sp,
+			struct stack_info *info)
+{
+	unsigned long low = (unsigned long)raw_cpu_read(sdei_stack_normal_ptr);
+	unsigned long high = low + SDEI_STACK_SIZE;
+
+	if (low <= sp && sp < high) {
+		if (info) {
+			info->low = low;
+			info->high = high;
+			info->type = STACK_TYPE_SDEI_NORMAL;
+		}
+		return true;
+	}
+
+	return false;
+}
+
+bool on_sdei_critical_stack(unsigned long sp,
+			struct stack_info *info)
+{
+	unsigned long low = (unsigned long)raw_cpu_read(sdei_stack_critical_ptr);
+	unsigned long high = low + SDEI_STACK_SIZE;
+
+	if (low <= sp && sp < high) {

Implementation of stackleak based heavily on the x86 version

Signed-off-by Laura Abbott <[email protected]>

---
v2: Convert to adjusted on_acessible_stack APIs. Fixed alloca check to just panic. Dropped the extra include per Kees. I also didn't add the Reviewed-by since the APIs did change and I wanted another pass.

---
 arch/arm64/Kconfig                    |  1 +
 arch/arm64/include/asm/processor.h    | 15 +++++++++++++++
 arch/arm64/kernel/entry.S             |  7 +++++++
 arch/arm64/kernel/process.c           | 17 +++++++++++++++++
 arch/arm64/kvm/hyp/Makefile           |  3 ++-
 drivers/firmware/efi/libstub/Makefile |  3 ++-
 6 files changed, 44 insertions(+), 2 deletions(-)

arch/arm64/Kconfig +1 -0

index 42c090cf0292..216d36a49ab5 100644
@@ -96,6 +96,7 @@ config ARM64
        select HAVE_ARCH_MMAP_RND_BITS
        select HAVE_ARCH_MMAP_RND_COMPAT_BITS if COMPAT
        select HAVE_ARCH_SECCOMP_FILTER
+       select HAVE_ARCH_STACKLEAK
        select HAVE_ARCH_THREAD_STRUCT_WHITELIST
        select HAVE_ARCH_TRACEHOOK
        select HAVE_ARCH_TRANSPARENT_HUGEPAGE

arch/arm64/include/asm/processor.h +15 -0

index a73ae1e49200..0061450a793b 100644
@@ -266,5 +266,20 @@ extern void __init minsigstksz_setup(void);
 #define SVE_SET_VL(arg)        sve_set_current_vl(arg)
 #define SVE_GET_VL()   sve_get_current_vl()
 
+/*
+ * For CONFIG_GCC_PLUGIN_STACKLEAK
+ *
+ * These need to be macros because otherwise we get stuck in a nightmare
+ * of header definitions for the use of task_stack_page.
+ */
+
+#define current_top_of_stack()                                                 \
+({                                                                             \
+       struct stack_info _info;                                                \
+       BUG_ON(!on_accessible_stack(current, current_stack_pointer, &_info));   \
+       _info.high;                                                             \
+})
+#define on_thread_stack()      (on_task_stack(current, current_stack_pointer, NULL))
+
 #endif /* __ASSEMBLY__ */
 #endif /* __ASM_PROCESSOR_H */

arch/arm64/kernel/entry.S +7 -0

index 28ad8799406f..67d12016063d 100644
@@ -431,6 +431,11 @@ tsk        .req    x28             // current thread_info
 
        .text
 
+       .macro  stackleak_erase
+#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
+       bl      stackleak_erase
+#endif
+       .endm
 /*
  * Exception vectors.
  */

@@ -910,6 +915,7 @@ ret_fast_syscall:
        and     x2, x1, #_TIF_WORK_MASK
        cbnz    x2, work_pending
        enable_step_tsk x1, x2
+       stackleak_erase
        kernel_exit 0
 ret_fast_syscall_trace:
        enable_daif

@@ -936,6 +942,7 @@ ret_to_user:
        cbnz    x2, work_pending
 finish_ret_to_user:
        enable_step_tsk x1, x2
+       stackleak_erase
        kernel_exit 0
 ENDPROC(ret_to_user)
 

arch/arm64/kernel/process.c +17 -0

index e10bc363f533..2724e4d31b16 100644
@@ -493,3 +493,20 @@ void arch_setup_new_exec(void)
 {
        current->mm->context.flags = is_compat_task() ? MMCF_AARCH32 : 0;
 }
+
+#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
+void __used stackleak_check_alloca(unsigned long size)
+{
+       unsigned long stack_left;
+       unsigned long current_sp = current_stack_pointer;
+       struct stack_info info;
+
+       BUG_ON(!on_accessible_stack(current, current_sp, &info));
+
+       stack_left = current_sp - info.low;
+
+       if (size >= stack_left)
+               panic("alloca() over the kernel stack boundary\n");
+}
+EXPORT_SYMBOL(stackleak_check_alloca);
+#endif

arch/arm64/kvm/hyp/Makefile +2 -1

index 4313f7475333..2fabc2dc1966 100644
@@ -3,7 +3,8 @@
 # Makefile for Kernel-based Virtual Machine module, HYP part
 #
 
-ccflags-y += -fno-stack-protector -DDISABLE_BRANCH_PROFILING
+ccflags-y += -fno-stack-protector -DDISABLE_BRANCH_PROFILING \
+               $(DISABLE_STACKLEAK_PLUGIN)
 
 KVM=../../../../virt/kvm
 

drivers/firmware/efi/libstub/Makefile +2 -2

index a34e9290a699..25dd2a14560d 100644
@@ -20,7 +20,8 @@ cflags-$(CONFIG_EFI_ARMSTUB)  += -I$(srctree)/scripts/dtc/libfdt
 KBUILD_CFLAGS                  := $(cflags-y) -DDISABLE_BRANCH_PROFILING \
                                   -D__NO_FORTIFY \
                                   $(call cc-option,-ffreestanding) \
-                                  $(call cc-option,-fno-stack-protector)
+                                  $(call cc-option,-fno-stack-protector) \
+                                  $(DISABLE_STACKLEAK_PLUGIN)
 
 GCOV_PROFILE                   := n
 KASAN_SANITIZE                 := n
--

2.17.1

Implementation of stackleak based heavily on the x86 version

Signed-off-by Laura Abbott <[email protected]>

---
v2: Convert to adjusted on_acessible_stack APIs. Fixed alloca check to just panic. Dropped the extra include per Kees. I also didn't add the Reviewed-by since the APIs did change and I wanted another pass.

---
 arch/arm64/Kconfig                    |  1 +
 arch/arm64/include/asm/processor.h    | 15 +++++++++++++++
 arch/arm64/kernel/entry.S             |  7 +++++++
 arch/arm64/kernel/process.c           | 17 +++++++++++++++++
 arch/arm64/kvm/hyp/Makefile           |  3 ++-
 drivers/firmware/efi/libstub/Makefile |  3 ++-
 6 files changed, 44 insertions(+), 2 deletions(-)

arch/arm64/Kconfig +1 -0

index 42c090cf0292..216d36a49ab5 100644
@@ -96,6 +96,7 @@ config ARM64
 	select HAVE_ARCH_MMAP_RND_BITS
 	select HAVE_ARCH_MMAP_RND_COMPAT_BITS if COMPAT
 	select HAVE_ARCH_SECCOMP_FILTER
+	select HAVE_ARCH_STACKLEAK
 	select HAVE_ARCH_THREAD_STRUCT_WHITELIST
 	select HAVE_ARCH_TRACEHOOK
 	select HAVE_ARCH_TRANSPARENT_HUGEPAGE

arch/arm64/include/asm/processor.h +15 -0

index a73ae1e49200..0061450a793b 100644
@@ -266,5 +266,20 @@ extern void __init minsigstksz_setup(void);
 #define SVE_SET_VL(arg)	sve_set_current_vl(arg)
 #define SVE_GET_VL()	sve_get_current_vl()
 
+/*
+ * For CONFIG_GCC_PLUGIN_STACKLEAK
+ *
+ * These need to be macros because otherwise we get stuck in a nightmare
+ * of header definitions for the use of task_stack_page.
+ */
+
+#define current_top_of_stack()							\
+({										\
+	struct stack_info _info;						\
+	BUG_ON(!on_accessible_stack(current, current_stack_pointer, &_info));	\
+	_info.high;								\
+})
+#define on_thread_stack()	(on_task_stack(current, current_stack_pointer, NULL))
+
 #endif /* __ASSEMBLY__ */
 #endif /* __ASM_PROCESSOR_H */

arch/arm64/kernel/entry.S +7 -0

index 28ad8799406f..67d12016063d 100644
@@ -431,6 +431,11 @@ tsk	.req	x28		// current thread_info
 
 	.text
 
+	.macro	stackleak_erase
+#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
+	bl	stackleak_erase
+#endif
+	.endm
 /*
  * Exception vectors.
  */

@@ -910,6 +915,7 @@ ret_fast_syscall:
 	and	x2, x1, #_TIF_WORK_MASK
 	cbnz	x2, work_pending
 	enable_step_tsk x1, x2
+	stackleak_erase
 	kernel_exit 0
 ret_fast_syscall_trace:
 	enable_daif

@@ -936,6 +942,7 @@ ret_to_user:
 	cbnz	x2, work_pending
 finish_ret_to_user:
 	enable_step_tsk x1, x2
+	stackleak_erase
 	kernel_exit 0
 ENDPROC(ret_to_user)
 

arch/arm64/kernel/process.c +17 -0

index e10bc363f533..2724e4d31b16 100644
@@ -493,3 +493,20 @@ void arch_setup_new_exec(void)
 {
 	current->mm->context.flags = is_compat_task() ? MMCF_AARCH32 : 0;
 }
+
+#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
+void __used stackleak_check_alloca(unsigned long size)
+{
+	unsigned long stack_left;
+	unsigned long current_sp = current_stack_pointer;
+	struct stack_info info;
+
+	BUG_ON(!on_accessible_stack(current, current_sp, &info));
+
+	stack_left = current_sp - info.low;
+
+	if (size >= stack_left)
+		panic("alloca() over the kernel stack boundary\n");
+}
+EXPORT_SYMBOL(stackleak_check_alloca);
+#endif

arch/arm64/kvm/hyp/Makefile +2 -1

index 4313f7475333..2fabc2dc1966 100644
@@ -3,7 +3,8 @@
 # Makefile for Kernel-based Virtual Machine module, HYP part
 #
 
-ccflags-y += -fno-stack-protector -DDISABLE_BRANCH_PROFILING
+ccflags-y += -fno-stack-protector -DDISABLE_BRANCH_PROFILING \
+		$(DISABLE_STACKLEAK_PLUGIN)
 
 KVM=../../../../virt/kvm
 

drivers/firmware/efi/libstub/Makefile +2 -1

index a34e9290a699..25dd2a14560d 100644
@@ -20,7 +20,8 @@ cflags-$(CONFIG_EFI_ARMSTUB)	+= -I$(srctree)/scripts/dtc/libfdt
 KBUILD_CFLAGS			:= $(cflags-y) -DDISABLE_BRANCH_PROFILING \
 				   -D__NO_FORTIFY \
 				   $(call cc-option,-ffreestanding) \
-				   $(call cc-option,-fno-stack-protector)
+				   $(call cc-option,-fno-stack-protector) \
+				   $(DISABLE_STACKLEAK_PLUGIN)
 
 GCOV_PROFILE			:= n
 KASAN_SANITIZE			:= n

Signature · 1 line

2.17.1

This adds support for the STACKLEAK gcc plugin to arm64 by implementing stackleak_check_alloca(), based heavily on the x86 version, and adding the two helpers used by the stackleak common code: current_top_of_stack() and on_thread_stack(). The stack erasure calls are made at syscall returns. Additionally, this disables the plugin in hypervisor and EFI stub code, which are out of scope for the protection.

Reviewed-by Mark Rutland <[email protected]>

Reviewed-by Kees Cook <[email protected]>

Signed-off-by Laura Abbott <[email protected]>

---
v3: Actual commit text courtesy of Kees. A comment explaining why we panic

---
 arch/arm64/Kconfig                    |  1 +
 arch/arm64/include/asm/processor.h    | 15 +++++++++++++++
 arch/arm64/kernel/entry.S             |  7 +++++++
 arch/arm64/kernel/process.c           | 22 ++++++++++++++++++++++
 arch/arm64/kvm/hyp/Makefile           |  3 ++-
 drivers/firmware/efi/libstub/Makefile |  3 ++-
 6 files changed, 49 insertions(+), 2 deletions(-)

Quoted 13 lines · Laura Abbott

This adds support for the STACKLEAK gcc plugin to arm64 by implementing stackleak_check_alloca(), based heavily on the x86 version, and adding the two helpers used by the stackleak common code: current_top_of_stack() and on_thread_stack(). The stack erasure calls are made at syscall returns. Additionally, this disables the plugin in hypervisor and EFI stub code, which are out of scope for the protection.

Reviewed-by Mark Rutland <[email protected]>

Reviewed-by Kees Cook <[email protected]>

Signed-off-by Laura Abbott <[email protected]>

---
v3: Actual commit text courtesy of Kees. A comment explaining why we panic

Quoted 8 lines

---
 arch/arm64/Kconfig                    |  1 +
 arch/arm64/include/asm/processor.h    | 15 +++++++++++++++
 arch/arm64/kernel/entry.S             |  7 +++++++
 arch/arm64/kernel/process.c           | 22 ++++++++++++++++++++++
 arch/arm64/kvm/hyp/Makefile           |  3 ++-
 drivers/firmware/efi/libstub/Makefile |  3 ++-
 6 files changed, 49 insertions(+), 2 deletions(-)

Laura, thanks for your work!

I've reviewed and tested this patch on my LeMaker HiKey board (HiSilicon Kirin 620 SoC). The lkdtm tests for STACKLEAK work fine.

Acked-by Alexander Popov <[email protected]>

For testing I applied your patches above Kees' for-next/kspp:
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=for-next/kspp

I've had one trouble with building CONFIG_STACKLEAK_RUNTIME_DISABLE on arm64. Kees, could you please fold this into the 7th patch of the series?

---- >8 ----

kernel/stackleak.c +1 -0

index f731c9a..03031f7a 100644
@@ -16,6 +16,7 @@
 
 #ifdef CONFIG_STACKLEAK_RUNTIME_DISABLE
 #include <linux/jump_label.h>
+#include <linux/sysctl.h>
 
 static DEFINE_STATIC_KEY_FALSE(stack_erasing_bypass);
 

This is the version of stackleak for arm64, hopefully ready for queueing

Quoted 1 line · Laura Abbott

This is the version of stackleak for arm64, hopefully ready for queueing

Thanks. I'll push these into linux-next tomorrow, once I've had a chance to test my conflict resolution in entry.S.

Hi Laura,

Quoted 5 lines · Will Deacon

Quoted 1 line · Laura Abbott

This is the version of stackleak for arm64, hopefully ready for queueing

Thanks. I'll push these into linux-next tomorrow, once I've had a chance to test my conflict resolution in entry.S.

I've run into a couple of issues with this series:

1. I had to install libmpc-dev to get GCC_PLUGINS to appear, otherwise the hostcc check would silently fail. I guess that's a general observation, but it might be nice to print a message about the missing dependencies.

2. It breaks arm64 allmodconfig build. Log below.

Please can you take a look at the build failure? Otherwise, the patches look good to me.

Cheers,

Will

--->8

arch/arm64/kernel/sdei.c: In function ‘on_sdei_normal_stack’:
arch/arm64/kernel/sdei.c:101:7: error: dereferencing pointer to incomplete type ‘struct stack_info’

    info->low = low;
        ^~
arch/arm64/kernel/sdei.c:103:16: error: ‘STACK_TYPE_SDEI_NORMAL’ undeclared (first use in this function); did you mean ‘SCHED_NORMAL’?
    info->type = STACK_TYPE_SDEI_NORMAL;
                 ^~~~~~~~~~~~~~~~~~~~~~
                 SCHED_NORMAL
arch/arm64/kernel/sdei.c:103:16: note: each undeclared identifier is reported only once for each function it appears in
arch/arm64/kernel/sdei.c: In function ‘on_sdei_critical_stack’:
arch/arm64/kernel/sdei.c:121:16: error: ‘STACK_TYPE_SDEI_CRITICAL’ undeclared (first use in this function)
    info->type = STACK_TYPE_SDEI_CRITICAL;
                 ^~~~~~~~~~~~~~~~~~~~~~~~
arch/arm64/kernel/sdei.c: At top level:
arch/arm64/kernel/sdei.c:127:6: error: conflicting types for ‘_on_sdei_stack’
  bool _on_sdei_stack(unsigned long sp,
       ^~~~~~~~~~~~~~
In file included from ./include/linux/arm_sdei.h:14:0,
                  from arch/arm64/kernel/sdei.c:5:
./arch/arm64/include/asm/sdei.h:45:6: note: previous declaration of ‘_on_sdei_stack’ was here
  bool _on_sdei_stack(unsigned long sp, struct stack_info *info);
       ^~~~~~~~~~~~~~
arch/arm64/kernel/sdei.c: In function ‘_on_sdei_stack’:
arch/arm64/kernel/sdei.c:136:33: error: ‘info’ undeclared (first use in this function); did you mean ‘int’?
   if (on_sdei_critical_stack(sp, info))
                                  ^~~~
                                  int
arch/arm64/kernel/sdei.c:131:21: warning: unused variable ‘high’ [-Wunused-variable]
   unsigned long low, high;
                      ^~~~
arch/arm64/kernel/sdei.c:131:16: warning: unused variable ‘low’ [-Wunused-variable]
   unsigned long low, high;
                 ^~~

make[1]: *** [arch/arm64/kernel/sdei.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [arch/arm64/kernel] Error 2

Quoted 20 lines · Will Deacon

arch/arm64/kernel/sdei.c: At top level:
arch/arm64/kernel/sdei.c:127:6: error: conflicting types for ‘_on_sdei_stack’
bool _on_sdei_stack(unsigned long sp,

      ^~~~~~~~~~~~~~
In file included from ./include/linux/arm_sdei.h:14:0,
                 from arch/arm64/kernel/sdei.c:5:
./arch/arm64/include/asm/sdei.h:45:6: note: previous declaration of ‘_on_sdei_stack’ was here
 bool _on_sdei_stack(unsigned long sp, struct stack_info *info);
      ^~~~~~~~~~~~~~
arch/arm64/kernel/sdei.c: In function ‘_on_sdei_stack’:
arch/arm64/kernel/sdei.c:136:33: error: ‘info’ undeclared (first use in this function); did you mean ‘int’?
  if (on_sdei_critical_stack(sp, info))
                                 ^~~~
                                 int
arch/arm64/kernel/sdei.c:131:21: warning: unused variable ‘high’ [-Wunused-variable]
  unsigned long low, high;
                     ^~~~
arch/arm64/kernel/sdei.c:131:16: warning: unused variable ‘low’ [-Wunused-variable]
  unsigned long low, high;
                ^~~

Quoted 4 lines

make[1]: *** [arch/arm64/kernel/sdei.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [arch/arm64/kernel] Error 2

Ugh this was a failure that I missed folding in, sorry about that