[PATCH v2 5/8] arm64: stacktrace: rework stack boundary discovery

[Mark Rutland <mark.rutland@arm.com>]

In subsequent patches we'll want to acquire the stack boundaries
ahead-of-time, and we'll need to be able to acquire the relevant
stack_info regardless of whether we have an object the happens to be on
the stack.

This patch replaces the on_XXX_stack() helpers with stackinfo_get_XXX()
helpers, with the caller being responsible for the checking whether an
object is on a relevant stack. For the moment this is moved into the
on_accessible_stack() functions, making these slightly larger;
subsequent patches will remove the on_accessible_stack() functions and
simplify the logic.

The on_irq_stack() and on_task_stack() helpers are kept as these are
used by IRQ entry sequences and stackleak respectively. As they're only
used as predicates, the stack_info pointer parameter is removed in both
cases.

As the on_accessible_stack() functions are always passed a non-NULL info
pointer, these now update info unconditionally. When updating the type
to STACK_TYPE_UNKNOWN, the low/high bounds are also modified, but as
these will not be consumed this should have no adverse affect.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Kalesh Singh <kaleshsingh@google.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
Cc: Fuad Tabba <tabba@google.com>
Cc: Madhavan T. Venkataraman <madvenka@linux.microsoft.com>
Cc: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/include/asm/processor.h         |  2 +-
 arch/arm64/include/asm/stacktrace.h        | 78 +++++++++++++---------
 arch/arm64/include/asm/stacktrace/common.h | 28 +++-----
 arch/arm64/kernel/ptrace.c                 |  2 +-
 arch/arm64/kernel/stacktrace.c             | 65 +++++++++++-------
 arch/arm64/kvm/hyp/nvhe/stacktrace.c       | 37 +++++++---
 arch/arm64/kvm/stacktrace.c                | 37 +++++++---
 7 files changed, 153 insertions(+), 96 deletions(-)

diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 9e58749db21df..5035e0394a8a0 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -409,7 +409,7 @@ long get_tagged_addr_ctrl(struct task_struct *task);
  * The top of the current task's task stack
  */
 #define current_top_of_stack()	((unsigned long)current->stack + THREAD_SIZE)
-#define on_thread_stack()	(on_task_stack(current, current_stack_pointer, 1, NULL))
+#define on_thread_stack()	(on_task_stack(current, current_stack_pointer, 1))
 
 #endif /* __ASSEMBLY__ */
 #endif /* __ASM_PROCESSOR_H */
diff --git a/arch/arm64/include/asm/stacktrace.h b/arch/arm64/include/asm/stacktrace.h
index fa2df1ea22ebc..aad0c6258721d 100644
--- a/arch/arm64/include/asm/stacktrace.h
+++ b/arch/arm64/include/asm/stacktrace.h
@@ -22,77 +22,91 @@ 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, unsigned long size,
-				struct stack_info *info)
+static inline struct stack_info stackinfo_get_irq(void)
 {
 	unsigned long low = (unsigned long)raw_cpu_read(irq_stack_ptr);
 	unsigned long high = low + IRQ_STACK_SIZE;
 
-	return on_stack(sp, size, low, high, STACK_TYPE_IRQ, info);
+	return (struct stack_info) {
+		.low = low,
+		.high = high,
+		.type = STACK_TYPE_IRQ,
+	};
 }
 
-static inline bool on_task_stack(const struct task_struct *tsk,
-				 unsigned long sp, unsigned long size,
-				 struct stack_info *info)
+static inline bool on_irq_stack(unsigned long sp, unsigned long size)
+{
+	struct stack_info info = stackinfo_get_irq();
+	return stackinfo_on_stack(&info, sp, size);
+}
+
+static inline struct stack_info stackinfo_get_task(const struct task_struct *tsk)
 {
 	unsigned long low = (unsigned long)task_stack_page(tsk);
 	unsigned long high = low + THREAD_SIZE;
 
-	return on_stack(sp, size, low, high, STACK_TYPE_TASK, info);
+	return (struct stack_info) {
+		.low = low,
+		.high = high,
+		.type = STACK_TYPE_TASK,
+	};
+}
+
+static inline bool on_task_stack(const struct task_struct *tsk,
+				 unsigned long sp, unsigned long size)
+{
+	struct stack_info info = stackinfo_get_task(tsk);
+	return stackinfo_on_stack(&info, sp, size);
 }
 
 #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, unsigned long size,
