Hi,
This is hopefully the final version of the stackleak plugin for arm64.
Acks are appreciated!
Laura Abbott (2):
arm64: Add stack information to on_accessible_stack
arm64: Clear the stack
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/processor.h | 15 ++++++
arch/arm64/include/asm/sdei.h | 9 ++--
arch/arm64/include/asm/stacktrace.h | 73 ++++++++++++++++++++++-----
arch/arm64/kernel/entry.S | 7 +++
arch/arm64/kernel/process.c | 17 +++++++
arch/arm64/kernel/ptrace.c | 2 +-
arch/arm64/kernel/sdei.c | 51 ++++++++++++++++---
arch/arm64/kernel/stacktrace.c | 2 +-
arch/arm64/kvm/hyp/Makefile | 3 +-
drivers/firmware/efi/libstub/Makefile | 3 +-
11 files changed, 156 insertions(+), 27 deletions(-)
--
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/sdei.h | 9 ++--
arch/arm64/include/asm/stacktrace.h | 73 ++++++++++++++++++++++++-----
arch/arm64/kernel/ptrace.c | 2 +-
arch/arm64/kernel/sdei.c | 51 ++++++++++++++++----
arch/arm64/kernel/stacktrace.c | 2 +-
5 files changed, 112 insertions(+), 25 deletions(-)
diff --git a/arch/arm64/include/asm/sdei.h b/arch/arm64/include/asm/sdei.h
index e073e6886685..ffe47d766c25 100644
--- a/arch/arm64/include/asm/sdei.h
+++ b/arch/arm64/include/asm/sdei.h
@@ -40,15 +40,18 @@ 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)
+struct stack_info;
+
+bool _on_sdei_stack(unsigned long sp, struct stack_info *info);
+static inline bool on_sdei_stack(unsigned long sp,
+ struct stack_info *info)
{
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, info);
return false;
}
diff --git a/arch/arm64/include/asm/stacktrace.h b/arch/arm64/include/asm/stacktrace.h
index 902f9edacbea..e86737b7c924 100644
--- a/arch/arm64/include/asm/stacktrace.h
+++ b/arch/arm64/include/asm/stacktrace.h
@@ -32,6 +32,21 @@ struct stackframe {
#endif
};
+enum stack_type {
+ STACK_TYPE_UNKNOWN,
+ STACK_TYPE_TASK,
+ STACK_TYPE_IRQ,
+ STACK_TYPE_OVERFLOW,
+ STACK_TYPE_SDEI_NORMAL,
+ STACK_TYPE_SDEI_CRITICAL,
+};
+
+struct stack_info {
+ unsigned long low;
+ unsigned long high;
+ enum stack_type type;
+};
+
extern int unwind_frame(struct task_struct *tsk, struct stackframe *frame);
extern void walk_stackframe(struct task_struct *tsk, struct stackframe *frame,
int (*fn)(struct stackframe *, void *), void *data);
@@ -39,7 +54,8 @@ 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,
+ 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;
}
-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,
+ struct stack_info *info)
{
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 (info) {
+ info->low = low;
+ info->high = high;
+ info->type = STACK_TYPE_TASK;
+ }
+
+ 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,
+ struct stack_info *info)
{
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 (info) {
+ info->low = low;
+ info->high = high;
+ info->type = STACK_TYPE_OVERFLOW;
+ }
+
+ return true;
}
#else
-static inline bool on_overflow_stack(unsigned long sp) { return false; }
+static inline bool on_overflow_stack(unsigned long sp,
+ struct stack_info *info) { return false; }
#endif
+
/*
* 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)
+static inline bool on_accessible_stack(struct task_struct *tsk,
+ unsigned long sp,
+ struct stack_info *info)
{
- if (on_task_stack(tsk, sp))
+ if (on_task_stack(tsk, sp, info))
return true;
if (tsk != current || preemptible())
return false;
- if (on_irq_stack(sp))
+ if (on_irq_stack(sp, info))
return true;
- if (on_overflow_stack(sp))
+ if (on_overflow_stack(sp, info))
return true;
- if (on_sdei_stack(sp))
+ if (on_sdei_stack(sp, info))
return true;
return false;
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 5c338ce5a7fa..cf94e1498ba6 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -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);
}
/**
diff --git a/arch/arm64/kernel/sdei.c b/arch/arm64/kernel/sdei.c
index 6b8d90d5ceae..f0787610f82e 100644
--- a/arch/arm64/kernel/sdei.c
+++ b/arch/arm64/kernel/sdei.c
@@ -88,23 +88,58 @@ static int init_sdei_stacks(void)
return err;
}
-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) {
+ if (info) {
+ info->low = low;
+ info->high = high;
+ info->type = STACK_TYPE_SDEI_CRITICAL;
+ }
+ return true;
+ }
+
+ return false;
+}
+
+bool _on_sdei_stack(unsigned long sp,
+ unsigned long *stack_low,
+ unsigned long *stack_high)
{
unsigned long low, high;
if (!IS_ENABLED(CONFIG_VMAP_STACK))
return false;
- low = (unsigned long)raw_cpu_read(sdei_stack_critical_ptr);
- high = low + SDEI_STACK_SIZE;
-
- if (low <= sp && sp < high)
+ if (on_sdei_critical_stack(sp, info))
return true;
- low = (unsigned long)raw_cpu_read(sdei_stack_normal_ptr);
- high = low + SDEI_STACK_SIZE;
+ if (on_sdei_normal_stack(sp, info))
+ return true;
- return (low <= sp && sp < high);
+ return false;
}
unsigned long sdei_arch_get_entry_point(int conduit)
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index d5718a060672..4989f7ea1e59 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -50,7 +50,7 @@ int notrace unwind_frame(struct task_struct *tsk, struct stackframe *frame)
if (!tsk)
tsk = current;
- if (!on_accessible_stack(tsk, fp))
+ if (!on_accessible_stack(tsk, fp, NULL))
return -EINVAL;
frame->fp = READ_ONCE_NOCHECK(*(unsigned long *)(fp));
--
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(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 42c090cf0292..216d36a49ab5 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -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
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index a73ae1e49200..0061450a793b 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -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 */
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 28ad8799406f..67d12016063d 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -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)
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index e10bc363f533..2724e4d31b16 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -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
diff --git a/arch/arm64/kvm/hyp/Makefile b/arch/arm64/kvm/hyp/Makefile
index 4313f7475333..2fabc2dc1966 100644
--- a/arch/arm64/kvm/hyp/Makefile
+++ b/arch/arm64/kvm/hyp/Makefile
@@ -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
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index a34e9290a699..25dd2a14560d 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -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
On Thu, Jul 19, 2018 at 4:28 PM, Laura Abbott <[email protected]> wrote:
>
> Implementation of stackleak based heavily on the x86 version
>
> Signed-off-by: Laura Abbott <[email protected]>
This is the commit message I wrote when I was using an earlier
version, which I think is more descriptive:
arm64: Add support for STACKLEAK gcc plugin
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.
Either way:
Reviewed-by: Kees Cook <[email protected]>
Thanks for getting this hammered out!
> ---
> 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.
Maybe the panic() should get a comment above it to describe why it's
there (i.e. summarize the thread where that change was discussed?) Or
maybe mention it in the commit log (instead of being only below the
--- line?)
-Kees
> ---
> 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(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 42c090cf0292..216d36a49ab5 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -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
> diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
> index a73ae1e49200..0061450a793b 100644
> --- a/arch/arm64/include/asm/processor.h
> +++ b/arch/arm64/include/asm/processor.h
> @@ -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 */
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 28ad8799406f..67d12016063d 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -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)
>
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index e10bc363f533..2724e4d31b16 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -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
> diff --git a/arch/arm64/kvm/hyp/Makefile b/arch/arm64/kvm/hyp/Makefile
> index 4313f7475333..2fabc2dc1966 100644
> --- a/arch/arm64/kvm/hyp/Makefile
> +++ b/arch/arm64/kvm/hyp/Makefile
> @@ -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
>
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index a34e9290a699..25dd2a14560d 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -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
>
--
Kees Cook
Pixel Security
On Thu, Jul 19, 2018 at 04:28:05PM -0700, Laura Abbott wrote:
>
> 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.
[...]
> -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;
> }
[...]
> -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) {
> + if (info) {
> + info->low = low;
> + info->high = high;
> + info->type = STACK_TYPE_SDEI_CRITICAL;
> + }
> + return true;
> + }
> +
> + return false;
> +}
Minor nit, but it would be good to avoid the nested conditionals for these two
by bailing out early when the SP is out of bounds, as with the other
on_<foo>_stack() functions, e.g.
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 (sp < low || sp >= high)
return false;
if (info) {
info->low = low;
info->high = high;
info->type = STACK_TYPE_SDEI_NORMAL;
}
return true;
}
Otherwise, this all looks good to me. With that:
Reviewed-by: Mark Rutland <[email protected]>
Thanks for working on this!
Mark.
On Thu, Jul 19, 2018 at 04:28:06PM -0700, Laura Abbott wrote:
>
> Implementation of stackleak based heavily on the x86 version
>
> Signed-off-by: Laura Abbott <[email protected]>
Reviewed-by: Mark Rutlamd <[email protected]>
Thanks for working on this!
Mark.
> ---
> 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(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 42c090cf0292..216d36a49ab5 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -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
> diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
> index a73ae1e49200..0061450a793b 100644
> --- a/arch/arm64/include/asm/processor.h
> +++ b/arch/arm64/include/asm/processor.h
> @@ -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 */
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 28ad8799406f..67d12016063d 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -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)
>
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index e10bc363f533..2724e4d31b16 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -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
> diff --git a/arch/arm64/kvm/hyp/Makefile b/arch/arm64/kvm/hyp/Makefile
> index 4313f7475333..2fabc2dc1966 100644
> --- a/arch/arm64/kvm/hyp/Makefile
> +++ b/arch/arm64/kvm/hyp/Makefile
> @@ -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
>
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index a34e9290a699..25dd2a14560d 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -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
>