* [PATCH v3 1/9] arm64: stacktrace: fix kerneldoc comments
2022-08-15 11:39 [PATCH v3 0/9] arm64: stacktrace: cleanups and improvements Mark Rutland
@ 2022-08-15 11:39 ` Mark Rutland
2022-08-16 15:59 ` Mark Brown
2022-08-23 5:11 ` Madhavan T. Venkataraman
2022-08-15 11:39 ` [PATCH v3 2/9] arm64: stacktrace: simplify unwind_next_common() Mark Rutland
` (7 subsequent siblings)
8 siblings, 2 replies; 12+ messages in thread
From: Mark Rutland @ 2022-08-15 11:39 UTC (permalink / raw)
To: linux-arm-kernel
Cc: broonie, catalin.marinas, james.morse, kaleshsingh, madvenka,
mark.rutland, maz, tabba, will
Many of the comment blocks in the arm64 stacktrace code are *almost*
kerneldoc, but not quite.
Convert them to kerneldoc, as was presumably originally intended.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Fuad Tabba <tabba@google.com>
Cc: Kalesh Singh <kaleshsingh@google.com>
Cc: Madhavan T. Venkataraman <madvenka@linux.microsoft.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Mark Brown <broonie@kernel.org>
---
arch/arm64/include/asm/stacktrace/common.h | 28 +++++++++++++---------
arch/arm64/include/asm/stacktrace/nvhe.h | 4 ++--
2 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/arch/arm64/include/asm/stacktrace/common.h b/arch/arm64/include/asm/stacktrace/common.h
index f58eb944c46fb..b8b20ef5aa5db 100644
--- a/arch/arm64/include/asm/stacktrace/common.h
+++ b/arch/arm64/include/asm/stacktrace/common.h
@@ -38,9 +38,8 @@ struct stack_info {
enum stack_type type;
};
-/*
- * A snapshot of a frame record or fp/lr register values, along with some
- * accounting information necessary for robust unwinding.
+/**
+ * struct unwind_state - state used for robust unwinding.
*
* @fp: The fp value in the frame record (or the real fp)
* @pc: The lr value in the frame record (or the real lr)
@@ -113,27 +112,34 @@ static inline void unwind_init_common(struct unwind_state *state,
state->prev_type = STACK_TYPE_UNKNOWN;
}
-/*
- * stack_trace_translate_fp_fn() - Translates a non-kernel frame pointer to
- * a kernel address.
+/**
+ * typedef stack_trace_translate_fp_fn() - Translates a non-kernel frame
+ * pointer to a kernel address.
*
* @fp: the frame pointer to be updated to its kernel address.
* @type: the stack type associated with frame pointer @fp
*
- * Returns true and success and @fp is updated to the corresponding
- * kernel virtual address; otherwise returns false.
+ * Return: true if the VA can be translated, false otherwise.
+ *
+ * Upon success @fp is updated to the corresponding kernel virtual address.
*/
typedef bool (*stack_trace_translate_fp_fn)(unsigned long *fp,
enum stack_type type);
-/*
- * on_accessible_stack_fn() - Check whether a stack range is on any
- * of the possible stacks.
+/**
+ * typedef on_accessible_stack_fn() - Check whether a stack range is on any of
+ * the possible stacks.
*
* @tsk: task whose stack is being unwound
* @sp: stack address being checked
* @size: size of the stack range being checked
* @info: stack unwinding context
+ *
+ * Return: true if the stack range is accessible, false otherwise.
+ *
+ * Upon success @info is updated with information for the relevant stack.
+ *
+ * Upon failure @info is updated with the UNKNOWN stack.
*/
typedef bool (*on_accessible_stack_fn)(const struct task_struct *tsk,
unsigned long sp, unsigned long size,
diff --git a/arch/arm64/include/asm/stacktrace/nvhe.h b/arch/arm64/include/asm/stacktrace/nvhe.h
index d5527b6003908..25ab83a315a76 100644
--- a/arch/arm64/include/asm/stacktrace/nvhe.h
+++ b/arch/arm64/include/asm/stacktrace/nvhe.h
@@ -20,8 +20,8 @@
#include <asm/stacktrace/common.h>
-/*
- * kvm_nvhe_unwind_init - Start an unwind from the given nVHE HYP fp and pc
+/**
+ * kvm_nvhe_unwind_init() - Start an unwind from the given nVHE HYP fp and pc
*
* @state : unwind_state to initialize
* @fp : frame pointer at which to start the unwinding.
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v3 1/9] arm64: stacktrace: fix kerneldoc comments
2022-08-15 11:39 ` [PATCH v3 1/9] arm64: stacktrace: fix kerneldoc comments Mark Rutland
@ 2022-08-16 15:59 ` Mark Brown
2022-08-23 5:11 ` Madhavan T. Venkataraman
1 sibling, 0 replies; 12+ messages in thread
From: Mark Brown @ 2022-08-16 15:59 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-arm-kernel, catalin.marinas, james.morse, kaleshsingh,
madvenka, maz, tabba, will
[-- Attachment #1.1: Type: text/plain, Size: 207 bytes --]
On Mon, Aug 15, 2022 at 12:39:14PM +0100, Mark Rutland wrote:
> Many of the comment blocks in the arm64 stacktrace code are *almost*
> kerneldoc, but not quite.
Reviewed-by: Mark Brown <broonie@kernel.org>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/9] arm64: stacktrace: fix kerneldoc comments
2022-08-15 11:39 ` [PATCH v3 1/9] arm64: stacktrace: fix kerneldoc comments Mark Rutland
2022-08-16 15:59 ` Mark Brown
@ 2022-08-23 5:11 ` Madhavan T. Venkataraman
1 sibling, 0 replies; 12+ messages in thread
From: Madhavan T. Venkataraman @ 2022-08-23 5:11 UTC (permalink / raw)
To: Mark Rutland, linux-arm-kernel
Cc: broonie, catalin.marinas, james.morse, kaleshsingh, maz, tabba,
will
Reviewed-by: Madhavan T. Venkataraman <madvenka@linux.microsoft.com>
On 8/15/22 06:39, Mark Rutland wrote:
> Many of the comment blocks in the arm64 stacktrace code are *almost*
> kerneldoc, but not quite.
>
> Convert them to kerneldoc, as was presumably originally intended.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Fuad Tabba <tabba@google.com>
> Cc: Kalesh Singh <kaleshsingh@google.com>
> Cc: Madhavan T. Venkataraman <madvenka@linux.microsoft.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Mark Brown <broonie@kernel.org>
> ---
> arch/arm64/include/asm/stacktrace/common.h | 28 +++++++++++++---------
> arch/arm64/include/asm/stacktrace/nvhe.h | 4 ++--
> 2 files changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/arch/arm64/include/asm/stacktrace/common.h b/arch/arm64/include/asm/stacktrace/common.h
> index f58eb944c46fb..b8b20ef5aa5db 100644
> --- a/arch/arm64/include/asm/stacktrace/common.h
> +++ b/arch/arm64/include/asm/stacktrace/common.h
> @@ -38,9 +38,8 @@ struct stack_info {
> enum stack_type type;
> };
>
> -/*
> - * A snapshot of a frame record or fp/lr register values, along with some
> - * accounting information necessary for robust unwinding.
> +/**
> + * struct unwind_state - state used for robust unwinding.
> *
> * @fp: The fp value in the frame record (or the real fp)
> * @pc: The lr value in the frame record (or the real lr)
> @@ -113,27 +112,34 @@ static inline void unwind_init_common(struct unwind_state *state,
> state->prev_type = STACK_TYPE_UNKNOWN;
> }
>
> -/*
> - * stack_trace_translate_fp_fn() - Translates a non-kernel frame pointer to
> - * a kernel address.
> +/**
> + * typedef stack_trace_translate_fp_fn() - Translates a non-kernel frame
> + * pointer to a kernel address.
> *
> * @fp: the frame pointer to be updated to its kernel address.
> * @type: the stack type associated with frame pointer @fp
> *
> - * Returns true and success and @fp is updated to the corresponding
> - * kernel virtual address; otherwise returns false.
> + * Return: true if the VA can be translated, false otherwise.
> + *
> + * Upon success @fp is updated to the corresponding kernel virtual address.
> */
> typedef bool (*stack_trace_translate_fp_fn)(unsigned long *fp,
> enum stack_type type);
>
> -/*
> - * on_accessible_stack_fn() - Check whether a stack range is on any
> - * of the possible stacks.
> +/**
> + * typedef on_accessible_stack_fn() - Check whether a stack range is on any of
> + * the possible stacks.
> *
> * @tsk: task whose stack is being unwound
> * @sp: stack address being checked
> * @size: size of the stack range being checked
> * @info: stack unwinding context
> + *
> + * Return: true if the stack range is accessible, false otherwise.
> + *
> + * Upon success @info is updated with information for the relevant stack.
> + *
> + * Upon failure @info is updated with the UNKNOWN stack.
> */
> typedef bool (*on_accessible_stack_fn)(const struct task_struct *tsk,
> unsigned long sp, unsigned long size,
> diff --git a/arch/arm64/include/asm/stacktrace/nvhe.h b/arch/arm64/include/asm/stacktrace/nvhe.h
> index d5527b6003908..25ab83a315a76 100644
> --- a/arch/arm64/include/asm/stacktrace/nvhe.h
> +++ b/arch/arm64/include/asm/stacktrace/nvhe.h
> @@ -20,8 +20,8 @@
>
> #include <asm/stacktrace/common.h>
>
> -/*
> - * kvm_nvhe_unwind_init - Start an unwind from the given nVHE HYP fp and pc
> +/**
> + * kvm_nvhe_unwind_init() - Start an unwind from the given nVHE HYP fp and pc
> *
> * @state : unwind_state to initialize
> * @fp : frame pointer at which to start the unwinding.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 2/9] arm64: stacktrace: simplify unwind_next_common()
2022-08-15 11:39 [PATCH v3 0/9] arm64: stacktrace: cleanups and improvements Mark Rutland
2022-08-15 11:39 ` [PATCH v3 1/9] arm64: stacktrace: fix kerneldoc comments Mark Rutland
@ 2022-08-15 11:39 ` Mark Rutland
2022-08-15 11:39 ` [PATCH v3 3/9] arm64: stacktrace: rename unwind_next_common() -> unwind_next_frame_record() Mark Rutland
` (6 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Mark Rutland @ 2022-08-15 11:39 UTC (permalink / raw)
To: linux-arm-kernel
Cc: broonie, catalin.marinas, james.morse, kaleshsingh, madvenka,
mark.rutland, maz, tabba, will
Currently unwind_next_common() takes a pointer to a stack_info which is
only ever used within unwind_next_common().
Make it a local variable and simplify callers.
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: Madhavan T. Venkataraman <madvenka@linux.microsoft.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
Cc: Fuad Tabba <tabba@google.com>
Cc: Marc Zyngier <maz@kernel.org>
---
arch/arm64/include/asm/stacktrace/common.h | 12 ++++++------
arch/arm64/kernel/stacktrace.c | 3 +--
arch/arm64/kvm/hyp/nvhe/stacktrace.c | 4 +---
arch/arm64/kvm/stacktrace.c | 4 +---
4 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/arch/arm64/include/asm/stacktrace/common.h b/arch/arm64/include/asm/stacktrace/common.h
index b8b20ef5aa5db..0fbae7c78b70d 100644
--- a/arch/arm64/include/asm/stacktrace/common.h
+++ b/arch/arm64/include/asm/stacktrace/common.h
@@ -146,27 +146,27 @@ typedef bool (*on_accessible_stack_fn)(const struct task_struct *tsk,
struct stack_info *info);
static inline int unwind_next_common(struct unwind_state *state,
- struct stack_info *info,
on_accessible_stack_fn accessible,
stack_trace_translate_fp_fn translate_fp)
{
+ struct stack_info info;
unsigned long fp = state->fp, kern_fp = fp;
struct task_struct *tsk = state->task;
if (fp & 0x7)
return -EINVAL;
- if (!accessible(tsk, fp, 16, info))
+ if (!accessible(tsk, fp, 16, &info))
return -EINVAL;
- if (test_bit(info->type, state->stacks_done))
+ if (test_bit(info.type, state->stacks_done))
return -EINVAL;
/*
* If fp is not from the current address space perform the necessary
* translation before dereferencing it to get the next fp.
*/
- if (translate_fp && !translate_fp(&kern_fp, info->type))
+ if (translate_fp && !translate_fp(&kern_fp, info.type))
return -EINVAL;
/*
@@ -183,7 +183,7 @@ static inline int unwind_next_common(struct unwind_state *state,
* stack to another, it's never valid to unwind back to that first
* stack.
*/
- if (info->type == state->prev_type) {
+ if (info.type == state->prev_type) {
if (fp <= state->prev_fp)
return -EINVAL;
} else {
@@ -197,7 +197,7 @@ static inline int unwind_next_common(struct unwind_state *state,
state->fp = READ_ONCE(*(unsigned long *)(kern_fp));
state->pc = READ_ONCE(*(unsigned long *)(kern_fp + 8));
state->prev_fp = fp;
- state->prev_type = info->type;
+ state->prev_type = info.type;
return 0;
}
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index ce190ee18a201..056fb045d0e0c 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -103,14 +103,13 @@ static int notrace unwind_next(struct unwind_state *state)
{
struct task_struct *tsk = state->task;
unsigned long fp = state->fp;
- struct stack_info info;
int err;
/* Final frame; nothing to unwind */
if (fp == (unsigned long)task_pt_regs(tsk)->stackframe)
return -ENOENT;
- err = unwind_next_common(state, &info, on_accessible_stack, NULL);
+ err = unwind_next_common(state, on_accessible_stack, NULL);
if (err)
return err;
diff --git a/arch/arm64/kvm/hyp/nvhe/stacktrace.c b/arch/arm64/kvm/hyp/nvhe/stacktrace.c
index 58f645ad66bcb..50a1fa6b5c044 100644
--- a/arch/arm64/kvm/hyp/nvhe/stacktrace.c
+++ b/arch/arm64/kvm/hyp/nvhe/stacktrace.c
@@ -71,9 +71,7 @@ static bool on_accessible_stack(const struct task_struct *tsk,
static int unwind_next(struct unwind_state *state)
{
- struct stack_info info;
-
- return unwind_next_common(state, &info, on_accessible_stack, NULL);
+ return unwind_next_common(state, on_accessible_stack, NULL);
}
static void notrace unwind(struct unwind_state *state,
diff --git a/arch/arm64/kvm/stacktrace.c b/arch/arm64/kvm/stacktrace.c
index 949d19d603fba..b9cf551d9d31d 100644
--- a/arch/arm64/kvm/stacktrace.c
+++ b/arch/arm64/kvm/stacktrace.c
@@ -97,9 +97,7 @@ static bool on_accessible_stack(const struct task_struct *tsk,
static int unwind_next(struct unwind_state *state)
{
- struct stack_info info;
-
- return unwind_next_common(state, &info, on_accessible_stack,
+ return unwind_next_common(state, on_accessible_stack,
kvm_nvhe_stack_kern_va);
}
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3 3/9] arm64: stacktrace: rename unwind_next_common() -> unwind_next_frame_record()
2022-08-15 11:39 [PATCH v3 0/9] arm64: stacktrace: cleanups and improvements Mark Rutland
2022-08-15 11:39 ` [PATCH v3 1/9] arm64: stacktrace: fix kerneldoc comments Mark Rutland
2022-08-15 11:39 ` [PATCH v3 2/9] arm64: stacktrace: simplify unwind_next_common() Mark Rutland
@ 2022-08-15 11:39 ` Mark Rutland
2022-08-15 11:39 ` [PATCH v3 4/9] arm64: stacktrace: move SDEI stack helpers to stacktrace code Mark Rutland
` (5 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Mark Rutland @ 2022-08-15 11:39 UTC (permalink / raw)
To: linux-arm-kernel
Cc: broonie, catalin.marinas, james.morse, kaleshsingh, madvenka,
mark.rutland, maz, tabba, will
The unwind_next_common() function unwinds a single frame record. There
are other unwind steps (e.g. unwinding through trampolines) which are
handled in the regular kernel unwinder, and in future there may be other
common unwind helpers.
Clarify the purpose of unwind_next_common() by renaming it to
unwind_next_frame_record(). At the same time, add commentary, and delete
the redundant comment at the top of asm/stacktrace/common.h.
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: Madhavan T. Venkataraman <madvenka@linux.microsoft.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
Cc: Fuad Tabba <tabba@google.com>
Cc: Marc Zyngier <maz@kernel.org>
---
arch/arm64/include/asm/stacktrace/common.h | 23 ++++++++++++----------
arch/arm64/kernel/stacktrace.c | 2 +-
arch/arm64/kvm/hyp/nvhe/stacktrace.c | 2 +-
arch/arm64/kvm/stacktrace.c | 4 ++--
4 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/arch/arm64/include/asm/stacktrace/common.h b/arch/arm64/include/asm/stacktrace/common.h
index 0fbae7c78b70d..a74fa301fe953 100644
--- a/arch/arm64/include/asm/stacktrace/common.h
+++ b/arch/arm64/include/asm/stacktrace/common.h
@@ -2,13 +2,6 @@
/*
* Common arm64 stack unwinder code.
*
- * To implement a new arm64 stack unwinder:
- * 1) Include this header
- *
- * 2) Call into unwind_next_common() from your top level unwind
- * function, passing it the validation and translation callbacks
- * (though the later can be NULL if no translation is required).
- *
* See: arch/arm64/kernel/stacktrace.c for the reference implementation.
*
* Copyright (C) 2012 ARM Ltd.
@@ -145,9 +138,19 @@ typedef bool (*on_accessible_stack_fn)(const struct task_struct *tsk,
unsigned long sp, unsigned long size,
struct stack_info *info);
-static inline int unwind_next_common(struct unwind_state *state,
- on_accessible_stack_fn accessible,
- stack_trace_translate_fp_fn translate_fp)
+/**
+ * unwind_next_frame_record() - Unwind to the next frame record.
+ *
+ * @state: the current unwind state.
+ * @accessible: determines whether the frame record is accessible
+ * @translate_fp: translates the fp prior to access (may be NULL)
+ *
+ * Return: 0 upon success, an error code otherwise.
+ */
+static inline int
+unwind_next_frame_record(struct unwind_state *state,
+ on_accessible_stack_fn accessible,
+ stack_trace_translate_fp_fn translate_fp)
{
struct stack_info info;
unsigned long fp = state->fp, kern_fp = fp;
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index 056fb045d0e0c..4c8865e495fea 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -109,7 +109,7 @@ static int notrace unwind_next(struct unwind_state *state)
if (fp == (unsigned long)task_pt_regs(tsk)->stackframe)
return -ENOENT;
- err = unwind_next_common(state, on_accessible_stack, NULL);
+ err = unwind_next_frame_record(state, on_accessible_stack, NULL);
if (err)
return err;
diff --git a/arch/arm64/kvm/hyp/nvhe/stacktrace.c b/arch/arm64/kvm/hyp/nvhe/stacktrace.c
index 50a1fa6b5c044..579b46aa9a553 100644
--- a/arch/arm64/kvm/hyp/nvhe/stacktrace.c
+++ b/arch/arm64/kvm/hyp/nvhe/stacktrace.c
@@ -71,7 +71,7 @@ static bool on_accessible_stack(const struct task_struct *tsk,
static int unwind_next(struct unwind_state *state)
{
- return unwind_next_common(state, on_accessible_stack, NULL);
+ return unwind_next_frame_record(state, on_accessible_stack, NULL);
}
static void notrace unwind(struct unwind_state *state,
diff --git a/arch/arm64/kvm/stacktrace.c b/arch/arm64/kvm/stacktrace.c
index b9cf551d9d31d..b69c18a26567d 100644
--- a/arch/arm64/kvm/stacktrace.c
+++ b/arch/arm64/kvm/stacktrace.c
@@ -97,8 +97,8 @@ static bool on_accessible_stack(const struct task_struct *tsk,
static int unwind_next(struct unwind_state *state)
{
- return unwind_next_common(state, on_accessible_stack,
- kvm_nvhe_stack_kern_va);
+ return unwind_next_frame_record(state, on_accessible_stack,
+ kvm_nvhe_stack_kern_va);
}
static void unwind(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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3 4/9] arm64: stacktrace: move SDEI stack helpers to stacktrace code
2022-08-15 11:39 [PATCH v3 0/9] arm64: stacktrace: cleanups and improvements Mark Rutland
` (2 preceding siblings ...)
2022-08-15 11:39 ` [PATCH v3 3/9] arm64: stacktrace: rename unwind_next_common() -> unwind_next_frame_record() Mark Rutland
@ 2022-08-15 11:39 ` Mark Rutland
2022-08-15 11:39 ` [PATCH v3 5/9] arm64: stacktrace: add stackinfo_on_stack() helper Mark Rutland
` (4 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Mark Rutland @ 2022-08-15 11:39 UTC (permalink / raw)
To: linux-arm-kernel
Cc: broonie, catalin.marinas, james.morse, kaleshsingh, madvenka,
mark.rutland, maz, tabba, will
For clarity and ease of maintenance, it would be helpful for all the
stack helpers to be in the same place.
Move the SDEI stack helpers into the stacktrace code where all the other
stack helpers live.
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: Madhavan T. Venkataraman <madvenka@linux.microsoft.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Fuad Tabba <tabba@google.com>
Cc: James Morse <james.morse@arm.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/sdei.h | 17 ------------
arch/arm64/include/asm/stacktrace.h | 40 ++++++++++++++++++++++++++++-
arch/arm64/kernel/sdei.c | 32 -----------------------
arch/arm64/kernel/stacktrace.c | 11 ++++++--
4 files changed, 48 insertions(+), 52 deletions(-)
diff --git a/arch/arm64/include/asm/sdei.h b/arch/arm64/include/asm/sdei.h
index 7bea1d705dd64..4292d9bafb9d2 100644
--- a/arch/arm64/include/asm/sdei.h
+++ b/arch/arm64/include/asm/sdei.h
@@ -43,22 +43,5 @@ unsigned long do_sdei_event(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)
-struct stack_info;
-
-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))
- return false;
- if (!IS_ENABLED(CONFIG_ARM_SDE_INTERFACE))
- return false;
- if (in_nmi())
- return _on_sdei_stack(sp, size, info);
-
- return false;
-}
-
#endif /* __ASSEMBLY__ */
#endif /* __ASM_SDEI_H */
diff --git a/arch/arm64/include/asm/stacktrace.h b/arch/arm64/include/asm/stacktrace.h
index 6ebdcdff77f56..fa2df1ea22ebc 100644
--- a/arch/arm64/include/asm/stacktrace.h
+++ b/arch/arm64/include/asm/stacktrace.h
@@ -54,7 +54,45 @@ static inline bool on_overflow_stack(unsigned long sp, unsigned long size,
}
#else
static inline bool on_overflow_stack(unsigned long sp, unsigned long size,
- struct stack_info *info) { return false; }
+ struct stack_info *info)
+{
+ return false;
+}
+#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)
+{
+ 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);
+}
+
+static inline 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, size, low, high, STACK_TYPE_SDEI_CRITICAL, info);
+}
+#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;
+}
#endif
#endif /* __ASM_STACKTRACE_H */
diff --git a/arch/arm64/kernel/sdei.c b/arch/arm64/kernel/sdei.c
index d20620a1c51a4..d56e170e1ca7c 100644
--- a/arch/arm64/kernel/sdei.c
+++ b/arch/arm64/kernel/sdei.c
@@ -162,38 +162,6 @@ static int init_sdei_scs(void)
return err;
}
-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, size, low, high, STACK_TYPE_SDEI_NORMAL, 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, size, low, high, STACK_TYPE_SDEI_CRITICAL, 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, size, info))
- return true;
-
- if (on_sdei_normal_stack(sp, size, info))
- return true;
-
- 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 4c8865e495fea..edf9edca20552 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -86,8 +86,15 @@ static bool on_accessible_stack(const struct task_struct *tsk,
return true;
if (on_overflow_stack(sp, size, info))
return true;
- if (on_sdei_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;
+ }
return false;
}
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3 5/9] arm64: stacktrace: add stackinfo_on_stack() helper
2022-08-15 11:39 [PATCH v3 0/9] arm64: stacktrace: cleanups and improvements Mark Rutland
` (3 preceding siblings ...)
2022-08-15 11:39 ` [PATCH v3 4/9] arm64: stacktrace: move SDEI stack helpers to stacktrace code Mark Rutland
@ 2022-08-15 11:39 ` Mark Rutland
2022-08-15 11:39 ` [PATCH v3 6/9] arm64: stacktrace: rework stack boundary discovery Mark Rutland
` (3 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Mark Rutland @ 2022-08-15 11:39 UTC (permalink / raw)
To: linux-arm-kernel
Cc: broonie, catalin.marinas, james.morse, kaleshsingh, madvenka,
mark.rutland, maz, tabba, will
Factor the core predicate out of on_stack() into a helper which can be
used on a pre-populated stack_info.
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: Madhavan T. Venkataraman <madvenka@linux.microsoft.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
Cc: Fuad Tabba <tabba@google.com>
Cc: Marc Zyngier <maz@kernel.org>
---
arch/arm64/include/asm/stacktrace/common.h | 29 ++++++++++++++++------
1 file changed, 21 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/include/asm/stacktrace/common.h b/arch/arm64/include/asm/stacktrace/common.h
index a74fa301fe953..81c21378b1ac9 100644
--- a/arch/arm64/include/asm/stacktrace/common.h
+++ b/arch/arm64/include/asm/stacktrace/common.h
@@ -65,21 +65,34 @@ struct unwind_state {
struct task_struct *task;
};
+static inline bool stackinfo_on_stack(const struct stack_info *info,
+ unsigned long sp, unsigned long size)
+{
+ if (!info->low)
+ return false;
+
+ if (sp < info->low || sp + size < sp || sp + size > info->high)
+ return false;
+
+ 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)
{
- if (!low)
- return false;
+ struct stack_info tmp = {
+ .low = low,
+ .high = high,
+ .type = type,
+ };
- if (sp < low || sp + size < sp || sp + size > high)
+ if (!stackinfo_on_stack(&tmp, sp, size))
return false;
- if (info) {
- info->low = low;
- info->high = high;
- info->type = type;
- }
+ if (info)
+ *info = tmp;
+
return true;
}
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3 6/9] arm64: stacktrace: rework stack boundary discovery
2022-08-15 11:39 [PATCH v3 0/9] arm64: stacktrace: cleanups and improvements Mark Rutland
` (4 preceding siblings ...)
2022-08-15 11:39 ` [PATCH v3 5/9] arm64: stacktrace: add stackinfo_on_stack() helper Mark Rutland
@ 2022-08-15 11:39 ` Mark Rutland
2022-08-15 11:39 ` [PATCH v3 7/9] arm64: stacktrace: remove stack type from fp translator Mark Rutland
` (2 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Mark Rutland @ 2022-08-15 11:39 UTC (permalink / raw)
To: linux-arm-kernel
Cc: broonie, catalin.marinas, james.morse, kaleshsingh, madvenka,
mark.rutland, maz, tabba, will
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: Madhavan T. Venkataraman <madvenka@linux.microsoft.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
Cc: Fuad Tabba <tabba@google.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 86eb0bfe3b380..61883518fc50b 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -410,7 +410,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 81c21378b1ac9..3c3bda34bb872 100644
--- a/arch/arm64/include/asm/stacktrace/common.h
+++ b/arch/arm64/include/asm/stacktrace/common.h
@@ -65,6 +65,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)
{
@@ -77,25 +86,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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3 7/9] arm64: stacktrace: remove stack type from fp translator
2022-08-15 11:39 [PATCH v3 0/9] arm64: stacktrace: cleanups and improvements Mark Rutland
` (5 preceding siblings ...)
2022-08-15 11:39 ` [PATCH v3 6/9] arm64: stacktrace: rework stack boundary discovery Mark Rutland
@ 2022-08-15 11:39 ` Mark Rutland
2022-08-15 11:39 ` [PATCH v3 8/9] arm64: stacktrace: track all stack boundaries explicitly Mark Rutland
2022-08-15 11:39 ` [PATCH v3 9/9] arm64: stacktrace: track hyp stacks in unwinder's address space Mark Rutland
8 siblings, 0 replies; 12+ messages in thread
From: Mark Rutland @ 2022-08-15 11:39 UTC (permalink / raw)
To: linux-arm-kernel
Cc: broonie, catalin.marinas, james.morse, kaleshsingh, madvenka,
mark.rutland, maz, tabba, will
In subsequent patches we'll remove the stack_type enum, and move the FP
translation logic out of the raw FP unwind code.
In preparation for doing so, this patch removes the type parameter from
the FP translation callback, and modifies kvm_nvhe_stack_kern_va() to
determine the relevant stack directly.
So that kvm_nvhe_stack_kern_va() can use the stackinfo_*() helpers,
these are moved earlier in the file, but are not modified in any way.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Kalesh Singh <kaleshsingh@google.com>
Reviewed-by: Madhavan T. Venkataraman <madvenka@linux.microsoft.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
Cc: Fuad Tabba <tabba@google.com>
Cc: Marc Zyngier <maz@kernel.org>
---
arch/arm64/include/asm/stacktrace/common.h | 6 +-
arch/arm64/kvm/stacktrace.c | 82 +++++++++++++---------
2 files changed, 49 insertions(+), 39 deletions(-)
diff --git a/arch/arm64/include/asm/stacktrace/common.h b/arch/arm64/include/asm/stacktrace/common.h
index 3c3bda34bb872..2f972441f572e 100644
--- a/arch/arm64/include/asm/stacktrace/common.h
+++ b/arch/arm64/include/asm/stacktrace/common.h
@@ -113,14 +113,12 @@ static inline void unwind_init_common(struct unwind_state *state,
* pointer to a kernel address.
*
* @fp: the frame pointer to be updated to its kernel address.
- * @type: the stack type associated with frame pointer @fp
*
* Return: true if the VA can be translated, false otherwise.
*
* Upon success @fp is updated to the corresponding kernel virtual address.
*/
-typedef bool (*stack_trace_translate_fp_fn)(unsigned long *fp,
- enum stack_type type);
+typedef bool (*stack_trace_translate_fp_fn)(unsigned long *fp);
/**
* typedef on_accessible_stack_fn() - Check whether a stack range is on any of
@@ -172,7 +170,7 @@ unwind_next_frame_record(struct unwind_state *state,
* If fp is not from the current address space perform the necessary
* translation before dereferencing it to get the next fp.
*/
- if (translate_fp && !translate_fp(&kern_fp, info.type))
+ if (translate_fp && !translate_fp(&kern_fp))
return -EINVAL;
/*
diff --git a/arch/arm64/kvm/stacktrace.c b/arch/arm64/kvm/stacktrace.c
index 26927344a2632..7658c5db47c1d 100644
--- a/arch/arm64/kvm/stacktrace.c
+++ b/arch/arm64/kvm/stacktrace.c
@@ -21,6 +21,34 @@
#include <asm/stacktrace/nvhe.h>
+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 (struct stack_info) {
+ .low = low,
+ .high = high,
+ .type = STACK_TYPE_OVERFLOW,
+ };
+}
+
+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 (struct stack_info) {
+ .low = low,
+ .high = high,
+ .type = STACK_TYPE_HYP,
+ };
+}
+
/*
* kvm_nvhe_stack_kern_va - Convert KVM nVHE HYP stack addresses to a kernel VAs
*
@@ -34,27 +62,31 @@
* Returns true on success and updates @addr to its corresponding kernel VA;
* otherwise returns false.
*/
-static bool kvm_nvhe_stack_kern_va(unsigned long *addr,
- enum stack_type type)
+static bool kvm_nvhe_stack_kern_va(unsigned long *addr, unsigned long size)
{
struct kvm_nvhe_stacktrace_info *stacktrace_info;
unsigned long hyp_base, kern_base, hyp_offset;
+ struct stack_info stack;
stacktrace_info = this_cpu_ptr_nvhe_sym(kvm_stacktrace_info);
- switch (type) {
- case STACK_TYPE_HYP:
+ stack = stackinfo_get_hyp();
+ if (stackinfo_on_stack(&stack, *addr, size)) {
kern_base = (unsigned long)*this_cpu_ptr(&kvm_arm_hyp_stack_page);
hyp_base = (unsigned long)stacktrace_info->stack_base;
- break;
- case STACK_TYPE_OVERFLOW:
+ goto found;
+ }
+
+ stack = stackinfo_get_overflow();
+ if (stackinfo_on_stack(&stack, *addr, size)) {
kern_base = (unsigned long)this_cpu_ptr_nvhe_sym(overflow_stack);
hyp_base = (unsigned long)stacktrace_info->overflow_stack_base;
- break;
- default:
- return false;
+ goto found;
}
+ return false;
+
+found:
hyp_offset = *addr - hyp_base;
*addr = kern_base + hyp_offset;
@@ -62,32 +94,12 @@ static bool kvm_nvhe_stack_kern_va(unsigned long *addr,
return true;
}
-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 (struct stack_info) {
- .low = low,
- .high = high,
- .type = STACK_TYPE_OVERFLOW,
- };
-}
-
-static struct stack_info stackinfo_get_hyp(void)
+/*
+ * Convert a KVN nVHE HYP frame record address to a kernel VA
+ */
+static bool kvm_nvhe_stack_kern_record_va(unsigned long *addr)
{
- 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 (struct stack_info) {
- .low = low,
- .high = high,
- .type = STACK_TYPE_HYP,
- };
+ return kvm_nvhe_stack_kern_va(addr, 16);
}
static bool on_accessible_stack(const struct task_struct *tsk,
@@ -115,7 +127,7 @@ static bool on_accessible_stack(const struct task_struct *tsk,
static int unwind_next(struct unwind_state *state)
{
return unwind_next_frame_record(state, on_accessible_stack,
- kvm_nvhe_stack_kern_va);
+ kvm_nvhe_stack_kern_record_va);
}
static void unwind(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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3 8/9] arm64: stacktrace: track all stack boundaries explicitly
2022-08-15 11:39 [PATCH v3 0/9] arm64: stacktrace: cleanups and improvements Mark Rutland
` (6 preceding siblings ...)
2022-08-15 11:39 ` [PATCH v3 7/9] arm64: stacktrace: remove stack type from fp translator Mark Rutland
@ 2022-08-15 11:39 ` Mark Rutland
2022-08-15 11:39 ` [PATCH v3 9/9] arm64: stacktrace: track hyp stacks in unwinder's address space Mark Rutland
8 siblings, 0 replies; 12+ messages in thread
From: Mark Rutland @ 2022-08-15 11:39 UTC (permalink / raw)
To: linux-arm-kernel
Cc: broonie, catalin.marinas, james.morse, kaleshsingh, madvenka,
mark.rutland, maz, tabba, will
Currently we call an on_accessible_stack() callback for each step of the
unwinder, requiring redundant work to be performed in the core of the
unwind loop (e.g. disabling preemption around accesses to per-cpu
variables containing stack boundaries). To prevent unwind loops which go
through a stack multiple times, we have to track the set of unwound
stacks, requiring a stack_type enum which needs to cater for all the
stacks of all possible callees. To prevent loops within a stack, we must
track the prior FP values.
This patch reworks the unwinder to minimize the work in the core of the
unwinder, and to remove the need for the stack_type enum. The set of
accessible stacks (and their boundaries) are determined at the start of
the unwind, and the current stack is tracked during the unwind, with
completed stacks removed from the set of accessible stacks. This makes
the boundary checks more accurate (e.g. detecting overlapped frame
records), and removes the need for separate tracking of the prior FP and
visited stacks.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Kalesh Singh <kaleshsingh@google.com>
Reviewed-by: Madhavan T. Venkataraman <madvenka@linux.microsoft.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
Cc: Fuad Tabba <tabba@google.com>
Cc: Marc Zyngier <maz@kernel.org>
---
arch/arm64/include/asm/stacktrace.h | 5 -
arch/arm64/include/asm/stacktrace/common.h | 164 ++++++++++-----------
arch/arm64/kernel/stacktrace.c | 91 +++++-------
arch/arm64/kvm/hyp/nvhe/stacktrace.c | 35 ++---
arch/arm64/kvm/stacktrace.c | 36 ++---
5 files changed, 132 insertions(+), 199 deletions(-)
diff --git a/arch/arm64/include/asm/stacktrace.h b/arch/arm64/include/asm/stacktrace.h
index aad0c6258721d..5a0edb064ea47 100644
--- a/arch/arm64/include/asm/stacktrace.h
+++ b/arch/arm64/include/asm/stacktrace.h
@@ -30,7 +30,6 @@ static inline struct stack_info stackinfo_get_irq(void)
return (struct stack_info) {
.low = low,
.high = high,
- .type = STACK_TYPE_IRQ,
};
}
@@ -48,7 +47,6 @@ static inline struct stack_info stackinfo_get_task(const struct task_struct *tsk
return (struct stack_info) {
.low = low,
.high = high,
- .type = STACK_TYPE_TASK,
};
}
@@ -70,7 +68,6 @@ static inline struct stack_info stackinfo_get_overflow(void)
return (struct stack_info) {
.low = low,
.high = high,
- .type = STACK_TYPE_OVERFLOW,
};
}
#else
@@ -89,7 +86,6 @@ static inline struct stack_info stackinfo_get_sdei_normal(void)
return (struct stack_info) {
.low = low,
.high = high,
- .type = STACK_TYPE_SDEI_NORMAL,
};
}
@@ -101,7 +97,6 @@ static inline struct stack_info stackinfo_get_sdei_critical(void)
return (struct stack_info) {
.low = low,
.high = high,
- .type = STACK_TYPE_SDEI_CRITICAL,
};
}
#else
diff --git a/arch/arm64/include/asm/stacktrace/common.h b/arch/arm64/include/asm/stacktrace/common.h
index 2f972441f572e..638008f485972 100644
--- a/arch/arm64/include/asm/stacktrace/common.h
+++ b/arch/arm64/include/asm/stacktrace/common.h
@@ -9,26 +9,12 @@
#ifndef __ASM_STACKTRACE_COMMON_H
#define __ASM_STACKTRACE_COMMON_H
-#include <linux/bitmap.h>
-#include <linux/bitops.h>
#include <linux/kprobes.h>
#include <linux/types.h>
-enum stack_type {
- STACK_TYPE_UNKNOWN,
- STACK_TYPE_TASK,
- STACK_TYPE_IRQ,
- STACK_TYPE_OVERFLOW,
- STACK_TYPE_SDEI_NORMAL,
- STACK_TYPE_SDEI_CRITICAL,
- STACK_TYPE_HYP,
- __NR_STACK_TYPES
-};
-
struct stack_info {
unsigned long low;
unsigned long high;
- enum stack_type type;
};
/**
@@ -37,32 +23,27 @@ struct stack_info {
* @fp: The fp value in the frame record (or the real fp)
* @pc: The lr value in the frame record (or the real lr)
*
- * @stacks_done: Stacks which have been entirely unwound, for which it is no
- * longer valid to unwind to.
- *
- * @prev_fp: The fp that pointed to this frame record, or a synthetic value
- * of 0. This is used to ensure that within a stack, each
- * subsequent frame record is at an increasing address.
- * @prev_type: The type of stack this frame record was on, or a synthetic
- * value of STACK_TYPE_UNKNOWN. This is used to detect a
- * transition from one stack to another.
- *
* @kr_cur: When KRETPROBES is selected, holds the kretprobe instance
* associated with the most recently encountered replacement lr
* value.
*
* @task: The task being unwound.
+ *
+ * @stack: The stack currently being unwound.
+ * @stacks: An array of stacks which can be unwound.
+ * @nr_stacks: The number of stacks in @stacks.
*/
struct unwind_state {
unsigned long fp;
unsigned long pc;
- DECLARE_BITMAP(stacks_done, __NR_STACK_TYPES);
- unsigned long prev_fp;
- enum stack_type prev_type;
#ifdef CONFIG_KRETPROBES
struct llist_node *kr_cur;
#endif
struct task_struct *task;
+
+ struct stack_info stack;
+ struct stack_info *stacks;
+ int nr_stacks;
};
static inline struct stack_info stackinfo_get_unknown(void)
@@ -70,7 +51,6 @@ static inline struct stack_info stackinfo_get_unknown(void)
return (struct stack_info) {
.low = 0,
.high = 0,
- .type = STACK_TYPE_UNKNOWN,
};
}
@@ -94,18 +74,7 @@ static inline void unwind_init_common(struct unwind_state *state,
state->kr_cur = NULL;
#endif
- /*
- * Prime the first unwind.
- *
- * In unwind_next() we'll check that the FP points to a valid stack,
- * which can't be STACK_TYPE_UNKNOWN, and the first unwind will be
- * treated as a transition to whichever stack that happens to be. The
- * prev_fp value won't be used, but we set it to 0 such that it is
- * definitely not an accessible stack address.
- */
- bitmap_zero(state->stacks_done, __NR_STACK_TYPES);
- state->prev_fp = 0;
- state->prev_type = STACK_TYPE_UNKNOWN;
+ state->stack = stackinfo_get_unknown();
}
/**
@@ -120,51 +89,94 @@ static inline void unwind_init_common(struct unwind_state *state,
*/
typedef bool (*stack_trace_translate_fp_fn)(unsigned long *fp);
+static struct stack_info *unwind_find_next_stack(const struct unwind_state *state,
+ unsigned long sp,
+ unsigned long size)
+{
+ for (int i = 0; i < state->nr_stacks; i++) {
+ struct stack_info *info = &state->stacks[i];
+
+ if (stackinfo_on_stack(info, sp, size))
+ return info;
+ }
+
+ return NULL;
+}
+
/**
- * typedef on_accessible_stack_fn() - Check whether a stack range is on any of
- * the possible stacks.
- *
- * @tsk: task whose stack is being unwound
- * @sp: stack address being checked
- * @size: size of the stack range being checked
- * @info: stack unwinding context
- *
- * Return: true if the stack range is accessible, false otherwise.
+ * unwind_consume_stack() - Check if an object is on an accessible stack,
+ * updating stack boundaries so that future unwind steps cannot consume this
+ * object again.
*
- * Upon success @info is updated with information for the relevant stack.
+ * @state: the current unwind state.
+ * @sp: the base address of the object.
+ * @size: the size of the object.
*
- * Upon failure @info is updated with the UNKNOWN stack.
+ * Return: 0 upon success, an error code otherwise.
*/
-typedef bool (*on_accessible_stack_fn)(const struct task_struct *tsk,
- unsigned long sp, unsigned long size,
- struct stack_info *info);
+static inline int unwind_consume_stack(struct unwind_state *state,
+ unsigned long sp,
+ unsigned long size)
+{
+ struct stack_info *next;
+
+ if (stackinfo_on_stack(&state->stack, sp, size))
+ goto found;
+
+ next = unwind_find_next_stack(state, sp, size);
+ if (!next)
+ return -EINVAL;
+
+ /*
+ * Stack transitions are strictly one-way, and once we've
+ * transitioned from one stack to another, it's never valid to
+ * unwind back to the old stack.
+ *
+ * Remove the current stack from the list of stacks so that it cannot
+ * be found on a subsequent transition.
+ *
+ * Note that stacks can nest in several valid orders, e.g.
+ *
+ * TASK -> IRQ -> OVERFLOW -> SDEI_NORMAL
+ * TASK -> SDEI_NORMAL -> SDEI_CRITICAL -> OVERFLOW
+ * HYP -> OVERFLOW
+ *
+ * ... so we do not check the specific order of stack
+ * transitions.
+ */
+ state->stack = *next;
+ *next = stackinfo_get_unknown();
+
+found:
+ /*
+ * Future unwind steps can only consume stack above this frame record.
+ * Update the current stack to start immediately above it.
+ */
+ state->stack.low = sp + size;
+ return 0;
+}
/**
* unwind_next_frame_record() - Unwind to the next frame record.
*
* @state: the current unwind state.
- * @accessible: determines whether the frame record is accessible
* @translate_fp: translates the fp prior to access (may be NULL)
*
* Return: 0 upon success, an error code otherwise.
*/
static inline int
unwind_next_frame_record(struct unwind_state *state,
- on_accessible_stack_fn accessible,
stack_trace_translate_fp_fn translate_fp)
{
- struct stack_info info;
unsigned long fp = state->fp, kern_fp = fp;
- struct task_struct *tsk = state->task;
+ int err;
if (fp & 0x7)
return -EINVAL;
- if (!accessible(tsk, fp, 16, &info))
- return -EINVAL;
-
- if (test_bit(info.type, state->stacks_done))
- return -EINVAL;
+ err = unwind_consume_stack(state, fp, 16);
+ if (err)
+ return err;
/*
* If fp is not from the current address space perform the necessary
@@ -174,34 +186,10 @@ unwind_next_frame_record(struct unwind_state *state,
return -EINVAL;
/*
- * As stacks grow downward, any valid record on the same stack must be
- * at a strictly higher address than the prior record.
- *
- * Stacks can nest in several valid orders, e.g.
- *
- * TASK -> IRQ -> OVERFLOW -> SDEI_NORMAL
- * TASK -> SDEI_NORMAL -> SDEI_CRITICAL -> OVERFLOW
- * HYP -> OVERFLOW
- *
- * ... but the nesting itself is strict. Once we transition from one
- * stack to another, it's never valid to unwind back to that first
- * stack.
- */
- if (info.type == state->prev_type) {
- if (fp <= state->prev_fp)
- return -EINVAL;
- } else {
- __set_bit(state->prev_type, state->stacks_done);
- }
-
- /*
- * Record this frame record's values and location. The prev_fp and
- * prev_type are only meaningful to the next unwind_next() invocation.
+ * Record this frame record's values.
*/
state->fp = READ_ONCE(*(unsigned long *)(kern_fp));
state->pc = READ_ONCE(*(unsigned long *)(kern_fp + 8));
- state->prev_fp = fp;
- state->prev_type = info.type;
return 0;
}
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index ca56fd732c2a9..9c8820f242625 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -67,57 +67,6 @@ static inline void unwind_init_from_task(struct unwind_state *state,
state->pc = thread_saved_pc(task);
}
-static bool on_accessible_stack(const struct task_struct *tsk,
- unsigned long sp, unsigned long size,
- struct stack_info *info)
-{
- struct stack_info tmp;
-
- 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;
-}
-
/*
* Unwind from one frame record (A) to the next frame record (B).
*
@@ -135,7 +84,7 @@ static int notrace unwind_next(struct unwind_state *state)
if (fp == (unsigned long)task_pt_regs(tsk)->stackframe)
return -ENOENT;
- err = unwind_next_frame_record(state, on_accessible_stack, NULL);
+ err = unwind_next_frame_record(state, NULL);
if (err)
return err;
@@ -215,11 +164,47 @@ void show_stack(struct task_struct *tsk, unsigned long *sp, const char *loglvl)
barrier();
}
+/*
+ * Per-cpu stacks are only accessible when unwinding the current task in a
+ * non-preemptible context.
+ */
+#define STACKINFO_CPU(name) \
+ ({ \
+ ((task == current) && !preemptible()) \
+ ? stackinfo_get_##name() \
+ : stackinfo_get_unknown(); \
+ })
+
+/*
+ * SDEI stacks are only accessible when unwinding the current task in an NMI
+ * context.
+ */
+#define STACKINFO_SDEI(name) \
+ ({ \
+ ((task == current) && in_nmi()) \
+ ? stackinfo_get_sdei_##name() \
+ : stackinfo_get_unknown(); \
+ })
+
noinline notrace void arch_stack_walk(stack_trace_consume_fn consume_entry,
void *cookie, struct task_struct *task,
struct pt_regs *regs)
{
- struct unwind_state state;
+ struct stack_info stacks[] = {
+ stackinfo_get_task(task),
+ STACKINFO_CPU(irq),
+#if defined(CONFIG_VMAP_STACK)
+ STACKINFO_CPU(overflow),
+#endif
+#if defined(CONFIG_VMAP_STACK) && defined(CONFIG_ARM_SDE_INTERFACE)
+ STACKINFO_SDEI(normal),
+ STACKINFO_SDEI(critical),
+#endif
+ };
+ struct unwind_state state = {
+ .stacks = stacks,
+ .nr_stacks = ARRAY_SIZE(stacks),
+ };
if (regs) {
if (task != current)
diff --git a/arch/arm64/kvm/hyp/nvhe/stacktrace.c b/arch/arm64/kvm/hyp/nvhe/stacktrace.c
index 5da0d44f61b73..08e1325ead73f 100644
--- a/arch/arm64/kvm/hyp/nvhe/stacktrace.c
+++ b/arch/arm64/kvm/hyp/nvhe/stacktrace.c
@@ -47,7 +47,6 @@ static struct stack_info stackinfo_get_overflow(void)
return (struct stack_info) {
.low = low,
.high = high,
- .type = STACK_TYPE_OVERFLOW,
};
}
@@ -60,35 +59,12 @@ static struct stack_info stackinfo_get_hyp(void)
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)
-{
- struct stack_info tmp;
-
- 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)
{
- return unwind_next_frame_record(state, on_accessible_stack, NULL);
+ return unwind_next_frame_record(state, NULL);
}
static void notrace unwind(struct unwind_state *state,
@@ -144,7 +120,14 @@ static bool pkvm_save_backtrace_entry(void *arg, unsigned long where)
*/
static void pkvm_save_backtrace(unsigned long fp, unsigned long pc)
{
- struct unwind_state state;
+ struct stack_info stacks[] = {
+ stackinfo_get_overflow(),
+ stackinfo_get_hyp(),
+ };
+ struct unwind_state state = {
+ .stacks = stacks,
+ .nr_stacks = ARRAY_SIZE(stacks),
+ };
int idx = 0;
kvm_nvhe_unwind_init(&state, fp, pc);
diff --git a/arch/arm64/kvm/stacktrace.c b/arch/arm64/kvm/stacktrace.c
index 7658c5db47c1d..0b4703945780f 100644
--- a/arch/arm64/kvm/stacktrace.c
+++ b/arch/arm64/kvm/stacktrace.c
@@ -31,7 +31,6 @@ static struct stack_info stackinfo_get_overflow(void)
return (struct stack_info) {
.low = low,
.high = high,
- .type = STACK_TYPE_OVERFLOW,
};
}
@@ -45,7 +44,6 @@ static struct stack_info stackinfo_get_hyp(void)
return (struct stack_info) {
.low = low,
.high = high,
- .type = STACK_TYPE_HYP,
};
}
@@ -102,32 +100,9 @@ static bool kvm_nvhe_stack_kern_record_va(unsigned long *addr)
return kvm_nvhe_stack_kern_va(addr, 16);
}
-static bool on_accessible_stack(const struct task_struct *tsk,
- unsigned long sp, unsigned long size,
- struct stack_info *info)
-{
- struct stack_info tmp;
-
- 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)
{
- return unwind_next_frame_record(state, on_accessible_stack,
- kvm_nvhe_stack_kern_record_va);
+ return unwind_next_frame_record(state, kvm_nvhe_stack_kern_record_va);
}
static void unwind(struct unwind_state *state,
@@ -185,7 +160,14 @@ static void kvm_nvhe_dump_backtrace_end(void)
static void hyp_dump_backtrace(unsigned long hyp_offset)
{
struct kvm_nvhe_stacktrace_info *stacktrace_info;
- struct unwind_state state;
+ struct stack_info stacks[] = {
+ stackinfo_get_overflow(),
+ stackinfo_get_hyp(),
+ };
+ struct unwind_state state = {
+ .stacks = stacks,
+ .nr_stacks = ARRAY_SIZE(stacks),
+ };
stacktrace_info = this_cpu_ptr_nvhe_sym(kvm_stacktrace_info);
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3 9/9] arm64: stacktrace: track hyp stacks in unwinder's address space
2022-08-15 11:39 [PATCH v3 0/9] arm64: stacktrace: cleanups and improvements Mark Rutland
` (7 preceding siblings ...)
2022-08-15 11:39 ` [PATCH v3 8/9] arm64: stacktrace: track all stack boundaries explicitly Mark Rutland
@ 2022-08-15 11:39 ` Mark Rutland
8 siblings, 0 replies; 12+ messages in thread
From: Mark Rutland @ 2022-08-15 11:39 UTC (permalink / raw)
To: linux-arm-kernel
Cc: broonie, catalin.marinas, james.morse, kaleshsingh, madvenka,
mark.rutland, maz, tabba, will
Currently unwind_next_frame_record() has an optional callback to convert
the address space of the FP. This is necessary for the NVHE unwinder,
which tracks the stacks in the hyp VA space, but accesses the frame
records in the kernel VA space.
This is a bit unfortunate since it clutters unwind_next_frame_record(),
which will get in the way of future rework.
Instead, this patch changes the NVHE unwinder to track the stacks in the
kernel's VA space and translate to FP prior to calling
unwind_next_frame_record(). This removes the need for the translate_fp()
callback, as all unwinders consistently track stacks in the native
address space of the unwinder.
At the same time, this patch consolidates the generation of the stack
addresses behind the stackinfo_get_*() helpers.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Kalesh Singh <kaleshsingh@google.com>
Reviewed-by: Madhavan T. Venkataraman <madvenka@linux.microsoft.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
Cc: Fuad Tabba <tabba@google.com>
Cc: Marc Zyngier <maz@kernel.org>
---
arch/arm64/include/asm/stacktrace/common.h | 29 ++--------
arch/arm64/kernel/stacktrace.c | 2 +-
arch/arm64/kvm/hyp/nvhe/stacktrace.c | 2 +-
arch/arm64/kvm/stacktrace.c | 62 ++++++++++++++--------
4 files changed, 46 insertions(+), 49 deletions(-)
diff --git a/arch/arm64/include/asm/stacktrace/common.h b/arch/arm64/include/asm/stacktrace/common.h
index 638008f485972..508f734de46ee 100644
--- a/arch/arm64/include/asm/stacktrace/common.h
+++ b/arch/arm64/include/asm/stacktrace/common.h
@@ -77,18 +77,6 @@ static inline void unwind_init_common(struct unwind_state *state,
state->stack = stackinfo_get_unknown();
}
-/**
- * typedef stack_trace_translate_fp_fn() - Translates a non-kernel frame
- * pointer to a kernel address.
- *
- * @fp: the frame pointer to be updated to its kernel address.
- *
- * Return: true if the VA can be translated, false otherwise.
- *
- * Upon success @fp is updated to the corresponding kernel virtual address.
- */
-typedef bool (*stack_trace_translate_fp_fn)(unsigned long *fp);
-
static struct stack_info *unwind_find_next_stack(const struct unwind_state *state,
unsigned long sp,
unsigned long size)
@@ -160,15 +148,13 @@ static inline int unwind_consume_stack(struct unwind_state *state,
* unwind_next_frame_record() - Unwind to the next frame record.
*
* @state: the current unwind state.
- * @translate_fp: translates the fp prior to access (may be NULL)
*
* Return: 0 upon success, an error code otherwise.
*/
static inline int
-unwind_next_frame_record(struct unwind_state *state,
- stack_trace_translate_fp_fn translate_fp)
+unwind_next_frame_record(struct unwind_state *state)
{
- unsigned long fp = state->fp, kern_fp = fp;
+ unsigned long fp = state->fp;
int err;
if (fp & 0x7)
@@ -178,18 +164,11 @@ unwind_next_frame_record(struct unwind_state *state,
if (err)
return err;
- /*
- * If fp is not from the current address space perform the necessary
- * translation before dereferencing it to get the next fp.
- */
- if (translate_fp && !translate_fp(&kern_fp))
- return -EINVAL;
-
/*
* Record this frame record's values.
*/
- state->fp = READ_ONCE(*(unsigned long *)(kern_fp));
- state->pc = READ_ONCE(*(unsigned long *)(kern_fp + 8));
+ state->fp = READ_ONCE(*(unsigned long *)(fp));
+ state->pc = READ_ONCE(*(unsigned long *)(fp + 8));
return 0;
}
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index 9c8820f242625..634279b3b03d1 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -84,7 +84,7 @@ static int notrace unwind_next(struct unwind_state *state)
if (fp == (unsigned long)task_pt_regs(tsk)->stackframe)
return -ENOENT;
- err = unwind_next_frame_record(state, NULL);
+ err = unwind_next_frame_record(state);
if (err)
return err;
diff --git a/arch/arm64/kvm/hyp/nvhe/stacktrace.c b/arch/arm64/kvm/hyp/nvhe/stacktrace.c
index 08e1325ead73f..ed6b58b19cfa5 100644
--- a/arch/arm64/kvm/hyp/nvhe/stacktrace.c
+++ b/arch/arm64/kvm/hyp/nvhe/stacktrace.c
@@ -64,7 +64,7 @@ static struct stack_info stackinfo_get_hyp(void)
static int unwind_next(struct unwind_state *state)
{
- return unwind_next_frame_record(state, NULL);
+ return unwind_next_frame_record(state);
}
static void notrace unwind(struct unwind_state *state,
diff --git a/arch/arm64/kvm/stacktrace.c b/arch/arm64/kvm/stacktrace.c
index 0b4703945780f..3ace5b75813bd 100644
--- a/arch/arm64/kvm/stacktrace.c
+++ b/arch/arm64/kvm/stacktrace.c
@@ -34,6 +34,17 @@ static struct stack_info stackinfo_get_overflow(void)
};
}
+static struct stack_info stackinfo_get_overflow_kern_va(void)
+{
+ unsigned long low = (unsigned long)this_cpu_ptr_nvhe_sym(overflow_stack);
+ unsigned long high = low + OVERFLOW_STACK_SIZE;
+
+ return (struct stack_info) {
+ .low = low,
+ .high = high,
+ };
+}
+
static struct stack_info stackinfo_get_hyp(void)
{
struct kvm_nvhe_stacktrace_info *stacktrace_info
@@ -47,6 +58,17 @@ static struct stack_info stackinfo_get_hyp(void)
};
}
+static struct stack_info stackinfo_get_hyp_kern_va(void)
+{
+ unsigned long low = (unsigned long)*this_cpu_ptr(&kvm_arm_hyp_stack_page);
+ unsigned long high = low + PAGE_SIZE;
+
+ return (struct stack_info) {
+ .low = low,
+ .high = high,
+ };
+}
+
/*
* kvm_nvhe_stack_kern_va - Convert KVM nVHE HYP stack addresses to a kernel VAs
*
@@ -62,33 +84,22 @@ static struct stack_info stackinfo_get_hyp(void)
*/
static bool kvm_nvhe_stack_kern_va(unsigned long *addr, unsigned long size)
{
- struct kvm_nvhe_stacktrace_info *stacktrace_info;
- unsigned long hyp_base, kern_base, hyp_offset;
- struct stack_info stack;
+ struct stack_info stack_hyp, stack_kern;
- stacktrace_info = this_cpu_ptr_nvhe_sym(kvm_stacktrace_info);
-
- stack = stackinfo_get_hyp();
- if (stackinfo_on_stack(&stack, *addr, size)) {
- kern_base = (unsigned long)*this_cpu_ptr(&kvm_arm_hyp_stack_page);
- hyp_base = (unsigned long)stacktrace_info->stack_base;
+ stack_hyp = stackinfo_get_hyp();
+ stack_kern = stackinfo_get_hyp_kern_va();
+ if (stackinfo_on_stack(&stack_hyp, *addr, size))
goto found;
- }
- stack = stackinfo_get_overflow();
- if (stackinfo_on_stack(&stack, *addr, size)) {
- kern_base = (unsigned long)this_cpu_ptr_nvhe_sym(overflow_stack);
- hyp_base = (unsigned long)stacktrace_info->overflow_stack_base;
+ stack_hyp = stackinfo_get_overflow();
+ stack_kern = stackinfo_get_overflow_kern_va();
+ if (stackinfo_on_stack(&stack_hyp, *addr, size))
goto found;
- }
return false;
found:
- hyp_offset = *addr - hyp_base;
-
- *addr = kern_base + hyp_offset;
-
+ *addr = *addr - stack_hyp.low + stack_kern.low;
return true;
}
@@ -102,7 +113,14 @@ static bool kvm_nvhe_stack_kern_record_va(unsigned long *addr)
static int unwind_next(struct unwind_state *state)
{
- return unwind_next_frame_record(state, kvm_nvhe_stack_kern_record_va);
+ /*
+ * The FP is in the hypervisor VA space. Convert it to the kernel VA
+ * space so it can be unwound by the regular unwind functions.
+ */
+ if (!kvm_nvhe_stack_kern_record_va(&state->fp))
+ return -EINVAL;
+
+ return unwind_next_frame_record(state);
}
static void unwind(struct unwind_state *state,
@@ -161,8 +179,8 @@ static void hyp_dump_backtrace(unsigned long hyp_offset)
{
struct kvm_nvhe_stacktrace_info *stacktrace_info;
struct stack_info stacks[] = {
- stackinfo_get_overflow(),
- stackinfo_get_hyp(),
+ stackinfo_get_overflow_kern_va(),
+ stackinfo_get_hyp_kern_va(),
};
struct unwind_state state = {
.stacks = stacks,
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 12+ messages in thread