On Thu, Jul 10, 2025 at 06:35:13PM +0200, Jens Remus wrote:
quoted hunk ↗ jump to hunk
+++ 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?
quoted hunk ↗ jump to hunk
@@ -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.
quoted hunk ↗ jump to hunk
@@ -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