From: madvenka@linux.microsoft.com To: mark.rutland@arm.com, broonie@kernel.org, jpoimboe@redhat.com, ardb@kernel.org, nobuta.keiya@fujitsu.com, sjitindarsingh@gmail.com, catalin.marinas@arm.com, will@kernel.org, jamorris@linux.microsoft.com, linux-arm-kernel@lists.infradead.org, live-patching@vger.kernel.org, linux-kernel@vger.kernel.org, madvenka@linux.microsoft.com Subject: [PATCH v16 1/1] arm64: Make the unwind loop similar to other architectures Date: Thu, 7 Jul 2022 10:01:34 -0500 [thread overview] Message-ID: <20220707150134.4614-2-madvenka@linux.microsoft.com> (raw) In-Reply-To: <20220707150134.4614-1-madvenka@linux.microsoft.com> From: "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com> Change the unwind loop to this ============================== for (unwind_start(&state, task, regs); !unwind_done(state); unwind_next(state)) { if (unwind_failed(state)) { /* PC is suspect. Cannot consume it. */ return; } if (!consume_entry(cookie, state->pc)) { /* Caller terminated the unwind. */ return; } } unwind_start() ============== Define this new function to perform all of the initialization prior to doing a stack trace. So, the unwind_init_*() functions will be called from here. unwind_done() ============= Define this new helper function to return true upon reaching the end of the stack successfully. unwind_failed() =============== Define this new helper function to return true if any type of stack corruption is detected. unwind_next() ============= Change this function to record stack corruption or other failures in the state rather than return an error. Use the unwind loop directly in arch_stack_walk() ================================================= Remove the unwind() function. Instead, inline the unwind loop in arch_stack_walk(). In the future, arch_stack_walk_reliable() will also inline the unwind loop and add reliability checks to the loop. Signed-off-by: Madhavan T. Venkataraman <madvenka@linux.microsoft.com> --- arch/arm64/kernel/stacktrace.c | 121 ++++++++++++++++++++++----------- 1 file changed, 81 insertions(+), 40 deletions(-) diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c index fcaa151b81f1..3ddf0f9ae081 100644 --- a/arch/arm64/kernel/stacktrace.c +++ b/arch/arm64/kernel/stacktrace.c @@ -40,6 +40,12 @@ * value. * * @task: The task being unwound. + * + * @final_fp: Pointer to the final frame. + * + * @done: Unwind completed successfully. + * + * @failed: Unwind failed. */ struct unwind_state { unsigned long fp; @@ -51,6 +57,9 @@ struct unwind_state { struct llist_node *kr_cur; #endif struct task_struct *task; + unsigned long final_fp; + bool done; + bool failed; }; static void unwind_init_common(struct unwind_state *state, @@ -73,6 +82,26 @@ static void unwind_init_common(struct unwind_state *state, bitmap_zero(state->stacks_done, __NR_STACK_TYPES); state->prev_fp = 0; state->prev_type = STACK_TYPE_UNKNOWN; + state->done = false; + state->failed = false; + + /* Stack trace terminates here. */ + state->final_fp = (unsigned long)task_pt_regs(task)->stackframe; +} + +static inline bool unwind_final_frame(struct unwind_state *state) +{ + return state->fp == state->final_fp; +} + +static inline bool unwind_done(struct unwind_state *state) +{ + return state->done; +} + +static inline bool unwind_failed(struct unwind_state *state) +{ + return state->failed; } /* @@ -133,24 +162,31 @@ static inline void unwind_init_from_task(struct unwind_state *state, * records (e.g. a cycle), determined based on the location and fp value of A * and the location (but not the fp value) of B. */ -static int notrace unwind_next(struct unwind_state *state) +static void notrace unwind_next(struct unwind_state *state) { struct task_struct *tsk = state->task; unsigned long fp = state->fp; struct stack_info info; - /* Final frame; nothing to unwind */ - if (fp == (unsigned long)task_pt_regs(tsk)->stackframe) - return -ENOENT; + if (unwind_final_frame(state)) { + state->done = true; + return; + } - if (fp & 0x7) - return -EINVAL; + if (fp & 0x7) { + state->failed = true; + return; + } - if (!on_accessible_stack(tsk, fp, 16, &info)) - return -EINVAL; + if (!on_accessible_stack(tsk, fp, 16, &info)) { + state->failed = true; + return; + } - if (test_bit(info.type, state->stacks_done)) - return -EINVAL; + if (test_bit(info.type, state->stacks_done)) { + state->failed = true; + return; + } /* * As stacks grow downward, any valid record on the same stack must be @@ -166,8 +202,10 @@ static int notrace unwind_next(struct unwind_state *state) * stack. */ if (info.type == state->prev_type) { - if (fp <= state->prev_fp) - return -EINVAL; + if (fp <= state->prev_fp) { + state->failed = true; + return; + } } else { __set_bit(state->prev_type, state->stacks_done); } @@ -195,8 +233,10 @@ static int notrace unwind_next(struct unwind_state *state) */ orig_pc = ftrace_graph_ret_addr(tsk, NULL, state->pc, (void *)state->fp); - if (WARN_ON_ONCE(state->pc == orig_pc)) - return -EINVAL; + if (WARN_ON_ONCE(state->pc == orig_pc)) { + state->failed = true; + return; + } state->pc = orig_pc; } #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ @@ -204,26 +244,9 @@ static int notrace unwind_next(struct unwind_state *state) if (is_kretprobe_trampoline(state->pc)) state->pc = kretprobe_find_ret_addr(tsk, (void *)state->fp, &state->kr_cur); #endif - - return 0; } NOKPROBE_SYMBOL(unwind_next); -static void notrace unwind(struct unwind_state *state, - stack_trace_consume_fn consume_entry, void *cookie) -{ - while (1) { - int ret; - - if (!consume_entry(cookie, state->pc)) - break; - ret = unwind_next(state); - if (ret < 0) - break; - } -} -NOKPROBE_SYMBOL(unwind); - static bool dump_backtrace_entry(void *arg, unsigned long where) { char *loglvl = arg; @@ -257,21 +280,39 @@ void show_stack(struct task_struct *tsk, unsigned long *sp, const char *loglvl) barrier(); } +static __always_inline void unwind_start(struct unwind_state *state, + struct task_struct *task, + struct pt_regs *regs) +{ + if (regs) + unwind_init_from_regs(state, regs); + else if (task == current) + unwind_init_from_caller(state); + else + unwind_init_from_task(state, task); + +} + 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; - if (regs) { - if (task != current) + if (regs && task != current) + return; + + for (unwind_start(&state, task, regs); !unwind_done(&state); + unwind_next(&state)) { + + if (unwind_failed(&state)) { + /* PC is suspect. Cannot consume it. */ return; - unwind_init_from_regs(&state, regs); - } else if (task == current) { - unwind_init_from_caller(&state); - } else { - unwind_init_from_task(&state, task); - } + } - unwind(&state, consume_entry, cookie); + if (!consume_entry(cookie, state.pc)) { + /* Caller terminated the unwind. */ + return; + } + } } -- 2.25.1
WARNING: multiple messages have this Message-ID (diff)
From: madvenka@linux.microsoft.com To: mark.rutland@arm.com, broonie@kernel.org, jpoimboe@redhat.com, ardb@kernel.org, nobuta.keiya@fujitsu.com, sjitindarsingh@gmail.com, catalin.marinas@arm.com, will@kernel.org, jamorris@linux.microsoft.com, linux-arm-kernel@lists.infradead.org, live-patching@vger.kernel.org, linux-kernel@vger.kernel.org, madvenka@linux.microsoft.com Subject: [PATCH v16 1/1] arm64: Make the unwind loop similar to other architectures Date: Thu, 7 Jul 2022 10:01:34 -0500 [thread overview] Message-ID: <20220707150134.4614-2-madvenka@linux.microsoft.com> (raw) In-Reply-To: <20220707150134.4614-1-madvenka@linux.microsoft.com> From: "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com> Change the unwind loop to this ============================== for (unwind_start(&state, task, regs); !unwind_done(state); unwind_next(state)) { if (unwind_failed(state)) { /* PC is suspect. Cannot consume it. */ return; } if (!consume_entry(cookie, state->pc)) { /* Caller terminated the unwind. */ return; } } unwind_start() ============== Define this new function to perform all of the initialization prior to doing a stack trace. So, the unwind_init_*() functions will be called from here. unwind_done() ============= Define this new helper function to return true upon reaching the end of the stack successfully. unwind_failed() =============== Define this new helper function to return true if any type of stack corruption is detected. unwind_next() ============= Change this function to record stack corruption or other failures in the state rather than return an error. Use the unwind loop directly in arch_stack_walk() ================================================= Remove the unwind() function. Instead, inline the unwind loop in arch_stack_walk(). In the future, arch_stack_walk_reliable() will also inline the unwind loop and add reliability checks to the loop. Signed-off-by: Madhavan T. Venkataraman <madvenka@linux.microsoft.com> --- arch/arm64/kernel/stacktrace.c | 121 ++++++++++++++++++++++----------- 1 file changed, 81 insertions(+), 40 deletions(-) diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c index fcaa151b81f1..3ddf0f9ae081 100644 --- a/arch/arm64/kernel/stacktrace.c +++ b/arch/arm64/kernel/stacktrace.c @@ -40,6 +40,12 @@ * value. * * @task: The task being unwound. + * + * @final_fp: Pointer to the final frame. + * + * @done: Unwind completed successfully. + * + * @failed: Unwind failed. */ struct unwind_state { unsigned long fp; @@ -51,6 +57,9 @@ struct unwind_state { struct llist_node *kr_cur; #endif struct task_struct *task; + unsigned long final_fp; + bool done; + bool failed; }; static void unwind_init_common(struct unwind_state *state, @@ -73,6 +82,26 @@ static void unwind_init_common(struct unwind_state *state, bitmap_zero(state->stacks_done, __NR_STACK_TYPES); state->prev_fp = 0; state->prev_type = STACK_TYPE_UNKNOWN; + state->done = false; + state->failed = false; + + /* Stack trace terminates here. */ + state->final_fp = (unsigned long)task_pt_regs(task)->stackframe; +} + +static inline bool unwind_final_frame(struct unwind_state *state) +{ + return state->fp == state->final_fp; +} + +static inline bool unwind_done(struct unwind_state *state) +{ + return state->done; +} + +static inline bool unwind_failed(struct unwind_state *state) +{ + return state->failed; } /* @@ -133,24 +162,31 @@ static inline void unwind_init_from_task(struct unwind_state *state, * records (e.g. a cycle), determined based on the location and fp value of A * and the location (but not the fp value) of B. */ -static int notrace unwind_next(struct unwind_state *state) +static void notrace unwind_next(struct unwind_state *state) { struct task_struct *tsk = state->task; unsigned long fp = state->fp; struct stack_info info; - /* Final frame; nothing to unwind */ - if (fp == (unsigned long)task_pt_regs(tsk)->stackframe) - return -ENOENT; + if (unwind_final_frame(state)) { + state->done = true; + return; + } - if (fp & 0x7) - return -EINVAL; + if (fp & 0x7) { + state->failed = true; + return; + } - if (!on_accessible_stack(tsk, fp, 16, &info)) - return -EINVAL; + if (!on_accessible_stack(tsk, fp, 16, &info)) { + state->failed = true; + return; + } - if (test_bit(info.type, state->stacks_done)) - return -EINVAL; + if (test_bit(info.type, state->stacks_done)) { + state->failed = true; + return; + } /* * As stacks grow downward, any valid record on the same stack must be @@ -166,8 +202,10 @@ static int notrace unwind_next(struct unwind_state *state) * stack. */ if (info.type == state->prev_type) { - if (fp <= state->prev_fp) - return -EINVAL; + if (fp <= state->prev_fp) { + state->failed = true; + return; + } } else { __set_bit(state->prev_type, state->stacks_done); } @@ -195,8 +233,10 @@ static int notrace unwind_next(struct unwind_state *state) */ orig_pc = ftrace_graph_ret_addr(tsk, NULL, state->pc, (void *)state->fp); - if (WARN_ON_ONCE(state->pc == orig_pc)) - return -EINVAL; + if (WARN_ON_ONCE(state->pc == orig_pc)) { + state->failed = true; + return; + } state->pc = orig_pc; } #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ @@ -204,26 +244,9 @@ static int notrace unwind_next(struct unwind_state *state) if (is_kretprobe_trampoline(state->pc)) state->pc = kretprobe_find_ret_addr(tsk, (void *)state->fp, &state->kr_cur); #endif - - return 0; } NOKPROBE_SYMBOL(unwind_next); -static void notrace unwind(struct unwind_state *state, - stack_trace_consume_fn consume_entry, void *cookie) -{ - while (1) { - int ret; - - if (!consume_entry(cookie, state->pc)) - break; - ret = unwind_next(state); - if (ret < 0) - break; - } -} -NOKPROBE_SYMBOL(unwind); - static bool dump_backtrace_entry(void *arg, unsigned long where) { char *loglvl = arg; @@ -257,21 +280,39 @@ void show_stack(struct task_struct *tsk, unsigned long *sp, const char *loglvl) barrier(); } +static __always_inline void unwind_start(struct unwind_state *state, + struct task_struct *task, + struct pt_regs *regs) +{ + if (regs) + unwind_init_from_regs(state, regs); + else if (task == current) + unwind_init_from_caller(state); + else + unwind_init_from_task(state, task); + +} + 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; - if (regs) { - if (task != current) + if (regs && task != current) + return; + + for (unwind_start(&state, task, regs); !unwind_done(&state); + unwind_next(&state)) { + + if (unwind_failed(&state)) { + /* PC is suspect. Cannot consume it. */ return; - unwind_init_from_regs(&state, regs); - } else if (task == current) { - unwind_init_from_caller(&state); - } else { - unwind_init_from_task(&state, task); - } + } - unwind(&state, consume_entry, cookie); + if (!consume_entry(cookie, state.pc)) { + /* Caller terminated the unwind. */ + return; + } + } } -- 2.25.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-07-07 15:01 UTC|newest] Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <1be3f2d391cd5d29da988242375c5fbc79aebb8f> 2022-07-07 15:01 ` [PATCH v16 0/1] arm64: Reorganize the unwinder madvenka 2022-07-07 15:01 ` madvenka 2022-07-07 15:01 ` madvenka [this message] 2022-07-07 15:01 ` [PATCH v16 1/1] arm64: Make the unwind loop similar to other architectures madvenka
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20220707150134.4614-2-madvenka@linux.microsoft.com \ --to=madvenka@linux.microsoft.com \ --cc=ardb@kernel.org \ --cc=broonie@kernel.org \ --cc=catalin.marinas@arm.com \ --cc=jamorris@linux.microsoft.com \ --cc=jpoimboe@redhat.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=live-patching@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=nobuta.keiya@fujitsu.com \ --cc=sjitindarsingh@gmail.com \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.