* Re: [PATCH v14 08/19] unwind_user: Stop when reaching an outermost frame
[not found] ` <20260505124034.B339EC2BCB4@smtp.kernel.org>
@ 2026-05-06 15:01 ` Jens Remus
2026-05-06 15:40 ` Steven Rostedt
0 siblings, 1 reply; 2+ messages in thread
From: Jens Remus @ 2026-05-06 15:01 UTC (permalink / raw)
To: Steven Rostedt, Josh Poimboeuf; +Cc: bpf, sashiko, Indu Bhagat
On 5/5/2026 2:40 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The newly added `outermost` field in `struct unwind_user_frame` is read uninitialized during SFrame unwinding, which can randomly truncate stack traces.
> --
>
> commit 84ee374fd6a62072e8d61545c4ceae42822e4e37
> Author: Jens Remus <jremus@linux.ibm.com>
>
> unwind_user: Stop when reaching an outermost frame
>
> This commit adds an outermost frame indicator to the unwind user frame
> structure, and modifies the unwinder to stop when it is reached.
>
>> diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c
>> index 1fb2724197332..fdb1001e37505 100644
>> --- a/kernel/unwind/user.c
>> +++ b/kernel/unwind/user.c
>> @@ -32,6 +32,12 @@ static int unwind_user_next_common(struct unwind_user_state *state,
>> {
>> unsigned long cfa, fp, ra;
>>
>> + /* Stop unwinding when reaching an outermost frame. */
>> + if (frame->outermost) {
>
> Could this evaluate uninitialized stack memory during SFrame unwinding?
>
> In kernel/unwind/user.c, unwind_user_next_sframe() declares the frame
> variable on the stack without zero-initializing it:
>
> unwind_user_next_sframe() {
> struct unwind_user_frame frame;
Should unwind_user_next_sframe() zero-initialize frame to resolve below?
> ...
> err = sframe_find(..., &frame);
> ...
> return unwind_user_next_common(state, &frame);
> }
>
> Since sframe_find() does not populate the newly added outermost field at this
> point in the patch series, frame->outermost contains uninitialized memory
> when checked here.
>
> If this evaluates to true, does it erroneously set state->done and silently
> truncate stack traces?
>
> I note that this is fixed later in the series by the commit
> "unwind_user/sframe: Add support for outermost frame indication", but
> introducing the uninitialized read in this commit could break kernel
> bisectability.
Or should I merge this and the following commit into one? Or disregard?
>
>> + state->done = true;
>> + return 0;
>> + }
>> +
>> /* Get the Canonical Frame Address (CFA) */
>
Thanks and regards,
Jens
--
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com
IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/
^ permalink raw reply [flat|nested] 2+ messages in thread