Re: [RFC PATCH v2 19/24] unwind_user/eh_frame/x86: Enable eh_frame unwinding on x86

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Enabling `.eh_frame` unwinding on x86_64 breaks user stack traces for 32-bit compat binaries due to incorrect error handling in the generic unwind loop.
--

--- Patch [19]: [RFC PATCH v2 19/24] unwind_user/eh_frame/x86: Enable eh_frame unwinding on x86 ---
commit 26326eedb731dafef6946d5d887c188233f39043
Author: Jens Remus <[email protected]>

unwind_user/eh_frame/x86: Enable eh_frame unwinding on x86

This patch enables .eh_frame user stack unwinding for x86-64 by selecting
HAVE_UNWIND_USER_EH_FRAME and providing the necessary register accessors.

> diff --git a/arch/x86/include/asm/unwind_user.h b/arch/x86/include/asm/unwind_user.h
> index c96645c824d1b..df1966bcb0f6e 100644
> --- a/arch/x86/include/asm/unwind_user.h
> +++ b/arch/x86/include/asm/unwind_user.h

[ ... ]

> @@ -15,6 +16,45 @@ static inline int unwind_user_word_size(struct pt_regs *regs)
>  	return user_64bit_mode(regs) ? 8 : 4;
>  }
>  
> +#ifdef CONFIG_X86_64
> +
> +static inline int unwind_user_get_reg(unsigned long *val, unsigned int regnum)
> +{
> +	struct pt_regs *regs = task_pt_regs(current);
> +
> +	/* TODO: eh_frame is currently limited to x86-64 */
> +	if (!user_64bit_mode(regs))
> +		return -EINVAL;

[Severity: High]
Does returning -EINVAL here break the stack trace fallback mechanism for
32-bit compat tasks?

In kernel/unwind/user.c:unwind_user_next(), the generic unwinder only
continues to the next method (like the frame pointer unwinder) if it receives
-ENOENT:

kernel/unwind/user.c:unwind_user_next() {
...
        case UNWIND_USER_TYPE_EH_FRAME:
                switch (unwind_user_next_eh_frame(state)) {
                case 0:
                        return 0;
                case -ENOENT:
                        continue;       /* Try next method. */
                default:
                        state->done = true;
                }
                break;
...
}

By returning -EINVAL for 32-bit tasks, the unwinder hits the default case and
aborts entirely by setting state->done = true. This truncates the user stack
trace instead of safely falling back to the FP unwinder.

Would returning -ENOENT be more appropriate here when the current task is
not 64-bit to ensure the fallback pipeline continues correctly?

> +
> +	switch (regnum) {
> +	/* DWARF register numbers 0..15 */
> +	case  0: *val = regs->ax; break;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=19