[PATCH v2 0/6] unwind, arm64: add sframe unwinder for kernel

[PATCH v2 0/6] unwind, arm64: add sframe unwinder for kernel

[Dylan Hatch]

This patchset implements a generic kernel sframe-based [1] unwinder.
The main goal is to support reliable stacktraces on arm64.

On x86 orc unwinder provides reliable stacktraces. But arm64 misses the
required support from objtool: it cannot generate orc unwind tables for
arm64.

Currently, there's already a sframe unwinder proposed for userspace: [2].
Since the sframe unwind table algorithm is similar, these two proposals
could integrate common functionality in the future.

Currently, only GCC supports sframe.

These patches are based on v6.17-rc4 and are available on github [3].

Ref:
[1]: https://sourceware.org/binutils/docs/sframe-spec.html
[2]: https://lore.kernel.org/lkml/cover.1730150953.git.jpoimboe@kernel.org/
[3]: https://github.com/dylanbhatch/linux/tree/sframe-v2

Changes since v1:
https://lore.kernel.org/live-patching/20250127213310.2496133-1-wnliu@google.com/

 - Fixed detection of sframe support in compiler (Josh, Jens)
 - Adapt latest sframe v2 header definition from userspace patch series
   (Josh)
 - Folded together unwinder/stacktrace patches (Prasanna)
 - Fix "orphan section" warnings for .init.sframe sections (Puranjay,
   Indu, Josh)
 - Build VDSO without sframe (Dylan)
 - Added support for modules (Weinan)

Dylan Hatch (2):
  unwind: build kernel with sframe info
  unwind: add sframe v2 header

Weinan Liu (4):
  arm64: entry: add unwind info for various kernel entries
  unwind: Implement generic sframe unwinder library
  arm64/module, unwind: Add sframe support for modules.
  unwind: arm64: Add reliable stacktrace with sframe unwinder.

 Makefile                                   |   8 +
 arch/Kconfig                               |   6 +
 arch/arm64/Kconfig.debug                   |  10 +
 arch/arm64/include/asm/module.h            |   6 +
 arch/arm64/include/asm/stacktrace/common.h |   6 +
 arch/arm64/kernel/entry.S                  |  10 +
 arch/arm64/kernel/module.c                 |   5 +
 arch/arm64/kernel/setup.c                  |   2 +
 arch/arm64/kernel/stacktrace.c             | 102 +++++++++
 arch/arm64/kernel/vdso/Makefile            |   2 +-
 include/asm-generic/vmlinux.lds.h          |  15 ++
 include/linux/sframe_lookup.h              |  45 ++++
 kernel/Makefile                            |   1 +
 kernel/sframe.h                            |  75 +++++++
 kernel/sframe_lookup.c                     | 232 +++++++++++++++++++++
 15 files changed, 524 insertions(+), 1 deletion(-)
 create mode 100644 include/linux/sframe_lookup.h
 create mode 100644 kernel/sframe.h
 create mode 100644 kernel/sframe_lookup.c

-- 
2.51.0.355.g5224444f11-goog

[PATCH v2 1/6] unwind: build kernel with sframe info

[Dylan Hatch]

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

[PATCH v2 2/6] arm64: entry: add unwind info for various kernel entries

[Dylan Hatch]

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

[PATCH v2 3/6] unwind: add sframe v2 header

[Dylan Hatch]

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

[PATCH v2 4/6] unwind: Implement generic sframe unwinder library

[Dylan Hatch]

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

[PATCH v2 5/6] arm64/module, unwind: Add sframe support for modules.

[Dylan Hatch]

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

[PATCH v2 6/6] unwind: arm64: Add reliable stacktrace with sframe unwinder.

[Dylan Hatch]

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

Re: [PATCH v2 4/6] unwind: Implement generic sframe unwinder library

[Puranjay Mohan]

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

Re: [PATCH v2 4/6] unwind: Implement generic sframe unwinder library

[Indu Bhagat]

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
> 

Re: [PATCH v2 6/6] unwind: arm64: Add reliable stacktrace with sframe unwinder.

[Josh Poimboeuf]

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