-				struct stack_info *info)
+static inline struct stack_info stackinfo_get_overflow(void)
 {
 	unsigned long low = (unsigned long)raw_cpu_ptr(overflow_stack);
 	unsigned long high = low + OVERFLOW_STACK_SIZE;
 
-	return on_stack(sp, size, low, high, STACK_TYPE_OVERFLOW, info);
+	return (struct stack_info) {
+		.low = low,
+		.high = high,
+		.type = STACK_TYPE_OVERFLOW,
+	};
 }
 #else
-static inline bool on_overflow_stack(unsigned long sp, unsigned long size,
-				     struct stack_info *info)
-{
-	return false;
-}
+#define stackinfo_get_overflow()	stackinfo_get_unknown()
 #endif
 
 #if defined(CONFIG_ARM_SDE_INTERFACE) && defined(CONFIG_VMAP_STACK)
 DECLARE_PER_CPU(unsigned long *, sdei_stack_normal_ptr);
 DECLARE_PER_CPU(unsigned long *, sdei_stack_critical_ptr);
 
-static inline bool on_sdei_normal_stack(unsigned long sp, unsigned long size,
-					struct stack_info *info)
+static inline struct stack_info stackinfo_get_sdei_normal(void)
 {
 	unsigned long low = (unsigned long)raw_cpu_read(sdei_stack_normal_ptr);
 	unsigned long high = low + SDEI_STACK_SIZE;
 
-	return on_stack(sp, size, low, high, STACK_TYPE_SDEI_NORMAL, info);
+	return (struct stack_info) {
+		.low = low,
+		.high = high,
+		.type = STACK_TYPE_SDEI_NORMAL,
+	};
 }
 
-static inline bool on_sdei_critical_stack(unsigned long sp, unsigned long size,
-					  struct stack_info *info)
+static inline struct stack_info stackinfo_get_sdei_critical(void)
 {
 	unsigned long low = (unsigned long)raw_cpu_read(sdei_stack_critical_ptr);
 	unsigned long high = low + SDEI_STACK_SIZE;
 
-	return on_stack(sp, size, low, high, STACK_TYPE_SDEI_CRITICAL, info);
+	return (struct stack_info) {
+		.low = low,
+		.high = high,
+		.type = STACK_TYPE_SDEI_CRITICAL,
+	};
 }
 #else
-static inline bool on_sdei_normal_stack(unsigned long sp, unsigned long size,
-					struct stack_info *info)
-{
-	return false;
-}
-
-static inline bool on_sdei_critical_stack(unsigned long sp, unsigned long size,
-					  struct stack_info *info)
-{
-	return false;
-}
+#define stackinfo_get_sdei_normal()	stackinfo_get_unknown()
+#define stackinfo_get_sdei_critical()	stackinfo_get_unknown()
 #endif
 
 #endif	/* __ASM_STACKTRACE_H */
diff --git a/arch/arm64/include/asm/stacktrace/common.h b/arch/arm64/include/asm/stacktrace/common.h
index 9ed7feb493a36..0071f2459c703 100644
--- a/arch/arm64/include/asm/stacktrace/common.h
+++ b/arch/arm64/include/asm/stacktrace/common.h
@@ -66,6 +66,15 @@ struct unwind_state {
 	struct task_struct *task;
 };
 
