| Message ID | 20231125072739.3151-1-qiwu.chen@transsion.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] arm64: Add USER_STACKTRACE support | expand |
On Fri, Nov 24, 2023 at 11:27:39PM -0800, qiwuchen55@gmail.com wrote: > From: chenqiwu <qiwu.chen@transsion.com> > > Use the perf_callchain_user() code as blueprint to implement > arch_stack_walk_user() which add ftrace userstacktrace support > on arm64. > With this patch, tracer can get userstacktrace by below callchain: > ftrace_trace_userstack -> > stack_trace_save_user -> > aasrch_stack_walk_user > An example test case is as shown below: > # cd /sys/kernel/debug/tracing > # echo 1 > options/userstacktrace > # echo 1 > options/sym-userobj > # echo 1 > events/sched/sched_process_fork/enable > # cat trace > ...... > bash-418 [000] ..... 121.820661: sched_process_fork: comm=bash pid=418 child_comm=bash child_pid=441 > bash-418 [000] ..... 121.821340: <user stack trace> > => /lib/aarch64-linux-gnu/libc-2.32.so[+0xa76d8] > => /bin/bash[+0x5f354] > => /bin/bash[+0x47fe8] > => /bin/bash[+0x493f8] > => /bin/bash[+0x4aec4] > => /bin/bash[+0x4c31c] > => /bin/bash[+0x339b0] > => /bin/bash[+0x322f8] > > changes in v2: > - Remove useless arch_dump_user_stacktrace(). > - Rework arch_stack_walk_user() implementation. > - Modify the commit message. > Tested-by: chenqiwu <qiwu.chen@transsion.com> > Signed-off-by: chenqiwu <qiwu.chen@transsion.com> > --- > arch/arm64/Kconfig | 1 + > arch/arm64/kernel/stacktrace.c | 120 +++++++++++++++++++++++++++++++++ > 2 files changed, 121 insertions(+) Thanks for this, it looks much better than v1. However, as I mentioned on v1, we want to have one copy of this code. Rather than just copying the logic from perf_callchain.c, please also update perf_callchain_user() to use arch_stack_walk_user() and delete the redundant code. Thanks, Mark. > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 7b071a00425d..4c5066f88dd2 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -255,6 +255,7 @@ config ARM64 > select TRACE_IRQFLAGS_SUPPORT > select TRACE_IRQFLAGS_NMI_SUPPORT > select HAVE_SOFTIRQ_ON_OWN_STACK > + select USER_STACKTRACE_SUPPORT > help > ARM 64-bit (AArch64) Linux support. > > diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c > index 17f66a74c745..7f9ab5a37096 100644 > --- a/arch/arm64/kernel/stacktrace.c > +++ b/arch/arm64/kernel/stacktrace.c > @@ -240,3 +240,123 @@ void show_stack(struct task_struct *tsk, unsigned long *sp, const char *loglvl) > dump_backtrace(NULL, tsk, loglvl); > barrier(); > } > + > +/* > + * The struct defined for userspace stack frame in AARCH64 mode. > + */ > +struct frame_tail { > + struct frame_tail __user *fp; > + unsigned long lr; > +} __attribute__((packed)); > + > +/* > + * Get the return address for a single stackframe and return a pointer to the > + * next frame tail. > + */ > +static struct frame_tail __user * > +unwind_user_frame(struct frame_tail __user *tail, void *cookie, > + stack_trace_consume_fn consume_entry) > +{ > + struct frame_tail buftail; > + unsigned long err; > + unsigned long lr; > + > + /* Also check accessibility of one struct frame_tail beyond */ > + if (!access_ok(tail, sizeof(buftail))) > + return NULL; > + > + pagefault_disable(); > + err = __copy_from_user_inatomic(&buftail, tail, sizeof(buftail)); > + pagefault_enable(); > + > + if (err) > + return NULL; > + > + lr = ptrauth_strip_user_insn_pac(buftail.lr); > + > + if (!consume_entry(cookie, lr)) > + return NULL; > + > + /* > + * Frame pointers should strictly progress back up the stack > + * (towards higher addresses). > + */ > + if (tail >= buftail.fp) > + return NULL; > + > + return buftail.fp; > +} > + > +#ifdef CONFIG_COMPAT > +/* > + * The registers we're interested in are at the end of the variable > + * length saved register structure. The fp points at the end of this > + * structure so the address of this struct is: > + * (struct compat_frame_tail *)(xxx->fp)-1 > + * > + * This code has been adapted from the ARM OProfile support. > + */ > +struct compat_frame_tail { > + compat_uptr_t fp; /* a (struct compat_frame_tail *) in compat mode */ > + u32 sp; > + u32 lr; > +} __attribute__((packed)); > + > +static struct compat_frame_tail __user * > +unwind_compat_user_frame(struct compat_frame_tail __user *tail, void *cookie, > + stack_trace_consume_fn consume_entry) > +{ > + struct compat_frame_tail buftail; > + unsigned long err; > + > + /* Also check accessibility of one struct frame_tail beyond */ > + if (!access_ok(tail, sizeof(buftail))) > + return NULL; > + > + pagefault_disable(); > + err = __copy_from_user_inatomic(&buftail, tail, sizeof(buftail)); > + pagefault_enable(); > + > + if (err) > + return NULL; > + > + if (!consume_entry(cookie, buftail.lr)) > + return NULL; > + > + /* > + * Frame pointers should strictly progress back up the stack > + * (towards higher addresses). > + */ > + if (tail + 1 >= (struct compat_frame_tail __user *) > + compat_ptr(buftail.fp)) > + return NULL; > + > + return (struct compat_frame_tail __user *)compat_ptr(buftail.fp) - 1; > +} > +#endif /* CONFIG_COMPAT */ > + > + > +void arch_stack_walk_user(stack_trace_consume_fn consume_entry, void *cookie, > + const struct pt_regs *regs) > +{ > + if (!consume_entry(cookie, regs->pc)) > + return; > + > + if (!compat_user_mode(regs)) { > + /* AARCH64 mode */ > + struct frame_tail __user *tail; > + > + tail = (struct frame_tail __user *)regs->regs[29]; > + while (tail && !((unsigned long)tail & 0x7)) > + tail = unwind_user_frame(tail, cookie, consume_entry); > + } else { > +#ifdef CONFIG_COMPAT > + /* AARCH32 compat mode */ > + struct compat_frame_tail __user *tail; > + > + tail = (struct compat_frame_tail __user *)regs->compat_fp - 1; > + while (tail && !((unsigned long)tail & 0x3)) > + tail = unwind_compat_user_frame(tail, cookie, consume_entry); > +#endif > + } > +} > -- > 2.25.1 >
On Tue, Nov 28, 2023 at 02:48:32PM +0000, Mark Rutland wrote: > On Fri, Nov 24, 2023 at 11:27:39PM -0800, qiwuchen55@gmail.com wrote: > > From: chenqiwu <qiwu.chen@transsion.com> > > > > Use the perf_callchain_user() code as blueprint to implement > > arch_stack_walk_user() which add ftrace userstacktrace support > > on arm64. > > With this patch, tracer can get userstacktrace by below callchain: > > ftrace_trace_userstack -> > > stack_trace_save_user -> > > aasrch_stack_walk_user > > An example test case is as shown below: > > # cd /sys/kernel/debug/tracing > > # echo 1 > options/userstacktrace > > # echo 1 > options/sym-userobj > > # echo 1 > events/sched/sched_process_fork/enable > > # cat trace > > ...... > > bash-418 [000] ..... 121.820661: sched_process_fork: comm=bash pid=418 child_comm=bash child_pid=441 > > bash-418 [000] ..... 121.821340: <user stack trace> > > => /lib/aarch64-linux-gnu/libc-2.32.so[+0xa76d8] > > => /bin/bash[+0x5f354] > > => /bin/bash[+0x47fe8] > > => /bin/bash[+0x493f8] > > => /bin/bash[+0x4aec4] > > => /bin/bash[+0x4c31c] > > => /bin/bash[+0x339b0] > > => /bin/bash[+0x322f8] > > > > changes in v2: > > - Remove useless arch_dump_user_stacktrace(). > > - Rework arch_stack_walk_user() implementation. > > - Modify the commit message. > > Tested-by: chenqiwu <qiwu.chen@transsion.com> > > Signed-off-by: chenqiwu <qiwu.chen@transsion.com> > > --- > > arch/arm64/Kconfig | 1 + > > arch/arm64/kernel/stacktrace.c | 120 +++++++++++++++++++++++++++++++++ > > 2 files changed, 121 insertions(+) > > Thanks for this, it looks much better than v1. > However, as I mentioned on v1, we want to have one copy of this code. > > Rather than just copying the logic from perf_callchain.c, please also update > perf_callchain_user() to use arch_stack_walk_user() and delete the redundant code. > > Thanks, > Mark. > Hi Mark, Thanks for your professional advice, I make an improvement in patch v3. Please help review this again: https://patchwork.kernel.org/project/linux-arm-kernel/patch/20231129160119.60829-1-qiwu.chen@transsion.com/ Thanks Qiwu
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 7b071a00425d..4c5066f88dd2 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -255,6 +255,7 @@ config ARM64 select TRACE_IRQFLAGS_SUPPORT select TRACE_IRQFLAGS_NMI_SUPPORT select HAVE_SOFTIRQ_ON_OWN_STACK + select USER_STACKTRACE_SUPPORT help ARM 64-bit (AArch64) Linux support. diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c index 17f66a74c745..7f9ab5a37096 100644 --- a/arch/arm64/kernel/stacktrace.c +++ b/arch/arm64/kernel/stacktrace.c @@ -240,3 +240,123 @@ void show_stack(struct task_struct *tsk, unsigned long *sp, const char *loglvl) dump_backtrace(NULL, tsk, loglvl); barrier(); } + +/* + * The struct defined for userspace stack frame in AARCH64 mode. + */ +struct frame_tail { + struct frame_tail __user *fp; + unsigned long lr; +} __attribute__((packed)); + +/* + * Get the return address for a single stackframe and return a pointer to the + * next frame tail. + */ +static struct frame_tail __user * +unwind_user_frame(struct frame_tail __user *tail, void *cookie, + stack_trace_consume_fn consume_entry) +{ + struct frame_tail buftail; + unsigned long err; + unsigned long lr; + + /* Also check accessibility of one struct frame_tail beyond */ + if (!access_ok(tail, sizeof(buftail))) + return NULL; + + pagefault_disable(); + err = __copy_from_user_inatomic(&buftail, tail, sizeof(buftail)); + pagefault_enable(); + + if (err) + return NULL; + + lr = ptrauth_strip_user_insn_pac(buftail.lr); + + if (!consume_entry(cookie, lr)) + return NULL; + + /* + * Frame pointers should strictly progress back up the stack + * (towards higher addresses). + */ + if (tail >= buftail.fp) + return NULL; + + return buftail.fp; +} + +#ifdef CONFIG_COMPAT +/* + * The registers we're interested in are at the end of the variable + * length saved register structure. The fp points at the end of this + * structure so the address of this struct is: + * (struct compat_frame_tail *)(xxx->fp)-1 + * + * This code has been adapted from the ARM OProfile support. + */ +struct compat_frame_tail { + compat_uptr_t fp; /* a (struct compat_frame_tail *) in compat mode */ + u32 sp; + u32 lr; +} __attribute__((packed)); + +static struct compat_frame_tail __user * +unwind_compat_user_frame(struct compat_frame_tail __user *tail, void *cookie, + stack_trace_consume_fn consume_entry) +{ + struct compat_frame_tail buftail; + unsigned long err; + + /* Also check accessibility of one struct frame_tail beyond */ + if (!access_ok(tail, sizeof(buftail))) + return NULL; + + pagefault_disable(); + err = __copy_from_user_inatomic(&buftail, tail, sizeof(buftail)); + pagefault_enable(); + + if (err) + return NULL; + + if (!consume_entry(cookie, buftail.lr)) + return NULL; + + /* + * Frame pointers should strictly progress back up the stack + * (towards higher addresses). + */ + if (tail + 1 >= (struct compat_frame_tail __user *) + compat_ptr(buftail.fp)) + return NULL; + + return (struct compat_frame_tail __user *)compat_ptr(buftail.fp) - 1; +} +#endif /* CONFIG_COMPAT */ + + +void arch_stack_walk_user(stack_trace_consume_fn consume_entry, void *cookie, + const struct pt_regs *regs) +{ + if (!consume_entry(cookie, regs->pc)) + return; + + if (!compat_user_mode(regs)) { + /* AARCH64 mode */ + struct frame_tail __user *tail; + + tail = (struct frame_tail __user *)regs->regs[29]; + while (tail && !((unsigned long)tail & 0x7)) + tail = unwind_user_frame(tail, cookie, consume_entry); + } else { +#ifdef CONFIG_COMPAT + /* AARCH32 compat mode */ + struct compat_frame_tail __user *tail; + + tail = (struct compat_frame_tail __user *)regs->compat_fp - 1; + while (tail && !((unsigned long)tail & 0x3)) + tail = unwind_compat_user_frame(tail, cookie, consume_entry); +#endif + } +}