Re: [PATCH v2 0/6] unwind, arm64: add sframe unwinder for kernel

[Song Liu]

On Thu, Sep 4, 2025 at 3:39 PM Dylan Hatch <dylanbhatch@google.com> wrote:
>
> This patchset implements a generic kernel sframe-based [1] unwinder.
> The main goal is to support reliable stacktraces on arm64.
>
> On x86 orc unwinder provides reliable stacktraces. But arm64 misses the
> required support from objtool: it cannot generate orc unwind tables for
> arm64.
>
> Currently, there's already a sframe unwinder proposed for userspace: [2].
> Since the sframe unwind table algorithm is similar, these two proposals
> could integrate common functionality in the future.
>
> Currently, only GCC supports sframe.
>
> These patches are based on v6.17-rc4 and are available on github [3].
>
> Ref:
> [1]: https://sourceware.org/binutils/docs/sframe-spec.html
> [2]: https://lore.kernel.org/lkml/cover.1730150953.git.jpoimboe@kernel.org/
> [3]: https://github.com/dylanbhatch/linux/tree/sframe-v2

I run the following test on this sframe-v2 branch:

bpftrace -e 'kprobe:security_file_open {printf("%s",
kstack);@count+=1; if (@count > 1) {exit();}}'

        security_file_open+0
        bpf_prog_eaca355a0dcdca7f_kprobe_security_file_open_1+16641632@./bpftrace.bpf.o:0
        path_openat+1892
        do_filp_open+132
        do_open_execat+84
        alloc_bprm+44
        do_execveat_common.isra.0+116
        __arm64_sys_execve+72
        invoke_syscall+76
        el0_svc_common.constprop.0+68
        do_el0_svc+32
        el0_svc+56
        el0t_64_sync_handler+152
        el0t_64_sync+388

This looks wrong. The right call trace should be:

  do_filp_open
    => path_openat
      => vfs_open
        => do_dentry_open
          => security_file_open
            => bpf_prog_eaca355a0dcdca7f_...

I am not sure whether this is just a problem with the bpf program,
or also with something else.

Thanks,
Song

Re: [PATCH v2 0/6] unwind, arm64: add sframe unwinder for kernel

[Puranjay Mohan]

On Mon, Sep 29, 2025 at 9:46 PM Song Liu <song@kernel.org> wrote:
>
> On Thu, Sep 4, 2025 at 3:39 PM Dylan Hatch <dylanbhatch@google.com> wrote:
> >
> > This patchset implements a generic kernel sframe-based [1] unwinder.
> > The main goal is to support reliable stacktraces on arm64.
> >
> > On x86 orc unwinder provides reliable stacktraces. But arm64 misses the
> > required support from objtool: it cannot generate orc unwind tables for
> > arm64.
> >
> > Currently, there's already a sframe unwinder proposed for userspace: [2].
> > Since the sframe unwind table algorithm is similar, these two proposals
> > could integrate common functionality in the future.
> >
> > Currently, only GCC supports sframe.
> >
> > These patches are based on v6.17-rc4 and are available on github [3].
> >
> > Ref:
> > [1]: https://sourceware.org/binutils/docs/sframe-spec.html
> > [2]: https://lore.kernel.org/lkml/cover.1730150953.git.jpoimboe@kernel.org/
> > [3]: https://github.com/dylanbhatch/linux/tree/sframe-v2
>
> I run the following test on this sframe-v2 branch:
>
> bpftrace -e 'kprobe:security_file_open {printf("%s",
> kstack);@count+=1; if (@count > 1) {exit();}}'
>
>         security_file_open+0
>         bpf_prog_eaca355a0dcdca7f_kprobe_security_file_open_1+16641632@./bpftrace.bpf.o:0
>         path_openat+1892
>         do_filp_open+132
>         do_open_execat+84
>         alloc_bprm+44
>         do_execveat_common.isra.0+116
>         __arm64_sys_execve+72
>         invoke_syscall+76
>         el0_svc_common.constprop.0+68
>         do_el0_svc+32
>         el0_svc+56
>         el0t_64_sync_handler+152
>         el0t_64_sync+388
>
> This looks wrong. The right call trace should be:
>
>   do_filp_open
>     => path_openat
>       => vfs_open
>         => do_dentry_open
>           => security_file_open
>             => bpf_prog_eaca355a0dcdca7f_...
>
> I am not sure whether this is just a problem with the bpf program,
> or also with something else.

