Re: [PATCH v2] arm64: Add USER_STACKTRACE support

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

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
> 

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