arm64: Change the on_*stack functions to take a size argument

unwind_frame() was previously implicitly checking that the frame
record is in bounds of the stack by enforcing that FP is both aligned
to 16 and in bounds of the stack. Once the FP alignment requirement
is relaxed to 8 this will not be sufficient because it does not
account for the case where FP points to 8 bytes before the end of the
stack.

Make the check explicit by changing the on_*stack functions to take a
size argument and adjusting the callers to pass the appropriate sizes.

Signed-off-by: Peter Collingbourne <pcc@google.com>
Link: https://linux-review.googlesource.com/id/Ib7a3eb3eea41b0687ffaba045ceb2012d077d8b4
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
Link: https://lore.kernel.org/r/20210526174927.2477847-1-pcc@google.com
Signed-off-by: Will Deacon <will@kernel.org>

diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 9df3feeee8909b5a0794a8d15c6331d855e072f4..7a094aafec200ed8be4f57f939c958a9e17cf5e1 100644 (file)
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -329,13 +329,13 @@ long get_tagged_addr_ctrl(struct task_struct *task);
  * 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 current_top_of_stack()                                                         \
+({                                                                                     \
+       struct stack_info _info;                                                        \
+       BUG_ON(!on_accessible_stack(current, current_stack_pointer, 1, &_info));        \
+       _info.high;                                                                     \
 })
-#define on_thread_stack()      (on_task_stack(current, current_stack_pointer, NULL))
+#define on_thread_stack()      (on_task_stack(current, current_stack_pointer, 1, NULL))
 
 #endif /* __ASSEMBLY__ */
 #endif /* __ASM_PROCESSOR_H */

diff --git a/arch/arm64/include/asm/sdei.h b/arch/arm64/include/asm/sdei.h
index 63e0b92a5fbb069bc232a93e32e17a39d6df9bd1..8bc30a5c456939e28cba34e07eb8c4a670473f5c 100644 (file)
--- a/arch/arm64/include/asm/sdei.h
+++ b/arch/arm64/include/asm/sdei.h
@@ -42,8 +42,9 @@ unsigned long sdei_arch_get_entry_point(int conduit);
 
 struct stack_info;
 