I will try to debug this more but am just curious about BPF's
interactions with sframe.
The sframe data for bpf programs doesn't exist, so we would need to
add that support
and that wouldn't be trivial, given the BPF programs are JITed.

Thanks,
Puranjay

Re: [PATCH v2 1/6] unwind: build kernel with sframe info

[Will Deacon]

On Thu, Sep 04, 2025 at 10:38:45PM +0000, Dylan Hatch wrote:
> 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)

Is it possible to extend this check so that we reject assemblers that
emit the unsupported "sframe version one" format?

> +config SFRAME_UNWIND_TABLE
> +	bool

Is this extra option actually needed for anything?

>  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

Shouldn't there be an arch dependency here as well? Since architectures
need to make use of sframe in their unwinders, I was expecting something
like 'depends on ARCH_SUPPORTS_SFRAME_UNWINDER' here.

Will

Re: [PATCH v2 6/6] unwind: arm64: Add reliable stacktrace with sframe unwinder.

[Dylan Hatch]

Sorry for the slow reply on this, I'm going to try and get a v3 out
sometime after next week.

On Wed, Sep 17, 2025 at 4:41 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> As far I can tell, the *only* error condition being checked is if it
> (successfully) fell back to frame pointers.

By checking/handling error conditions, do you mean just marking the
state as unreliable in any case where the unwind isn't successful with
SFrame? I'm thinking if I can make the unwind_next_frame_sframe() code
path handle the end of the stack correctly on its own, I can more
strictly mark the trace as unreliable if it encounters any error.

>
> 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).

