Re: [PATCH v15 07/20] unwind_user/sframe: Wire up unwind_user to sframe

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incorrect FRE size calculation completely breaks SFrame parsing for functions with multiple FREs.
--

commit 8adeb8aec136e0b90e11e4b9aa74e9464240b9f3
Author: Josh Poimboeuf <jpoimboe@kernel.org>

unwind_user/sframe: Wire up unwind_user to sframe

This commit hooks up the sframe infrastructure to the unwind_user
interface. It introduces unwind_user_next_sframe() to utilize
sframe_find() and wires it into the user unwinding logic.

[ ... ]

> diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c
> index 90ab3c1a205ef..1fb2724197332 100644
> --- a/kernel/unwind/user.c
> +++ b/kernel/unwind/user.c

[ ... ]

> @@ -82,6 +83,16 @@ static int unwind_user_next_fp(struct unwind_user_state *state)
>  	return unwind_user_next_common(state, &fp_frame);
>  }
>  
> +static int unwind_user_next_sframe(struct unwind_user_state *state)
> +{
> +	struct unwind_user_frame frame;
> +
> +	/* sframe expects the frame to be local storage */
> +	if (sframe_find(state->ip, &frame))

[Severity: High]
This is a pre-existing issue, but does the SFrame parsing logic calculate
the FRE size incorrectly for functions with multiple FREs?

Looking at the call chain from sframe_find() into __find_fre() and
__read_fre(), dataword_count is decremented as datawords are parsed until
it reaches 0. This is explicitly enforced by returning -EINVAL if
dataword_count is not zero.

Immediately after, the FRE size is computed using the now-zero
dataword_count:

fre->size = addr_size + 1 + (dataword_count * dataword_size);

Because dataword_count is always 0 here, the size of the datawords is
omitted from the size calculation.

When __find_fre() advances to the next FRE using fre_addr += fre->size,
will it start reading from the middle of the current FRE's datawords
instead of the next FRE header?

I note this is addressed later in the patch series by commit 9c045164b82cb
("unwind_user/sframe: Separate reading of FRE from reading of FRE data
words"), which refactors the code to avoid decrementing dataword_count.

> +		return -ENOENT;
> +	return unwind_user_next_common(state, &frame);
> +}
> +

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520154004.3845823-1-jremus@linux.ibm.com?part=7