* [PATCH v2 1/6] unwind: build kernel with sframe info
2025-09-04 22:38 [PATCH v2 0/6] unwind, arm64: add sframe unwinder for kernel Dylan Hatch
@ 2025-09-04 22:38 ` Dylan Hatch
2025-09-04 22:38 ` [PATCH v2 2/6] arm64: entry: add unwind info for various kernel entries Dylan Hatch
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Dylan Hatch @ 2025-09-04 22:38 UTC (permalink / raw)
To: Josh Poimboeuf, Steven Rostedt, Indu Bhagat, Peter Zijlstra,
Will Deacon, Catalin Marinas, Jiri Kosina
Cc: Dylan Hatch, Roman Gushchin, Weinan Liu, Mark Rutland, Ian Rogers,
linux-toolchains, linux-kernel, live-patching, joe.lawrence,
Puranjay Mohan, Song Liu, Prasanna Kumar T S M
Use the -Wa,--gsframe flags to build the code, so GAS will generate
a new .sframe section for the stack trace information.
Currently, the sframe format only supports arm64 and x86_64
architectures. Add this configuration on arm64 to enable sframe
unwinder in the future.
Signed-off-by: Weinan Liu <wnliu@google.com>
Signed-off-by: Dylan Hatch <dylanbhatch@google.com>
Reviewed-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
---
Makefile | 8 ++++++++
arch/Kconfig | 6 ++++++
arch/arm64/Kconfig.debug | 10 ++++++++++
arch/arm64/kernel/vdso/Makefile | 2 +-
include/asm-generic/vmlinux.lds.h | 15 +++++++++++++++
5 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index b9c661913250..09972c71a3e8 100644
--- a/Makefile
+++ b/Makefile
@@ -1078,6 +1078,14 @@ endif
# Ensure compilers do not transform certain loops into calls to wcslen()
KBUILD_CFLAGS += -fno-builtin-wcslen
+# build with sframe table
+ifdef CONFIG_SFRAME_UNWIND_TABLE
+CC_FLAGS_SFRAME := -Wa,--gsframe
+KBUILD_CFLAGS += $(CC_FLAGS_SFRAME)
+KBUILD_AFLAGS += $(CC_FLAGS_SFRAME)
+export CC_FLAGS_SFRAME
+endif
+
# change __FILE__ to the relative path to the source directory
ifdef building_out_of_srctree
KBUILD_CPPFLAGS += $(call cc-option,-fmacro-prefix-map=$(srcroot)/=)
diff --git a/arch/Kconfig b/arch/Kconfig
index d1b4ffd6e085..4362d2f49d91 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1782,4 +1782,10 @@ config ARCH_WANTS_PRE_LINK_VMLINUX
config ARCH_HAS_CPU_ATTACK_VECTORS
bool
+config AS_SFRAME
+ def_bool $(as-instr,.cfi_sections .sframe\n.cfi_startproc\n.cfi_endproc)
+
+config SFRAME_UNWIND_TABLE
+ bool
+
endmenu
diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
index 265c4461031f..d64bf58457de 100644
--- a/arch/arm64/Kconfig.debug
+++ b/arch/arm64/Kconfig.debug
@@ -20,4 +20,14 @@ config ARM64_RELOC_TEST
depends on m
tristate "Relocation testing module"
+config SFRAME_UNWINDER
+ bool "Sframe unwinder"
+ depends on AS_SFRAME
+ depends on 64BIT
+ select SFRAME_UNWIND_TABLE
+ help
+ This option enables the sframe (Simple Frame) unwinder for unwinding
+ kernel stack traces. It uses unwind table that is directly generated
+ by toolchain based on DWARF CFI information.
+
source "drivers/hwtracing/coresight/Kconfig"
diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
index 7dec05dd33b7..c60ef921956f 100644
--- a/arch/arm64/kernel/vdso/Makefile
+++ b/arch/arm64/kernel/vdso/Makefile
@@ -38,7 +38,7 @@ ccflags-y += -DDISABLE_BRANCH_PROFILING -DBUILD_VDSO
CC_FLAGS_REMOVE_VDSO := $(CC_FLAGS_FTRACE) -Os $(CC_FLAGS_SCS) \
$(RANDSTRUCT_CFLAGS) $(KSTACK_ERASE_CFLAGS) \
$(GCC_PLUGINS_CFLAGS) \
- $(CC_FLAGS_LTO) $(CC_FLAGS_CFI) \
+ $(CC_FLAGS_LTO) $(CC_FLAGS_CFI) $(CC_FLAGS_SFRAME) \
-Wmissing-prototypes -Wmissing-declarations
CC_FLAGS_ADD_VDSO := -O2 -mcmodel=tiny -fasynchronous-unwind-tables
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index ae2d2359b79e..4f486080e4fb 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -473,6 +473,8 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
*(.rodata1) \
} \
\
+ SFRAME \
+ \
/* PCI quirks */ \
.pci_fixup : AT(ADDR(.pci_fixup) - LOAD_OFFSET) { \
BOUNDED_SECTION_PRE_LABEL(.pci_fixup_early, _pci_fixups_early, __start, __end) \
@@ -891,6 +893,19 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
#define TRACEDATA
#endif
+#ifdef CONFIG_SFRAME_UNWIND_TABLE
+#define SFRAME \
+ /* sframe */ \
+ .sframe : AT(ADDR(.sframe) - LOAD_OFFSET) { \
+ __start_sframe_header = .; \
+ KEEP(*(.sframe)) \
+ KEEP(*(.init.sframe)) \
+ __stop_sframe_header = .; \
+ }
+#else
+#define SFRAME
+#endif
+
#ifdef CONFIG_PRINTK_INDEX
#define PRINTK_INDEX \
.printk_index : AT(ADDR(.printk_index) - LOAD_OFFSET) { \
--
2.51.0.355.g5224444f11-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 2/6] arm64: entry: add unwind info for various kernel entries
2025-09-04 22:38 [PATCH v2 0/6] unwind, arm64: add sframe unwinder for kernel Dylan Hatch
2025-09-04 22:38 ` [PATCH v2 1/6] unwind: build kernel with sframe info Dylan Hatch
@ 2025-09-04 22:38 ` Dylan Hatch
2025-09-04 22:38 ` [PATCH v2 3/6] unwind: add sframe v2 header Dylan Hatch
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Dylan Hatch @ 2025-09-04 22:38 UTC (permalink / raw)
To: Josh Poimboeuf, Steven Rostedt, Indu Bhagat, Peter Zijlstra,
Will Deacon, Catalin Marinas, Jiri Kosina
Cc: Dylan Hatch, Roman Gushchin, Weinan Liu, Mark Rutland, Ian Rogers,
linux-toolchains, linux-kernel, live-patching, joe.lawrence,
Puranjay Mohan, Song Liu
From: Weinan Liu <wnliu@google.com>
DWARF CFI (Call Frame Information) specifies how to recover the return
address and callee-saved registers at each PC in a given function.
Compilers are able to generate the CFI annotations when they compile
the code to assembly language. For handcrafted assembly, we need to
annotate them by hand.
Annotate CFI unwind info for assembly for interrupt and exception
handlers.
Signed-off-by: Weinan Liu <wnliu@google.com>
Signed-off-by: Dylan Hatch <dylanbhatch@google.com>
---
arch/arm64/kernel/entry.S | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index f8018b5c1f9a..3148ede8c2c6 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -575,7 +575,12 @@ SYM_CODE_START_LOCAL(el\el\ht\()_\regsize\()_\label)
.if \el == 0
b ret_to_user
.else
+ .cfi_startproc
+ .cfi_def_cfa_offset PT_REGS_SIZE
+ .cfi_offset 29, S_FP - PT_REGS_SIZE
+ .cfi_offset 30, S_LR - PT_REGS_SIZE
b ret_to_kernel
+ .cfi_endproc
.endif
SYM_CODE_END(el\el\ht\()_\regsize\()_\label)
.endm
@@ -889,6 +894,10 @@ SYM_FUNC_START(call_on_irq_stack)
add sp, x16, #IRQ_STACK_SIZE
restore_irq x9
blr x1
+ .cfi_startproc
+ .cfi_def_cfa 29, 16
+ .cfi_offset 29, -16
+ .cfi_offset 30, -8
save_and_disable_daif x9
/*
@@ -900,6 +909,7 @@ SYM_FUNC_START(call_on_irq_stack)
scs_load_current
restore_irq x9
ret
+ .cfi_endproc
SYM_FUNC_END(call_on_irq_stack)
NOKPROBE(call_on_irq_stack)
--
2.51.0.355.g5224444f11-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 3/6] unwind: add sframe v2 header
2025-09-04 22:38 [PATCH v2 0/6] unwind, arm64: add sframe unwinder for kernel Dylan Hatch
2025-09-04 22:38 ` [PATCH v2 1/6] unwind: build kernel with sframe info Dylan Hatch
2025-09-04 22:38 ` [PATCH v2 2/6] arm64: entry: add unwind info for various kernel entries Dylan Hatch
@ 2025-09-04 22:38 ` Dylan Hatch
2025-09-04 22:38 ` [PATCH v2 4/6] unwind: Implement generic sframe unwinder library Dylan Hatch
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Dylan Hatch @ 2025-09-04 22:38 UTC (permalink / raw)
To: Josh Poimboeuf, Steven Rostedt, Indu Bhagat, Peter Zijlstra,
Will Deacon, Catalin Marinas, Jiri Kosina
Cc: Dylan Hatch, Roman Gushchin, Weinan Liu, Mark Rutland, Ian Rogers,
linux-toolchains, linux-kernel, live-patching, joe.lawrence,
Puranjay Mohan, Song Liu, Prasanna Kumar T S M
Add sframe header so that we know how to access the sframe section
generated by compilers.
This is the sframe header file borrowed from the patchset [1]
Josh Poimboeuf according to sframe v2 spec [2].
[1]: https://lore.kernel.org/all/f27e8463783febfa0dabb0432a3dd6be8ad98412.1737511963.git.jpoimboe@kernel.org/
[2]: https://sourceware.org/binutils/docs/sframe-spec.html
Signed-off-by: Weinan Liu <wnliu@google.com>
Signed-off-by: Dylan Hatch <dylanbhatch@google.com>
Reviewed-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
---
kernel/sframe.h | 75 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 75 insertions(+)
create mode 100644 kernel/sframe.h
diff --git a/kernel/sframe.h b/kernel/sframe.h
new file mode 100644
index 000000000000..e9045f980fee
--- /dev/null
+++ b/kernel/sframe.h
@@ -0,0 +1,75 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * From https://www.sourceware.org/binutils/docs/sframe-spec.html
+ */
+#ifndef _SFRAME_H
+#define _SFRAME_H
+
+#include <linux/types.h>
+
+#define SFRAME_VERSION_1 1
+#define SFRAME_VERSION_2 2
+#define SFRAME_MAGIC 0xdee2
+
+#define SFRAME_F_FDE_SORTED 0x1
+#define SFRAME_F_FRAME_POINTER 0x2
+
+#define SFRAME_ABI_AARCH64_ENDIAN_BIG 1
+#define SFRAME_ABI_AARCH64_ENDIAN_LITTLE 2
+#define SFRAME_ABI_AMD64_ENDIAN_LITTLE 3
+
+#define SFRAME_FRE_TYPE_ADDR1 0
+#define SFRAME_FRE_TYPE_ADDR2 1
+#define SFRAME_FRE_TYPE_ADDR4 2
+
+#define SFRAME_FDE_TYPE_PCINC 0
+#define SFRAME_FDE_TYPE_PCMASK 1
+
+struct sframe_preamble {
+ u16 magic;
+ u8 version;
+ u8 flags;
+} __packed;
+
+struct sframe_header {
+ struct sframe_preamble preamble;
+ u8 abi_arch;
+ s8 cfa_fixed_fp_offset;
+ s8 cfa_fixed_ra_offset;
+ u8 auxhdr_len;
+ u32 num_fdes;
+ u32 num_fres;
+ u32 fre_len;
+ u32 fdes_off;
+ u32 fres_off;
+} __packed;
+
+#define SFRAME_HEADER_SIZE(header) \
+ ((sizeof(struct sframe_header) + (header).auxhdr_len))
+
+#define SFRAME_AARCH64_PAUTH_KEY_A 0
+#define SFRAME_AARCH64_PAUTH_KEY_B 1
+
+struct sframe_fde {
+ s32 start_addr;
+ u32 func_size;
+ u32 fres_off;
+ u32 fres_num;
+ u8 info;
+ u8 rep_size;
+ u16 padding;
+} __packed;
+
+#define SFRAME_FUNC_FRE_TYPE(data) (data & 0xf)
+#define SFRAME_FUNC_FDE_TYPE(data) ((data >> 4) & 0x1)
+#define SFRAME_FUNC_PAUTH_KEY(data) ((data >> 5) & 0x1)
+
+#define SFRAME_BASE_REG_FP 0
+#define SFRAME_BASE_REG_SP 1
+
+#define SFRAME_FRE_CFA_BASE_REG_ID(data) (data & 0x1)
+#define SFRAME_FRE_OFFSET_COUNT(data) ((data >> 1) & 0xf)
+#define SFRAME_FRE_OFFSET_SIZE(data) ((data >> 5) & 0x3)
+#define SFRAME_FRE_MANGLED_RA_P(data) ((data >> 7) & 0x1)
+
+#endif /* _SFRAME_H */
--
2.51.0.355.g5224444f11-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 4/6] unwind: Implement generic sframe unwinder library
2025-09-04 22:38 [PATCH v2 0/6] unwind, arm64: add sframe unwinder for kernel Dylan Hatch
` (2 preceding siblings ...)
2025-09-04 22:38 ` [PATCH v2 3/6] unwind: add sframe v2 header Dylan Hatch
@ 2025-09-04 22:38 ` Dylan Hatch
2025-09-09 16:44 ` Puranjay Mohan
2025-09-04 22:38 ` [PATCH v2 5/6] arm64/module, unwind: Add sframe support for modules Dylan Hatch
2025-09-04 22:38 ` [PATCH v2 6/6] unwind: arm64: Add reliable stacktrace with sframe unwinder Dylan Hatch
5 siblings, 1 reply; 10+ messages in thread
From: Dylan Hatch @ 2025-09-04 22:38 UTC (permalink / raw)
To: Josh Poimboeuf, Steven Rostedt, Indu Bhagat, Peter Zijlstra,
Will Deacon, Catalin Marinas, Jiri Kosina
Cc: Dylan Hatch, Roman Gushchin, Weinan Liu, Mark Rutland, Ian Rogers,
linux-toolchains, linux-kernel, live-patching, joe.lawrence,
Puranjay Mohan, Song Liu, Prasanna Kumar T S M
From: Weinan Liu <wnliu@google.com>
This change introduces a kernel space unwinder using sframe table for
architectures without ORC unwinder support.
The implementation is adapted from Josh's userspace sframe unwinder
proposal[1] according to the sframe v2 spec[2].
[1] https://lore.kernel.org/lkml/42c0a99236af65c09c8182e260af7bcf5aa1e158.1730150953.git.jpoimboe@kernel.org/
[2] https://sourceware.org/binutils/docs/sframe-spec.html
Signed-off-by: Weinan Liu <wnliu@google.com>
Signed-off-by: Dylan Hatch <dylanbhatch@google.com>
Reviewed-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
---
include/linux/sframe_lookup.h | 43 ++++++++
kernel/Makefile | 1 +
kernel/sframe_lookup.c | 196 ++++++++++++++++++++++++++++++++++
3 files changed, 240 insertions(+)
create mode 100644 include/linux/sframe_lookup.h
create mode 100644 kernel/sframe_lookup.c
diff --git a/include/linux/sframe_lookup.h b/include/linux/sframe_lookup.h
new file mode 100644
index 000000000000..1c26cf1f38d4
--- /dev/null
+++ b/include/linux/sframe_lookup.h
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_SFRAME_LOOKUP_H
+#define _LINUX_SFRAME_LOOKUP_H
+
+/**
+ * struct sframe_ip_entry - sframe unwind info for given ip
+ * @cfa_offset: Offset for the Canonical Frame Address(CFA) from Frame
+ * Pointer(FP) or Stack Pointer(SP)
+ * @ra_offset: Offset for the Return Address from CFA.
+ * @fp_offset: Offset for the Frame Pointer (FP) from CFA.
+ * @use_fp: Use FP to get next CFA or not
+ */
+struct sframe_ip_entry {
+ int32_t cfa_offset;
+ int32_t ra_offset;
+ int32_t fp_offset;
+ bool use_fp;
+};
+
+/**
+ * struct sframe_table - sframe struct of a table
+ * @sfhdr_p: Pointer to sframe header
+ * @fde_p: Pointer to the first of sframe frame description entry(FDE).
+ * @fre_p: Pointer to the first of sframe frame row entry(FRE).
+ */
+struct sframe_table {
+ struct sframe_header *sfhdr_p;
+ struct sframe_fde *fde_p;
+ char *fre_p;
+};
+
+#ifdef CONFIG_SFRAME_UNWINDER
+void init_sframe_table(void);
+int sframe_find_pc(unsigned long pc, struct sframe_ip_entry *entry);
+#else
+static inline void init_sframe_table(void) {}
+static inline int sframe_find_pc(unsigned long pc, struct sframe_ip_entry *entry)
+{
+ return -EINVAL;
+}
+#endif
+
+#endif /* _LINUX_SFRAME_LOOKUP_H */
diff --git a/kernel/Makefile b/kernel/Makefile
index c60623448235..17e9cfe09dc0 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -138,6 +138,7 @@ obj-$(CONFIG_WATCH_QUEUE) += watch_queue.o
obj-$(CONFIG_RESOURCE_KUNIT_TEST) += resource_kunit.o
obj-$(CONFIG_SYSCTL_KUNIT_TEST) += sysctl-test.o
+obj-$(CONFIG_SFRAME_UNWINDER) += sframe_lookup.o
CFLAGS_kstack_erase.o += $(DISABLE_KSTACK_ERASE)
CFLAGS_kstack_erase.o += $(call cc-option,-mgeneral-regs-only)
diff --git a/kernel/sframe_lookup.c b/kernel/sframe_lookup.c
new file mode 100644
index 000000000000..51cd24a75956
--- /dev/null
+++ b/kernel/sframe_lookup.c
@@ -0,0 +1,196 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#define pr_fmt(fmt) "sframe: " fmt
+
+#include <linux/module.h>
+#include <linux/sort.h>
+#include <linux/sframe_lookup.h>
+#include <linux/kallsyms.h>
+#include "sframe.h"
+
+extern char __start_sframe_header[];
+extern char __stop_sframe_header[];
+
+static bool sframe_init __ro_after_init;
+static struct sframe_table sftbl;
+
+#define SFRAME_READ_TYPE(in, out, type) \
+({ \
+ type __tmp; \
+ memcpy(&__tmp, in, sizeof(__tmp)); \
+ in += sizeof(__tmp); \
+ out = __tmp; \
+})
+
+#define SFRAME_READ_ROW_ADDR(in_addr, out_addr, type) \
+({ \
+ switch (type) { \
+ case SFRAME_FRE_TYPE_ADDR1: \
+ SFRAME_READ_TYPE(in_addr, out_addr, u8); \
+ break; \
+ case SFRAME_FRE_TYPE_ADDR2: \
+ SFRAME_READ_TYPE(in_addr, out_addr, u16); \
+ break; \
+ case SFRAME_FRE_TYPE_ADDR4: \
+ SFRAME_READ_TYPE(in_addr, out_addr, u32); \
+ break; \
+ default: \
+ break; \
+ } \
+})
+
+#define SFRAME_READ_ROW_OFFSETS(in_addr, out_addr, size) \
+({ \
+ switch (size) { \
+ case 1: \
+ SFRAME_READ_TYPE(in_addr, out_addr, s8); \
+ break; \
+ case 2: \
+ SFRAME_READ_TYPE(in_addr, out_addr, s16); \
+ break; \
+ case 4: \
+ SFRAME_READ_TYPE(in_addr, out_addr, s32); \
+ break; \
+ default: \
+ break; \
+ } \
+})
+
+static struct sframe_fde *find_fde(const struct sframe_table *tbl, unsigned long pc)
+{
+ int l, r, m, f;
+ int32_t ip;
+ struct sframe_fde *fdep;
+
+ if (!tbl || !tbl->sfhdr_p || !tbl->fde_p)
+ return NULL;
+
+ ip = (pc - (unsigned long)tbl->sfhdr_p);
+
+ /* Do a binary range search to find the rightmost FDE start_addr < ip */
+ l = m = f = 0;
+ r = tbl->sfhdr_p->num_fdes;
+ while (l < r) {
+ m = l + ((r - l) / 2);
+ fdep = tbl->fde_p + m;
+ if (fdep->start_addr > ip)
+ r = m;
+ else
+ l = m + 1;
+ }
+ /* use l - 1 because l will be the first item fdep->start_addr > ip */
+ f = l - 1;
+ if (f >= tbl->sfhdr_p->num_fdes || f < 0)
+ return NULL;
+ fdep = tbl->fde_p + f;
+ if (ip < fdep->start_addr || ip > fdep->start_addr + fdep->func_size)
+ return NULL;
+
+ return fdep;
+}
+
+static int find_fre(const struct sframe_table *tbl, unsigned long pc,
+ const struct sframe_fde *fdep, struct sframe_ip_entry *entry)
+{
+ int i, offset_size, offset_count;
+ char *fres, *offsets_loc;
+ int32_t ip_off;
+ uint32_t next_row_ip_off;
+ uint8_t fre_info, fde_type = SFRAME_FUNC_FDE_TYPE(fdep->info),
+ fre_type = SFRAME_FUNC_FRE_TYPE(fdep->info);
+
+ fres = tbl->fre_p + fdep->fres_off;
+
+ /* Whether PCs in the FREs should be treated as masks or not */
+ if (fde_type == SFRAME_FDE_TYPE_PCMASK)
+ ip_off = pc % fdep->rep_size;
+ else
+ ip_off = (int32_t)(pc - (unsigned long)tbl->sfhdr_p) - fdep->start_addr;
+
+ if (ip_off < 0 || ip_off > fdep->func_size)
+ return -EINVAL;
+
+ /*
+ * FRE structure starts by address of the entry with variants length. Use
+ * two pointers to track current head(fres) and the address of last
+ * offset(offsets_loc)
+ */
+ for (i = 0; i < fdep->fres_num; i++) {
+ SFRAME_READ_ROW_ADDR(fres, next_row_ip_off, fre_type);
+ if (ip_off < next_row_ip_off)
+ break;
+ SFRAME_READ_TYPE(fres, fre_info, u8);
+ offsets_loc = fres;
+ /*
+ * jump to the start of next fre
+ * fres += fre_offets_cnt*offset_size
+ */
+ fres += SFRAME_FRE_OFFSET_COUNT(fre_info) << SFRAME_FRE_OFFSET_SIZE(fre_info);
+ }
+
+ offset_size = 1 << SFRAME_FRE_OFFSET_SIZE(fre_info);
+ offset_count = SFRAME_FRE_OFFSET_COUNT(fre_info);
+
+ if (offset_count > 0) {
+ SFRAME_READ_ROW_OFFSETS(offsets_loc, entry->cfa_offset, offset_size);
+ offset_count--;
+ }
+ if (offset_count > 0 && !entry->ra_offset) {
+ SFRAME_READ_ROW_OFFSETS(offsets_loc, entry->ra_offset, offset_size);
+ offset_count--;
+ }
+ if (offset_count > 0 && !entry->fp_offset) {
+ SFRAME_READ_ROW_OFFSETS(offsets_loc, entry->fp_offset, offset_size);
+ offset_count--;
+ }
+ if (offset_count)
+ return -EINVAL;
+
+ entry->use_fp = SFRAME_FRE_CFA_BASE_REG_ID(fre_info) == SFRAME_BASE_REG_FP;
+
+ return 0;
+}
+
+int sframe_find_pc(unsigned long pc, struct sframe_ip_entry *entry)
+{
+ struct sframe_fde *fdep;
+ struct sframe_table *sftbl_p = &sftbl;
+ int err;
+
+ if (!sframe_init)
+ return -EINVAL;
+
+ memset(entry, 0, sizeof(*entry));
+ entry->ra_offset = sftbl_p->sfhdr_p->cfa_fixed_ra_offset;
+ entry->fp_offset = sftbl_p->sfhdr_p->cfa_fixed_fp_offset;
+
+ fdep = find_fde(sftbl_p, pc);
+ if (!fdep)
+ return -EINVAL;
+ err = find_fre(sftbl_p, pc, fdep, entry);
+ if (err)
+ return err;
+
+ return 0;
+}
+
+void __init init_sframe_table(void)
+{
+ size_t sframe_size = (void *)__stop_sframe_header - (void *)__start_sframe_header;
+ void *sframe_buf = __start_sframe_header;
+
+ if (sframe_size <= 0)
+ return;
+ sftbl.sfhdr_p = sframe_buf;
+ if (!sftbl.sfhdr_p || sftbl.sfhdr_p->preamble.magic != SFRAME_MAGIC ||
+ sftbl.sfhdr_p->preamble.version != SFRAME_VERSION_2 ||
+ !(sftbl.sfhdr_p->preamble.flags & SFRAME_F_FDE_SORTED)) {
+ pr_warn("WARNING: Unable to read sframe header. Disabling unwinder.\n");
+ return;
+ }
+
+ sftbl.fde_p = (struct sframe_fde *)(__start_sframe_header + SFRAME_HEADER_SIZE(*sftbl.sfhdr_p)
+ + sftbl.sfhdr_p->fdes_off);
+ sftbl.fre_p = __start_sframe_header + SFRAME_HEADER_SIZE(*sftbl.sfhdr_p)
+ + sftbl.sfhdr_p->fres_off;
+ sframe_init = true;
+}
--
2.51.0.355.g5224444f11-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v2 4/6] unwind: Implement generic sframe unwinder library
2025-09-04 22:38 ` [PATCH v2 4/6] unwind: Implement generic sframe unwinder library Dylan Hatch
@ 2025-09-09 16:44 ` Puranjay Mohan
2025-09-09 18:39 ` Indu Bhagat
0 siblings, 1 reply; 10+ messages in thread
From: Puranjay Mohan @ 2025-09-09 16:44 UTC (permalink / raw)
To: Dylan Hatch, Josh Poimboeuf, Steven Rostedt, Indu Bhagat,
Peter Zijlstra, Will Deacon, Catalin Marinas, Jiri Kosina
Cc: Dylan Hatch, Roman Gushchin, Weinan Liu, Mark Rutland, Ian Rogers,
linux-toolchains, linux-kernel, live-patching, joe.lawrence,
Song Liu, Prasanna Kumar T S M
Dylan Hatch <dylanbhatch@google.com> writes:
> From: Weinan Liu <wnliu@google.com>
>
> This change introduces a kernel space unwinder using sframe table for
> architectures without ORC unwinder support.
>
> The implementation is adapted from Josh's userspace sframe unwinder
> proposal[1] according to the sframe v2 spec[2].
>
> [1] https://lore.kernel.org/lkml/42c0a99236af65c09c8182e260af7bcf5aa1e158.1730150953.git.jpoimboe@kernel.org/
> [2] https://sourceware.org/binutils/docs/sframe-spec.html
>
> Signed-off-by: Weinan Liu <wnliu@google.com>
> Signed-off-by: Dylan Hatch <dylanbhatch@google.com>
> Reviewed-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
> ---
> include/linux/sframe_lookup.h | 43 ++++++++
> kernel/Makefile | 1 +
> kernel/sframe_lookup.c | 196 ++++++++++++++++++++++++++++++++++
> 3 files changed, 240 insertions(+)
> create mode 100644 include/linux/sframe_lookup.h
> create mode 100644 kernel/sframe_lookup.c
>
> diff --git a/include/linux/sframe_lookup.h b/include/linux/sframe_lookup.h
> new file mode 100644
> index 000000000000..1c26cf1f38d4
> --- /dev/null
> +++ b/include/linux/sframe_lookup.h
> @@ -0,0 +1,43 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_SFRAME_LOOKUP_H
> +#define _LINUX_SFRAME_LOOKUP_H
> +
> +/**
> + * struct sframe_ip_entry - sframe unwind info for given ip
> + * @cfa_offset: Offset for the Canonical Frame Address(CFA) from Frame
> + * Pointer(FP) or Stack Pointer(SP)
> + * @ra_offset: Offset for the Return Address from CFA.
> + * @fp_offset: Offset for the Frame Pointer (FP) from CFA.
> + * @use_fp: Use FP to get next CFA or not
> + */
> +struct sframe_ip_entry {
> + int32_t cfa_offset;
> + int32_t ra_offset;
> + int32_t fp_offset;
> + bool use_fp;
> +};
> +
> +/**
> + * struct sframe_table - sframe struct of a table
> + * @sfhdr_p: Pointer to sframe header
> + * @fde_p: Pointer to the first of sframe frame description entry(FDE).
> + * @fre_p: Pointer to the first of sframe frame row entry(FRE).
> + */
> +struct sframe_table {
> + struct sframe_header *sfhdr_p;
> + struct sframe_fde *fde_p;
> + char *fre_p;
> +};
> +
> +#ifdef CONFIG_SFRAME_UNWINDER
> +void init_sframe_table(void);
> +int sframe_find_pc(unsigned long pc, struct sframe_ip_entry *entry);
> +#else
> +static inline void init_sframe_table(void) {}
> +static inline int sframe_find_pc(unsigned long pc, struct sframe_ip_entry *entry)
> +{
> + return -EINVAL;
> +}
> +#endif
> +
> +#endif /* _LINUX_SFRAME_LOOKUP_H */
> diff --git a/kernel/Makefile b/kernel/Makefile
> index c60623448235..17e9cfe09dc0 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -138,6 +138,7 @@ obj-$(CONFIG_WATCH_QUEUE) += watch_queue.o
>
> obj-$(CONFIG_RESOURCE_KUNIT_TEST) += resource_kunit.o
> obj-$(CONFIG_SYSCTL_KUNIT_TEST) += sysctl-test.o
> +obj-$(CONFIG_SFRAME_UNWINDER) += sframe_lookup.o
>
> CFLAGS_kstack_erase.o += $(DISABLE_KSTACK_ERASE)
> CFLAGS_kstack_erase.o += $(call cc-option,-mgeneral-regs-only)
> diff --git a/kernel/sframe_lookup.c b/kernel/sframe_lookup.c
> new file mode 100644
> index 000000000000..51cd24a75956
> --- /dev/null
> +++ b/kernel/sframe_lookup.c
> @@ -0,0 +1,196 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#define pr_fmt(fmt) "sframe: " fmt
> +
> +#include <linux/module.h>
> +#include <linux/sort.h>
> +#include <linux/sframe_lookup.h>
> +#include <linux/kallsyms.h>
> +#include "sframe.h"
> +
> +extern char __start_sframe_header[];
> +extern char __stop_sframe_header[];
> +
> +static bool sframe_init __ro_after_init;
> +static struct sframe_table sftbl;
> +
> +#define SFRAME_READ_TYPE(in, out, type) \
> +({ \
> + type __tmp; \
> + memcpy(&__tmp, in, sizeof(__tmp)); \
> + in += sizeof(__tmp); \
> + out = __tmp; \
> +})
> +
> +#define SFRAME_READ_ROW_ADDR(in_addr, out_addr, type) \
> +({ \
> + switch (type) { \
> + case SFRAME_FRE_TYPE_ADDR1: \
> + SFRAME_READ_TYPE(in_addr, out_addr, u8); \
> + break; \
> + case SFRAME_FRE_TYPE_ADDR2: \
> + SFRAME_READ_TYPE(in_addr, out_addr, u16); \
> + break; \
> + case SFRAME_FRE_TYPE_ADDR4: \
> + SFRAME_READ_TYPE(in_addr, out_addr, u32); \
> + break; \
> + default: \
> + break; \
> + } \
> +})
> +
> +#define SFRAME_READ_ROW_OFFSETS(in_addr, out_addr, size) \
> +({ \
> + switch (size) { \
> + case 1: \
> + SFRAME_READ_TYPE(in_addr, out_addr, s8); \
> + break; \
> + case 2: \
> + SFRAME_READ_TYPE(in_addr, out_addr, s16); \
> + break; \
> + case 4: \
> + SFRAME_READ_TYPE(in_addr, out_addr, s32); \
> + break; \
> + default: \
> + break; \
> + } \
> +})
> +
> +static struct sframe_fde *find_fde(const struct sframe_table *tbl, unsigned long pc)
> +{
> + int l, r, m, f;
> + int32_t ip;
> + struct sframe_fde *fdep;
> +
> + if (!tbl || !tbl->sfhdr_p || !tbl->fde_p)
> + return NULL;
> +
> + ip = (pc - (unsigned long)tbl->sfhdr_p);
> +
> + /* Do a binary range search to find the rightmost FDE start_addr < ip */
> + l = m = f = 0;
> + r = tbl->sfhdr_p->num_fdes;
> + while (l < r) {
> + m = l + ((r - l) / 2);
> + fdep = tbl->fde_p + m;
> + if (fdep->start_addr > ip)
> + r = m;
> + else
> + l = m + 1;
> + }
The above logic doesn't correctly work for the new scheme with
SFRAME_F_FDE_FUNC_START_PCREL, see [1]
If SFRAME_F_FDE_FUNC_START_PCREL is set in flags then function start
address in SFrame FDE is encoded as the distance from the location of
the sfde_func_start_address to the start PC of the function.
And for modules, sframes will only work if compiled with [1] with
SFRAME_F_FDE_FUNC_START_PCREL flag set as ET_DYN, ET_EXEC, and ET_REL
(relocatable links) generated by ld have sfde_func_start_address as
offset from field itself. see [2] for more details.
So, for the in kernel sframe unwinder that should support both normal
links (kernel) and relocatable links (modules), we need to reject the
sframe section if this flag is not set in init_sframe_table() and in
sframe_module_init().
Then we can fix find_fde() like:
use pc in place of ip directly.
and the check will become
if (fdep->start_addr > (s32)(pc - fdep))
I hope I am not missing something,
Indu,
Do you agree with my comments above?
Thanks,
Puranjay
[1] https://sourceware.org/pipermail/binutils/2025-July/142222.html
[2] https://sourceware.org/bugzilla/show_bug.cgi?id=32666
> + /* use l - 1 because l will be the first item fdep->start_addr > ip */
> + f = l - 1;
> + if (f >= tbl->sfhdr_p->num_fdes || f < 0)
> + return NULL;
> + fdep = tbl->fde_p + f;
> + if (ip < fdep->start_addr || ip > fdep->start_addr + fdep->func_size)
> + return NULL;
> +
> + return fdep;
> +}
> +
> +static int find_fre(const struct sframe_table *tbl, unsigned long pc,
> + const struct sframe_fde *fdep, struct sframe_ip_entry *entry)
> +{
> + int i, offset_size, offset_count;
> + char *fres, *offsets_loc;
> + int32_t ip_off;
> + uint32_t next_row_ip_off;
> + uint8_t fre_info, fde_type = SFRAME_FUNC_FDE_TYPE(fdep->info),
> + fre_type = SFRAME_FUNC_FRE_TYPE(fdep->info);
> +
> + fres = tbl->fre_p + fdep->fres_off;
> +
> + /* Whether PCs in the FREs should be treated as masks or not */
> + if (fde_type == SFRAME_FDE_TYPE_PCMASK)
> + ip_off = pc % fdep->rep_size;
> + else
> + ip_off = (int32_t)(pc - (unsigned long)tbl->sfhdr_p) - fdep->start_addr;
> +
> + if (ip_off < 0 || ip_off > fdep->func_size)
> + return -EINVAL;
> +
> + /*
> + * FRE structure starts by address of the entry with variants length. Use
> + * two pointers to track current head(fres) and the address of last
> + * offset(offsets_loc)
> + */
> + for (i = 0; i < fdep->fres_num; i++) {
> + SFRAME_READ_ROW_ADDR(fres, next_row_ip_off, fre_type);
> + if (ip_off < next_row_ip_off)
> + break;
> + SFRAME_READ_TYPE(fres, fre_info, u8);
> + offsets_loc = fres;
> + /*
> + * jump to the start of next fre
> + * fres += fre_offets_cnt*offset_size
> + */
> + fres += SFRAME_FRE_OFFSET_COUNT(fre_info) << SFRAME_FRE_OFFSET_SIZE(fre_info);
> + }
> +
> + offset_size = 1 << SFRAME_FRE_OFFSET_SIZE(fre_info);
> + offset_count = SFRAME_FRE_OFFSET_COUNT(fre_info);
> +
> + if (offset_count > 0) {
> + SFRAME_READ_ROW_OFFSETS(offsets_loc, entry->cfa_offset, offset_size);
> + offset_count--;
> + }
> + if (offset_count > 0 && !entry->ra_offset) {
> + SFRAME_READ_ROW_OFFSETS(offsets_loc, entry->ra_offset, offset_size);
> + offset_count--;
> + }
> + if (offset_count > 0 && !entry->fp_offset) {
> + SFRAME_READ_ROW_OFFSETS(offsets_loc, entry->fp_offset, offset_size);
> + offset_count--;
> + }
> + if (offset_count)
> + return -EINVAL;
> +
> + entry->use_fp = SFRAME_FRE_CFA_BASE_REG_ID(fre_info) == SFRAME_BASE_REG_FP;
> +
> + return 0;
> +}
> +
> +int sframe_find_pc(unsigned long pc, struct sframe_ip_entry *entry)
> +{
> + struct sframe_fde *fdep;
> + struct sframe_table *sftbl_p = &sftbl;
> + int err;
> +
> + if (!sframe_init)
> + return -EINVAL;
> +
> + memset(entry, 0, sizeof(*entry));
> + entry->ra_offset = sftbl_p->sfhdr_p->cfa_fixed_ra_offset;
> + entry->fp_offset = sftbl_p->sfhdr_p->cfa_fixed_fp_offset;
> +
> + fdep = find_fde(sftbl_p, pc);
> + if (!fdep)
> + return -EINVAL;
> + err = find_fre(sftbl_p, pc, fdep, entry);
> + if (err)
> + return err;
> +
> + return 0;
> +}
> +
> +void __init init_sframe_table(void)
> +{
> + size_t sframe_size = (void *)__stop_sframe_header - (void *)__start_sframe_header;
> + void *sframe_buf = __start_sframe_header;
> +
> + if (sframe_size <= 0)
> + return;
> + sftbl.sfhdr_p = sframe_buf;
> + if (!sftbl.sfhdr_p || sftbl.sfhdr_p->preamble.magic != SFRAME_MAGIC ||
> + sftbl.sfhdr_p->preamble.version != SFRAME_VERSION_2 ||
> + !(sftbl.sfhdr_p->preamble.flags & SFRAME_F_FDE_SORTED)) {
> + pr_warn("WARNING: Unable to read sframe header. Disabling unwinder.\n");
> + return;
> + }
> +
> + sftbl.fde_p = (struct sframe_fde *)(__start_sframe_header + SFRAME_HEADER_SIZE(*sftbl.sfhdr_p)
> + + sftbl.sfhdr_p->fdes_off);
> + sftbl.fre_p = __start_sframe_header + SFRAME_HEADER_SIZE(*sftbl.sfhdr_p)
> + + sftbl.sfhdr_p->fres_off;
> + sframe_init = true;
> +}
> --
> 2.51.0.355.g5224444f11-goog
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2 4/6] unwind: Implement generic sframe unwinder library
2025-09-09 16:44 ` Puranjay Mohan
@ 2025-09-09 18:39 ` Indu Bhagat
0 siblings, 0 replies; 10+ messages in thread
From: Indu Bhagat @ 2025-09-09 18:39 UTC (permalink / raw)
To: Puranjay Mohan, Dylan Hatch, Josh Poimboeuf, Steven Rostedt,
Peter Zijlstra, Will Deacon, Catalin Marinas, Jiri Kosina
Cc: Roman Gushchin, Weinan Liu, Mark Rutland, Ian Rogers,
linux-toolchains, linux-kernel, live-patching, joe.lawrence,
Song Liu, Prasanna Kumar T S M, Jens Remus
On 9/9/25 9:44 AM, Puranjay Mohan wrote:
> Dylan Hatch <dylanbhatch@google.com> writes:
>
>> From: Weinan Liu <wnliu@google.com>
>>
>> This change introduces a kernel space unwinder using sframe table for
>> architectures without ORC unwinder support.
>>
>> The implementation is adapted from Josh's userspace sframe unwinder
>> proposal[1] according to the sframe v2 spec[2].
>>
>> [1] https://lore.kernel.org/lkml/42c0a99236af65c09c8182e260af7bcf5aa1e158.1730150953.git.jpoimboe@kernel.org/
>> [2] https://sourceware.org/binutils/docs/sframe-spec.html
>>
>> Signed-off-by: Weinan Liu <wnliu@google.com>
>> Signed-off-by: Dylan Hatch <dylanbhatch@google.com>
>> Reviewed-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
>> ---
>> include/linux/sframe_lookup.h | 43 ++++++++
>> kernel/Makefile | 1 +
>> kernel/sframe_lookup.c | 196 ++++++++++++++++++++++++++++++++++
>> 3 files changed, 240 insertions(+)
>> create mode 100644 include/linux/sframe_lookup.h
>> create mode 100644 kernel/sframe_lookup.c
>>
>> diff --git a/include/linux/sframe_lookup.h b/include/linux/sframe_lookup.h
>> new file mode 100644
>> index 000000000000..1c26cf1f38d4
>> --- /dev/null
>> +++ b/include/linux/sframe_lookup.h
>> @@ -0,0 +1,43 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef _LINUX_SFRAME_LOOKUP_H
>> +#define _LINUX_SFRAME_LOOKUP_H
>> +
>> +/**
>> + * struct sframe_ip_entry - sframe unwind info for given ip
>> + * @cfa_offset: Offset for the Canonical Frame Address(CFA) from Frame
>> + * Pointer(FP) or Stack Pointer(SP)
>> + * @ra_offset: Offset for the Return Address from CFA.
>> + * @fp_offset: Offset for the Frame Pointer (FP) from CFA.
>> + * @use_fp: Use FP to get next CFA or not
>> + */
>> +struct sframe_ip_entry {
>> + int32_t cfa_offset;
>> + int32_t ra_offset;
>> + int32_t fp_offset;
>> + bool use_fp;
>> +};
>> +
>> +/**
>> + * struct sframe_table - sframe struct of a table
>> + * @sfhdr_p: Pointer to sframe header
>> + * @fde_p: Pointer to the first of sframe frame description entry(FDE).
>> + * @fre_p: Pointer to the first of sframe frame row entry(FRE).
>> + */
>> +struct sframe_table {
>> + struct sframe_header *sfhdr_p;
>> + struct sframe_fde *fde_p;
>> + char *fre_p;
>> +};
>> +
>> +#ifdef CONFIG_SFRAME_UNWINDER
>> +void init_sframe_table(void);
>> +int sframe_find_pc(unsigned long pc, struct sframe_ip_entry *entry);
>> +#else
>> +static inline void init_sframe_table(void) {}
>> +static inline int sframe_find_pc(unsigned long pc, struct sframe_ip_entry *entry)
>> +{
>> + return -EINVAL;
>> +}
>> +#endif
>> +
>> +#endif /* _LINUX_SFRAME_LOOKUP_H */
>> diff --git a/kernel/Makefile b/kernel/Makefile
>> index c60623448235..17e9cfe09dc0 100644
>> --- a/kernel/Makefile
>> +++ b/kernel/Makefile
>> @@ -138,6 +138,7 @@ obj-$(CONFIG_WATCH_QUEUE) += watch_queue.o
>>
>> obj-$(CONFIG_RESOURCE_KUNIT_TEST) += resource_kunit.o
>> obj-$(CONFIG_SYSCTL_KUNIT_TEST) += sysctl-test.o
>> +obj-$(CONFIG_SFRAME_UNWINDER) += sframe_lookup.o
>>
>> CFLAGS_kstack_erase.o += $(DISABLE_KSTACK_ERASE)
>> CFLAGS_kstack_erase.o += $(call cc-option,-mgeneral-regs-only)
>> diff --git a/kernel/sframe_lookup.c b/kernel/sframe_lookup.c
>> new file mode 100644
>> index 000000000000..51cd24a75956
>> --- /dev/null
>> +++ b/kernel/sframe_lookup.c
>> @@ -0,0 +1,196 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +#define pr_fmt(fmt) "sframe: " fmt
>> +
>> +#include <linux/module.h>
>> +#include <linux/sort.h>
>> +#include <linux/sframe_lookup.h>
>> +#include <linux/kallsyms.h>
>> +#include "sframe.h"
>> +
>> +extern char __start_sframe_header[];
>> +extern char __stop_sframe_header[];
>> +
>> +static bool sframe_init __ro_after_init;
>> +static struct sframe_table sftbl;
>> +
>> +#define SFRAME_READ_TYPE(in, out, type) \
>> +({ \
>> + type __tmp; \
>> + memcpy(&__tmp, in, sizeof(__tmp)); \
>> + in += sizeof(__tmp); \
>> + out = __tmp; \
>> +})
>> +
>> +#define SFRAME_READ_ROW_ADDR(in_addr, out_addr, type) \
>> +({ \
>> + switch (type) { \
>> + case SFRAME_FRE_TYPE_ADDR1: \
>> + SFRAME_READ_TYPE(in_addr, out_addr, u8); \
>> + break; \
>> + case SFRAME_FRE_TYPE_ADDR2: \
>> + SFRAME_READ_TYPE(in_addr, out_addr, u16); \
>> + break; \
>> + case SFRAME_FRE_TYPE_ADDR4: \
>> + SFRAME_READ_TYPE(in_addr, out_addr, u32); \
>> + break; \
>> + default: \
>> + break; \
>> + } \
>> +})
>> +
>> +#define SFRAME_READ_ROW_OFFSETS(in_addr, out_addr, size) \
>> +({ \
>> + switch (size) { \
>> + case 1: \
>> + SFRAME_READ_TYPE(in_addr, out_addr, s8); \
>> + break; \
>> + case 2: \
>> + SFRAME_READ_TYPE(in_addr, out_addr, s16); \
>> + break; \
>> + case 4: \
>> + SFRAME_READ_TYPE(in_addr, out_addr, s32); \
>> + break; \
>> + default: \
>> + break; \
>> + } \
>> +})
>> +
>> +static struct sframe_fde *find_fde(const struct sframe_table *tbl, unsigned long pc)
>> +{
>> + int l, r, m, f;
>> + int32_t ip;
>> + struct sframe_fde *fdep;
>> +
>> + if (!tbl || !tbl->sfhdr_p || !tbl->fde_p)
>> + return NULL;
>> +
>> + ip = (pc - (unsigned long)tbl->sfhdr_p);
>> +
>> + /* Do a binary range search to find the rightmost FDE start_addr < ip */
>> + l = m = f = 0;
>> + r = tbl->sfhdr_p->num_fdes;
>> + while (l < r) {
>> + m = l + ((r - l) / 2);
>> + fdep = tbl->fde_p + m;
>> + if (fdep->start_addr > ip)
>> + r = m;
>> + else
>> + l = m + 1;
>> + }
>
> The above logic doesn't correctly work for the new scheme with
> SFRAME_F_FDE_FUNC_START_PCREL, see [1]
>
> If SFRAME_F_FDE_FUNC_START_PCREL is set in flags then function start
> address in SFrame FDE is encoded as the distance from the location of
> the sfde_func_start_address to the start PC of the function.
>
> And for modules, sframes will only work if compiled with [1] with
> SFRAME_F_FDE_FUNC_START_PCREL flag set as ET_DYN, ET_EXEC, and ET_REL
> (relocatable links) generated by ld have sfde_func_start_address as
> offset from field itself. see [2] for more details.
>
Yes.
The SFrame reader patches need to be refreshed with changes to do the
right thing when SFRAME_F_FDE_FUNC_START_PCREL flag is set.
Jens had posted a patch sometime ago (patch to update the SFrame reader
routines to work with Binutils 2.45) to serve as a starting point.
+CC: Jens Remus
> So, for the in kernel sframe unwinder that should support both normal
> links (kernel) and relocatable links (modules), we need to reject the
> sframe section if this flag is not set in init_sframe_table() and in
> sframe_module_init().
>
> Then we can fix find_fde() like:
>
> use pc in place of ip directly.
>
> and the check will become
>
> if (fdep->start_addr > (s32)(pc - fdep))
>
> I hope I am not missing something,
>
> Indu,
> Do you agree with my comments above?
>
> Thanks,
> Puranjay
>
> [1] https://sourceware.org/pipermail/binutils/2025-July/142222.html
> [2] https://sourceware.org/bugzilla/show_bug.cgi?id=32666
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 5/6] arm64/module, unwind: Add sframe support for modules.
2025-09-04 22:38 [PATCH v2 0/6] unwind, arm64: add sframe unwinder for kernel Dylan Hatch
` (3 preceding siblings ...)
2025-09-04 22:38 ` [PATCH v2 4/6] unwind: Implement generic sframe unwinder library Dylan Hatch
@ 2025-09-04 22:38 ` Dylan Hatch
2025-09-04 22:38 ` [PATCH v2 6/6] unwind: arm64: Add reliable stacktrace with sframe unwinder Dylan Hatch
5 siblings, 0 replies; 10+ messages in thread
From: Dylan Hatch @ 2025-09-04 22:38 UTC (permalink / raw)
To: Josh Poimboeuf, Steven Rostedt, Indu Bhagat, Peter Zijlstra,
Will Deacon, Catalin Marinas, Jiri Kosina
Cc: Dylan Hatch, Roman Gushchin, Weinan Liu, Mark Rutland, Ian Rogers,
linux-toolchains, linux-kernel, live-patching, joe.lawrence,
Puranjay Mohan, Song Liu
From: Weinan Liu <wnliu@google.com>
Add sframe table to mod_arch_specific and support sframe unwind when
.sframe section can be found on incoming modules.
Signed-off-by: Dylan Hatch <dylanbhatch@google.com>
Signed-off-by: Weinan Liu <wnliu@google.com>
---
arch/arm64/include/asm/module.h | 6 ++++++
arch/arm64/kernel/module.c | 5 +++++
include/linux/sframe_lookup.h | 2 ++
kernel/sframe_lookup.c | 38 ++++++++++++++++++++++++++++++++-
4 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/module.h b/arch/arm64/include/asm/module.h
index 79550b22ba19..e3781fcdc620 100644
--- a/arch/arm64/include/asm/module.h
+++ b/arch/arm64/include/asm/module.h
@@ -6,6 +6,7 @@
#define __ASM_MODULE_H
#include <asm-generic/module.h>
+#include <linux/sframe_lookup.h>
struct mod_plt_sec {
int plt_shndx;
@@ -17,6 +18,11 @@ struct mod_arch_specific {
struct mod_plt_sec core;
struct mod_plt_sec init;
+#ifdef CONFIG_SFRAME_UNWINDER
+ struct sframe_table sftbl;
+ bool sframe_init;
+#endif
+
/* for CONFIG_DYNAMIC_FTRACE */
struct plt_entry *ftrace_trampolines;
};
diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c
index 40148d2725ce..d0adeb4cf63d 100644
--- a/arch/arm64/kernel/module.c
+++ b/arch/arm64/kernel/module.c
@@ -18,6 +18,7 @@
#include <linux/moduleloader.h>
#include <linux/random.h>
#include <linux/scs.h>
+#include <linux/sframe_lookup.h>
#include <asm/alternative.h>
#include <asm/insn.h>
@@ -491,5 +492,9 @@ int module_finalize(const Elf_Ehdr *hdr,
}
}
+ s = find_section(hdr, sechdrs, ".sframe");
+ if (s)
+ sframe_module_init(me, (void *)s->sh_addr, s->sh_size);
+
return module_init_ftrace_plt(hdr, sechdrs, me);
}
diff --git a/include/linux/sframe_lookup.h b/include/linux/sframe_lookup.h
index 1c26cf1f38d4..f84c1f41a421 100644
--- a/include/linux/sframe_lookup.h
+++ b/include/linux/sframe_lookup.h
@@ -31,9 +31,11 @@ struct sframe_table {
#ifdef CONFIG_SFRAME_UNWINDER
void init_sframe_table(void);
+void sframe_module_init(struct module *mod, void *_sframe, size_t _sframe_size);
int sframe_find_pc(unsigned long pc, struct sframe_ip_entry *entry);
#else
static inline void init_sframe_table(void) {}
+static inline void sframe_module_init(struct module *mod, void *_sframe, size_t _sframe_size) {}
static inline int sframe_find_pc(unsigned long pc, struct sframe_ip_entry *entry)
{
return -EINVAL;
diff --git a/kernel/sframe_lookup.c b/kernel/sframe_lookup.c
index 51cd24a75956..c87a94f01891 100644
--- a/kernel/sframe_lookup.c
+++ b/kernel/sframe_lookup.c
@@ -156,10 +156,20 @@ int sframe_find_pc(unsigned long pc, struct sframe_ip_entry *entry)
struct sframe_table *sftbl_p = &sftbl;
int err;
- if (!sframe_init)
+ if (!entry || !sframe_init)
return -EINVAL;
memset(entry, 0, sizeof(*entry));
+
+ if (!is_ksym_addr(pc)) {
+ struct module *mod;
+
+ mod = __module_address(pc);
+ if (!mod || !mod->arch.sframe_init)
+ return -EINVAL;
+ sftbl_p = &mod->arch.sftbl;
+ }
+
entry->ra_offset = sftbl_p->sfhdr_p->cfa_fixed_ra_offset;
entry->fp_offset = sftbl_p->sfhdr_p->cfa_fixed_fp_offset;
@@ -194,3 +204,29 @@ void __init init_sframe_table(void)
+ sftbl.sfhdr_p->fres_off;
sframe_init = true;
}
+
+void sframe_module_init(struct module *mod, void *_sframe, size_t _sframe_size)
+{
+ size_t sframe_size = _sframe_size;
+ void *sframe_buf = _sframe;
+ struct sframe_table _sftbl;
+
+
+ if (sframe_size <= 0)
+ return;
+ _sftbl.sfhdr_p = sframe_buf;
+ if (!_sftbl.sfhdr_p || _sftbl.sfhdr_p->preamble.magic != SFRAME_MAGIC ||
+ _sftbl.sfhdr_p->preamble.version != SFRAME_VERSION_2 ||
+ !(_sftbl.sfhdr_p->preamble.flags & SFRAME_F_FDE_SORTED)) {
+ pr_warn("WARNING: Unable to read sframe header. Disabling unwinder.\n");
+ return;
+ }
+
+ _sftbl.fde_p = (struct sframe_fde *)(sframe_buf + SFRAME_HEADER_SIZE(*_sftbl.sfhdr_p)
+ + _sftbl.sfhdr_p->fdes_off);
+ _sftbl.fre_p = sframe_buf + SFRAME_HEADER_SIZE(*_sftbl.sfhdr_p)
+ + _sftbl.sfhdr_p->fres_off;
+
+ mod->arch.sftbl = _sftbl;
+ mod->arch.sframe_init = true;
+}
--
2.51.0.355.g5224444f11-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 6/6] unwind: arm64: Add reliable stacktrace with sframe unwinder.
2025-09-04 22:38 [PATCH v2 0/6] unwind, arm64: add sframe unwinder for kernel Dylan Hatch
` (4 preceding siblings ...)
2025-09-04 22:38 ` [PATCH v2 5/6] arm64/module, unwind: Add sframe support for modules Dylan Hatch
@ 2025-09-04 22:38 ` Dylan Hatch
2025-09-17 23:41 ` Josh Poimboeuf
5 siblings, 1 reply; 10+ messages in thread
From: Dylan Hatch @ 2025-09-04 22:38 UTC (permalink / raw)
To: Josh Poimboeuf, Steven Rostedt, Indu Bhagat, Peter Zijlstra,
Will Deacon, Catalin Marinas, Jiri Kosina
Cc: Dylan Hatch, Roman Gushchin, Weinan Liu, Mark Rutland, Ian Rogers,
linux-toolchains, linux-kernel, live-patching, joe.lawrence,
Puranjay Mohan, Song Liu, Prasanna Kumar T S M
From: Weinan Liu <wnliu@google.com>
Add unwind_next_frame_sframe() function to unwind by sframe info.
Built with GNU Binutils 2.42 to verify that this sframe unwinder can
backtrace correctly on arm64.
To support livepatch, we need to add arch_stack_walk_reliable to
support reliable stacktrace according to
https://docs.kernel.org/livepatch/reliable-stacktrace.html#requirements
report stacktrace is not reliable if we are not able to unwind the stack
by sframe unwinder and fallback to FP based unwinder
Signed-off-by: Weinan Liu <wnliu@google.com>
Signed-off-by: Dylan Hatch <dylanbhatch@google.com>
Reviewed-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
---
arch/arm64/include/asm/stacktrace/common.h | 6 ++
arch/arm64/kernel/setup.c | 2 +
arch/arm64/kernel/stacktrace.c | 102 +++++++++++++++++++++
3 files changed, 110 insertions(+)
diff --git a/arch/arm64/include/asm/stacktrace/common.h b/arch/arm64/include/asm/stacktrace/common.h
index 821a8fdd31af..26449cd402db 100644
--- a/arch/arm64/include/asm/stacktrace/common.h
+++ b/arch/arm64/include/asm/stacktrace/common.h
@@ -25,6 +25,8 @@ struct stack_info {
* @stack: The stack currently being unwound.
* @stacks: An array of stacks which can be unwound.
* @nr_stacks: The number of stacks in @stacks.
+ * @cfa: The sp value at the call site of the current function.
+ * @unreliable: Stacktrace is unreliable.
*/
struct unwind_state {
unsigned long fp;
@@ -33,6 +35,10 @@ struct unwind_state {
struct stack_info stack;
struct stack_info *stacks;
int nr_stacks;
+#ifdef CONFIG_SFRAME_UNWINDER
+ unsigned long cfa;
+ bool unreliable;
+#endif
};
static inline struct stack_info stackinfo_get_unknown(void)
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 77c7926a4df6..ac1da45da532 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -32,6 +32,7 @@
#include <linux/sched/task.h>
#include <linux/scs.h>
#include <linux/mm.h>
+#include <linux/sframe_lookup.h>
#include <asm/acpi.h>
#include <asm/fixmap.h>
@@ -375,6 +376,7 @@ void __init __no_sanitize_address setup_arch(char **cmdline_p)
"This indicates a broken bootloader or old kernel\n",
boot_args[1], boot_args[2], boot_args[3]);
}
+ init_sframe_table();
}
static inline bool cpu_can_disable(unsigned int cpu)
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index 3ebcf8c53fb0..72e78024d05e 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -14,6 +14,7 @@
#include <linux/sched/debug.h>
#include <linux/sched/task_stack.h>
#include <linux/stacktrace.h>
+#include <linux/sframe_lookup.h>
#include <asm/efi.h>
#include <asm/irq.h>
@@ -244,6 +245,53 @@ kunwind_next_frame_record(struct kunwind_state *state)
return 0;
}
+#ifdef CONFIG_SFRAME_UNWINDER
+/*
+ * Unwind to the next frame according to sframe.
+ */
+static __always_inline int
+unwind_next_frame_sframe(struct unwind_state *state)
+{
+ unsigned long fp = state->fp, ip = state->pc;
+ unsigned long base_reg, cfa;
+ unsigned long pc_addr, fp_addr;
+ struct sframe_ip_entry entry;
+ struct stack_info *info;
+ struct frame_record *record = (struct frame_record *)fp;
+
+ int err;
+
+ /* frame record alignment 8 bytes */
+ if (fp & 0x7)
+ return -EINVAL;
+
+ info = unwind_find_stack(state, fp, sizeof(*record));
+ if (!info)
+ return -EINVAL;
+
+ err = sframe_find_pc(ip, &entry);
+ if (err)
+ return -EINVAL;
+
+ unwind_consume_stack(state, info, fp, sizeof(*record));
+
+ base_reg = entry.use_fp ? fp : state->cfa;
+
+ /* Set up the initial CFA using fp based info if CFA is not set */
+ if (!state->cfa)
+ cfa = fp - entry.fp_offset;
+ else
+ cfa = base_reg + entry.cfa_offset;
+ fp_addr = cfa + entry.fp_offset;
+ pc_addr = cfa + entry.ra_offset;
+ state->cfa = cfa;
+ state->fp = READ_ONCE(*(unsigned long *)(fp_addr));
+ state->pc = READ_ONCE(*(unsigned long *)(pc_addr));
+
+ return 0;
+}
+#endif
+
/*
* Unwind from one frame record (A) to the next frame record (B).
*
@@ -263,7 +311,20 @@ kunwind_next(struct kunwind_state *state)
case KUNWIND_SOURCE_CALLER:
case KUNWIND_SOURCE_TASK:
case KUNWIND_SOURCE_REGS_PC:
+#ifdef CONFIG_SFRAME_UNWINDER
+ if (!state->common.unreliable)
+ err = unwind_next_frame_sframe(&state->common);
+
+ /* Fallback to FP based unwinder */
+ if (err || state->common.unreliable) {
err = kunwind_next_frame_record(state);
+ /* Mark its stacktrace result as unreliable if it is unwindable via FP */
+ if (!err)
+ state->common.unreliable = true;
+ }
+#else
+ err = kunwind_next_frame_record(state);
+#endif
break;
default:
err = -EINVAL;
@@ -350,6 +411,9 @@ kunwind_stack_walk(kunwind_consume_fn consume_state,
.common = {
.stacks = stacks,
.nr_stacks = ARRAY_SIZE(stacks),
+#ifdef CONFIG_SFRAME_UNWINDER
+ .cfa = 0,
+#endif
},
};
@@ -390,6 +454,43 @@ noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry,
kunwind_stack_walk(arch_kunwind_consume_entry, &data, task, regs);
}
+#ifdef CONFIG_SFRAME_UNWINDER
+struct kunwind_reliable_consume_entry_data {
+ stack_trace_consume_fn consume_entry;
+ void *cookie;
+ bool unreliable;
+};
+
+static __always_inline bool
+arch_kunwind_reliable_consume_entry(const struct kunwind_state *state, void *cookie)
+{
+ struct kunwind_reliable_consume_entry_data *data = cookie;
+
+ if (state->common.unreliable) {
+ data->unreliable = true;
+ return false;
+ }
+ return data->consume_entry(data->cookie, state->common.pc);
+}
+
+noinline notrace int arch_stack_walk_reliable(
+ stack_trace_consume_fn consume_entry,
+ void *cookie, struct task_struct *task)
+{
+ struct kunwind_reliable_consume_entry_data data = {
+ .consume_entry = consume_entry,
+ .cookie = cookie,
+ .unreliable = false,
+ };
+
+ kunwind_stack_walk(arch_kunwind_reliable_consume_entry, &data, task, NULL);
+
+ if (data.unreliable)
+ return -EINVAL;
+
+ return 0;
+}
+#else
static __always_inline bool
arch_reliable_kunwind_consume_entry(const struct kunwind_state *state, void *cookie)
{
@@ -419,6 +520,7 @@ noinline noinstr int arch_stack_walk_reliable(stack_trace_consume_fn consume_ent
return kunwind_stack_walk(arch_reliable_kunwind_consume_entry, &data,
task, NULL);
}
+#endif
struct bpf_unwind_consume_entry_data {
bool (*consume_entry)(void *cookie, u64 ip, u64 sp, u64 fp);
--
2.51.0.355.g5224444f11-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v2 6/6] unwind: arm64: Add reliable stacktrace with sframe unwinder.
2025-09-04 22:38 ` [PATCH v2 6/6] unwind: arm64: Add reliable stacktrace with sframe unwinder Dylan Hatch
@ 2025-09-17 23:41 ` Josh Poimboeuf
0 siblings, 0 replies; 10+ messages in thread
From: Josh Poimboeuf @ 2025-09-17 23:41 UTC (permalink / raw)
To: Dylan Hatch
Cc: Steven Rostedt, Indu Bhagat, Peter Zijlstra, Will Deacon,
Catalin Marinas, Jiri Kosina, Roman Gushchin, Weinan Liu,
Mark Rutland, Ian Rogers, linux-toolchains, linux-kernel,
live-patching, joe.lawrence, Puranjay Mohan, Song Liu,
Prasanna Kumar T S M
On Thu, Sep 04, 2025 at 10:38:50PM +0000, Dylan Hatch wrote:
> +noinline notrace int arch_stack_walk_reliable(
> + stack_trace_consume_fn consume_entry,
> + void *cookie, struct task_struct *task)
> +{
> + struct kunwind_reliable_consume_entry_data data = {
> + .consume_entry = consume_entry,
> + .cookie = cookie,
> + .unreliable = false,
> + };
> +
> + kunwind_stack_walk(arch_kunwind_reliable_consume_entry, &data, task, NULL);
> +
> + if (data.unreliable)
> + return -EINVAL;
As far I can tell, the *only* error condition being checked is if it
(successfully) fell back to frame pointers.
What if there was some bad or missing sframe data? Or some unexpected
condition on the stack?
Also, does the exception handling code have correct cfi/sframe metadata?
In order for it to be "reliable", we need to know the unwind reached the
end of the stack (e.g., the task pt_regs frame, from entry-from-user).
--
Josh
^ permalink raw reply [flat|nested] 10+ messages in thread