Re: [PATCH v14 07/19] unwind_user/sframe: Wire up unwind_user to sframe

[Jens Remus <jremus@linux.ibm.com>]

On 5/5/2026 8:55 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Unbounded linear scan of user-controlled sframe entries can cause a CPU hard lockup in NMI context.
> - [Medium] The commit message claims to refactor `unwind_user_next_common()` by removing an unused `pt_regs` parameter, but this change is entirely absent from the code modifications.
> --
> 
> commit 2e1e17bbaa1c0426d41d66ce7930a944f3e86ae7
> Author: Josh Poimboeuf <jpoimboe@kernel.org>
> 
> unwind_user/sframe: Wire up unwind_user to sframe
> 
> This commit hooks up the sframe infrastructure into the unwind_user interface
> to support user-space stack unwinding via sframe. It adds the
> UNWIND_USER_TYPE_SFRAME type and updates the unwind_user_next() logic to
> attempt sframe unwinding when available.
> 
>>     [ Jens Remus: Remove unused pt_regs from unwind_user_next_common() and
>>     its callers.  Simplify unwind_user_next_sframe(). ]
> 
> This isn't a bug, but does this commit message match the changes?
> Looking at the diff and the code, there are no modifications to
> unwind_user_next_common() or its callers to remove a pt_regs parameter.

Sashiko AI review failed to understand this commit message comment.

> 
>> 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))
> 
> Can this sequence cause a hard lockup in NMI context?
> 
> Since this unwinder is often called from NMI context (like perf), sframe_find()
> eventually calls __find_fre() to search for Frame Row Entries. If __find_fre()
> performs a linear scan over an unbounded, user-controlled number of entries
> (fde->fres_num), it could spin for a long time without yielding.
> 
> If a malicious userspace program maps a large VMA and provides hundreds of
> millions of FREs, would this loop calling unsafe_get_user() on every
> iteration tie up the CPU long enough in NMI context to trigger the hard
> lockup detector?

Please advise.

A user could craft .sframe with a large number of FDEs (up to UINT32_MAX)
and large number of FREs per FDE (up to UINT16_MAX).  Each FDE could point
at the same FDA and FREs to save section space.  Each FDE would require
16 bytes in the FDE table. All FDEs could share a single FDA of 5 bytes
and share FREs of minimum 5 bytes each (when using the 16-bit start
address offset to have UINT16_MAX FREs) in the FRE table.

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

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/