+static inline struct stack_info stackinfo_get_unknown(void)
+{
+	return (struct stack_info) {
+		.low = 0,
+		.high = 0,
+		.type = STACK_TYPE_UNKNOWN,
+	};
+}
+
 static inline bool stackinfo_on_stack(const struct stack_info *info,
 				      unsigned long sp, unsigned long size)
 {
@@ -78,25 +87,6 @@ static inline bool stackinfo_on_stack(const struct stack_info *info,
 	return true;
 }
 
-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)
-{
-	struct stack_info tmp = {
-		.low = low,
-		.high = high,
-		.type = type,
-	};
-
-	if (!stackinfo_on_stack(&tmp, sp, size))
-		return false;
-
-	if (info)
-		*info = tmp;
-
-	return true;
-}
-
 static inline void unwind_init_common(struct unwind_state *state,
 				      struct task_struct *task)
 {
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 21da83187a602..2e1b721497794 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -121,7 +121,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, sizeof(unsigned long), NULL);
+		on_irq_stack(addr, sizeof(unsigned long));
 }
 
 /**
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index edf9edca20552..ca56fd732c2a9 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -67,36 +67,55 @@ static inline void unwind_init_from_task(struct unwind_state *state,
 	state->pc = thread_saved_pc(task);
 }
 
-/*
- * We can only safely access per-cpu stacks from current in a non-preemptible
- * context.
- */
 static bool on_accessible_stack(const struct task_struct *tsk,
 				unsigned long sp, unsigned long size,
 				struct stack_info *info)
 {
-	if (info)
-		info->type = STACK_TYPE_UNKNOWN;
+	struct stack_info tmp;
 
-	if (on_task_stack(tsk, sp, size, info))
-		return true;
-	if (tsk != current || preemptible())
-		return false;
-	if (on_irq_stack(sp, size, info))
-		return true;
-	if (on_overflow_stack(sp, size, info))
-		return true;
-
-	if (IS_ENABLED(CONFIG_VMAP_STACK) &&
-	    IS_ENABLED(CONFIG_ARM_SDE_INTERFACE) &&
-	    in_nmi()) {
-		if (on_sdei_critical_stack(sp, size, info))
-			return true;
-		if (on_sdei_normal_stack(sp, size, info))
-			return true;
-	}
+	tmp = stackinfo_get_task(tsk);
+	if (stackinfo_on_stack(&tmp, sp, size))
+		goto found;
 
+	/*
+	 * We can only safely access per-cpu stacks when unwinding the current
+	 * task in a non-preemptible context.
+	 */
+	if (tsk != current || preemptible())
+		goto not_found;
+
+	tmp = stackinfo_get_irq();
+	if (stackinfo_on_stack(&tmp, sp, size))
+		goto found;
+
+	tmp = stackinfo_get_overflow();
+	if (stackinfo_on_stack(&tmp, sp, size))
+		goto found;
+
+	/*
+	 * We can only safely access SDEI stacks which unwinding the current
+	 * task in an NMI context.
+	 */
+	if (!IS_ENABLED(CONFIG_VMAP_STACK) ||
+	    !IS_ENABLED(CONFIG_ARM_SDE_INTERFACE) ||
+	    !in_nmi())
+		goto not_found;
+
+	tmp = stackinfo_get_sdei_normal();
+	if (stackinfo_on_stack(&tmp, sp, size))
+		goto found;
+
+	tmp = stackinfo_get_sdei_critical();
+	if (stackinfo_on_stack(&tmp, sp, size))
+		goto found;
+
+not_found:
+	*info = stackinfo_get_unknown();
 	return false;
+
+found:
+	*info = tmp;
+	return true;
 }
 
 /*
diff --git a/arch/arm64/kvm/hyp/nvhe/stacktrace.c b/arch/arm64/kvm/hyp/nvhe/stacktrace.c
index 579b46aa9a553..5da0d44f61b73 100644
--- a/arch/arm64/kvm/hyp/nvhe/stacktrace.c
+++ b/arch/arm64/kvm/hyp/nvhe/stacktrace.c
@@ -39,34 +39,51 @@ static void hyp_prepare_backtrace(unsigned long fp, unsigned long pc)
 
 DEFINE_PER_CPU(unsigned long [NVHE_STACKTRACE_SIZE/sizeof(long)], pkvm_stacktrace);
 
-static bool on_overflow_stack(unsigned long sp, unsigned long size,
-			      struct stack_info *info)
+static struct stack_info stackinfo_get_overflow(void)
 {
 	unsigned long low = (unsigned long)this_cpu_ptr(overflow_stack);
 	unsigned long high = low + OVERFLOW_STACK_SIZE;
 
-	return on_stack(sp, size, low, high, STACK_TYPE_OVERFLOW, info);
+	return (struct stack_info) {
+		.low = low,
+		.high = high,
+		.type = STACK_TYPE_OVERFLOW,
+	};
 }
 
-static bool on_hyp_stack(unsigned long sp, unsigned long size,
-			      struct stack_info *info)
+static struct stack_info stackinfo_get_hyp(void)
 {
 	struct kvm_nvhe_init_params *params = this_cpu_ptr(&kvm_init_params);
 	unsigned long high = params->stack_hyp_va;
 	unsigned long low = high - PAGE_SIZE;
 
-	return on_stack(sp, size, low, high, STACK_TYPE_HYP, info);
+	return (struct stack_info) {
+		.low = low,
+		.high = high,
+		.type = STACK_TYPE_HYP,
+	};
 }
 
 static bool on_accessible_stack(const struct task_struct *tsk,
 				unsigned long sp, unsigned long size,
 				struct stack_info *info)
 {
-	if (info)
-		info->type = STACK_TYPE_UNKNOWN;
+	struct stack_info tmp;
 
-	return (on_overflow_stack(sp, size, info) ||
-		on_hyp_stack(sp, size, info));
+	tmp = stackinfo_get_overflow();
+	if (stackinfo_on_stack(&tmp, sp, size))
+		goto found;
+
+	tmp = stackinfo_get_hyp();
+	if (stackinfo_on_stack(&tmp, sp, size))
+		goto found;
+
+	*info = stackinfo_get_unknown();
+	return false;
+
+found:
+	*info = tmp;
+	return true;
 }
 
 static int unwind_next(struct unwind_state *state)
diff --git a/arch/arm64/kvm/stacktrace.c b/arch/arm64/kvm/stacktrace.c
index b69c18a26567d..26927344a2632 100644
--- a/arch/arm64/kvm/stacktrace.c
+++ b/arch/arm64/kvm/stacktrace.c
@@ -62,37 +62,54 @@ static bool kvm_nvhe_stack_kern_va(unsigned long *addr,
 	return true;
 }
 
-static bool on_overflow_stack(unsigned long sp, unsigned long size,
-			      struct stack_info *info)
+static struct stack_info stackinfo_get_overflow(void)
 {
 	struct kvm_nvhe_stacktrace_info *stacktrace_info
 				= this_cpu_ptr_nvhe_sym(kvm_stacktrace_info);
 	unsigned long low = (unsigned long)stacktrace_info->overflow_stack_base;
 	unsigned long high = low + OVERFLOW_STACK_SIZE;
 
-	return on_stack(sp, size, low, high, STACK_TYPE_OVERFLOW, info);
+	return (struct stack_info) {
+		.low = low,
+		.high = high,
+		.type = STACK_TYPE_OVERFLOW,
+	};
 }
 
-static bool on_hyp_stack(unsigned long sp, unsigned long size,
-			 struct stack_info *info)
+static struct stack_info stackinfo_get_hyp(void)
 {
 	struct kvm_nvhe_stacktrace_info *stacktrace_info
 				= this_cpu_ptr_nvhe_sym(kvm_stacktrace_info);
 	unsigned long low = (unsigned long)stacktrace_info->stack_base;
 	unsigned long high = low + PAGE_SIZE;
 
-	return on_stack(sp, size, low, high, STACK_TYPE_HYP, info);
+	return (struct stack_info) {
+		.low = low,
+		.high = high,
+		.type = STACK_TYPE_HYP,
+	};
 }
 
 static bool on_accessible_stack(const struct task_struct *tsk,
 				unsigned long sp, unsigned long size,
 				struct stack_info *info)
 {
-	if (info)
-		info->type = STACK_TYPE_UNKNOWN;
+	struct stack_info tmp;
 
-	return (on_overflow_stack(sp, size, info) ||
-		on_hyp_stack(sp, size, info));
+	tmp = stackinfo_get_overflow();
+	if (stackinfo_on_stack(&tmp, sp, size))
+		goto found;
+
+	tmp = stackinfo_get_hyp();
+	if (stackinfo_on_stack(&tmp, sp, size))
+		goto found;
+
+	*info = stackinfo_get_unknown();
+	return false;
+
+found:
+	*info = tmp;
+	return true;
 }
 
 static int unwind_next(struct unwind_state *state)
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel