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

[sashiko-bot@kernel.org]

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.

> 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?

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

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