It looks like the frame-pointer based method of handling the end of
the stack involves calling kunwind_next_frame_record_meta() to extract
and check frame_record_meta::type for FRAME_META_TYPE_FINAL. I think
this currently assumes (based on the definition of 'struct
frame_record') that the next FP and PC are right next to each other,
alongside the meta type. But the sframe format stores separate entries
for the FP and RA offsets, which makes extracting the meta type from
this information a little bit murky to me.

Would it make sense to fall back to the frame pointer method for the
final stack frame? Or I guess I could define a new sframe-friendly
meta frame record format?

Thanks,
Dylan

Re: [PATCH v2 0/6] unwind, arm64: add sframe unwinder for kernel

[Dylan Hatch]

On Mon, Sep 29, 2025 at 12:55 PM Puranjay Mohan <puranjay12@gmail.com> wrote:
>
> I will try to debug this more but am just curious about BPF's
> interactions with sframe.
> The sframe data for bpf programs doesn't exist, so we would need to
> add that support
> and that wouldn't be trivial, given the BPF programs are JITed.
>
> Thanks,
> Puranjay

From what I can tell, the ORC unwinder in x86 falls back to using
frame pointers in cases of generated code, like BPF. Would matching
this behavior in the sframe unwinder be a reasonable approach, at
least for the purposes of enabling reliable unwind for livepatch?

Thanks,
Dylan

Re: [PATCH v2 6/6] unwind: arm64: Add reliable stacktrace with sframe unwinder.

[Josh Poimboeuf]

On Fri, Nov 14, 2025 at 10:44:20PM -0800, Dylan Hatch wrote:
> Sorry for the slow reply on this, I'm going to try and get a v3 out
> sometime after next week.
> 
> On Wed, Sep 17, 2025 at 4:41 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> > As far I can tell, the *only* error condition being checked is if it
> > (successfully) fell back to frame pointers.
> 
> By checking/handling error conditions, do you mean just marking the
> state as unreliable in any case where the unwind isn't successful with
> SFrame?

Right, any sframe error it encounters along the way (including missing
sframe) would be a reason to mark it as unreliable.

> I'm thinking if I can make the unwind_next_frame_sframe() code
> path handle the end of the stack correctly on its own, I can more
> strictly mark the trace as unreliable if it encounters any error.
> 
> >
> > 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).
> 
> It looks like the frame-pointer based method of handling the end of
> the stack involves calling kunwind_next_frame_record_meta() to extract
> and check frame_record_meta::type for FRAME_META_TYPE_FINAL. I think
> this currently assumes (based on the definition of 'struct
> frame_record') that the next FP and PC are right next to each other,
> alongside the meta type. But the sframe format stores separate entries
> for the FP and RA offsets, which makes extracting the meta type from
> this information a little bit murky to me.
> 
> Would it make sense to fall back to the frame pointer method for the
> final stack frame? Or I guess I could define a new sframe-friendly
> meta frame record format?

For sframe v3, I believe Indu is planning to add support for marking the
outermost frame.  That would be one definitive way to know that the
stack trace made it to the end.

Or, if the entry-from-user pt_regs frame is always stored at a certain
offset compared to the end of the task stack page, that might be another
way.

-- 
Josh

Re: [PATCH v2 0/6] unwind, arm64: add sframe unwinder for kernel

[Josh Poimboeuf]

On Fri, Nov 14, 2025 at 10:50:16PM -0800, Dylan Hatch wrote:
> On Mon, Sep 29, 2025 at 12:55 PM Puranjay Mohan <puranjay12@gmail.com> wrote:
> >
> > I will try to debug this more but am just curious about BPF's
> > interactions with sframe.
> > The sframe data for bpf programs doesn't exist, so we would need to
> > add that support
> > and that wouldn't be trivial, given the BPF programs are JITed.
> >
> > Thanks,
> > Puranjay
> 
> From what I can tell, the ORC unwinder in x86 falls back to using
> frame pointers in cases of generated code, like BPF. Would matching
> this behavior in the sframe unwinder be a reasonable approach, at
> least for the purposes of enabling reliable unwind for livepatch?

The ORC unwinder marks the unwind "unreliable" if it has to fall back to
frame pointers.

But that's not a problem for livepatch because it only[*] unwinds
blocked/sleeping tasks, which shouldn't have BPF on their stack anyway.

[*] with one exception: the task calling into livepatch

-- 
Josh

Re: [PATCH v2 0/6] unwind, arm64: add sframe unwinder for kernel

[Steven Rostedt]

On Mon, 17 Nov 2025 15:06:32 -0800
Josh Poimboeuf <jpoimboe@kernel.org> wrote:

> The ORC unwinder marks the unwind "unreliable" if it has to fall back to
> frame pointers.
> 
> But that's not a problem for livepatch because it only[*] unwinds
> blocked/sleeping tasks, which shouldn't have BPF on their stack anyway.
> 
> [*] with one exception: the task calling into livepatch

It may be a problem with preempted tasks right? I believe with PREEMPT_LAZY
(and definitely with PREEMPT_RT) BPF programs can be preempted.

-- Steve

Re: [PATCH v2 0/6] unwind, arm64: add sframe unwinder for kernel

[Puranjay Mohan]

On Tue, Nov 18, 2025 at 12:06 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Fri, Nov 14, 2025 at 10:50:16PM -0800, Dylan Hatch wrote:
> > On Mon, Sep 29, 2025 at 12:55 PM Puranjay Mohan <puranjay12@gmail.com> wrote:
> > >
> > > I will try to debug this more but am just curious about BPF's
> > > interactions with sframe.
> > > The sframe data for bpf programs doesn't exist, so we would need to
> > > add that support
> > > and that wouldn't be trivial, given the BPF programs are JITed.
> > >
> > > Thanks,
> > > Puranjay
> >
> > From what I can tell, the ORC unwinder in x86 falls back to using
> > frame pointers in cases of generated code, like BPF. Would matching
> > this behavior in the sframe unwinder be a reasonable approach, at
> > least for the purposes of enabling reliable unwind for livepatch?
>
> The ORC unwinder marks the unwind "unreliable" if it has to fall back to
> frame pointers.
>
> But that's not a problem for livepatch because it only[*] unwinds
> blocked/sleeping tasks, which shouldn't have BPF on their stack anyway.
>

BPF programs can sleep, so wouldn't they show up in the stack?
Like if I am tracing a syscall with a bpf program attached using
fentry and the BPF program calls a bpf_arena_alloc_pages(), which can
sleep.

Re: [PATCH v2 0/6] unwind, arm64: add sframe unwinder for kernel

[Josh Poimboeuf]

On Mon, Nov 17, 2025 at 06:42:23PM -0500, Steven Rostedt wrote:
> On Mon, 17 Nov 2025 15:06:32 -0800
> Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> 
> > The ORC unwinder marks the unwind "unreliable" if it has to fall back to
> > frame pointers.
> > 
> > But that's not a problem for livepatch because it only[*] unwinds
> > blocked/sleeping tasks, which shouldn't have BPF on their stack anyway.
> > 
> > [*] with one exception: the task calling into livepatch
> 
> It may be a problem with preempted tasks right? I believe with PREEMPT_LAZY
> (and definitely with PREEMPT_RT) BPF programs can be preempted.

In that case, then yes, that stack would be marked unreliable and
livepatch would have to go try and patch the task later.

If it were an isolated case, that would be fine, but if BPF were
consistently on the same task's stack, it could stall the completion of
the livepatch indefinitely.

I haven't (yet?) heard of BPF-induced livepatch stalls happening in
reality, but maybe it's only a matter of time :-/

To fix that, I suppose we would need some kind of dynamic ORC
registration interface.  Similar to what has been discussed with
sframe+JIT.

If BPF were to always use frame pointers then there would be only a very
limited set of ORC entries (either "frame pointer" or "undefined") for a
given BPF function and it shouldn't be too complicated.

-- 
Josh

Re: [PATCH v2 0/6] unwind, arm64: add sframe unwinder for kernel

[Puranjay Mohan]

On Tue, Nov 18, 2025 at 1:10 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Mon, Nov 17, 2025 at 06:42:23PM -0500, Steven Rostedt wrote:
> > On Mon, 17 Nov 2025 15:06:32 -0800
> > Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> > > The ORC unwinder marks the unwind "unreliable" if it has to fall back to
> > > frame pointers.
> > >
> > > But that's not a problem for livepatch because it only[*] unwinds
> > > blocked/sleeping tasks, which shouldn't have BPF on their stack anyway.
> > >
> > > [*] with one exception: the task calling into livepatch
> >
> > It may be a problem with preempted tasks right? I believe with PREEMPT_LAZY
> > (and definitely with PREEMPT_RT) BPF programs can be preempted.
>
> In that case, then yes, that stack would be marked unreliable and
> livepatch would have to go try and patch the task later.
>
> If it were an isolated case, that would be fine, but if BPF were
> consistently on the same task's stack, it could stall the completion of
> the livepatch indefinitely.
>
> I haven't (yet?) heard of BPF-induced livepatch stalls happening in
> reality, but maybe it's only a matter of time :-/
>
> To fix that, I suppose we would need some kind of dynamic ORC
> registration interface.  Similar to what has been discussed with
> sframe+JIT.

I work with the BPF JITs and would be interested in exploring this further,
can you point me to this discussion if it happened on the list.

>
> If BPF were to always use frame pointers then there would be only a very
> limited set of ORC entries (either "frame pointer" or "undefined") for a
> given BPF function and it shouldn't be too complicated.
>
> --
> Josh

Re: [PATCH v2 0/6] unwind, arm64: add sframe unwinder for kernel

[Josh Poimboeuf]

On Tue, Nov 18, 2025 at 01:49:06AM +0100, Puranjay Mohan wrote:
> On Tue, Nov 18, 2025 at 1:10 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> > On Mon, Nov 17, 2025 at 06:42:23PM -0500, Steven Rostedt wrote:
> > > On Mon, 17 Nov 2025 15:06:32 -0800
> > > Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > >
> > > > The ORC unwinder marks the unwind "unreliable" if it has to fall back to
> > > > frame pointers.
> > > >
> > > > But that's not a problem for livepatch because it only[*] unwinds
> > > > blocked/sleeping tasks, which shouldn't have BPF on their stack anyway.
> > > >
> > > > [*] with one exception: the task calling into livepatch
> > >
> > > It may be a problem with preempted tasks right? I believe with PREEMPT_LAZY
> > > (and definitely with PREEMPT_RT) BPF programs can be preempted.
> >
> > In that case, then yes, that stack would be marked unreliable and
> > livepatch would have to go try and patch the task later.
> >
> > If it were an isolated case, that would be fine, but if BPF were
> > consistently on the same task's stack, it could stall the completion of
> > the livepatch indefinitely.
> >
> > I haven't (yet?) heard of BPF-induced livepatch stalls happening in
> > reality, but maybe it's only a matter of time :-/
> >
> > To fix that, I suppose we would need some kind of dynamic ORC
> > registration interface.  Similar to what has been discussed with
> > sframe+JIT.
> 
> I work with the BPF JITs and would be interested in exploring this further,
> can you point me to this discussion if it happened on the list.

Sorry, nothing specific has been discussed that I'm aware of :-)

-- 
Josh

Re: [PATCH v2 0/6] unwind, arm64: add sframe unwinder for kernel

[Steven Rostedt]

On Mon, 17 Nov 2025 21:18:41 -0800
Josh Poimboeuf <jpoimboe@kernel.org> wrote:

> > > To fix that, I suppose we would need some kind of dynamic ORC
> > > registration interface.  Similar to what has been discussed with
> > > sframe+JIT.  
> > 
> > I work with the BPF JITs and would be interested in exploring this further,
> > can you point me to this discussion if it happened on the list.  
> 
> Sorry, nothing specific has been discussed that I'm aware of :-)

Right, the only discussions have been at the monthly Sframe meetings about
needing to be able to handle this. But the actual implementation details
have not been figured out yet.

-- Steve

Re: [PATCH v2 0/6] unwind, arm64: add sframe unwinder for kernel

[Indu Bhagat]

On 11/17/25 4:49 PM, Puranjay Mohan wrote:
> On Tue, Nov 18, 2025 at 1:10 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>>
>> On Mon, Nov 17, 2025 at 06:42:23PM -0500, Steven Rostedt wrote:
>>> On Mon, 17 Nov 2025 15:06:32 -0800
>>> Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>>>
>>>> The ORC unwinder marks the unwind "unreliable" if it has to fall back to
>>>> frame pointers.
>>>>
>>>> But that's not a problem for livepatch because it only[*] unwinds
>>>> blocked/sleeping tasks, which shouldn't have BPF on their stack anyway.
>>>>
>>>> [*] with one exception: the task calling into livepatch
>>>
>>> It may be a problem with preempted tasks right? I believe with PREEMPT_LAZY
>>> (and definitely with PREEMPT_RT) BPF programs can be preempted.
>>
>> In that case, then yes, that stack would be marked unreliable and
>> livepatch would have to go try and patch the task later.
>>
>> If it were an isolated case, that would be fine, but if BPF were
>> consistently on the same task's stack, it could stall the completion of
>> the livepatch indefinitely.
>>
>> I haven't (yet?) heard of BPF-induced livepatch stalls happening in
>> reality, but maybe it's only a matter of time :-/
>>
>> To fix that, I suppose we would need some kind of dynamic ORC
>> registration interface.  Similar to what has been discussed with
>> sframe+JIT.
> 
> I work with the BPF JITs and would be interested in exploring this further,
> can you point me to this discussion if it happened on the list.
> 

We discussed SFrame/JIT topic earlier this year in our monthly SFrame 
meetings.  I can point you to the meeting notes in a separate email.  We 
had some discussion around:

   - SFrame specification: Allow efficient addition, removal and update 
of data in SFrame sections.  A part of the challenge is in representing 
the variety of frames a JIT may use.
   - SFrame APIs with JIT: Efficient SFrame stack trace data 
manipulation by JIT.
   - Interface with Linux kernel: Efficient SFrame stack trace data 
registration and update stack trace data.

It will be great to have more collaboration and brainstorming, and to 
include BPF/JIT in the discussions.

>>
>> If BPF were to always use frame pointers then there would be only a very
>> limited set of ORC entries (either "frame pointer" or "undefined") for a
>> given BPF function and it shouldn't be too complicated.
>>
>> --
>> Josh

Re: [PATCH v2 6/6] unwind: arm64: Add reliable stacktrace with sframe unwinder.

[Dylan Hatch]

On Mon, Nov 17, 2025 at 3:01 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Fri, Nov 14, 2025 at 10:44:20PM -0800, Dylan Hatch wrote:
> > Sorry for the slow reply on this, I'm going to try and get a v3 out
> > sometime after next week.
> >
> > On Wed, Sep 17, 2025 at 4:41 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > >
> > > As far I can tell, the *only* error condition being checked is if it
> > > (successfully) fell back to frame pointers.
> >
> > By checking/handling error conditions, do you mean just marking the
> > state as unreliable in any case where the unwind isn't successful with
> > SFrame?
>
> Right, any sframe error it encounters along the way (including missing
> sframe) would be a reason to mark it as unreliable.
>
> > I'm thinking if I can make the unwind_next_frame_sframe() code
> > path handle the end of the stack correctly on its own, I can more
> > strictly mark the trace as unreliable if it encounters any error.
> >
> > >
> > > 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).
> >
> > It looks like the frame-pointer based method of handling the end of
> > the stack involves calling kunwind_next_frame_record_meta() to extract
> > and check frame_record_meta::type for FRAME_META_TYPE_FINAL. I think
> > this currently assumes (based on the definition of 'struct
> > frame_record') that the next FP and PC are right next to each other,
> > alongside the meta type. But the sframe format stores separate entries
> > for the FP and RA offsets, which makes extracting the meta type from
> > this information a little bit murky to me.
> >
> > Would it make sense to fall back to the frame pointer method for the
> > final stack frame? Or I guess I could define a new sframe-friendly
> > meta frame record format?
>
> For sframe v3, I believe Indu is planning to add support for marking the
> outermost frame.  That would be one definitive way to know that the
> stack trace made it to the end.

How would this work? Is there a way of determining at compile time
which functions would end up being the outermost frame?

>
> Or, if the entry-from-user pt_regs frame is always stored at a certain
> offset compared to the end of the task stack page, that might be another
> way.

It looks like kunwind_next_frame_record_meta() uses this strategy
already. It checks that 'fp == &task_pt_regs(tsk)->stackframe' to
validate that it has in fact reached the end of the stack. It seems
like we need alternate versions of kunwind_next_frame_record_meta()
and kunwind_next_regs_pc() that use the CFA calculated from the sframe
data (instead of the frame pointer). Does that sound right?

Thanks,
Dylan

On Mon, Nov 17, 2025 at 3:01 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Fri, Nov 14, 2025 at 10:44:20PM -0800, Dylan Hatch wrote:
> > Sorry for the slow reply on this, I'm going to try and get a v3 out
> > sometime after next week.
> >
> > On Wed, Sep 17, 2025 at 4:41 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > >
> > > As far I can tell, the *only* error condition being checked is if it
> > > (successfully) fell back to frame pointers.
> >
> > By checking/handling error conditions, do you mean just marking the
> > state as unreliable in any case where the unwind isn't successful with
> > SFrame?
>
> Right, any sframe error it encounters along the way (including missing
> sframe) would be a reason to mark it as unreliable.
>
> > I'm thinking if I can make the unwind_next_frame_sframe() code
> > path handle the end of the stack correctly on its own, I can more
> > strictly mark the trace as unreliable if it encounters any error.
> >
> > >
> > > 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).
> >
> > It looks like the frame-pointer based method of handling the end of
> > the stack involves calling kunwind_next_frame_record_meta() to extract
> > and check frame_record_meta::type for FRAME_META_TYPE_FINAL. I think
> > this currently assumes (based on the definition of 'struct
> > frame_record') that the next FP and PC are right next to each other,
> > alongside the meta type. But the sframe format stores separate entries
> > for the FP and RA offsets, which makes extracting the meta type from
> > this information a little bit murky to me.
> >
> > Would it make sense to fall back to the frame pointer method for the
> > final stack frame? Or I guess I could define a new sframe-friendly
> > meta frame record format?
>
> For sframe v3, I believe Indu is planning to add support for marking the
> outermost frame.  That would be one definitive way to know that the
> stack trace made it to the end.
>
> Or, if the entry-from-user pt_regs frame is always stored at a certain
> offset compared to the end of the task stack page, that might be another
> way.
>
> --
> Josh

Re: [PATCH v2 6/6] unwind: arm64: Add reliable stacktrace with sframe unwinder.

[Indu Bhagat]

On 11/18/25 7:17 PM, Dylan Hatch wrote:
>> For sframe v3, I believe Indu is planning to add support for marking the
>> outermost frame.  That would be one definitive way to know that the
>> stack trace made it to the end.
> How would this work? Is there a way of determining at compile time
> which functions would end up being the outermost frame?

No, the compiler does not emit such a marker.

SFrame information is generated by assembler using the .cfi_* 
directives.  For the outermost functions, they need to be marked with a:
    .cfi_undefined RA
where RA is the default return address register for the ABI.

This mechanism is formalised in the DWARF standard:
"If a Return Address register is defined in the virtual unwind table, 
and its rule is undefined (for example, by DW_CFA_undefined), then there 
is no return address and no call address, and the virtual unwind of 
stack activations is complete."

SFrame relies on this to emit a marker for identifying outermost frame.

Re: [PATCH v2 1/6] unwind: build kernel with sframe info

[Jens Remus]

Hello Dylan!

On 9/5/2025 12:38 AM, Dylan Hatch wrote:
> 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>

> diff --git 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)

