2018-07-20 21:43:07

by Laura Abbott

[permalink] [raw]
Subject: [PATCHv3 0/2] Stackleak for arm64

Hi,

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

Laura Abbott (2):
arm64: Add stack information to on_accessible_stack
arm64: Add support for STACKLEAK gcc plugin

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 | 22 ++++++++
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, 161 insertions(+), 27 deletions(-)

--
2.17.1



2018-07-20 21:43:09

by Laura Abbott

[permalink] [raw]
Subject: [PATCHv3 1/2] arm64: Add stack information to on_accessible_stack


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.

Reviewed-by: Mark Rutland <[email protected]>
Signed-off-by: Laura Abbott <[email protected]>
---
v3: Switched some logic to reduce if nesting
---
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..7154fee1cb2b 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 (sp < low || sp >= high)
+ return false;
+
+ if (info) {
+ info->low = low;
+ info->high = high;
+ info->type = STACK_TYPE_SDEI_NORMAL;
+ }
+
+ return true;
+}
+
+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 (sp < low || sp >= high)
+ return false;
+
+ if (info) {
+ info->low = low;
+ info->high = high;
+ info->type = STACK_TYPE_SDEI_CRITICAL;
+ }
+
+ return true;
+}
+
+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


2018-07-20 21:43:11

by Laura Abbott

[permalink] [raw]
Subject: [PATCHv3 2/2] 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.

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

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..f0ad00fb6a71 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -493,3 +493,25 @@ 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;
+
+ /*
+ * There's a good chance we're almost out of stack space if this
+ * is true. Using panic() over BUG() is more likely to give
+ * reliable debugging output.
+ */
+ 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


2018-07-24 12:45:34

by Alexander Popov

[permalink] [raw]
Subject: Re: [PATCHv3 2/2] arm64: Add support for STACKLEAK gcc plugin

On 21.07.2018 00:41, Laura Abbott wrote:
> 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(-)

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

diff --git a/kernel/stackleak.c b/kernel/stackleak.c
index f731c9a..03031f7a 100644
--- a/kernel/stackleak.c
+++ b/kernel/stackleak.c
@@ -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);


2018-07-24 16:37:43

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCHv3 2/2] arm64: Add support for STACKLEAK gcc plugin

On Tue, Jul 24, 2018 at 5:44 AM, Alexander Popov <[email protected]> wrote:
> On 21.07.2018 00:41, Laura Abbott wrote:
>> 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(-)
>
> 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?

Sure thing!

-Kees

>
> ---- >8 ----
>
> diff --git a/kernel/stackleak.c b/kernel/stackleak.c
> index f731c9a..03031f7a 100644
> --- a/kernel/stackleak.c
> +++ b/kernel/stackleak.c
> @@ -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);
>



--
Kees Cook
Pixel Security

2018-07-24 16:39:19

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCHv3 0/2] Stackleak for arm64

Hi Laura,

On Fri, Jul 20, 2018 at 02:41:52PM -0700, Laura Abbott wrote:
> 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.

Will

2018-07-25 11:51:06

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCHv3 0/2] Stackleak for arm64

Hi Laura,

On Tue, Jul 24, 2018 at 05:38:07PM +0100, Will Deacon wrote:
> On Fri, Jul 20, 2018 at 02:41:52PM -0700, Laura Abbott wrote:
> > 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

2018-07-25 22:07:05

by Laura Abbott

[permalink] [raw]
Subject: Re: [PATCHv3 0/2] Stackleak for arm64

On 07/25/2018 04:49 AM, Will Deacon wrote:
> Hi Laura,
>
> On Tue, Jul 24, 2018 at 05:38:07PM +0100, Will Deacon wrote:
>> On Fri, Jul 20, 2018 at 02:41:52PM -0700, Laura Abbott wrote:
>>> 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
>

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

--- 8< ----

diff --git a/arch/arm64/kernel/sdei.c b/arch/arm64/kernel/sdei.c
index 7154fee1cb2b..a94a868f0532 100644
--- a/arch/arm64/kernel/sdei.c
+++ b/arch/arm64/kernel/sdei.c
@@ -13,6 +13,7 @@
#include <asm/mmu.h>
#include <asm/ptrace.h>
#include <asm/sections.h>
+#include <asm/stacktrace.h>
#include <asm/sysreg.h>
#include <asm/vmap_stack.h>

@@ -125,11 +126,8 @@ bool on_sdei_critical_stack(unsigned long sp,
}

bool _on_sdei_stack(unsigned long sp,
- unsigned long *stack_low,
- unsigned long *stack_high)
+ struct stack_info *info)
{
- unsigned long low, high;
-
if (!IS_ENABLED(CONFIG_VMAP_STACK))
return false;


2018-07-26 09:57:06

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCHv3 0/2] Stackleak for arm64

Hi Laura,

On Wed, Jul 25, 2018 at 03:05:58PM -0700, Laura Abbott wrote:
> On 07/25/2018 04:49 AM, Will Deacon wrote:
> >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
> >
>
> Ugh this was a failure that I missed folding in, sorry about that

That's ok, thanks for the quick fixup. I'll fold it in and push this out
later on.

Cheers,

Will