From: Josh Poimboeuf <jpoimboe@kernel.org>
Add optional support for user space frame pointer unwinding. If
supported, the arch needs to enable CONFIG_HAVE_UNWIND_USER_FP and
define ARCH_INIT_USER_FP_FRAME.
By encoding the frame offsets in struct unwind_user_frame, much of this
code can also be reused for future unwinder implementations like sframe.
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Co-developed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
Changes since v12: https://lore.kernel.org/20250701005450.888492528@goodmis.org
- Added test when use_fp is true to make sure fp < sp (Jens Remus)
- Make sure the address read is word aligned (Linus Torvalds)
arch/Kconfig | 4 ++
include/asm-generic/Kbuild | 1 +
include/asm-generic/unwind_user.h | 5 +++
include/linux/unwind_user.h | 5 +++
include/linux/unwind_user_types.h | 1 +
kernel/unwind/user.c | 65 ++++++++++++++++++++++++++++++-
6 files changed, 79 insertions(+), 2 deletions(-)
create mode 100644 include/asm-generic/unwind_user.h
diff --git a/arch/Kconfig b/arch/Kconfig
index ea59e5d7cc69..8e3fd723bd74 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -438,6 +438,10 @@ config HAVE_HARDLOCKUP_DETECTOR_ARCH
config UNWIND_USER
bool
+config HAVE_UNWIND_USER_FP
+ bool
+ select UNWIND_USER
+
config HAVE_PERF_REGS
bool
help
diff --git a/include/asm-generic/Kbuild b/include/asm-generic/Kbuild
index 8675b7b4ad23..295c94a3ccc1 100644
--- a/include/asm-generic/Kbuild
+++ b/include/asm-generic/Kbuild
@@ -59,6 +59,7 @@ mandatory-y += tlbflush.h
mandatory-y += topology.h
mandatory-y += trace_clock.h
mandatory-y += uaccess.h
+mandatory-y += unwind_user.h
mandatory-y += vermagic.h
mandatory-y += vga.h
mandatory-y += video.h
diff --git a/include/asm-generic/unwind_user.h b/include/asm-generic/unwind_user.h
new file mode 100644
index 000000000000..b8882b909944
--- /dev/null
+++ b/include/asm-generic/unwind_user.h
@@ -0,0 +1,5 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_GENERIC_UNWIND_USER_H
+#define _ASM_GENERIC_UNWIND_USER_H
+
+#endif /* _ASM_GENERIC_UNWIND_USER_H */
diff --git a/include/linux/unwind_user.h b/include/linux/unwind_user.h
index 1d77bed8de2c..7f7282516bf5 100644
--- a/include/linux/unwind_user.h
+++ b/include/linux/unwind_user.h
@@ -3,6 +3,11 @@
#define _LINUX_UNWIND_USER_H
#include <linux/unwind_user_types.h>
+#include <asm/unwind_user.h>
+
+#ifndef ARCH_INIT_USER_FP_FRAME
+ #define ARCH_INIT_USER_FP_FRAME
+#endif
int unwind_user(struct unwind_stacktrace *trace, unsigned int max_entries);
diff --git a/include/linux/unwind_user_types.h b/include/linux/unwind_user_types.h
index 6ed1b4ae74e1..65bd070eb6b0 100644
--- a/include/linux/unwind_user_types.h
+++ b/include/linux/unwind_user_types.h
@@ -6,6 +6,7 @@
enum unwind_user_type {
UNWIND_USER_TYPE_NONE,
+ UNWIND_USER_TYPE_FP,
};
struct unwind_stacktrace {
diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c
index d0cf3ee2706d..62b3ef37d71b 100644
--- a/kernel/unwind/user.c
+++ b/kernel/unwind/user.c
@@ -6,13 +6,71 @@
#include <linux/sched.h>
#include <linux/sched/task_stack.h>
#include <linux/unwind_user.h>
+#include <linux/uaccess.h>
+
+static struct unwind_user_frame fp_frame = {
+ ARCH_INIT_USER_FP_FRAME
+};
+
+static inline bool fp_state(struct unwind_user_state *state)
+{
+ return IS_ENABLED(CONFIG_HAVE_UNWIND_USER_FP) &&
+ state->type == UNWIND_USER_TYPE_FP;
+}
#define for_each_user_frame(state) \
for (unwind_user_start(state); !(state)->done; unwind_user_next(state))
static int unwind_user_next(struct unwind_user_state *state)
{
- /* no implementation yet */
+ struct unwind_user_frame *frame;
+ unsigned long cfa = 0, fp, ra = 0;
+ unsigned int shift;
+
+ if (state->done)
+ return -EINVAL;
+
+ if (fp_state(state))
+ frame = &fp_frame;
+ else
+ goto done;
+
+ if (frame->use_fp) {
+ if (state->fp < state->sp)
+ goto done;
+ cfa = state->fp;
+ } else {
+ cfa = state->sp;
+ }
+
+ /* Get the Canonical Frame Address (CFA) */
+ cfa += frame->cfa_off;
+
+ /* stack going in wrong direction? */
+ if (cfa <= state->sp)
+ goto done;
+
+ /* Make sure that the address is word aligned */
+ shift = sizeof(long) == 4 ? 2 : 3;
+ if ((cfa + frame->ra_off) & ((1 << shift) - 1))
+ goto done;
+
+ /* Find the Return Address (RA) */
+ if (get_user(ra, (unsigned long *)(cfa + frame->ra_off)))
+ goto done;
+
+ if (frame->fp_off && get_user(fp, (unsigned long __user *)(cfa + frame->fp_off)))
+ goto done;
+
+ state->ip = ra;
+ state->sp = cfa;
+ if (frame->fp_off)
+ state->fp = fp;
+
+ return 0;
+
+done:
+ state->done = true;
return -EINVAL;
}
@@ -27,7 +85,10 @@ static int unwind_user_start(struct unwind_user_state *state)
return -EINVAL;
}
- state->type = UNWIND_USER_TYPE_NONE;
+ if (IS_ENABLED(CONFIG_HAVE_UNWIND_USER_FP))
+ state->type = UNWIND_USER_TYPE_FP;
+ else
+ state->type = UNWIND_USER_TYPE_NONE;
state->ip = instruction_pointer(regs);
state->sp = user_stack_pointer(regs);
--
2.47.2
On 08.07.2025 03:22, Steven Rostedt wrote: > From: Josh Poimboeuf <jpoimboe@kernel.org> > > Add optional support for user space frame pointer unwinding. If > supported, the arch needs to enable CONFIG_HAVE_UNWIND_USER_FP and > define ARCH_INIT_USER_FP_FRAME. > > By encoding the frame offsets in struct unwind_user_frame, much of this > code can also be reused for future unwinder implementations like sframe. > > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> > Co-developed-by: Steven Rostedt (Google) <rostedt@goodmis.org> > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c > @@ -6,13 +6,71 @@ > #include <linux/sched.h> > #include <linux/sched/task_stack.h> > #include <linux/unwind_user.h> > +#include <linux/uaccess.h> > + > +static struct unwind_user_frame fp_frame = { > + ARCH_INIT_USER_FP_FRAME > +}; > + > +static inline bool fp_state(struct unwind_user_state *state) > +{ > + return IS_ENABLED(CONFIG_HAVE_UNWIND_USER_FP) && > + state->type == UNWIND_USER_TYPE_FP; > +} > > #define for_each_user_frame(state) \ > for (unwind_user_start(state); !(state)->done; unwind_user_next(state)) > > static int unwind_user_next(struct unwind_user_state *state) > { > - /* no implementation yet */ > + struct unwind_user_frame *frame; > + unsigned long cfa = 0, fp, ra = 0; > + unsigned int shift; > + > + if (state->done) > + return -EINVAL; > + > + if (fp_state(state)) > + frame = &fp_frame; > + else > + goto done; > + > + if (frame->use_fp) { > + if (state->fp < state->sp) if (state->fp <= state->sp) I meanwhile came to the conclusion that for architectures, such as s390, where SP at function entry == SP at call site, the FP may be equal to the SP. At least for the brief period where the FP has been setup and stack allocation did not yet take place. For most architectures this can probably only occur in the topmost frame. For s390 the FP is setup after static stack allocation, so --fno-omit-frame-pointer would enforce FP==SP in any frame that does not perform dynamic stack allocation. > + goto done; > + cfa = state->fp; > + } else { > + cfa = state->sp; > + } > + > + /* Get the Canonical Frame Address (CFA) */ > + cfa += frame->cfa_off; > + > + /* stack going in wrong direction? */ > + if (cfa <= state->sp) > + goto done; > + > + /* Make sure that the address is word aligned */ > + shift = sizeof(long) == 4 ? 2 : 3; > + if ((cfa + frame->ra_off) & ((1 << shift) - 1)) > + goto done; Do all architectures/ABI mandate register stack save slots to be aligned? s390 does. > + > + /* Find the Return Address (RA) */ > + if (get_user(ra, (unsigned long *)(cfa + frame->ra_off))) > + goto done; > + Why not validate the FP stack save slot address as well? > + if (frame->fp_off && get_user(fp, (unsigned long __user *)(cfa + frame->fp_off))) > + goto done; > + > + state->ip = ra; > + state->sp = cfa; > + if (frame->fp_off) > + state->fp = fp; > + > + return 0; > + > +done: > + state->done = true; > return -EINVAL; > } Thanks and regards, Jens -- Jens Remus Linux on Z Development (D3303) +49-7031-16-1128 Office jremus@de.ibm.com IBM IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294 IBM Data Privacy Statement: https://www.ibm.com/privacy/
On Wed, 9 Jul 2025 12:01:14 +0200 Jens Remus <jremus@linux.ibm.com> wrote: > > static int unwind_user_next(struct unwind_user_state *state) > > { > > - /* no implementation yet */ > > + struct unwind_user_frame *frame; > > + unsigned long cfa = 0, fp, ra = 0; > > + unsigned int shift; > > + > > + if (state->done) > > + return -EINVAL; > > + > > + if (fp_state(state)) > > + frame = &fp_frame; > > + else > > + goto done; > > + > > + if (frame->use_fp) { > > + if (state->fp < state->sp) > > if (state->fp <= state->sp) > > I meanwhile came to the conclusion that for architectures, such as s390, > where SP at function entry == SP at call site, the FP may be equal to > the SP. At least for the brief period where the FP has been setup and > stack allocation did not yet take place. For most architectures this > can probably only occur in the topmost frame. For s390 the FP is setup > after static stack allocation, so --fno-omit-frame-pointer would enforce > FP==SP in any frame that does not perform dynamic stack allocation. From your latest email, I take it I can ignore the above? > > > + goto done; > > + cfa = state->fp; > > + } else { > > + cfa = state->sp; > > + } > > + > > + /* Get the Canonical Frame Address (CFA) */ > > + cfa += frame->cfa_off; > > + > > + /* stack going in wrong direction? */ > > + if (cfa <= state->sp) > > + goto done; > > + > > + /* Make sure that the address is word aligned */ > > + shift = sizeof(long) == 4 ? 2 : 3; > > + if ((cfa + frame->ra_off) & ((1 << shift) - 1)) > > + goto done; > > Do all architectures/ABI mandate register stack save slots to be aligned? > s390 does. I believe so. > > > + > > + /* Find the Return Address (RA) */ > > + if (get_user(ra, (unsigned long *)(cfa + frame->ra_off))) > > + goto done; > > + > > Why not validate the FP stack save slot address as well? You mean to validate cfa + frame->fp_off? Isn't cfa the only real variable here? That is, if cfa + frame->ra_off works, wouldn't the same go for frame->fp_off, as both frame->ra_off and frame->fp_off are constants set by the architecture, and should be word aligned. -- Steve > > > + if (frame->fp_off && get_user(fp, (unsigned long __user *)(cfa + frame->fp_off))) > > + goto done; > > +
On 10.07.2025 17:21, Steven Rostedt wrote: > On Wed, 9 Jul 2025 12:01:14 +0200 > Jens Remus <jremus@linux.ibm.com> wrote: >>> + if (frame->use_fp) { >>> + if (state->fp < state->sp) >> >> if (state->fp <= state->sp) >> >> I meanwhile came to the conclusion that for architectures, such as s390, >> where SP at function entry == SP at call site, the FP may be equal to >> the SP. At least for the brief period where the FP has been setup and >> stack allocation did not yet take place. For most architectures this >> can probably only occur in the topmost frame. For s390 the FP is setup >> after static stack allocation, so --fno-omit-frame-pointer would enforce >> FP==SP in any frame that does not perform dynamic stack allocation. > > From your latest email, I take it I can ignore the above? Correct. >>> + /* Make sure that the address is word aligned */ >>> + shift = sizeof(long) == 4 ? 2 : 3; >>> + if ((cfa + frame->ra_off) & ((1 << shift) - 1)) >>> + goto done; >> >> Do all architectures/ABI mandate register stack save slots to be aligned? >> s390 does. > > I believe so. > >> >>> + >>> + /* Find the Return Address (RA) */ >>> + if (get_user(ra, (unsigned long *)(cfa + frame->ra_off))) >>> + goto done; >>> + >> >> Why not validate the FP stack save slot address as well? > > You mean to validate cfa + frame->fp_off? Yes. > Isn't cfa the only real variable here? That is, if cfa + frame->ra_off > works, wouldn't the same go for frame->fp_off, as both frame->ra_off > and frame->fp_off are constants set by the architecture, and should be > word aligned. cfa + frame->ra_off could be aligned by chance. So could cfa + frame->fp_off be as well of course. On s390 the CFA must be aligned (as the SP must be aligned) and the FP and RA offsets from CFA must be aligned, as pointer / 64-bit integers (such as 64-bit register values) must be aligned as well. So the CFA (and/or offset), FP offset, and RA offset could be validated individually. Not sure if that would be over engineering though. >>> + if (frame->fp_off && get_user(fp, (unsigned long __user *)(cfa + frame->fp_off))) >>> + goto done; Regards, Jens -- Jens Remus Linux on Z Development (D3303) +49-7031-16-1128 Office jremus@de.ibm.com IBM IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294 IBM Data Privacy Statement: https://www.ibm.com/privacy/
On Thu, 10 Jul 2025 17:41:36 +0200 Jens Remus <jremus@linux.ibm.com> wrote: > cfa + frame->ra_off could be aligned by chance. So could > cfa + frame->fp_off be as well of course. > > On s390 the CFA must be aligned (as the SP must be aligned) and the > FP and RA offsets from CFA must be aligned, as pointer / 64-bit integers > (such as 64-bit register values) must be aligned as well. > > So the CFA (and/or offset), FP offset, and RA offset could be validated > individually. Not sure if that would be over engineering though. I wonder if we should just validate that cfa is aligned? Would that work? I would think that ra_off and fp_off should be aligned as well and if cfa is aligned then it would still be aligned when adding those offsets. -- Steve
On 10.07.2025 19:08, Steven Rostedt wrote: > On Thu, 10 Jul 2025 17:41:36 +0200 > Jens Remus <jremus@linux.ibm.com> wrote: > >> cfa + frame->ra_off could be aligned by chance. So could >> cfa + frame->fp_off be as well of course. >> >> On s390 the CFA must be aligned (as the SP must be aligned) and the >> FP and RA offsets from CFA must be aligned, as pointer / 64-bit integers >> (such as 64-bit register values) must be aligned as well. >> >> So the CFA (and/or offset), FP offset, and RA offset could be validated >> individually. Not sure if that would be over engineering though. > > I wonder if we should just validate that cfa is aligned? Would that work? > > I would think that ra_off and fp_off should be aligned as well and if > cfa is aligned then it would still be aligned when adding those offsets. Makes sense, if the base assumption is that the SFrame information is valid and the primary intend is to check that the used CFA base register (i.e. SP or FP) was aligned. Regards, Jens -- Jens Remus Linux on Z Development (D3303) +49-7031-16-1128 Office jremus@de.ibm.com IBM IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294 IBM Data Privacy Statement: https://www.ibm.com/privacy/
On 09.07.2025 12:01, Jens Remus wrote: > On 08.07.2025 03:22, Steven Rostedt wrote: >> From: Josh Poimboeuf <jpoimboe@kernel.org> >> diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c >> static int unwind_user_next(struct unwind_user_state *state) >> { >> - /* no implementation yet */ >> + struct unwind_user_frame *frame; >> + unsigned long cfa = 0, fp, ra = 0; >> + unsigned int shift; >> + >> + if (state->done) >> + return -EINVAL; >> + >> + if (fp_state(state)) >> + frame = &fp_frame; >> + else >> + goto done; >> + >> + if (frame->use_fp) { >> + if (state->fp < state->sp) The initial check above is correct. I got the logic wrong. Sorry for the fuss! Do not change the check to what I came up with yesterday: > if (state->fp <= state->sp) > Below s390 particularity, that FP may be equal to FP in any frame, is only allowed with the initial check. > I meanwhile came to the conclusion that for architectures, such as s390, > where SP at function entry == SP at call site, the FP may be equal to > the SP. At least for the brief period where the FP has been setup and > stack allocation did not yet take place. For most architectures this > can probably only occur in the topmost frame. For s390 the FP is setup > after static stack allocation, so --fno-omit-frame-pointer would enforce > FP==SP in any frame that does not perform dynamic stack allocation. > >> + goto done; >> + cfa = state->fp; >> + } else { >> + cfa = state->sp; >> + } Regards, Jens -- Jens Remus Linux on Z Development (D3303) +49-7031-16-1128 Office jremus@de.ibm.com IBM IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294 IBM Data Privacy Statement: https://www.ibm.com/privacy/
© 2016 - 2025 Red Hat, Inc.