As you will soon be requiring SFrame V2 with the new PC-relative FDE
function start address encoding you may want to extend this check as
follows:

config AS_SFRAME
	def_bool y
	depends on $(as-instr,.cfi_sections .sframe\n.cfi_startproc\n.cfi_endproc)
	depends on $(success,printf "%b\n" ".cfi_sections .sframe\n.cfi_startproc\n.cfi_endproc" | $(CC) $(CLANG_FLAGS) -Wa$(comma)--fatal-warnings -c -x assembler-with-cpp -o "$$TMP" - && $(OBJDUMP) --sframe "$$TMP" | grep -q "SFRAME_VERSION_2")
	depends on $(success,printf "%b\n" ".cfi_sections .sframe\n.cfi_startproc\n.cfi_endproc" | $(CC) $(CLANG_FLAGS) -Wa$(comma)--fatal-warnings -c -x assembler-with-cpp -o "$$TMP" - && $(OBJDUMP) --sframe "$$TMP" | grep -q "SFRAME_F_FDE_FUNC_START_PCREL")


Or you could change it into multiple config options, which might be
overkill:

config AS_SFRAME
	def_bool $(as-instr,.cfi_sections .sframe\n.cfi_startproc\n.cfi_endproc)

config AS_SFRAME_V2
	def_bool y
	depends on AS_SFRAME
	depends on $(success,printf "%b\n" ".cfi_sections .sframe\n.cfi_startproc\n.cfi_endproc" | $(CC) $(CLANG_FLAGS) -Wa$(comma)--fatal-warnings -c -x assembler-with-cpp -o "$$TMP" - && $(OBJDUMP) --sframe "$$TMP" | grep -q "SFRAME_VERSION_2")

config AS_SFRAME_V2_PCREL_FDE
	def_bool y
	depends on AS_SFRAME_V2
	depends on $(success,printf "%b\n" ".cfi_sections .sframe\n.cfi_startproc\n.cfi_endproc" | $(CC) $(CLANG_FLAGS) -Wa$(comma)--fatal-warnings -c -x assembler-with-cpp -o "$$TMP" - && $(OBJDUMP) --sframe "$$TMP" | grep -q "SFRAME_F_FDE_FUNC_START_PCREL")

> +
> +config SFRAME_UNWIND_TABLE
> +	bool
> +
>  endmenu
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/