Re: [PATCH v14 08/19] unwind_user: Stop when reaching an outermost frame

On 5/5/2026 2:40 PM, [email protected] 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 <[email protected]>
> 
> 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)
[email protected] / [email protected]

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/