-bool _on_sdei_stack(unsigned long sp, struct stack_info *info);
-static inline bool on_sdei_stack(unsigned long sp,
+bool _on_sdei_stack(unsigned long sp, unsigned long size,
+                   struct stack_info *info);
+static inline bool on_sdei_stack(unsigned long sp, unsigned long size,
                                struct stack_info *info)
 {
        if (!IS_ENABLED(CONFIG_VMAP_STACK))
@@ -51,7 +52,7 @@ static inline bool on_sdei_stack(unsigned long sp,
        if (!IS_ENABLED(CONFIG_ARM_SDE_INTERFACE))
                return false;
        if (in_nmi())
-               return _on_sdei_stack(sp, info);
+               return _on_sdei_stack(sp, size, info);
 
        return false;
 }

diff --git a/arch/arm64/include/asm/stacktrace.h b/arch/arm64/include/asm/stacktrace.h
index 4b33ca6206793361fad25b23258c05ccfe06b024..1801399204d79fe011ec07e8653a3b7ca1bacdb3 100644 (file)
--- a/arch/arm64/include/asm/stacktrace.h
+++ b/arch/arm64/include/asm/stacktrace.h
@@ -69,14 +69,14 @@ extern void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk,
 
 DECLARE_PER_CPU(unsigned long *, irq_stack_ptr);
 
-static inline bool on_stack(unsigned long sp, unsigned long low,
-                               unsigned long high, enum stack_type type,
-                               struct stack_info *info)
+static inline bool on_stack(unsigned long sp, unsigned long size,
+                           unsigned long low, unsigned long high,
+                           enum stack_type type, struct stack_info *info)
 {
        if (!low)
                return false;
 
-       if (sp < low || sp >= high)
+       if (sp < low || sp + size < sp || sp + size > high)
                return false;
 
        if (info) {
@@ -87,38 +87,38 @@ static inline bool on_stack(unsigned long sp, unsigned long low,
        return true;
 }
 
-static inline bool on_irq_stack(unsigned long sp,
+static inline bool on_irq_stack(unsigned long sp, unsigned long size,
                                struct stack_info *info)
 {
        unsigned long low = (unsigned long)raw_cpu_read(irq_stack_ptr);
        unsigned long high = low + IRQ_STACK_SIZE;
 
-       return on_stack(sp, low, high, STACK_TYPE_IRQ, info);
+       return on_stack(sp, size, low, high, STACK_TYPE_IRQ, info);
 }
 
 static inline bool on_task_stack(const struct task_struct *tsk,
-                                unsigned long sp,
+                                unsigned long sp, unsigned long size,
                                 struct stack_info *info)
 {
        unsigned long low = (unsigned long)task_stack_page(tsk);
        unsigned long high = low + THREAD_SIZE;
 
-       return on_stack(sp, low, high, STACK_TYPE_TASK, info);
+       return on_stack(sp, size, low, high, STACK_TYPE_TASK, info);
 }
 
 #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 size,
                                struct stack_info *info)
 {
        unsigned long low = (unsigned long)raw_cpu_ptr(overflow_stack);
        unsigned long high = low + OVERFLOW_STACK_SIZE;
 
-       return on_stack(sp, low, high, STACK_TYPE_OVERFLOW, info);
+       return on_stack(sp, size, low, high, STACK_TYPE_OVERFLOW, info);
 }
 #else
-static inline bool on_overflow_stack(unsigned long sp,
+static inline bool on_overflow_stack(unsigned long sp, unsigned long size,
                        struct stack_info *info) { return false; }
 #endif
 
@@ -128,21 +128,21 @@ static inline bool on_overflow_stack(unsigned long sp,
  * context.
  */
 static inline bool on_accessible_stack(const struct task_struct *tsk,
-                                      unsigned long sp,
+                                      unsigned long sp, unsigned long size,
                                       struct stack_info *info)
 {
        if (info)
                info->type = STACK_TYPE_UNKNOWN;
 
-       if (on_task_stack(tsk, sp, info))
+       if (on_task_stack(tsk, sp, size, info))
                return true;
        if (tsk != current || preemptible())
                return false;
-       if (on_irq_stack(sp, info))
+       if (on_irq_stack(sp, size, info))
                return true;
-       if (on_overflow_stack(sp, info))
+       if (on_overflow_stack(sp, size, info))
                return true;
-       if (on_sdei_stack(sp, info))
+       if (on_sdei_stack(sp, size, info))
                return true;
 
        return false;

diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index eb2f73939b7bbbaf7feb4a5cb98dd127ad3c7a9b..499b6b2f9757f9ad198c655de743227f611c2562 100644 (file)
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -122,7 +122,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, NULL);
+               on_irq_stack(addr, sizeof(unsigned long), NULL);
 }
 
 /**

diff --git a/arch/arm64/kernel/sdei.c b/arch/arm64/kernel/sdei.c
index 2c7ca449dd5111206fa8559065fd95ce2858dcf9..c524f96f97c491e2aeaf9d172db350ee5d72b686 100644 (file)
--- a/arch/arm64/kernel/sdei.c
+++ b/arch/arm64/kernel/sdei.c
@@ -162,31 +162,33 @@ static int init_sdei_scs(void)
        return err;
 }
 
-static bool on_sdei_normal_stack(unsigned long sp, struct stack_info *info)
+static bool on_sdei_normal_stack(unsigned long sp, unsigned long size,
+                                struct stack_info *info)
 {
        unsigned long low = (unsigned long)raw_cpu_read(sdei_stack_normal_ptr);
        unsigned long high = low + SDEI_STACK_SIZE;
 
-       return on_stack(sp, low, high, STACK_TYPE_SDEI_NORMAL, info);
+       return on_stack(sp, size, low, high, STACK_TYPE_SDEI_NORMAL, info);
 }
 
-static bool on_sdei_critical_stack(unsigned long sp, struct stack_info *info)
+static bool on_sdei_critical_stack(unsigned long sp, unsigned long size,
+                                  struct stack_info *info)
 {
        unsigned long low = (unsigned long)raw_cpu_read(sdei_stack_critical_ptr);
        unsigned long high = low + SDEI_STACK_SIZE;
 
-       return on_stack(sp, low, high, STACK_TYPE_SDEI_CRITICAL, info);
+       return on_stack(sp, size, low, high, STACK_TYPE_SDEI_CRITICAL, info);
 }
 
-bool _on_sdei_stack(unsigned long sp, struct stack_info *info)
+bool _on_sdei_stack(unsigned long sp, unsigned long size, struct stack_info *info)
 {
        if (!IS_ENABLED(CONFIG_VMAP_STACK))
                return false;
 
-       if (on_sdei_critical_stack(sp, info))
+       if (on_sdei_critical_stack(sp, size, info))
                return true;
 
-       if (on_sdei_normal_stack(sp, info))
+       if (on_sdei_normal_stack(sp, size, info))
                return true;
 
        return false;

diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index 36cf05d5eb9ee97c3d0e2b0a4f1fd97479dd5a11..5c70f247645bf45d12a5d5fec0bb0ad4d01f44e3 100644 (file)
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -78,7 +78,7 @@ int notrace unwind_frame(struct task_struct *tsk, struct stackframe *frame)
        if (fp & 0xf)
                return -EINVAL;
 
-       if (!on_accessible_stack(tsk, fp, &info))
+       if (!on_accessible_stack(tsk, fp, 16, &info))
                return -EINVAL;
 
        if (test_bit(info.type, frame->stacks_done))