On Thu, Jul 10, 2025 at 06:35:13PM +0200, Jens Remus wrote:
> +++ b/arch/Kconfig
> @@ -450,6 +450,11 @@ config HAVE_UNWIND_USER_SFRAME
> bool
> select UNWIND_USER
>
> +config HAVE_USER_RA_REG
> + bool
> + help
> + The arch passes the return address (RA) in user space in a register.
How about "HAVE_UNWIND_USER_RA_REG" so it matches the existing
namespace?
> @@ -310,6 +307,12 @@ static __always_inline int __find_fre(struct
> sframe_section *sec,
> return -EINVAL;
> fre = prev_fre;
>
> + if ((!IS_ENABLED(CONFIG_HAVE_USER_RA_REG) || !topmost) && !fre->ra_off)
> {
> + dbg_sec_uaccess("fde addr 0x%x: zero ra_off\n",
> + fde->start_addr);
> + return -EINVAL;
> + }
The topmost frame doesn't necessarily (or even likely) come from before
the prologue, or from a leaf function, so this check would miss the case
where a non-leaf function wrongly has !ra_off after its prologue.
Which in reality is probably fine, as there are other guardrails in
place to catch such bad sframe data.
But then do we think the !ra_off check is still worth the effort? It
would be simpler to just always assume !ra_off is valid for the
CONFIG_HAVE_USER_RA_REG case.
I think I prefer the simplicity of removing the check, as the error
would be rare, and corrupt sframe would be caught in other ways.
> @@ -86,18 +88,28 @@ static int unwind_user_next(struct unwind_user_state
> *state)
>
> /* Get the Stack Pointer (SP) */
> sp = cfa + frame->sp_val_off;
> - /* Make sure that stack is not going in wrong direction */
> - if (sp <= state->sp)
> + /*
> + * Make sure that stack is not going in wrong direction. Allow SP
> + * to be unchanged for the topmost frame, by subtracting topmost,
> + * which is either 0 or 1.
> + */
> + if (sp <= state->sp - topmost)
> goto done;
>
> - /* Make sure that the address is word aligned */
> - shift = sizeof(long) == 4 || compat_fp_state(state) ? 2 : 3;
> - if ((cfa + frame->ra_off) & ((1 << shift) - 1))
> - goto done;
>
> /* Get the Return Address (RA) */
> - if (unwind_get_user_long(ra, cfa + frame->ra_off, state))
> - goto done;
> + if (frame->ra_off) {
> + /* Make sure that the address is word aligned */
> + shift = sizeof(long) == 4 || compat_fp_state(state) ? 2 : 3;
> + if ((cfa + frame->ra_off) & ((1 << shift) - 1))
> + goto done;
> + if (unwind_get_user_long(ra, cfa + frame->ra_off, state))
> + goto done;
> + } else {
> + if (!IS_ENABLED(CONFIG_HAVE_USER_RA_REG) || !topmost)
> + goto done;
I think this check is redundant with the one in __find_fre()?
--
Josh
