[PATCH v15 00/20] unwind_deferred: Implement sframe handling

[PATCH v15 00/20] unwind_deferred: Implement sframe handling

[Jens Remus]

This is the implementation of parsing the SFrame V3 stack trace information
from an .sframe section in an ELF file.  It's a continuation of Josh's and
Steve's work that can be found here:

   https://lore.kernel.org/all/cover.1737511963.git.jpoimboe@kernel.org/
   https://lore.kernel.org/all/20250827201548.448472904@kernel.org/

Currently the only way to get a user space stack trace from a stack
walk (and not just copying large amount of user stack into the kernel
ring buffer) is to use frame pointers. This has a few issues. The biggest
one is that compiling frame pointers into every application and library
has been shown to cause performance overhead.

Another issue is that the format of the frames may not always be consistent
between different compilers and some architectures (s390) has no defined
format to do a reliable stack walk. The only way to perform user space
profiling on these architectures is to copy the user stack into the kernel
buffer.

SFrame [1] is now supported in binutils (x86-64, ARM64, and s390). There is
discussions going on about supporting SFrame in LLVM. SFrame acts more like
ORC, and lives in the ELF executable file as its own section. Like ORC it
has two tables where the first table is sorted by instruction pointers (IP)
and using the current IP and finding it's entry in the first table, it will
take you to the second table which will tell you where the return address
of the current function is located and then you can use that address to
look it up in the first table to find the return address of that function,
and so on. This performs a user space stack walk.

Now because the .sframe section lives in the ELF file it needs to be faulted
into memory when it is used. This means that walking the user space stack
requires being in a faultable context. As profilers like perf request a stack
trace in interrupt or NMI context, it cannot do the walking when it is
requested. Instead it must be deferred until it is safe to fault in user
space. One place this is known to be safe is when the task is about to return
back to user space.

This series makes the deferred unwind user code implement SFrame format V3
and enables it on x86-64.

[1]: https://sourceware.org/binutils/wiki/sframe


This series applies on top of v7.1-rc4 tag:

  git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git  v7.1-rc4

The to be stack-traced user space programs (and libraries) need to be
built with the recent SFrame stack trace information format V3, as
generated by binutils 2.46+ with assembler option --gsframe-3.

Namhyung Kim's related perf tools deferred callchain support can be used
for testing ("perf record --call-graph fp,defer" and "perf report/script").


Changes in v15 (see patch notes for details):
- Rebase on v7.1-rc4.
- New patch to duplicate registered .sframe section data on clone/fork.
- Address Sashiko AI review feedback:
  - Fix sframe end passed to mtree_insert_range().
  - Fix outermost frame (FRE without datawords) handling.
  - Use GFP_KERNEL_ACCOUNT instead of GFP_KERNEL.
  - Improve text/sframe section start/end validation.
  - Always use guard(srcu) when accessing struct sframe_section fields.
  - Validate FDE repetition size for PCTYPE_MASK FDEs to be non-zero to
    prevent division by zero.
  - Only add sframe for text that is PT_LOAD in addition to PF_X.
  - Use pr_debug_once() instead of WARN_ON_ONCE() to prevent user-
    triggered warning/panic.
  - Add support for SP/FP-based CFA recovery rules with
    dereferencing.
  - Reject FRE control word with reserved_p=1.
  - x86-64: Fail unwind_user_get_reg() if !user_64bit_mode().
- Validate FDE PC type for supported values (i.e. PCTYPE_INC or
  PCTYPE_MASK).
- Validate FDE function end against text end.
- Validate FDE's number of FREs to be less or equal to FDE's function
  size, as each FRE must cover at least one byte. (Indu)
- Validate FRE function offset against FDE repetition size
  for PCTYPE_MASK.
- Change type of struct sframe_fde_internal field fres_num to the one of
  struct sframe_fda_v3 field fres_num.
- Return RC of sframe_init_[cfa_]rule_data() if bad RC.
- Normalize error code usage (.sframe is removed for all but ENOENT):
  ENOENT: No sframe or no FDE for IP found
          (FDE found but no FRE found is EINVAL)
  EFAULT: Bad address
  EINVAL: Invalid input or sframe
- Build-time checks for config options:
  - 64BIT: SFrame V3 only supports 64-bit architectures.
  - HAVE_EFFICIENT_UNALIGNED_ACCESS: Unaligned access to 16/32-bit
    SFrame FRE fields and datawords using unsafe_get_user(). (Steven)
- Add pr_debug_once() when restoring CFA/FP/RA from an unsupported
  register number.

Changes in v14 (see patch notes for details):
- Rebase on v7.1-rc2.
- Correct SFRAME_V3_FDE_TYPE_MASK value.
- Fix FDE function start address check in __read_fde().
- Rename SFrame V3 definitions accoring to final specification. (Indu)
- Improve comments on why UNWIND_USER_RULE_CFA_OFFSET is not
  implemented. (Mark Rutland)
- Add/update/improve sframe debug messages.
- Add generic and arch-specific unwind_user.h to MAINTAINERS.
- Add arch-specific unwind_user_sframe.h to MAINTAINERS.

Changes in v13 (see patch notes for details):
- Add support for SFrame V3, including its new flexible FDEs.  SFrame V2
  is not supported.

Changes in v12 (see patch notes for details):
- Adjust to Peter's latest undwind user enhancements.
- Simplify logic by using an internal SFrame FDE representation, whose
  FDE function start address field is an address instead of a PC-relative
  offset (from FDE).
- Rename struct sframe_fre to sframe_fre_internal to align with
  struct sframe_fde_internal.
- Remove unused pt_regs from unwind_user_next_common() and its
  callers. (Peter)
- Simplify unwind_user_next_sframe(). (Peter)
- Fix a few checkpatch errors and warnings.
- Minor cleanups (e.g. move includes, fix indentation).

Changes in v11:
- Support for SFrame V2 PC-relative FDE function start address.
- Support for SFrame V2 representing RA undefined as indication for
  outermost frames.


Patch 1 (new in v14), as a preparatory cleanup, adds the generic and
arch-specific unwind_user.h to MAINTAINERS.

Patches 2, 5, 12, and 19 have been updated to exclusively support the
latest SFrame V3 stack trace information format, that is generated by
binutils 2.46+.  Old SFrame V2 sections get rejected with dynamic debug
message "bad/unsupported sframe header".

Patches 8 and 9 add support to unwind user (sframe) for outermost frames.

Patches 13-16 add support to unwind user (sframe) for the new SFrame V3
flexible FDEs.

Patch 17 improves the performance of searching the SFrame FRE for an IP.

Patch 18 (new in v15) duplicates registered .sframe section data on
clone/fork from the parent to the child process.

Patch 20 is for test purposes only and will get replaced by a new
syscall, that Steven is working on:
[RFC][PATCH] unwind: Add stacktrace_setup system call
https://lore.kernel.org/all/20260429114355.6c712e6a@gandalf.local.home/

Regards,
Jens


Jens Remus (9):
  unwind_user: Add generic and arch-specific headers to MAINTAINERS
  unwind_user: Stop when reaching an outermost frame
  unwind_user/sframe: Add support for outermost frame indication
  unwind_user: Enable archs that pass RA in a register
  unwind_user: Flexible FP/RA recovery rules
  unwind_user: Flexible CFA recovery rules
  unwind_user/sframe: Add support for SFrame V3 flexible FDEs
  unwind_user/sframe: Separate reading of FRE from reading of FRE data
    words
  unwind_user/sframe: Duplicate registered .sframe section data on
    clone/fork

Josh Poimboeuf (11):
  unwind_user/sframe: Add support for reading .sframe headers
  unwind_user/sframe: Store .sframe section data in per-mm maple tree
  x86/uaccess: Add unsafe_copy_from_user() implementation
  unwind_user/sframe: Add support for reading .sframe contents
  unwind_user/sframe: Detect .sframe sections in executables
  unwind_user/sframe: Wire up unwind_user to sframe
  unwind_user/sframe: Remove .sframe section on detected corruption
  unwind_user/sframe: Show file name in debug output
  unwind_user/sframe: Add .sframe validation option
  unwind_user/sframe/x86: Enable sframe unwinding on x86
  unwind_user/sframe: Add prctl() interface for registering .sframe
    sections

 MAINTAINERS                               |   4 +
 arch/Kconfig                              |  23 +
 arch/x86/Kconfig                          |   1 +
 arch/x86/include/asm/mmu.h                |   2 +-
 arch/x86/include/asm/uaccess.h            |  39 +-
 arch/x86/include/asm/unwind_user.h        |  74 +-
 arch/x86/include/asm/unwind_user_sframe.h |  12 +
 fs/binfmt_elf.c                           |  48 +-
 include/linux/mm_types.h                  |   3 +
 include/linux/sframe.h                    |  65 ++
 include/linux/unwind_user.h               |  20 +
 include/linux/unwind_user_types.h         |  50 +-
 include/uapi/linux/elf.h                  |   1 +
 include/uapi/linux/prctl.h                |   4 +
 kernel/fork.c                             |  10 +
 kernel/sys.c                              |   9 +
 kernel/unwind/Makefile                    |   3 +-
 kernel/unwind/sframe.c                    | 949 ++++++++++++++++++++++
 kernel/unwind/sframe.h                    |  88 ++
 kernel/unwind/sframe_debug.h              |  75 ++
 kernel/unwind/user.c                      | 133 ++-
 mm/init-mm.c                              |   2 +
 mm/mmap.c                                 |   9 +
 23 files changed, 1586 insertions(+), 38 deletions(-)
 create mode 100644 arch/x86/include/asm/unwind_user_sframe.h
 create mode 100644 include/linux/sframe.h
 create mode 100644 kernel/unwind/sframe.c
 create mode 100644 kernel/unwind/sframe.h
 create mode 100644 kernel/unwind/sframe_debug.h

-- 
2.51.0

[PATCH v15 01/20] unwind_user: Add generic and arch-specific headers to MAINTAINERS

[Jens Remus]

Commit 71753c6ed2bf ("unwind_user: Add user space unwinding API with
frame pointer support") introduced include/asm-generic/unwind_user.h
without adding it to MAINTAINERS, as well as any future arch-specific
versions such as the one added by commit 49cf34c0815f
("unwind_user/x86: Enable frame pointer unwinding on x86") which
introduced arch/x86/include/asm/unwind_user.h.

Suggested-by: Dylan Hatch <dylanbhatch@google.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    Changes in v14:
    - New patch.

 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index c2c6d79275c6..7434e9d7b33f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27874,6 +27874,8 @@ USERSPACE STACK UNWINDING
 M:	Josh Poimboeuf <jpoimboe@kernel.org>
 M:	Steven Rostedt <rostedt@goodmis.org>
 S:	Maintained
+F:	arch/*/include/asm/unwind_user.h
+F:	include/asm-generic/unwind_user.h
 F:	include/linux/unwind*.h
 F:	kernel/unwind/
 
-- 
2.51.0

[PATCH v15 02/20] unwind_user/sframe: Add support for reading .sframe headers

[Jens Remus]

From: Josh Poimboeuf <jpoimboe@kernel.org>

In preparation for unwinding user space stacks with sframe, add basic
sframe compile infrastructure and support for reading the .sframe
section header.

sframe_add_section() reads the header and unconditionally returns an
error, so it's not very useful yet.  A subsequent patch will improve
that.

Link: https://lore.kernel.org/all/f27e8463783febfa0dabb0432a3dd6be8ad98412.1737511963.git.jpoimboe@kernel.org/

[ Jens Remus: Add support for SFrame V3.  Add support for PC-relative
FDE function start offset.  Cleanup includes and indentation. ]

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Reviewed-by: Indu Bhagat <ibhagatgnu@gmail.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    Changes in v15:
    - Improve text/sframe section start/end validation. (Sashiko AI)
    - Use GFP_KERNEL_ACCOUNT instead of GFP_KERNEL (see
      memory-allocation.rst, section "Get Free Page flags"). (Sashiko AI)
    
    Changes in v14:
    - Rename SFRAME_FDE_TYPE_REGULAR to SFRAME_FDE_TYPE_DEFAULT to match
      SFrame V3 specification. (Indu)
    - Correct SFRAME_V3_FDE_TYPE_MASK value.
    
    Changes in v13:
    - Update to SFrame V3:
      - Add and use SFRAME_VERSION_3 definition.
      - Add helper macros to access SFrame V3 FDE type.
      - Rename SFRAME_FUNC_*() macros to SFRAME_FDE_*().
      - Rename SFRAME_FDE_TYPE_PC* defines to SFRAME_FDE_PCTYPE_* and
        SFRAME_FUNC_FDE_TYPE() macro to SFRAME_V3_FDE_PCTYPE().
      - Reword OFFSET to DATAWORD in SFRAME_FRE_OFFSET_{COUNT|SIZE}()
        macros.
      - Rename version-specific SFRAME_*() macros to SFRAME_V3_*().
      - Update struct sframe_fde and rename to sframe_fde_v3:
        - Change field start_addr from s32 to s64 and rename to
          func_start_off.
        - Change field fres_num from u32 to u16.
        - New field u8 info2.
        - Remove u16 padding field.
      - Split FDE into function descriptor entry (struct sframe_fde_v3) and
        attributes (struct sframe_fde_v3).
      - Rename macro parameter "data" to "info" to hint at fde/fre info
        word and wrap it in parenthesis.
    - Group SFRAME_* definitions so that related ones are together.
    - Reword commit message (my changes).

 MAINTAINERS            |   1 +
 arch/Kconfig           |   3 +
 include/linux/sframe.h |  37 +++++++++++
 kernel/unwind/Makefile |   3 +-
 kernel/unwind/sframe.c | 136 +++++++++++++++++++++++++++++++++++++++++
 kernel/unwind/sframe.h |  81 ++++++++++++++++++++++++
 6 files changed, 260 insertions(+), 1 deletion(-)
 create mode 100644 include/linux/sframe.h
 create mode 100644 kernel/unwind/sframe.c
 create mode 100644 kernel/unwind/sframe.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 7434e9d7b33f..a9b42b67a88d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27876,6 +27876,7 @@ M:	Steven Rostedt <rostedt@goodmis.org>
 S:	Maintained
 F:	arch/*/include/asm/unwind_user.h
 F:	include/asm-generic/unwind_user.h
+F:	include/linux/sframe.h
 F:	include/linux/unwind*.h
 F:	kernel/unwind/
 
diff --git a/arch/Kconfig b/arch/Kconfig
index e86880045158..94b2d5e8e529 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -486,6 +486,9 @@ config HAVE_UNWIND_USER_FP
 	bool
 	select UNWIND_USER
 
+config HAVE_UNWIND_USER_SFRAME
+	bool
+
 config HAVE_PERF_REGS
 	bool
 	help
diff --git a/include/linux/sframe.h b/include/linux/sframe.h
new file mode 100644
index 000000000000..0642595534f9
--- /dev/null
+++ b/include/linux/sframe.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_SFRAME_H
+#define _LINUX_SFRAME_H
+
+#ifdef CONFIG_HAVE_UNWIND_USER_SFRAME
+
+struct sframe_section {
+	unsigned long	sframe_start;
+	unsigned long	sframe_end;
+	unsigned long	text_start;
+	unsigned long	text_end;
+
+	unsigned long	fdes_start;
+	unsigned long	fres_start;
+	unsigned long	fres_end;
+	unsigned int	num_fdes;
+
+	signed char	ra_off;
+	signed char	fp_off;
+};
+
+extern int sframe_add_section(unsigned long sframe_start, unsigned long sframe_end,
+			      unsigned long text_start, unsigned long text_end);
+extern int sframe_remove_section(unsigned long sframe_addr);
+
+#else /* !CONFIG_HAVE_UNWIND_USER_SFRAME */
+
+static inline int sframe_add_section(unsigned long sframe_start, unsigned long sframe_end,
+				     unsigned long text_start, unsigned long text_end)
+{
+	return -ENOSYS;
+}
+static inline int sframe_remove_section(unsigned long sframe_addr) { return -ENOSYS; }
+
+#endif /* CONFIG_HAVE_UNWIND_USER_SFRAME */
+
+#endif /* _LINUX_SFRAME_H */
diff --git a/kernel/unwind/Makefile b/kernel/unwind/Makefile
index eae37bea54fd..146038165865 100644
--- a/kernel/unwind/Makefile
+++ b/kernel/unwind/Makefile
@@ -1 +1,2 @@
- obj-$(CONFIG_UNWIND_USER)	+= user.o deferred.o
+ obj-$(CONFIG_UNWIND_USER)		+= user.o deferred.o
+ obj-$(CONFIG_HAVE_UNWIND_USER_SFRAME)	+= sframe.o
diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
new file mode 100644
index 000000000000..d24e9d4f8bef
--- /dev/null
+++ b/kernel/unwind/sframe.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Userspace sframe access functions
+ */
+
+#define pr_fmt(fmt)	"sframe: " fmt
+
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/srcu.h>
+#include <linux/uaccess.h>
+#include <linux/mm.h>
+#include <linux/string_helpers.h>
+#include <linux/sframe.h>
+#include <linux/unwind_user_types.h>
+
+#include "sframe.h"
+
+#define dbg(fmt, ...)							\
+	pr_debug("%s (%d): " fmt, current->comm, current->pid, ##__VA_ARGS__)
+
+static void free_section(struct sframe_section *sec)
+{
+	kfree(sec);
+}
+
+static int sframe_read_header(struct sframe_section *sec)
+{
+	unsigned long header_end, fdes_start, fdes_end, fres_start, fres_end;
+	struct sframe_header shdr;
+	unsigned int num_fdes;
+
+	if (copy_from_user(&shdr, (void __user *)sec->sframe_start, sizeof(shdr))) {
+		dbg("header usercopy failed\n");
+		return -EFAULT;
+	}
+
+	if (shdr.preamble.magic != SFRAME_MAGIC ||
+	    shdr.preamble.version != SFRAME_VERSION_3 ||
+	    !(shdr.preamble.flags & SFRAME_F_FDE_SORTED) ||
+	    !(shdr.preamble.flags & SFRAME_F_FDE_FUNC_START_PCREL) ||
+	    shdr.auxhdr_len) {
+		dbg("bad/unsupported sframe header\n");
+		return -EINVAL;
+	}
+
+	if (!shdr.num_fdes || !shdr.num_fres) {
+		dbg("no fde/fre entries\n");
+		return -EINVAL;
+	}
+
+	header_end = sec->sframe_start + SFRAME_HEADER_SIZE(shdr);
+	if (header_end >= sec->sframe_end) {
+		dbg("header doesn't fit in section\n");
+		return -EINVAL;
+	}
+
+	num_fdes   = shdr.num_fdes;
+	fdes_start = header_end + shdr.fdes_off;
+	fdes_end   = fdes_start + (num_fdes * sizeof(struct sframe_fde_v3));
+
+	fres_start = header_end + shdr.fres_off;
+	fres_end   = fres_start + shdr.fre_len;
+
+	if (fres_start < fdes_end || fres_end > sec->sframe_end) {
+		dbg("inconsistent fde/fre offsets\n");
+		return -EINVAL;
+	}
+
+	sec->num_fdes		= num_fdes;
+	sec->fdes_start		= fdes_start;
+	sec->fres_start		= fres_start;
+	sec->fres_end		= fres_end;
+
+	sec->ra_off		= shdr.cfa_fixed_ra_offset;
+	sec->fp_off		= shdr.cfa_fixed_fp_offset;
+
+	return 0;
+}
+
+int sframe_add_section(unsigned long sframe_start, unsigned long sframe_end,
+		       unsigned long text_start, unsigned long text_end)
+{
+	struct vm_area_struct *sframe_vma, *text_vma;
+	struct mm_struct *mm = current->mm;
+	struct sframe_section *sec;
+	int ret;
+
+	if (sframe_start >= sframe_end || text_start >= text_end) {
+		dbg("invalid sframe/text address\n");
+		return -EINVAL;
+	}
+
+	scoped_guard(mmap_read_lock, mm) {
+		sframe_vma = vma_lookup(mm, sframe_start);
+		if (!sframe_vma || sframe_end > sframe_vma->vm_end) {
+			dbg("bad sframe address (0x%lx - 0x%lx)\n",
+			    sframe_start, sframe_end);
+			return -EINVAL;
+		}
+
+		text_vma = vma_lookup(mm, text_start);
+		if (!text_vma ||
+		    !(text_vma->vm_flags & VM_EXEC) ||
+		    text_end > text_vma->vm_end) {
+			dbg("bad text address (0x%lx - 0x%lx)\n",
+			    text_start, text_end);
+			return -EINVAL;
+		}
+	}
+
+	sec = kzalloc(sizeof(*sec), GFP_KERNEL_ACCOUNT);
+	if (!sec)
+		return -ENOMEM;
+
+	sec->sframe_start	= sframe_start;
+	sec->sframe_end		= sframe_end;
+	sec->text_start		= text_start;
+	sec->text_end		= text_end;
+
+	ret = sframe_read_header(sec);
+	if (ret)
+		goto err_free;
+
+	/* TODO nowhere to store it yet - just free it and return an error */
+	ret = -ENOSYS;
+
+err_free:
+	free_section(sec);
+	return ret;
+}
+
+int sframe_remove_section(unsigned long sframe_start)
+{
+	return -ENOSYS;
+}
diff --git a/kernel/unwind/sframe.h b/kernel/unwind/sframe.h
new file mode 100644
index 000000000000..fc2908e92c7b
--- /dev/null
+++ b/kernel/unwind/sframe.h
@@ -0,0 +1,81 @@
+/* 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_VERSION_3			3
+#define SFRAME_MAGIC				0xdee2
+
+#define SFRAME_F_FDE_SORTED			0x1
+#define SFRAME_F_FRAME_POINTER			0x2
+#define SFRAME_F_FDE_FUNC_START_PCREL		0x4
+
+#define SFRAME_ABI_AARCH64_ENDIAN_BIG		1
+#define SFRAME_ABI_AARCH64_ENDIAN_LITTLE	2
+#define SFRAME_ABI_AMD64_ENDIAN_LITTLE		3
+
+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))
+
+struct sframe_fde_v3 {
+	s64	func_start_off;
+	u32	func_size;
+	u32	fres_off;
+} __packed;
+
+struct sframe_fda_v3 {
+	u16	fres_num;
+	u8	info;
+	u8	info2;
+	u8	rep_size;
+} __packed;
+
+#define SFRAME_FDE_PCTYPE_INC			0
+#define SFRAME_FDE_PCTYPE_MASK			1
+
+#define SFRAME_AARCH64_PAUTH_KEY_A		0
+#define SFRAME_AARCH64_PAUTH_KEY_B		1
+
+#define SFRAME_V3_FDE_FRE_TYPE(info)		((info) & 0xf)
+#define SFRAME_V3_FDE_PCTYPE(info)		(((info) >> 4) & 0x1)
+#define SFRAME_V3_AARCH64_FDE_PAUTH_KEY(info)	(((info) >> 5) & 0x1)
+
+#define SFRAME_FDE_TYPE_DEFAULT			0
+
+#define SFRAME_V3_FDE_TYPE_MASK			0x1f
+#define SFRAME_V3_FDE_TYPE(info2)		((info2) & SFRAME_V3_FDE_TYPE_MASK)
+
+#define SFRAME_BASE_REG_FP			0
+#define SFRAME_BASE_REG_SP			1
+
+#define SFRAME_V3_FRE_CFA_BASE_REG_ID(info)		((info) & 0x1)
+#define SFRAME_V3_FRE_DATAWORD_COUNT(info)		(((info) >> 1) & 0xf)
+#define SFRAME_V3_FRE_DATAWORD_SIZE(info)		(((info) >> 5) & 0x3)
+#define SFRAME_V3_AARCH64_FRE_MANGLED_RA_P(info)	(((info) >> 7) & 0x1)
+
+#endif /* _SFRAME_H */
-- 
2.51.0

[PATCH v15 03/20] unwind_user/sframe: Store .sframe section data in per-mm maple tree

[Jens Remus]

From: Josh Poimboeuf <jpoimboe@kernel.org>

Associate an .sframe section with its mm by adding it to a per-mm maple
tree which is indexed by the corresponding text address range.  A single
.sframe section can be associated with multiple text ranges.

[ Jens Remus: Minor cleanups. Reword commit subject/message. ]

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Reviewed-by: Indu Bhagat <ibhagatgnu@gmail.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    Changes in v15:
    - Fix text section end passed to mtree_insert_range() to be inclusive.
      (Sashiko AI)
    - sframe_remove_section(): Add guard(srcu) to guard access to
      sec->sframe_start.  This also guards access to sec->filename
      in __sframe_remove_section(). (Sashiko AI)
    - Use GFP_KERNEL_ACCOUNT instead of GFP_KERNEL (see
      memory-allocation.rst, section "Get Free Page flags"). (Sashiko AI)

 arch/x86/include/asm/mmu.h |  2 +-
 include/linux/mm_types.h   |  3 ++
 include/linux/sframe.h     | 15 ++++++++++
 kernel/fork.c              | 10 +++++++
 kernel/unwind/sframe.c     | 59 ++++++++++++++++++++++++++++++++++++--
 mm/init-mm.c               |  2 ++
 6 files changed, 87 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/mmu.h b/arch/x86/include/asm/mmu.h
index 0fe9c569d171..227a32899a59 100644
--- a/arch/x86/include/asm/mmu.h
+++ b/arch/x86/include/asm/mmu.h
@@ -87,7 +87,7 @@ typedef struct {
 	.context = {							\
 		.ctx_id = 1,						\
 		.lock = __MUTEX_INITIALIZER(mm.context.lock),		\
-	}
+	},
 
 void leave_mm(void);
 #define leave_mm leave_mm
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index a308e2c23b82..c1505356b6fc 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -1424,6 +1424,9 @@ struct mm_struct {
 #ifdef CONFIG_MM_ID
 		mm_id_t mm_id;
 #endif /* CONFIG_MM_ID */
+#ifdef CONFIG_HAVE_UNWIND_USER_SFRAME
+		struct maple_tree sframe_mt;
+#endif
 	} __randomize_layout;
 
 	/*
diff --git a/include/linux/sframe.h b/include/linux/sframe.h
index 0642595534f9..7ea6a97ed8af 100644
--- a/include/linux/sframe.h
+++ b/include/linux/sframe.h
@@ -2,6 +2,8 @@
 #ifndef _LINUX_SFRAME_H
 #define _LINUX_SFRAME_H
 
+#include <linux/mm_types.h>
+
 #ifdef CONFIG_HAVE_UNWIND_USER_SFRAME
 
 struct sframe_section {
@@ -19,18 +21,31 @@ struct sframe_section {
 	signed char	fp_off;
 };
 
+#define INIT_MM_SFRAME .sframe_mt = MTREE_INIT(sframe_mt, 0),
+extern void sframe_free_mm(struct mm_struct *mm);
+
 extern int sframe_add_section(unsigned long sframe_start, unsigned long sframe_end,
 			      unsigned long text_start, unsigned long text_end);
 extern int sframe_remove_section(unsigned long sframe_addr);
 
+static inline bool current_has_sframe(void)
+{
+	struct mm_struct *mm = current->mm;
+
+	return mm && !mtree_empty(&mm->sframe_mt);
+}
+
 #else /* !CONFIG_HAVE_UNWIND_USER_SFRAME */
 
+#define INIT_MM_SFRAME
+static inline void sframe_free_mm(struct mm_struct *mm) {}
 static inline int sframe_add_section(unsigned long sframe_start, unsigned long sframe_end,
 				     unsigned long text_start, unsigned long text_end)
 {
 	return -ENOSYS;
 }
 static inline int sframe_remove_section(unsigned long sframe_addr) { return -ENOSYS; }
+static inline bool current_has_sframe(void) { return false; }
 
 #endif /* CONFIG_HAVE_UNWIND_USER_SFRAME */
 
diff --git a/kernel/fork.c b/kernel/fork.c
index 5f3fdfdb14c7..8d8195561c95 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -110,6 +110,7 @@
 #include <linux/tick.h>
 #include <linux/unwind_deferred.h>
 #include <linux/pgalloc.h>
+#include <linux/sframe.h>
 #include <linux/uaccess.h>
 
 #include <asm/mmu_context.h>
@@ -735,6 +736,7 @@ void __mmdrop(struct mm_struct *mm)
 	mm_pasid_drop(mm);
 	mm_destroy_cid(mm);
 	percpu_counter_destroy_many(mm->rss_stat, NR_MM_COUNTERS);
+	sframe_free_mm(mm);
 
 	free_mm(mm);
 }
@@ -1072,6 +1074,13 @@ static void mmap_init_lock(struct mm_struct *mm)
 #endif
 }
 
+static void mm_init_sframe(struct mm_struct *mm)
+{
+#ifdef CONFIG_HAVE_UNWIND_USER_SFRAME
+	mt_init(&mm->sframe_mt);
+#endif
+}
+
 static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
 	struct user_namespace *user_ns)
 {
@@ -1100,6 +1109,7 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
 	mm->pmd_huge_pte = NULL;
 #endif
 	mm_init_uprobes_state(mm);
+	mm_init_sframe(mm);
 	hugetlb_count_init(mm);
 
 	mm_flags_clear_all(mm);
diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
index d24e9d4f8bef..6b3ce3f8966d 100644
--- a/kernel/unwind/sframe.c
+++ b/kernel/unwind/sframe.c
@@ -81,6 +81,7 @@ static int sframe_read_header(struct sframe_section *sec)
 int sframe_add_section(unsigned long sframe_start, unsigned long sframe_end,
 		       unsigned long text_start, unsigned long text_end)
 {
+	struct maple_tree *sframe_mt = &current->mm->sframe_mt;
 	struct vm_area_struct *sframe_vma, *text_vma;
 	struct mm_struct *mm = current->mm;
 	struct sframe_section *sec;
@@ -122,15 +123,67 @@ int sframe_add_section(unsigned long sframe_start, unsigned long sframe_end,
 	if (ret)
 		goto err_free;
 
-	/* TODO nowhere to store it yet - just free it and return an error */
-	ret = -ENOSYS;
+	ret = mtree_insert_range(sframe_mt, sec->text_start, sec->text_end - 1,
+				 sec, GFP_KERNEL_ACCOUNT);
+	if (ret) {
+		dbg("mtree_insert_range failed: text=%lx-%lx\n",
+		    sec->text_start, sec->text_end);
+		goto err_free;
+	}
+
+	return 0;
 
 err_free:
 	free_section(sec);
 	return ret;
 }
 
+static int __sframe_remove_section(struct mm_struct *mm,
+				   struct sframe_section *sec)
+{
+	if (!mtree_erase(&mm->sframe_mt, sec->text_start)) {
+		dbg("mtree_erase failed: text=%lx\n", sec->text_start);
+		return -EINVAL;
+	}
+
+	free_section(sec);
+
+	return 0;
+}
+
 int sframe_remove_section(unsigned long sframe_start)
 {
-	return -ENOSYS;
+	struct mm_struct *mm = current->mm;
+	struct sframe_section *sec;
+	unsigned long index = 0;
+	bool found = false;
+	int ret = 0;
+
+	guard(srcu)(&sframe_srcu);
+
+	mt_for_each(&mm->sframe_mt, sec, index, ULONG_MAX) {
+		if (sec->sframe_start == sframe_start) {
+			found = true;
+			ret |= __sframe_remove_section(mm, sec);
+		}
+	}
+
+	if (!found || ret)
+		return -EINVAL;
+
+	return 0;
+}
+
+void sframe_free_mm(struct mm_struct *mm)
+{
+	struct sframe_section *sec;
+	unsigned long index = 0;
+
+	if (!mm)
+		return;
+
+	mt_for_each(&mm->sframe_mt, sec, index, ULONG_MAX)
+		free_section(sec);
+
+	mtree_destroy(&mm->sframe_mt);
 }
diff --git a/mm/init-mm.c b/mm/init-mm.c
index c5556bb9d5f0..77909139162e 100644
--- a/mm/init-mm.c
+++ b/mm/init-mm.c
@@ -11,6 +11,7 @@
 #include <linux/atomic.h>
 #include <linux/user_namespace.h>
 #include <linux/iommu.h>
+#include <linux/sframe.h>
 #include <asm/mmu.h>
 
 #ifndef INIT_MM_CONTEXT
@@ -49,6 +50,7 @@ struct mm_struct init_mm = {
 #endif
 	.flexible_array	= MM_STRUCT_FLEXIBLE_ARRAY_INIT,
 	INIT_MM_CONTEXT(init_mm)
+	INIT_MM_SFRAME
 };
 
 void setup_initial_init_mm(void *start_code, void *end_code,
-- 
2.51.0

[PATCH v15 04/20] x86/uaccess: Add unsafe_copy_from_user() implementation

[Jens Remus]

From: Josh Poimboeuf <jpoimboe@kernel.org>

Add an x86 implementation of unsafe_copy_from_user() similar to the
existing unsafe_copy_to_user().

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Reviewed-by: Indu Bhagat <ibhagatgnu@gmail.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    Changes in v15:
    - unsafe_copy_from_user(): Use const void *__src. (Sashiko AI)

 arch/x86/include/asm/uaccess.h | 39 +++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index 3a0dd3c2b233..235886106f31 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -598,7 +598,7 @@ _label:									\
  * We want the unsafe accessors to always be inlined and use
  * the error labels - thus the macro games.
  */
-#define unsafe_copy_loop(dst, src, len, type, label)				\
+#define unsafe_copy_to_user_loop(dst, src, len, type, label)			\
 	while (len >= sizeof(type)) {						\
 		unsafe_put_user(*(type *)(src),(type __user *)(dst),label);	\
 		dst += sizeof(type);						\
@@ -606,15 +606,34 @@ _label:									\
 		len -= sizeof(type);						\
 	}
 
-#define unsafe_copy_to_user(_dst,_src,_len,label)			\
-do {									\
-	char __user *__ucu_dst = (_dst);				\
-	const char *__ucu_src = (_src);					\
-	size_t __ucu_len = (_len);					\
-	unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u64, label);	\
-	unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u32, label);	\
-	unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u16, label);	\
-	unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u8, label);	\
+#define unsafe_copy_to_user(_dst, _src, _len, label)				\
+do {										\
+	void __user *__dst = (_dst);						\
+	const void *__src = (_src);						\
+	size_t __len = (_len);							\
+	unsafe_copy_to_user_loop(__dst, __src, __len, u64, label);		\
+	unsafe_copy_to_user_loop(__dst, __src, __len, u32, label);		\
+	unsafe_copy_to_user_loop(__dst, __src, __len, u16, label);		\
+	unsafe_copy_to_user_loop(__dst, __src, __len, u8,  label);		\
+} while (0)
+
+#define unsafe_copy_from_user_loop(dst, src, len, type, label)			\
+	while (len >= sizeof(type)) {						\
+		unsafe_get_user(*(type *)(dst), (type __user *)(src), label);	\
+		dst += sizeof(type);						\
+		src += sizeof(type);						\
+		len -= sizeof(type);						\
+	}
+
+#define unsafe_copy_from_user(_dst, _src, _len, label)				\
+do {										\
+	void *__dst = (_dst);							\
+	const void __user *__src = (_src);					\
+	size_t __len = (_len);							\
+	unsafe_copy_from_user_loop(__dst, __src, __len, u64, label);		\
+	unsafe_copy_from_user_loop(__dst, __src, __len, u32, label);		\
+	unsafe_copy_from_user_loop(__dst, __src, __len, u16, label);		\
+	unsafe_copy_from_user_loop(__dst, __src, __len, u8,  label);		\
 } while (0)
 
 #ifdef CONFIG_CC_HAS_ASM_GOTO_OUTPUT
-- 
2.51.0

[PATCH v15 05/20] unwind_user/sframe: Add support for reading .sframe contents

[Jens Remus]

From: Josh Poimboeuf <jpoimboe@kernel.org>

In preparation for using sframe to unwind user space stacks, add an
sframe_find() interface for finding the sframe information associated
with a given text address.

For performance, use user_read_access_begin() and the corresponding
unsafe_*() accessors.  Note that use of pr_debug() in uaccess-enabled
regions would break noinstr validation, so there aren't any debug
messages yet.  That will be added in a subsequent commit.

Link: https://lore.kernel.org/all/77c0d1ec143bf2a53d66c4ecb190e7e0a576fbfd.1737511963.git.jpoimboe@kernel.org/
Link: https://lore.kernel.org/all/b35ca3a3-8de5-4d32-8d30-d4e562f6b0de@linux.ibm.com/

[ Jens Remus: Add initial support for SFrame V3 (limited to default
FDEs).  Add support for PC-relative FDE function start offset.  Simplify
logic by using an internal FDE representation.  Rename struct sframe_fre
to sframe_fre_internal to align with struct sframe_fde_internal.
Cleanup includes.  Fix checkpatch errors "spaces required around that
':'". ]

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Reviewed-by: Indu Bhagat <ibhagatgnu@gmail.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    Changes in v15:
    - __read_fde():
      - Validate FDE repetition size for PCTYPE_MASK FDEs to be non-zero to
        prevent division by zero. (Sashiko AI)
      - Validate FDE PC type for supported values (i.e. PCTYPE_INC or
        PCTYPE_MASK).
      - Validate FDE function end against text end.
      - Validate FDE's number of FREs to be less or equal to FDE's function
        size, as each FRE must cover at least one byte. (Indu)
    - __read_fre(): Validate FRE function offset against FDE repetition size
      for PCTYPE_MASK.
    - Change type of struct sframe_fde_internal field fres_num to the one of
      struct sframe_fda_v3 field fres_num.
    - Normalize error code usage (.sframe is removed for all but ENOENT):
      ENOENT: No sframe or no FDE for IP found
              (FDE found but no FRE found is EINVAL)
      EFAULT: Bad address
      EINVAL: Invalid input or sframe
    - Build-time checks for config options:
      - 64BIT: SFrame V3 only supports 64-bit architectures.
      - HAVE_EFFICIENT_UNALIGNED_ACCESS: Unaligned access to 16/32-bit
        SFrame FRE fields and datawords using unsafe_get_user(). (Steven)
    - Reword my changelog in commit message.
    
    Changes in v14:
    - Fix FDE function start address check in __read_fde().
    - Adjust to rename of SFRAME_FDE_TYPE_*.
    
    Changes in v13:
    - Update to SFrame V3:
      - Adjust to SFRAME_V3_*() macros and macro/define renames.
      - Adjust to struct sframe_fde_v3 rename.
      - Adjust to s64 FDE function start offset.
      - Rename local variables fde_type to fde_pctype.
      - Add and maintain struct sframe_fde_internal field u8 info2.
      - Adjust to FDE split into function descriptor entry
        (struct sframe_fde_v3) and attributes (struct sframe_fde_v3).
      - Rename offset_count/offset_size to dataword_count/dataword_count.
      - Limit __read_fre() to SFrame V3 regular FDEs (FDE_TYPE_REGULAR).  A
        subsequent patch will add support for flexible FDEs (FDE_TYPE_FLEX).
    - Rename struct sframe_fde_internal field func_start_addr to func_addr.
    - Add support u64/s64 in UNSAFE_GET_USER_INC() for s64 FDE function
      start offset.
    - Reduce indentation of assignments to fre.
    - Reword commit message (my changes).

 include/linux/sframe.h       |   6 +
 kernel/unwind/sframe.c       | 367 ++++++++++++++++++++++++++++++++++-
 kernel/unwind/sframe_debug.h |  35 ++++
 3 files changed, 404 insertions(+), 4 deletions(-)
 create mode 100644 kernel/unwind/sframe_debug.h

diff --git a/include/linux/sframe.h b/include/linux/sframe.h
index 7ea6a97ed8af..9a72209696f9 100644
--- a/include/linux/sframe.h
+++ b/include/linux/sframe.h
@@ -3,10 +3,14 @@
 #define _LINUX_SFRAME_H
 
 #include <linux/mm_types.h>
+#include <linux/srcu.h>
+#include <linux/unwind_user_types.h>
 
 #ifdef CONFIG_HAVE_UNWIND_USER_SFRAME
 
 struct sframe_section {
+	struct rcu_head	rcu;
+
 	unsigned long	sframe_start;
 	unsigned long	sframe_end;
 	unsigned long	text_start;
@@ -27,6 +31,7 @@ extern void sframe_free_mm(struct mm_struct *mm);
 extern int sframe_add_section(unsigned long sframe_start, unsigned long sframe_end,
 			      unsigned long text_start, unsigned long text_end);
 extern int sframe_remove_section(unsigned long sframe_addr);
+extern int sframe_find(unsigned long ip, struct unwind_user_frame *frame);
 
 static inline bool current_has_sframe(void)
 {
@@ -45,6 +50,7 @@ static inline int sframe_add_section(unsigned long sframe_start, unsigned long s
 	return -ENOSYS;
 }
 static inline int sframe_remove_section(unsigned long sframe_addr) { return -ENOSYS; }
+static inline int sframe_find(unsigned long ip, struct unwind_user_frame *frame) { return -ENOSYS; }
 static inline bool current_has_sframe(void) { return false; }
 
 #endif /* CONFIG_HAVE_UNWIND_USER_SFRAME */
diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
index 6b3ce3f8966d..a38f50a36363 100644
--- a/kernel/unwind/sframe.c
+++ b/kernel/unwind/sframe.c
@@ -15,9 +15,350 @@
 #include <linux/unwind_user_types.h>
 
 #include "sframe.h"
+#include "sframe_debug.h"
+
+struct sframe_fde_internal {
+	unsigned long	func_addr;
+	u32		func_size;
+	u32		fda_off;
+	u32		fres_off;
+	u16		fres_num;
+	u8		info;
+	u8		info2;
+	u8		rep_size;
+};
+
+struct sframe_fre_internal {
+	unsigned int	size;
+	u32		ip_off;
+	s32		cfa_off;
+	s32		ra_off;
+	s32		fp_off;
+	u8		info;
+};
+
+DEFINE_STATIC_SRCU(sframe_srcu);
+
+static __always_inline unsigned char fre_type_to_size(unsigned char fre_type)
+{
+	if (fre_type > 2)
+		return 0;
+	return 1 << fre_type;
+}
+
+static __always_inline unsigned char dataword_size_enum_to_size(unsigned char dataword_size)
+{
+	if (dataword_size > 2)
+		return 0;
+	return 1 << dataword_size;
+}
+
+static __always_inline int __read_fde(struct sframe_section *sec,
+				      unsigned int fde_num,
+				      struct sframe_fde_internal *fde)
+{
+	unsigned long fde_addr, fda_addr, func_start, func_end;
+	struct sframe_fde_v3 _fde;
+	struct sframe_fda_v3 _fda;
+	unsigned char fde_pctype;
+
+	fde_addr = sec->fdes_start + (fde_num * sizeof(struct sframe_fde_v3));
+	unsafe_copy_from_user(&_fde, (void __user *)fde_addr,
+			      sizeof(struct sframe_fde_v3), Efault);
+
+	func_start = fde_addr + _fde.func_start_off;
+	func_end = func_start + _fde.func_size;
+	if (func_start < sec->text_start || func_end > sec->text_end)
+		return -EFAULT;
+
+	fda_addr = sec->fres_start + _fde.fres_off;
+	if (fda_addr + sizeof(struct sframe_fda_v3) > sec->fres_end)
+		return -EFAULT;
+	unsafe_copy_from_user(&_fda, (void __user *)fda_addr,
+			      sizeof(struct sframe_fda_v3), Efault);
+
+	fde_pctype = SFRAME_V3_FDE_PCTYPE(_fda.info);
+	if (fde_pctype != SFRAME_FDE_PCTYPE_INC &&
+	    fde_pctype != SFRAME_FDE_PCTYPE_MASK)
+		return -EINVAL;
+	if (fde_pctype == SFRAME_FDE_PCTYPE_MASK && !_fda.rep_size)
+		return -EINVAL;
+	if (_fda.fres_num > _fde.func_size)
+		return -EINVAL;
+
+	fde->func_addr	= func_start;
+	fde->func_size	= _fde.func_size;
+	fde->fda_off	= _fde.fres_off;
+	fde->fres_off	= _fde.fres_off + sizeof(struct sframe_fda_v3);
+	fde->fres_num	= _fda.fres_num;
+	fde->info	= _fda.info;
+	fde->info2	= _fda.info2;
+	fde->rep_size	= _fda.rep_size;
 
-#define dbg(fmt, ...)							\
-	pr_debug("%s (%d): " fmt, current->comm, current->pid, ##__VA_ARGS__)
+	return 0;
+
+Efault:
+	return -EFAULT;
+}
+
+static __always_inline int __find_fde(struct sframe_section *sec,
+				      unsigned long ip,
+				      struct sframe_fde_internal *fde)
+{
+	unsigned long func_addr_low = 0, func_addr_high = ULONG_MAX;
+	struct sframe_fde_v3 __user *first, *low, *high, *found = NULL;
+	int ret;
+
+	first = (void __user *)sec->fdes_start;
+	low = first;
+	high = first + sec->num_fdes - 1;
+
+	while (low <= high) {
+		struct sframe_fde_v3 __user *mid;
+		s64 func_off;
+		unsigned long func_addr;
+
+		mid = low + ((high - low) / 2);
+
+		unsafe_get_user(func_off, (s64 __user *)mid, Efault);
+		func_addr = (unsigned long)mid + func_off;
+
+		if (ip >= func_addr) {
+			if (func_addr < func_addr_low)
+				return -EINVAL;
+
+			func_addr_low = func_addr;
+
+			found = mid;
+			low = mid + 1;
+		} else {
+			if (func_addr > func_addr_high)
+				return -EINVAL;
+
+			func_addr_high = func_addr;
+
+			high = mid - 1;
+		}
+	}
+
+	if (!found)
+		return -ENOENT;
+
+	ret = __read_fde(sec, found - first, fde);
+	if (ret)
+		return ret;
+
+	/* make sure it's not in a gap */
+	if (ip < fde->func_addr || ip >= fde->func_addr + fde->func_size)
+		return -ENOENT;
+
+	return 0;
+
+Efault:
+	return -EFAULT;
+}
+
+#define ____UNSAFE_GET_USER_INC(to, from, type, label)			\
+({									\
+	type __to;							\
+	unsafe_get_user(__to, (type __user *)from, label);		\
+	from += sizeof(__to);						\
+	to = __to;							\
+})
+
+#define __UNSAFE_GET_USER_INC(to, from, size, label, u_or_s)		\
+({									\
+	switch (size) {							\
+	case 1:								\
+		____UNSAFE_GET_USER_INC(to, from, u_or_s##8, label);	\
+		break;							\
+	case 2:								\
+		____UNSAFE_GET_USER_INC(to, from, u_or_s##16, label);	\
+		break;							\
+	case 4:								\
+		____UNSAFE_GET_USER_INC(to, from, u_or_s##32, label);	\
+		break;							\
+	default:							\
+		return -EFAULT;						\
+	}								\
+})
+
+#define UNSAFE_GET_USER_UNSIGNED_INC(to, from, size, label)		\
+	__UNSAFE_GET_USER_INC(to, from, size, label, u)
+
+#define UNSAFE_GET_USER_SIGNED_INC(to, from, size, label)		\
+	__UNSAFE_GET_USER_INC(to, from, size, label, s)
+
+#define UNSAFE_GET_USER_INC(to, from, size, label)				\
+	_Generic(to,								\
+		 u8 :	UNSAFE_GET_USER_UNSIGNED_INC(to, from, size, label),	\
+		 u16 :	UNSAFE_GET_USER_UNSIGNED_INC(to, from, size, label),	\
+		 u32 :	UNSAFE_GET_USER_UNSIGNED_INC(to, from, size, label),	\
+		 u64 :	UNSAFE_GET_USER_UNSIGNED_INC(to, from, size, label),	\
+		 s8 :	UNSAFE_GET_USER_SIGNED_INC(to, from, size, label),	\
+		 s16 :	UNSAFE_GET_USER_SIGNED_INC(to, from, size, label),	\
+		 s32 :	UNSAFE_GET_USER_SIGNED_INC(to, from, size, label),	\
+		 s64 :	UNSAFE_GET_USER_SIGNED_INC(to, from, size, label))
+
+static __always_inline int __read_fre(struct sframe_section *sec,
+				      struct sframe_fde_internal *fde,
+				      unsigned long fre_addr,
+				      struct sframe_fre_internal *fre)
+{
+	unsigned char fde_type = SFRAME_V3_FDE_TYPE(fde->info2);
+	unsigned char fde_pctype = SFRAME_V3_FDE_PCTYPE(fde->info);
+	unsigned char fre_type = SFRAME_V3_FDE_FRE_TYPE(fde->info);
+	unsigned char dataword_count, dataword_size;
+	s32 cfa_off, ra_off, fp_off;
+	unsigned long cur = fre_addr;
+	unsigned char addr_size;
+	u32 ip_off;
+	u8 info;
+
+	addr_size = fre_type_to_size(fre_type);
+	if (!addr_size)
+		return -EINVAL;
+
+	if (fre_addr + addr_size + 1 > sec->fres_end)
+		return -EFAULT;
+
+	UNSAFE_GET_USER_INC(ip_off, cur, addr_size, Efault);
+	if ((fde_pctype == SFRAME_FDE_PCTYPE_INC && ip_off >= fde->func_size) ||
+	    (fde_pctype == SFRAME_FDE_PCTYPE_MASK && ip_off >= fde->rep_size))
+		return -EINVAL;
+
+	UNSAFE_GET_USER_INC(info, cur, 1, Efault);
+	dataword_count = SFRAME_V3_FRE_DATAWORD_COUNT(info);
+	dataword_size  = dataword_size_enum_to_size(SFRAME_V3_FRE_DATAWORD_SIZE(info));
+	if (!dataword_count || !dataword_size)
+		return -EINVAL;
+
+	if (cur + (dataword_count * dataword_size) > sec->fres_end)
+		return -EFAULT;
+
+	/* TODO: Support for flexible FDEs not implemented yet. */
+	if (fde_type != SFRAME_FDE_TYPE_DEFAULT)
+		return -EINVAL;
+
+	UNSAFE_GET_USER_INC(cfa_off, cur, dataword_size, Efault);
+	dataword_count--;
+
+	ra_off = sec->ra_off;
+	if (!ra_off) {
+		if (!dataword_count--)
+			return -EINVAL;
+
+		UNSAFE_GET_USER_INC(ra_off, cur, dataword_size, Efault);
+	}
+
+	fp_off = sec->fp_off;
+	if (!fp_off && dataword_count) {
+		dataword_count--;
+		UNSAFE_GET_USER_INC(fp_off, cur, dataword_size, Efault);
+	}
+
+	if (dataword_count)
+		return -EINVAL;
+
+	fre->size	= addr_size + 1 + (dataword_count * dataword_size);
+	fre->ip_off	= ip_off;
+	fre->cfa_off	= cfa_off;
+	fre->ra_off	= ra_off;
+	fre->fp_off	= fp_off;
+	fre->info	= info;
+
+	return 0;
+
+Efault:
+	return -EFAULT;
+}
+
+static __always_inline int __find_fre(struct sframe_section *sec,
+				      struct sframe_fde_internal *fde,
+				      unsigned long ip,
+				      struct unwind_user_frame *frame)
+{
+	unsigned char fde_pctype = SFRAME_V3_FDE_PCTYPE(fde->info);
+	struct sframe_fre_internal *fre, *prev_fre = NULL;
+	struct sframe_fre_internal fres[2];
+	unsigned long fre_addr;
+	bool which = false;
+	unsigned int i;
+	u32 ip_off;
+
+	ip_off = ip - fde->func_addr;
+
+	if (fde_pctype == SFRAME_FDE_PCTYPE_MASK)
+		ip_off %= fde->rep_size;
+
+	fre_addr = sec->fres_start + fde->fres_off;
+
+	for (i = 0; i < fde->fres_num; i++) {
+		int ret;
+
+		/*
+		 * Alternate between the two fre_addr[] entries for 'fre' and
+		 * 'prev_fre'.
+		 */
+		fre = which ? fres : fres + 1;
+		which = !which;
+
+		ret = __read_fre(sec, fde, fre_addr, fre);
+		if (ret)
+			return ret;
+
+		fre_addr += fre->size;
+
+		if (prev_fre && fre->ip_off <= prev_fre->ip_off)
+			return -EINVAL;
+
+		if (fre->ip_off > ip_off)
+			break;
+
+		prev_fre = fre;
+	}
+
+	if (!prev_fre)
+		return -EINVAL;
+	fre = prev_fre;
+
+	frame->cfa_off = fre->cfa_off;
+	frame->ra_off  = fre->ra_off;
+	frame->fp_off  = fre->fp_off;
+	frame->use_fp  = SFRAME_V3_FRE_CFA_BASE_REG_ID(fre->info) == SFRAME_BASE_REG_FP;
+
+	return 0;
+}
+
+int sframe_find(unsigned long ip, struct unwind_user_frame *frame)
+{
+	struct mm_struct *mm = current->mm;
+	struct sframe_section *sec;
+	struct sframe_fde_internal fde;
+	int ret;
+
+	if (!mm)
+		return -EINVAL;
+
+	guard(srcu)(&sframe_srcu);
+
+	sec = mtree_load(&mm->sframe_mt, ip);
+	if (!sec)
+		return -ENOENT;
+
+	if (!user_read_access_begin((void __user *)sec->sframe_start,
+				    sec->sframe_end - sec->sframe_start))
+		return -EFAULT;
+
+	ret = __find_fde(sec, ip, &fde);
+	if (ret)
+		goto end;
+
+	ret = __find_fre(sec, &fde, ip, frame);
+end:
+	user_read_access_end();
+	return ret;
+}
 
 static void free_section(struct sframe_section *sec)
 {
@@ -30,6 +371,15 @@ static int sframe_read_header(struct sframe_section *sec)
 	struct sframe_header shdr;
 	unsigned int num_fdes;
 
+	/* SFrame V3 is only supported on 64-bit architectures */
+	BUILD_BUG_ON(!IS_ENABLED(CONFIG_64BIT));
+
+	/*
+	 * Unaligned access to 16/32-bit SFrame FRE fields and datawords
+	 * using unsafe_get_user() via UNSAFE_GET_USER_INC()
+	 */
+	BUILD_BUG_ON(!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS));
+
 	if (copy_from_user(&shdr, (void __user *)sec->sframe_start, sizeof(shdr))) {
 		dbg("header usercopy failed\n");
 		return -EFAULT;
@@ -120,8 +470,10 @@ int sframe_add_section(unsigned long sframe_start, unsigned long sframe_end,
 	sec->text_end		= text_end;
 
 	ret = sframe_read_header(sec);
-	if (ret)
+	if (ret) {
+		dbg_print_header(sec);
 		goto err_free;
+	}
 
 	ret = mtree_insert_range(sframe_mt, sec->text_start, sec->text_end - 1,
 				 sec, GFP_KERNEL_ACCOUNT);
@@ -138,6 +490,13 @@ int sframe_add_section(unsigned long sframe_start, unsigned long sframe_end,
 	return ret;
 }
 
+static void sframe_free_srcu(struct rcu_head *rcu)
+{
+	struct sframe_section *sec = container_of(rcu, struct sframe_section, rcu);
+
+	free_section(sec);
+}
+
 static int __sframe_remove_section(struct mm_struct *mm,
 				   struct sframe_section *sec)
 {
@@ -146,7 +505,7 @@ static int __sframe_remove_section(struct mm_struct *mm,
 		return -EINVAL;
 	}
 
-	free_section(sec);
+	call_srcu(&sframe_srcu, &sec->rcu, sframe_free_srcu);
 
 	return 0;
 }
diff --git a/kernel/unwind/sframe_debug.h b/kernel/unwind/sframe_debug.h
new file mode 100644
index 000000000000..36352124cde8
--- /dev/null
+++ b/kernel/unwind/sframe_debug.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _SFRAME_DEBUG_H
+#define _SFRAME_DEBUG_H
+
+#include <linux/sframe.h>
+#include "sframe.h"
+
+#ifdef CONFIG_DYNAMIC_DEBUG
+
+#define dbg(fmt, ...)							\
+	pr_debug("%s (%d): " fmt, current->comm, current->pid, ##__VA_ARGS__)
+
+static __always_inline void dbg_print_header(struct sframe_section *sec)
+{
+	unsigned long fdes_end;
+
+	fdes_end = sec->fdes_start + (sec->num_fdes * sizeof(struct sframe_fde_v3));
+
+	dbg("SEC: sframe:0x%lx-0x%lx text:0x%lx-0x%lx "
+	    "fdes:0x%lx-0x%lx fres:0x%lx-0x%lx "
+	    "ra_off:%d fp_off:%d\n",
+	    sec->sframe_start, sec->sframe_end, sec->text_start, sec->text_end,
+	    sec->fdes_start, fdes_end, sec->fres_start, sec->fres_end,
+	    sec->ra_off, sec->fp_off);
+}
+
+#else /* !CONFIG_DYNAMIC_DEBUG */
+
+#define dbg(args...)			no_printk(args)
+
+static inline void dbg_print_header(struct sframe_section *sec) {}
+
+#endif /* !CONFIG_DYNAMIC_DEBUG */
+
+#endif /* _SFRAME_DEBUG_H */
-- 
2.51.0

[PATCH v15 06/20] unwind_user/sframe: Detect .sframe sections in executables

[Jens Remus]

From: Josh Poimboeuf <jpoimboe@kernel.org>

When loading an ELF executable, automatically detect an .sframe section
and associate it with the mm_struct.

[ Jens Remus: Fix checkpatch warning "braces {} are not necessary for
single statement blocks". ]

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Reviewed-by: Indu Bhagat <ibhagatgnu@gmail.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    Changes in v15:
    - Only add sframe for text that is PT_LOAD in addition to PF_X.
      (Sashiko AI)

 fs/binfmt_elf.c          | 48 +++++++++++++++++++++++++++++++++++++---
 include/uapi/linux/elf.h |  1 +
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 16a56b6b3f6c..980a9f229cd1 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -48,6 +48,7 @@
 #include <linux/uaccess.h>
 #include <uapi/linux/rseq.h>
 #include <linux/rseq.h>
+#include <linux/sframe.h>
 #include <asm/param.h>
 #include <asm/page.h>
 
@@ -637,6 +638,21 @@ static inline int make_prot(u32 p_flags, struct arch_elf_state *arch_state,
 	return arch_elf_adjust_prot(prot, arch_state, has_interp, is_interp);
 }
 
+static void elf_add_sframe(struct elf_phdr *text, struct elf_phdr *sframe,
+			   unsigned long base_addr)
+{
+	unsigned long sframe_start, sframe_end, text_start, text_end;
+
+	sframe_start = base_addr + sframe->p_vaddr;
+	sframe_end   = sframe_start + sframe->p_memsz;
+
+	text_start   = base_addr + text->p_vaddr;
+	text_end     = text_start + text->p_memsz;
+
+	/* Ignore return value, sframe section isn't critical */
+	sframe_add_section(sframe_start, sframe_end, text_start, text_end);
+}
+
 /* This is much more generalized than the library routine read function,
    so we keep this separate.  Technically the library read function
    is only provided so that we can read a.out libraries that have
@@ -647,7 +663,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
 		unsigned long no_base, struct elf_phdr *interp_elf_phdata,
 		struct arch_elf_state *arch_state)
 {
-	struct elf_phdr *eppnt;
+	struct elf_phdr *eppnt, *sframe_phdr = NULL;
 	unsigned long load_addr = 0;
 	int load_addr_set = 0;
 	unsigned long error = ~0UL;
@@ -673,7 +689,8 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
 
 	eppnt = interp_elf_phdata;
 	for (i = 0; i < interp_elf_ex->e_phnum; i++, eppnt++) {
-		if (eppnt->p_type == PT_LOAD) {
+		switch (eppnt->p_type) {
+		case PT_LOAD: {
 			int elf_type = MAP_PRIVATE;
 			int elf_prot = make_prot(eppnt->p_flags, arch_state,
 						 true, true);
@@ -712,6 +729,19 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
 				error = -ENOMEM;
 				goto out;
 			}
+			break;
+		}
+		case PT_GNU_SFRAME:
+			sframe_phdr = eppnt;
+			break;
+		}
+	}
+
+	if (sframe_phdr) {
+		eppnt = interp_elf_phdata;
+		for (i = 0; i < interp_elf_ex->e_phnum; i++, eppnt++) {
+			if (eppnt->p_flags & PF_X && eppnt->p_type == PT_LOAD)
+				elf_add_sframe(eppnt, sframe_phdr, load_addr);
 		}
 	}
 
@@ -836,7 +866,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
 	int first_pt_load = 1;
 	unsigned long error;
 	struct elf_phdr *elf_ppnt, *elf_phdata, *interp_elf_phdata = NULL;
-	struct elf_phdr *elf_property_phdata = NULL;
+	struct elf_phdr *elf_property_phdata = NULL, *sframe_phdr = NULL;
 	unsigned long elf_brk;
 	bool brk_moved = false;
 	int retval, i;
@@ -945,6 +975,10 @@ static int load_elf_binary(struct linux_binprm *bprm)
 				executable_stack = EXSTACK_DISABLE_X;
 			break;
 
+		case PT_GNU_SFRAME:
+			sframe_phdr = elf_ppnt;
+			break;
+
 		case PT_LOPROC ... PT_HIPROC:
 			retval = arch_elf_pt_proc(elf_ex, elf_ppnt,
 						  bprm->file, false,
@@ -1242,6 +1276,14 @@ static int load_elf_binary(struct linux_binprm *bprm)
 			elf_brk = k;
 	}
 
+	if (sframe_phdr) {
+		for (i = 0, elf_ppnt = elf_phdata;
+		     i < elf_ex->e_phnum; i++, elf_ppnt++) {
+			if (elf_ppnt->p_flags & PF_X && elf_ppnt->p_type == PT_LOAD)
+				elf_add_sframe(elf_ppnt, sframe_phdr, load_bias);
+		}
+	}
+
 	e_entry = elf_ex->e_entry + load_bias;
 	phdr_addr += load_bias;
 	elf_brk += load_bias;
diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
index ee30dcd80901..e2a7dbed2e80 100644
--- a/include/uapi/linux/elf.h
+++ b/include/uapi/linux/elf.h
@@ -41,6 +41,7 @@ typedef __u16	Elf64_Versym;
 #define PT_GNU_STACK	(PT_LOOS + 0x474e551)
 #define PT_GNU_RELRO	(PT_LOOS + 0x474e552)
 #define PT_GNU_PROPERTY	(PT_LOOS + 0x474e553)
+#define PT_GNU_SFRAME	(PT_LOOS + 0x474e554)
 
 
 /* ARM MTE memory tag segment type */
-- 
2.51.0

[PATCH v15 07/20] unwind_user/sframe: Wire up unwind_user to sframe

[Jens Remus]

From: Josh Poimboeuf <jpoimboe@kernel.org>

Now that the sframe infrastructure is fully in place, make it work by
hooking it up to the unwind_user interface.

[ Jens Remus: Remove unused pt_regs from unwind_user_next_common() and
its callers.  Simplify unwind_user_next_sframe(). ]

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Reviewed-by: Indu Bhagat <ibhagatgnu@gmail.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---
 arch/Kconfig                      |  1 +
 include/linux/unwind_user_types.h |  4 +++-
 kernel/unwind/user.c              | 23 +++++++++++++++++++++++
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 94b2d5e8e529..37549832bd1f 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -488,6 +488,7 @@ config HAVE_UNWIND_USER_FP
 
 config HAVE_UNWIND_USER_SFRAME
 	bool
+	select UNWIND_USER
 
 config HAVE_PERF_REGS
 	bool
diff --git a/include/linux/unwind_user_types.h b/include/linux/unwind_user_types.h
index 412729a269bc..43e4b160883f 100644
--- a/include/linux/unwind_user_types.h
+++ b/include/linux/unwind_user_types.h
@@ -9,7 +9,8 @@
  * available.
  */
 enum unwind_user_type_bits {
-	UNWIND_USER_TYPE_FP_BIT =		0,
+	UNWIND_USER_TYPE_SFRAME_BIT =		0,
+	UNWIND_USER_TYPE_FP_BIT =		1,
 
 	NR_UNWIND_USER_TYPE_BITS,
 };
@@ -17,6 +18,7 @@ enum unwind_user_type_bits {
 enum unwind_user_type {
 	/* Type "none" for the start of stack walk iteration. */
 	UNWIND_USER_TYPE_NONE =			0,
+	UNWIND_USER_TYPE_SFRAME =		BIT(UNWIND_USER_TYPE_SFRAME_BIT),
 	UNWIND_USER_TYPE_FP =			BIT(UNWIND_USER_TYPE_FP_BIT),
 };
 
diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c
index 90ab3c1a205e..1fb272419733 100644
--- a/kernel/unwind/user.c
+++ b/kernel/unwind/user.c
@@ -7,6 +7,7 @@
 #include <linux/sched/task_stack.h>
 #include <linux/unwind_user.h>
 #include <linux/uaccess.h>
+#include <linux/sframe.h>
 
 #define for_each_user_frame(state) \
 	for (unwind_user_start(state); !(state)->done; unwind_user_next(state))
@@ -82,6 +83,16 @@ static int unwind_user_next_fp(struct unwind_user_state *state)
 	return unwind_user_next_common(state, &fp_frame);
 }
 
+static int unwind_user_next_sframe(struct unwind_user_state *state)
+{
+	struct unwind_user_frame frame;
+
+	/* sframe expects the frame to be local storage */
+	if (sframe_find(state->ip, &frame))
+		return -ENOENT;
+	return unwind_user_next_common(state, &frame);
+}
+
 static int unwind_user_next(struct unwind_user_state *state)
 {
 	unsigned long iter_mask = state->available_types;
@@ -95,6 +106,16 @@ static int unwind_user_next(struct unwind_user_state *state)
 
 		state->current_type = type;
 		switch (type) {
+		case UNWIND_USER_TYPE_SFRAME:
+			switch (unwind_user_next_sframe(state)) {
+			case 0:
+				return 0;
+			case -ENOENT:
+				continue;	/* Try next method. */
+			default:
+				state->done = true;
+			}
+			break;
 		case UNWIND_USER_TYPE_FP:
 			if (!unwind_user_next_fp(state))
 				return 0;
@@ -123,6 +144,8 @@ static int unwind_user_start(struct unwind_user_state *state)
 		return -EINVAL;
 	}
 
+	if (current_has_sframe())
+		state->available_types |= UNWIND_USER_TYPE_SFRAME;
 	if (IS_ENABLED(CONFIG_HAVE_UNWIND_USER_FP))
 		state->available_types |= UNWIND_USER_TYPE_FP;
 
-- 
2.51.0

[PATCH v15 08/20] unwind_user: Stop when reaching an outermost frame

[Jens Remus]

Add an indication for an outermost frame to the unwind user frame
structure and stop unwinding when reaching an outermost frame.

This will be used by unwind user sframe, as SFrame may represent an
undefined return address as indication for an outermost frame.

Reviewed-by: Indu Bhagat <ibhagatgnu@gmail.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---
 arch/x86/include/asm/unwind_user.h | 6 ++++--
 include/linux/unwind_user_types.h  | 1 +
 kernel/unwind/user.c               | 6 ++++++
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/unwind_user.h b/arch/x86/include/asm/unwind_user.h
index 6e469044e4de..2dfb5ef11e36 100644
--- a/arch/x86/include/asm/unwind_user.h
+++ b/arch/x86/include/asm/unwind_user.h
@@ -23,13 +23,15 @@ static inline int unwind_user_word_size(struct pt_regs *regs)
 	.cfa_off	=  2*(ws),			\
 	.ra_off		= -1*(ws),			\
 	.fp_off		= -2*(ws),			\
-	.use_fp		= true,
+	.use_fp		= true,				\
+	.outermost	= false,
 
 #define ARCH_INIT_USER_FP_ENTRY_FRAME(ws)		\
 	.cfa_off	=  1*(ws),			\
 	.ra_off		= -1*(ws),			\
 	.fp_off		= 0,				\
-	.use_fp		= false,
+	.use_fp		= false,			\
+	.outermost	= false,
 
 static inline bool unwind_user_at_function_start(struct pt_regs *regs)
 {
diff --git a/include/linux/unwind_user_types.h b/include/linux/unwind_user_types.h
index 43e4b160883f..616cc5ee4586 100644
--- a/include/linux/unwind_user_types.h
+++ b/include/linux/unwind_user_types.h
@@ -32,6 +32,7 @@ struct unwind_user_frame {
 	s32 ra_off;
 	s32 fp_off;
 	bool use_fp;
+	bool outermost;
 };
 
 struct unwind_user_state {
diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c
index 1fb272419733..fdb1001e3750 100644
--- a/kernel/unwind/user.c
+++ b/kernel/unwind/user.c
@@ -32,6 +32,12 @@ static int unwind_user_next_common(struct unwind_user_state *state,
 {
 	unsigned long cfa, fp, ra;
 
+	/* Stop unwinding when reaching an outermost frame. */
+	if (frame->outermost) {
+		state->done = true;
+		return 0;
+	}
+
 	/* Get the Canonical Frame Address (CFA) */
 	if (frame->use_fp) {
 		if (state->fp < state->sp)
-- 
2.51.0

[PATCH v15 09/20] unwind_user/sframe: Add support for outermost frame indication

[Jens Remus]

SFrame may represent an undefined return address (RA) as SFrame FRE
without any offsets as indication for an outermost frame.

Reviewed-by: Indu Bhagat <ibhagatgnu@gmail.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---
 kernel/unwind/sframe.c | 15 ++++++++++++++-
 kernel/unwind/sframe.h |  1 +
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
index a38f50a36363..f723c1a32f90 100644
--- a/kernel/unwind/sframe.c
+++ b/kernel/unwind/sframe.c
@@ -230,7 +230,7 @@ static __always_inline int __read_fre(struct sframe_section *sec,
 	UNSAFE_GET_USER_INC(info, cur, 1, Efault);
 	dataword_count = SFRAME_V3_FRE_DATAWORD_COUNT(info);
 	dataword_size  = dataword_size_enum_to_size(SFRAME_V3_FRE_DATAWORD_SIZE(info));
-	if (!dataword_count || !dataword_size)
+	if (!dataword_size)
 		return -EINVAL;
 
 	if (cur + (dataword_count * dataword_size) > sec->fres_end)
@@ -240,6 +240,17 @@ static __always_inline int __read_fre(struct sframe_section *sec,
 	if (fde_type != SFRAME_FDE_TYPE_DEFAULT)
 		return -EINVAL;
 
+	if (!dataword_count) {
+		/*
+		 * A FRE without data words indicates RA undefined /
+		 * outermost frame.
+		 */
+		cfa_off	= 0;
+		ra_off	= 0;
+		fp_off	= 0;
+		goto done;
+	}
+
 	UNSAFE_GET_USER_INC(cfa_off, cur, dataword_size, Efault);
 	dataword_count--;
 
@@ -260,6 +271,7 @@ static __always_inline int __read_fre(struct sframe_section *sec,
 	if (dataword_count)
 		return -EINVAL;
 
+done:
 	fre->size	= addr_size + 1 + (dataword_count * dataword_size);
 	fre->ip_off	= ip_off;
 	fre->cfa_off	= cfa_off;
@@ -326,6 +338,7 @@ static __always_inline int __find_fre(struct sframe_section *sec,
 	frame->ra_off  = fre->ra_off;
 	frame->fp_off  = fre->fp_off;
 	frame->use_fp  = SFRAME_V3_FRE_CFA_BASE_REG_ID(fre->info) == SFRAME_BASE_REG_FP;
+	frame->outermost = SFRAME_V3_FRE_RA_UNDEFINED_P(fre->info);
 
 	return 0;
 }
diff --git a/kernel/unwind/sframe.h b/kernel/unwind/sframe.h
index fc2908e92c7b..ed111fd0d702 100644
--- a/kernel/unwind/sframe.h
+++ b/kernel/unwind/sframe.h
@@ -77,5 +77,6 @@ struct sframe_fda_v3 {
 #define SFRAME_V3_FRE_DATAWORD_COUNT(info)		(((info) >> 1) & 0xf)
 #define SFRAME_V3_FRE_DATAWORD_SIZE(info)		(((info) >> 5) & 0x3)
 #define SFRAME_V3_AARCH64_FRE_MANGLED_RA_P(info)	(((info) >> 7) & 0x1)
+#define SFRAME_V3_FRE_RA_UNDEFINED_P(info)		(SFRAME_V3_FRE_DATAWORD_COUNT(info) == 0)
 
 #endif /* _SFRAME_H */
-- 
2.51.0

[PATCH v15 10/20] unwind_user/sframe: Remove .sframe section on detected corruption

[Jens Remus]

From: Josh Poimboeuf <jpoimboe@kernel.org>

To avoid continued attempted use of a bad .sframe section, remove it
on demand when the first sign of corruption is detected.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Reviewed-by: Indu Bhagat <ibhagatgnu@gmail.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    Changes in v15:
    - sframe_find(): Align to normalized error code usage and remove .sframe
      for all but ENOENT.  Also remove if user_read_access_begin() fails.

 kernel/unwind/sframe.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
index f723c1a32f90..02331956009a 100644
--- a/kernel/unwind/sframe.c
+++ b/kernel/unwind/sframe.c
@@ -360,16 +360,23 @@ int sframe_find(unsigned long ip, struct unwind_user_frame *frame)
 		return -ENOENT;
 
 	if (!user_read_access_begin((void __user *)sec->sframe_start,
-				    sec->sframe_end - sec->sframe_start))
-		return -EFAULT;
+				    sec->sframe_end - sec->sframe_start)) {
+		ret = -EFAULT;
+		goto end;
+	}
 
 	ret = __find_fde(sec, ip, &fde);
 	if (ret)
-		goto end;
+		goto end_uaccess;
 
 	ret = __find_fre(sec, &fde, ip, frame);
-end:
+end_uaccess:
 	user_read_access_end();
+
+end:
+	if (ret && ret != -ENOENT)
+		WARN_ON_ONCE(sframe_remove_section(sec->sframe_start));
+
 	return ret;
 }
 
-- 
2.51.0

[PATCH v15 11/20] unwind_user/sframe: Show file name in debug output

[Jens Remus]

From: Josh Poimboeuf <jpoimboe@kernel.org>

When debugging sframe issues, the error messages aren't all that helpful
without knowing what file a corresponding .sframe section belongs to.
Prefix debug output strings with the file name.

[ Jens Remus: Fix checkpatch error "space prohibited before that close
parenthesis ')'". ]

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Reviewed-by: Indu Bhagat <ibhagatgnu@gmail.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    Changes in v15:
    - Use GFP_KERNEL_ACCOUNT instead of GFP_KERNEL (see
      memory-allocation.rst, section "Get Free Page flags"). (Sashiko AI)
    
    Changes in v14:
    - Uppercase terms FDE and FRE in debug messages.

 include/linux/sframe.h       |  4 +++-
 kernel/unwind/sframe.c       | 23 ++++++++++--------
 kernel/unwind/sframe_debug.h | 45 +++++++++++++++++++++++++++++++-----
 3 files changed, 56 insertions(+), 16 deletions(-)

diff --git a/include/linux/sframe.h b/include/linux/sframe.h
index 9a72209696f9..b79c5ec09229 100644
--- a/include/linux/sframe.h
+++ b/include/linux/sframe.h
@@ -10,7 +10,9 @@
 
 struct sframe_section {
 	struct rcu_head	rcu;
-
+#ifdef CONFIG_DYNAMIC_DEBUG
+	const char	*filename;
+#endif
 	unsigned long	sframe_start;
 	unsigned long	sframe_end;
 	unsigned long	text_start;
diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
index 02331956009a..f931932bd34a 100644
--- a/kernel/unwind/sframe.c
+++ b/kernel/unwind/sframe.c
@@ -374,14 +374,17 @@ int sframe_find(unsigned long ip, struct unwind_user_frame *frame)
 	user_read_access_end();
 
 end:
-	if (ret && ret != -ENOENT)
+	if (ret && ret != -ENOENT) {
+		dbg_sec("removing bad .sframe section\n");
 		WARN_ON_ONCE(sframe_remove_section(sec->sframe_start));
+	}
 
 	return ret;
 }
 
 static void free_section(struct sframe_section *sec)
 {
+	dbg_free(sec);
 	kfree(sec);
 }
 
@@ -401,7 +404,7 @@ static int sframe_read_header(struct sframe_section *sec)
 	BUILD_BUG_ON(!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS));
 
 	if (copy_from_user(&shdr, (void __user *)sec->sframe_start, sizeof(shdr))) {
-		dbg("header usercopy failed\n");
+		dbg_sec("header usercopy failed\n");
 		return -EFAULT;
 	}
 
@@ -410,18 +413,18 @@ static int sframe_read_header(struct sframe_section *sec)
 	    !(shdr.preamble.flags & SFRAME_F_FDE_SORTED) ||
 	    !(shdr.preamble.flags & SFRAME_F_FDE_FUNC_START_PCREL) ||
 	    shdr.auxhdr_len) {
-		dbg("bad/unsupported sframe header\n");
+		dbg_sec("bad/unsupported sframe header\n");
 		return -EINVAL;
 	}
 
 	if (!shdr.num_fdes || !shdr.num_fres) {
-		dbg("no fde/fre entries\n");
+		dbg_sec("no FDE/FRE entries\n");
 		return -EINVAL;
 	}
 
 	header_end = sec->sframe_start + SFRAME_HEADER_SIZE(shdr);
 	if (header_end >= sec->sframe_end) {
-		dbg("header doesn't fit in section\n");
+		dbg_sec("header doesn't fit in section\n");
 		return -EINVAL;
 	}
 
@@ -433,7 +436,7 @@ static int sframe_read_header(struct sframe_section *sec)
 	fres_end   = fres_start + shdr.fre_len;
 
 	if (fres_start < fdes_end || fres_end > sec->sframe_end) {
-		dbg("inconsistent fde/fre offsets\n");
+		dbg_sec("inconsistent FDE/FRE offsets\n");
 		return -EINVAL;
 	}
 
@@ -489,6 +492,8 @@ int sframe_add_section(unsigned long sframe_start, unsigned long sframe_end,
 	sec->text_start		= text_start;
 	sec->text_end		= text_end;
 
+	dbg_init(sec);
+
 	ret = sframe_read_header(sec);
 	if (ret) {
 		dbg_print_header(sec);
@@ -498,8 +503,8 @@ int sframe_add_section(unsigned long sframe_start, unsigned long sframe_end,
 	ret = mtree_insert_range(sframe_mt, sec->text_start, sec->text_end - 1,
 				 sec, GFP_KERNEL_ACCOUNT);
 	if (ret) {
-		dbg("mtree_insert_range failed: text=%lx-%lx\n",
-		    sec->text_start, sec->text_end);
+		dbg_sec("mtree_insert_range failed: text=%lx-%lx\n",
+			sec->text_start, sec->text_end);
 		goto err_free;
 	}
 
@@ -521,7 +526,7 @@ static int __sframe_remove_section(struct mm_struct *mm,
 				   struct sframe_section *sec)
 {
 	if (!mtree_erase(&mm->sframe_mt, sec->text_start)) {
-		dbg("mtree_erase failed: text=%lx\n", sec->text_start);
+		dbg_sec("mtree_erase failed: text=%lx\n", sec->text_start);
 		return -EINVAL;
 	}
 
diff --git a/kernel/unwind/sframe_debug.h b/kernel/unwind/sframe_debug.h
index 36352124cde8..a63e75cccc70 100644
--- a/kernel/unwind/sframe_debug.h
+++ b/kernel/unwind/sframe_debug.h
@@ -10,26 +10,59 @@
 #define dbg(fmt, ...)							\
 	pr_debug("%s (%d): " fmt, current->comm, current->pid, ##__VA_ARGS__)
 
+#define dbg_sec(fmt, ...)						\
+	dbg("%s: " fmt, sec->filename, ##__VA_ARGS__)
+
 static __always_inline void dbg_print_header(struct sframe_section *sec)
 {
 	unsigned long fdes_end;
 
 	fdes_end = sec->fdes_start + (sec->num_fdes * sizeof(struct sframe_fde_v3));
 
-	dbg("SEC: sframe:0x%lx-0x%lx text:0x%lx-0x%lx "
-	    "fdes:0x%lx-0x%lx fres:0x%lx-0x%lx "
-	    "ra_off:%d fp_off:%d\n",
-	    sec->sframe_start, sec->sframe_end, sec->text_start, sec->text_end,
-	    sec->fdes_start, fdes_end, sec->fres_start, sec->fres_end,
-	    sec->ra_off, sec->fp_off);
+	dbg_sec("SEC: sframe:0x%lx-0x%lx text:0x%lx-0x%lx "
+		"fdes:0x%lx-0x%lx fres:0x%lx-0x%lx "
+		"ra_off:%d fp_off:%d\n",
+		sec->sframe_start, sec->sframe_end, sec->text_start, sec->text_end,
+		sec->fdes_start, fdes_end, sec->fres_start, sec->fres_end,
+		sec->ra_off, sec->fp_off);
+}
+
+static inline void dbg_init(struct sframe_section *sec)
+{
+	struct mm_struct *mm = current->mm;
+	struct vm_area_struct *vma;
+
+	guard(mmap_read_lock)(mm);
+	vma = vma_lookup(mm, sec->sframe_start);
+	if (!vma)
+		sec->filename = kstrdup("(vma gone???)", GFP_KERNEL_ACCOUNT);
+	else if (vma->vm_file)
+		sec->filename = kstrdup_quotable_file(vma->vm_file, GFP_KERNEL_ACCOUNT);
+	else if (vma->vm_ops && vma->vm_ops->name)
+		sec->filename = kstrdup(vma->vm_ops->name(vma), GFP_KERNEL_ACCOUNT);
+	else if (arch_vma_name(vma))
+		sec->filename = kstrdup(arch_vma_name(vma), GFP_KERNEL_ACCOUNT);
+	else if (!vma->vm_mm)
+		sec->filename = kstrdup("(vdso)", GFP_KERNEL_ACCOUNT);
+	else
+		sec->filename = kstrdup("(anonymous)", GFP_KERNEL_ACCOUNT);
+}
+
+static inline void dbg_free(struct sframe_section *sec)
+{
+	kfree(sec->filename);
 }
 
 #else /* !CONFIG_DYNAMIC_DEBUG */
 
 #define dbg(args...)			no_printk(args)
+#define dbg_sec(args...)		no_printk(args)
 
 static inline void dbg_print_header(struct sframe_section *sec) {}
 
+static inline void dbg_init(struct sframe_section *sec) {}
+static inline void dbg_free(struct sframe_section *sec) {}
+
 #endif /* !CONFIG_DYNAMIC_DEBUG */
 
 #endif /* _SFRAME_DEBUG_H */
-- 
2.51.0

[PATCH v15 12/20] unwind_user/sframe: Add .sframe validation option

[Jens Remus]

From: Josh Poimboeuf <jpoimboe@kernel.org>

Add a debug feature to validate all .sframe sections when first loading
the file rather than on demand.

[ Jens Remus: Add support for SFrame V3.  Add support for PC-relative
FDE function start offset.  Adjust to rename of struct sframe_fre to
sframe_fre_internal.  Use %#x/%#lx format specifiers. ]

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Reviewed-by: Indu Bhagat <ibhagatgnu@gmail.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    Changes in v15:
    - sframe_validate_section(): Fix format specifier for number of FREs
      in debug message. (Sashiko AI)
    - Normalize error code usage (.sframe is removed for all but ENOENT):
      ENOENT: No sframe or no FDE for IP found
              (FDE found but no FRE is EINVAL)
      EFAULT: Bad address
      EINVAL: Invalid input or sframe
    
    Changes in v14:
    - Add debug message if safe_read_fde() fails.
    - Update function names in debug messages.
    - Uppercase terms FDE and FRE in debug messages.
    
    Changes in v13:
    - Update to SFrame V3:
      - Print struct sframe_fde_internal fields fda_off and info2 in debug
        message.
    - Adjust to rename of struct sframe_fde_internal field func_start_addr
      to func_addr.
    - Use format strings "%#x" and "%#lx" instead of "0x%x" and "0x%lx".
    - Reword commit message (my changes).

 arch/Kconfig           |  19 ++++++++
 kernel/unwind/sframe.c | 101 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 120 insertions(+)

diff --git a/arch/Kconfig b/arch/Kconfig
index 37549832bd1f..132249d342a3 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -490,6 +490,25 @@ config HAVE_UNWIND_USER_SFRAME
 	bool
 	select UNWIND_USER
 
+config SFRAME_VALIDATION
+	bool "Enable .sframe section debugging"
+	depends on HAVE_UNWIND_USER_SFRAME
+	depends on DYNAMIC_DEBUG
+	help
+	  When adding an .sframe section for a task, validate the entire
+	  section immediately rather than on demand.
+
+	  This is a debug feature which is helpful for rooting out .sframe
+	  section issues.  If the .sframe section is corrupt, it will fail to
+	  load immediately, with more information provided in dynamic printks.
+
+	  This has a significant page cache footprint due to its reading of the
+	  entire .sframe section for every loaded executable and shared
+	  library.  Also, it's done for all processes, even those which don't
+	  get stack traced by the kernel.  Not recommended for general use.
+
+	  If unsure, say N.
+
 config HAVE_PERF_REGS
 	bool
 	help
diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
index f931932bd34a..b5f984fb2df2 100644
--- a/kernel/unwind/sframe.c
+++ b/kernel/unwind/sframe.c
@@ -382,6 +382,103 @@ int sframe_find(unsigned long ip, struct unwind_user_frame *frame)
 	return ret;
 }
 
+#ifdef CONFIG_SFRAME_VALIDATION
+
+static int safe_read_fde(struct sframe_section *sec,
+			 unsigned int fde_num, struct sframe_fde_internal *fde)
+{
+	int ret;
+
+	if (!user_read_access_begin((void __user *)sec->sframe_start,
+				    sec->sframe_end - sec->sframe_start))
+		return -EFAULT;
+	ret = __read_fde(sec, fde_num, fde);
+	user_read_access_end();
+	return ret;
+}
+
+static int safe_read_fre(struct sframe_section *sec,
+			 struct sframe_fde_internal *fde,
+			 unsigned long fre_addr,
+			 struct sframe_fre_internal *fre)
+{
+	int ret;
+
+	if (!user_read_access_begin((void __user *)sec->sframe_start,
+				    sec->sframe_end - sec->sframe_start))
+		return -EFAULT;
+	ret = __read_fre(sec, fde, fre_addr, fre);
+	user_read_access_end();
+	return ret;
+}
+
+static int sframe_validate_section(struct sframe_section *sec)
+{
+	unsigned long prev_ip = 0;
+	unsigned int i;
+
+	for (i = 0; i < sec->num_fdes; i++) {
+		struct sframe_fre_internal *fre, *prev_fre = NULL;
+		unsigned long ip, fre_addr;
+		struct sframe_fde_internal fde;
+		struct sframe_fre_internal fres[2];
+		bool which = false;
+		unsigned int j;
+		int ret;
+
+		ret = safe_read_fde(sec, i, &fde);
+		if (ret) {
+			dbg_sec("safe_read_fde(%u) failed\n", i);
+			return ret;
+		}
+
+		ip = fde.func_addr;
+		if (ip <= prev_ip) {
+			dbg_sec("FDE %u not sorted\n", i);
+			return -EINVAL;
+		}
+		prev_ip = ip;
+
+		fre_addr = sec->fres_start + fde.fres_off;
+		for (j = 0; j < fde.fres_num; j++) {
+			int ret;
+
+			fre = which ? fres : fres + 1;
+			which = !which;
+
+			ret = safe_read_fre(sec, &fde, fre_addr, fre);
+			if (ret) {
+				dbg_sec("FDE %u: safe_read_fre(%u) failed\n", i, j);
+				dbg_sec("FDE: func_addr:%#lx func_size:%#x fda_off:%#x fres_off:%#x fres_num:%u info:%u info2:%u rep_size:%u\n",
+					fde.func_addr, fde.func_size,
+					fde.fda_off,
+					fde.fres_off, fde.fres_num,
+					fde.info, fde.info2,
+					fde.rep_size);
+				return ret;
+			}
+
+			fre_addr += fre->size;
+
+			if (prev_fre && fre->ip_off <= prev_fre->ip_off) {
+				dbg_sec("FDE %u: FRE %u not sorted\n", i, j);
+				return -EINVAL;
+			}
+
+			prev_fre = fre;
+		}
+	}
+
+	return 0;
+}
+
+#else /*  !CONFIG_SFRAME_VALIDATION */
+
+static int sframe_validate_section(struct sframe_section *sec) { return 0; }
+
+#endif /* !CONFIG_SFRAME_VALIDATION */
+
+
 static void free_section(struct sframe_section *sec)
 {
 	dbg_free(sec);
@@ -500,6 +597,10 @@ int sframe_add_section(unsigned long sframe_start, unsigned long sframe_end,
 		goto err_free;
 	}
 
+	ret = sframe_validate_section(sec);
+	if (ret)
+		goto err_free;
+
 	ret = mtree_insert_range(sframe_mt, sec->text_start, sec->text_end - 1,
 				 sec, GFP_KERNEL_ACCOUNT);
 	if (ret) {
-- 
2.51.0

[PATCH v15 13/20] unwind_user: Enable archs that pass RA in a register

[Jens Remus]

Not all architectures/ABIs pass the return address (RA) on the stack on
function entry, like x86-64 does due to its CALL instruction pushing
the RA onto the stack.  Architectures/ABIs, such as s390, also do not
require the RA to be saved on the stack in the function prologue.  In
particular, the RA may never be saved to the stack at all, such as in
leaf functions.  Unwinding must therefore not assume the presence of a
RA saved on stack for the topmost frame.

Treat a RA offset from CFA of zero as indication that the RA is not
saved (on the stack).  For the topmost frame treat it as indication that
the RA is in the link/RA register, such as on arm64 and s390, and obtain
it from there.  For non-topmost frames treat it as error, as the RA must
be saved.

Additionally allow the SP to be unchanged in the topmost frame, for
architectures where SP at function entry == SP at call site, such as
arm64 and s390.

Note that treating a RA offset from CFA of zero as indication that
the RA is not saved on the stack additionally allows for architectures,
such as s390, where the frame pointer (FP) may be saved without the RA
being saved as well.  Provided that such architectures represent this
in SFrame by encoding the "missing" RA offset using a padding RA offset
with a value of zero.

Reviewed-by: Indu Bhagat <ibhagatgnu@gmail.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    Changes in v15:
    - Define pr_fmt().
    - unwind_user_get_ra_reg(): Use pr_debug_once() instead of
      WARN_ON_ONCE() to prevent user-triggered warning/panic. (Sashiko AI)
    - Reworded commit message. (Indu)

 include/linux/unwind_user.h | 10 ++++++++++
 kernel/unwind/sframe.c      |  6 ++----
 kernel/unwind/user.c        | 20 ++++++++++++++++----
 3 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/include/linux/unwind_user.h b/include/linux/unwind_user.h
index 64618618febd..7bf58f23aa64 100644
--- a/include/linux/unwind_user.h
+++ b/include/linux/unwind_user.h
@@ -23,6 +23,16 @@ static inline bool unwind_user_at_function_start(struct pt_regs *regs)
 #define unwind_user_at_function_start unwind_user_at_function_start
 #endif
 
+#ifndef unwind_user_get_ra_reg
+static inline int unwind_user_get_ra_reg(unsigned long *val)
+{
+	pr_debug_once("%s (%d): unwind_user_get_ra_reg() not implemented\n",
+		      current->comm, current->pid);
+	return -EINVAL;
+}
+#define unwind_user_get_ra_reg unwind_user_get_ra_reg
+#endif
+
 int unwind_user(struct unwind_stacktrace *trace, unsigned int max_entries);
 
 #endif /* _LINUX_UNWIND_USER_H */
diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
index b5f984fb2df2..4af533e9f980 100644
--- a/kernel/unwind/sframe.c
+++ b/kernel/unwind/sframe.c
@@ -255,10 +255,8 @@ static __always_inline int __read_fre(struct sframe_section *sec,
 	dataword_count--;
 
 	ra_off = sec->ra_off;
-	if (!ra_off) {
-		if (!dataword_count--)
-			return -EINVAL;
-
+	if (!ra_off && dataword_count) {
+		dataword_count--;
 		UNSAFE_GET_USER_INC(ra_off, cur, dataword_size, Efault);
 	}
 
diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c
index fdb1001e3750..afa7c6f6d9b4 100644
--- a/kernel/unwind/user.c
+++ b/kernel/unwind/user.c
@@ -2,6 +2,9 @@
 /*
 * Generic interfaces for unwinding user space
 */
+
+#define pr_fmt(fmt)	"unwind_user: " fmt
+
 #include <linux/kernel.h>
 #include <linux/sched.h>
 #include <linux/sched/task_stack.h>
@@ -48,8 +51,12 @@ static int unwind_user_next_common(struct unwind_user_state *state,
 	}
 	cfa += frame->cfa_off;
 
-	/* Make sure that stack is not going in wrong direction */
-	if (cfa <= state->sp)
+	/*
+	 * Make sure that stack is not going in wrong direction.  Allow SP
+	 * to be unchanged for the topmost frame, by subtracting topmost,
+	 * which is either 0 or 1.
+	 */
+	if (cfa <= state->sp - state->topmost)
 		return -EINVAL;
 
 	/* Make sure that the address is word aligned */
@@ -57,8 +64,13 @@ static int unwind_user_next_common(struct unwind_user_state *state,
 		return -EINVAL;
 
 	/* Get the Return Address (RA) */
-	if (get_user_word(&ra, cfa, frame->ra_off, state->ws))
-		return -EINVAL;
+	if (frame->ra_off) {
+		if (get_user_word(&ra, cfa, frame->ra_off, state->ws))
+			return -EINVAL;
+	} else {
+		if (!state->topmost || unwind_user_get_ra_reg(&ra))
+			return -EINVAL;
+	}
 
 	/* Get the Frame Pointer (FP) */
 	if (frame->fp_off && get_user_word(&fp, cfa, frame->fp_off, state->ws))
-- 
2.51.0

[PATCH v15 14/20] unwind_user: Flexible FP/RA recovery rules

[Jens Remus]

To enable support for SFrame V3 flexible FDEs with a subsequent patch,
add support for the following flexible frame pointer (FP) and return
address (RA) recovery rules:

  FP/RA = *(CFA + offset)
  FP/RA = register + offset
  FP/RA = *(register + offset)

Note that FP/RA recovery rules that use arbitrary register contents are
only valid when in the topmost frame, as their contents are otherwise
unknown.

This also enables unwinding of user space for architectures, such as
s390, that may save the frame pointer (FP) and/or return address (RA) in
other registers, for instance when in a leaf function.

Reviewed-by: Indu Bhagat <ibhagatgnu@gmail.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    Changes in v15:
    - Define dbg_once().
    - unwind_user_get_reg(): Use pr_debug_once() instead of WARN_ON_ONCE()
      to prevent user-triggered warning/panic. (Sashiko AI)
    - unwind_user_next_common(): Handle UNWIND_USER_RULE_CFA_OFFSET for RA
      and FP to use dbg_once() instead of WARN_ON_ONCE() to prevent user-
      triggered warning/panic. (Sashiko AI)
    
    Changes in v14:
    - Improve comment on why UNWIND_USER_RULE_CFA_OFFSET is not implemented.
      (Mark Rutland)

 arch/x86/include/asm/unwind_user.h | 21 +++++++--
 include/linux/unwind_user.h        | 10 +++++
 include/linux/unwind_user_types.h  | 23 +++++++++-
 kernel/unwind/sframe.c             | 16 ++++++-
 kernel/unwind/user.c               | 70 +++++++++++++++++++++++++++---
 5 files changed, 125 insertions(+), 15 deletions(-)

diff --git a/arch/x86/include/asm/unwind_user.h b/arch/x86/include/asm/unwind_user.h
index 2dfb5ef11e36..9c3417be4283 100644
--- a/arch/x86/include/asm/unwind_user.h
+++ b/arch/x86/include/asm/unwind_user.h
@@ -21,15 +21,26 @@ static inline int unwind_user_word_size(struct pt_regs *regs)
 
 #define ARCH_INIT_USER_FP_FRAME(ws)			\
 	.cfa_off	=  2*(ws),			\
-	.ra_off		= -1*(ws),			\
-	.fp_off		= -2*(ws),			\
+	.ra		= {				\
+		.rule		= UNWIND_USER_RULE_CFA_OFFSET_DEREF,\
+		.offset		= -1*(ws),		\
+			},				\
+	.fp		= {				\
+		.rule		= UNWIND_USER_RULE_CFA_OFFSET_DEREF,\
+		.offset		= -2*(ws),		\
+			},				\
 	.use_fp		= true,				\
 	.outermost	= false,
 
 #define ARCH_INIT_USER_FP_ENTRY_FRAME(ws)		\
 	.cfa_off	=  1*(ws),			\
-	.ra_off		= -1*(ws),			\
-	.fp_off		= 0,				\
+	.ra		= {				\
+		.rule		= UNWIND_USER_RULE_CFA_OFFSET_DEREF,\
+		.offset		= -1*(ws),		\
+			},				\
+	.fp		= {				\
+		.rule		= UNWIND_USER_RULE_RETAIN,\
+			},				\
 	.use_fp		= false,			\
 	.outermost	= false,
 
@@ -41,4 +52,6 @@ static inline bool unwind_user_at_function_start(struct pt_regs *regs)
 
 #endif /* CONFIG_HAVE_UNWIND_USER_FP */
 
+#include <asm-generic/unwind_user.h>
+
 #endif /* _ASM_X86_UNWIND_USER_H */
diff --git a/include/linux/unwind_user.h b/include/linux/unwind_user.h
index 7bf58f23aa64..6aca38f89ddd 100644
--- a/include/linux/unwind_user.h
+++ b/include/linux/unwind_user.h
@@ -33,6 +33,16 @@ static inline int unwind_user_get_ra_reg(unsigned long *val)
 #define unwind_user_get_ra_reg unwind_user_get_ra_reg
 #endif
 
+#ifndef unwind_user_get_reg
+static inline int unwind_user_get_reg(unsigned long *val, unsigned int regnum)
+{
+	pr_debug_once("%s (%d): unwind_user_get_reg(%u) not implemented\n",
+		      current->comm, current->pid, regnum);
+	return -EINVAL;
+}
+#define unwind_user_get_reg unwind_user_get_reg
+#endif
+
 int unwind_user(struct unwind_stacktrace *trace, unsigned int max_entries);
 
 #endif /* _LINUX_UNWIND_USER_H */
diff --git a/include/linux/unwind_user_types.h b/include/linux/unwind_user_types.h
index 616cc5ee4586..0d02714a1b5d 100644
--- a/include/linux/unwind_user_types.h
+++ b/include/linux/unwind_user_types.h
@@ -27,10 +27,29 @@ struct unwind_stacktrace {
 	unsigned long	*entries;
 };
 
+#define UNWIND_USER_RULE_DEREF			BIT(31)
+
+enum unwind_user_rule {
+	UNWIND_USER_RULE_RETAIN,		/* entity = entity */
+	UNWIND_USER_RULE_CFA_OFFSET,		/* entity = CFA + offset */
+	UNWIND_USER_RULE_REG_OFFSET,		/* entity = register + offset */
+	/* DEREF variants */
+	UNWIND_USER_RULE_CFA_OFFSET_DEREF =	/* entity = *(CFA + offset) */
+		UNWIND_USER_RULE_CFA_OFFSET | UNWIND_USER_RULE_DEREF,
+	UNWIND_USER_RULE_REG_OFFSET_DEREF =	/* entity = *(register + offset) */
+		UNWIND_USER_RULE_REG_OFFSET | UNWIND_USER_RULE_DEREF,
+};
+
+struct unwind_user_rule_data {
+	enum unwind_user_rule rule;
+	s32 offset;
+	unsigned int regnum;
+};
+
 struct unwind_user_frame {
 	s32 cfa_off;
-	s32 ra_off;
-	s32 fp_off;
+	struct unwind_user_rule_data ra;
+	struct unwind_user_rule_data fp;
 	bool use_fp;
 	bool outermost;
 };
diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
index 4af533e9f980..e82d1dcdd471 100644
--- a/kernel/unwind/sframe.c
+++ b/kernel/unwind/sframe.c
@@ -283,6 +283,18 @@ static __always_inline int __read_fre(struct sframe_section *sec,
 	return -EFAULT;
 }
 
+static __always_inline void
+sframe_init_rule_data(struct unwind_user_rule_data *rule_data,
+		      s32 offset)
+{
+	if (offset) {
+		rule_data->rule = UNWIND_USER_RULE_CFA_OFFSET_DEREF;
+		rule_data->offset = offset;
+	} else {
+		rule_data->rule = UNWIND_USER_RULE_RETAIN;
+	}
+}
+
 static __always_inline int __find_fre(struct sframe_section *sec,
 				      struct sframe_fde_internal *fde,
 				      unsigned long ip,
@@ -333,8 +345,8 @@ static __always_inline int __find_fre(struct sframe_section *sec,
 	fre = prev_fre;
 
 	frame->cfa_off = fre->cfa_off;
-	frame->ra_off  = fre->ra_off;
-	frame->fp_off  = fre->fp_off;
+	sframe_init_rule_data(&frame->ra, fre->ra_off);
+	sframe_init_rule_data(&frame->fp, fre->fp_off);
 	frame->use_fp  = SFRAME_V3_FRE_CFA_BASE_REG_ID(fre->info) == SFRAME_BASE_REG_FP;
 	frame->outermost = SFRAME_V3_FRE_RA_UNDEFINED_P(fre->info);
 
diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c
index afa7c6f6d9b4..c6a2abac78e0 100644
--- a/kernel/unwind/user.c
+++ b/kernel/unwind/user.c
@@ -12,6 +12,17 @@
 #include <linux/uaccess.h>
 #include <linux/sframe.h>
 
+#ifdef CONFIG_DYNAMIC_DEBUG
+
+#define dbg_once(fmt, ...)							\
+	pr_debug_once("%s (%d): " fmt, current->comm, current->pid, ##__VA_ARGS__)
+
+#else /* !CONFIG_DYNAMIC_DEBUG */
+
+#define dbg_once(args...)		no_printk(args)
+
+#endif /* !CONFIG_DYNAMIC_DEBUG */
+
 #define for_each_user_frame(state) \
 	for (unwind_user_start(state); !(state)->done; unwind_user_next(state))
 
@@ -64,22 +75,67 @@ static int unwind_user_next_common(struct unwind_user_state *state,
 		return -EINVAL;
 
 	/* Get the Return Address (RA) */
-	if (frame->ra_off) {
-		if (get_user_word(&ra, cfa, frame->ra_off, state->ws))
-			return -EINVAL;
-	} else {
+	switch (frame->ra.rule) {
+	case UNWIND_USER_RULE_RETAIN:
 		if (!state->topmost || unwind_user_get_ra_reg(&ra))
 			return -EINVAL;
+		break;
+	case UNWIND_USER_RULE_CFA_OFFSET:
+		/*
+		 * RA = CFA + offset does not make sense.
+		 * A return address cannot legitimately be a stack address.
+		 */
+		dbg_once("UNWIND_USER_RULE_CFA_OFFSET invalid for RA\n");
+		return -EINVAL;
+	case UNWIND_USER_RULE_CFA_OFFSET_DEREF:
+		ra = cfa + frame->ra.offset;
+		break;
+	case UNWIND_USER_RULE_REG_OFFSET:
+	case UNWIND_USER_RULE_REG_OFFSET_DEREF:
+		if (!state->topmost || unwind_user_get_reg(&ra, frame->ra.regnum))
+			return -EINVAL;
+		ra += frame->ra.offset;
+		break;
+	default:
+		WARN_ON_ONCE(1);
+		return -EINVAL;
 	}
+	if (frame->ra.rule & UNWIND_USER_RULE_DEREF &&
+	    get_user_word(&ra, ra, 0, state->ws))
+		return -EINVAL;
 
 	/* Get the Frame Pointer (FP) */
-	if (frame->fp_off && get_user_word(&fp, cfa, frame->fp_off, state->ws))
+	switch (frame->fp.rule) {
+	case UNWIND_USER_RULE_RETAIN:
+		fp = state->fp;
+		break;
+	case UNWIND_USER_RULE_CFA_OFFSET:
+		/*
+		 * FP = CFA + offset is currently not used for FP
+		 * (e.g. SFrame cannot represent this rule).
+		 */
+		dbg_once("UNWIND_USER_RULE_CFA_OFFSET unsupported for FP\n");
+		return -EINVAL;
+	case UNWIND_USER_RULE_CFA_OFFSET_DEREF:
+		fp = cfa + frame->fp.offset;
+		break;
+	case UNWIND_USER_RULE_REG_OFFSET:
+	case UNWIND_USER_RULE_REG_OFFSET_DEREF:
+		if (!state->topmost || unwind_user_get_reg(&fp, frame->fp.regnum))
+			return -EINVAL;
+		fp += frame->fp.offset;
+		break;
+	default:
+		WARN_ON_ONCE(1);
+		return -EINVAL;
+	}
+	if (frame->fp.rule & UNWIND_USER_RULE_DEREF &&
+	    get_user_word(&fp, fp, 0, state->ws))
 		return -EINVAL;
 
 	state->ip = ra;
 	state->sp = cfa;
-	if (frame->fp_off)
-		state->fp = fp;
+	state->fp = fp;
 	state->topmost = false;
 	return 0;
 }
-- 
2.51.0

[PATCH v15 15/20] unwind_user: Flexible CFA recovery rules

[Jens Remus]

To enable support for SFrame V3 flexible FDEs with a subsequent patch,
add support for the following flexible Canonical Frame Address (CFA)
recovery rules:

  CFA = SP + offset
  CFA = *(SP + offset)
  CFA = FP + offset
  CFA = *(FP + offset)
  CFA = register + offset
  CFA = *(register + offset)

Note that CFA recovery rules that use arbitrary register contents are
only valid when in the topmost frame, as their contents are otherwise
unknown.

Reviewed-by: Indu Bhagat <ibhagatgnu@gmail.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    Changes in v15:
    - enum unwind_user_cfa_rule, unwind_user_next_common(): Add support for
      SP/FP-based CFA recovery rules with dereferencing. (Sashiko AI)

 arch/x86/include/asm/unwind_user.h | 12 ++++++++----
 include/linux/unwind_user_types.h  | 22 ++++++++++++++++++++--
 kernel/unwind/sframe.c             | 15 +++++++++++++--
 kernel/unwind/user.c               | 24 ++++++++++++++++++++----
 4 files changed, 61 insertions(+), 12 deletions(-)

diff --git a/arch/x86/include/asm/unwind_user.h b/arch/x86/include/asm/unwind_user.h
index 9c3417be4283..f38f7c5ff1de 100644
--- a/arch/x86/include/asm/unwind_user.h
+++ b/arch/x86/include/asm/unwind_user.h
@@ -20,7 +20,10 @@ static inline int unwind_user_word_size(struct pt_regs *regs)
 #ifdef CONFIG_HAVE_UNWIND_USER_FP
 
 #define ARCH_INIT_USER_FP_FRAME(ws)			\
-	.cfa_off	=  2*(ws),			\
+	.cfa		= {				\
+		.rule		= UNWIND_USER_CFA_RULE_FP_OFFSET,\
+		.offset		=  2*(ws),		\
+			},				\
 	.ra		= {				\
 		.rule		= UNWIND_USER_RULE_CFA_OFFSET_DEREF,\
 		.offset		= -1*(ws),		\
@@ -29,11 +32,13 @@ static inline int unwind_user_word_size(struct pt_regs *regs)
 		.rule		= UNWIND_USER_RULE_CFA_OFFSET_DEREF,\
 		.offset		= -2*(ws),		\
 			},				\
-	.use_fp		= true,				\
 	.outermost	= false,
 
 #define ARCH_INIT_USER_FP_ENTRY_FRAME(ws)		\
-	.cfa_off	=  1*(ws),			\
+	.cfa		= {				\
+		.rule		= UNWIND_USER_CFA_RULE_SP_OFFSET,\
+		.offset		=  1*(ws),		\
+			},				\
 	.ra		= {				\
 		.rule		= UNWIND_USER_RULE_CFA_OFFSET_DEREF,\
 		.offset		= -1*(ws),		\
@@ -41,7 +46,6 @@ static inline int unwind_user_word_size(struct pt_regs *regs)
 	.fp		= {				\
 		.rule		= UNWIND_USER_RULE_RETAIN,\
 			},				\
-	.use_fp		= false,			\
 	.outermost	= false,
 
 static inline bool unwind_user_at_function_start(struct pt_regs *regs)
diff --git a/include/linux/unwind_user_types.h b/include/linux/unwind_user_types.h
index 0d02714a1b5d..c18be5b7d586 100644
--- a/include/linux/unwind_user_types.h
+++ b/include/linux/unwind_user_types.h
@@ -29,6 +29,25 @@ struct unwind_stacktrace {
 
 #define UNWIND_USER_RULE_DEREF			BIT(31)
 
+enum unwind_user_cfa_rule {
+	UNWIND_USER_CFA_RULE_SP_OFFSET,		/* CFA = SP + offset */
+	UNWIND_USER_CFA_RULE_FP_OFFSET,		/* CFA = FP + offset */
+	UNWIND_USER_CFA_RULE_REG_OFFSET,	/* CFA = register + offset */
+	/* DEREF variants */
+	UNWIND_USER_CFA_RULE_SP_OFFSET_DEREF =	/* CFA = *(SP + offset) */
+		UNWIND_USER_CFA_RULE_SP_OFFSET | UNWIND_USER_RULE_DEREF,
+	UNWIND_USER_CFA_RULE_FP_OFFSET_DEREF =	/* CFA = *(FP + offset) */
+		UNWIND_USER_CFA_RULE_FP_OFFSET | UNWIND_USER_RULE_DEREF,
+	UNWIND_USER_CFA_RULE_REG_OFFSET_DEREF =	/* CFA = *(register + offset) */
+		UNWIND_USER_CFA_RULE_REG_OFFSET | UNWIND_USER_RULE_DEREF,
+};
+
+struct unwind_user_cfa_rule_data {
+	enum unwind_user_cfa_rule rule;
+	s32 offset;
+	unsigned int regnum;
+};
+
 enum unwind_user_rule {
 	UNWIND_USER_RULE_RETAIN,		/* entity = entity */
 	UNWIND_USER_RULE_CFA_OFFSET,		/* entity = CFA + offset */
@@ -47,10 +66,9 @@ struct unwind_user_rule_data {
 };
 
 struct unwind_user_frame {
-	s32 cfa_off;
+	struct unwind_user_cfa_rule_data cfa;
 	struct unwind_user_rule_data ra;
 	struct unwind_user_rule_data fp;
-	bool use_fp;
 	bool outermost;
 };
 
diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
index e82d1dcdd471..6187379750db 100644
--- a/kernel/unwind/sframe.c
+++ b/kernel/unwind/sframe.c
@@ -283,6 +283,18 @@ static __always_inline int __read_fre(struct sframe_section *sec,
 	return -EFAULT;
 }
 
+static __always_inline void
+sframe_init_cfa_rule_data(struct unwind_user_cfa_rule_data *cfa_rule_data,
+			  unsigned char fre_info,
+			  s32 offset)
+{
+	if (SFRAME_V3_FRE_CFA_BASE_REG_ID(fre_info) == SFRAME_BASE_REG_FP)
+		cfa_rule_data->rule = UNWIND_USER_CFA_RULE_FP_OFFSET;
+	else
+		cfa_rule_data->rule = UNWIND_USER_CFA_RULE_SP_OFFSET;
+	cfa_rule_data->offset = offset;
+}
+
 static __always_inline void
 sframe_init_rule_data(struct unwind_user_rule_data *rule_data,
 		      s32 offset)
@@ -344,10 +356,9 @@ static __always_inline int __find_fre(struct sframe_section *sec,
 		return -EINVAL;
 	fre = prev_fre;
 
-	frame->cfa_off = fre->cfa_off;
+	sframe_init_cfa_rule_data(&frame->cfa, fre->info, fre->cfa_off);
 	sframe_init_rule_data(&frame->ra, fre->ra_off);
 	sframe_init_rule_data(&frame->fp, fre->fp_off);
-	frame->use_fp  = SFRAME_V3_FRE_CFA_BASE_REG_ID(fre->info) == SFRAME_BASE_REG_FP;
 	frame->outermost = SFRAME_V3_FRE_RA_UNDEFINED_P(fre->info);
 
 	return 0;
diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c
index c6a2abac78e0..447061b10613 100644
--- a/kernel/unwind/user.c
+++ b/kernel/unwind/user.c
@@ -53,14 +53,30 @@ static int unwind_user_next_common(struct unwind_user_state *state,
 	}
 
 	/* Get the Canonical Frame Address (CFA) */
-	if (frame->use_fp) {
+	switch (frame->cfa.rule) {
+	case UNWIND_USER_CFA_RULE_SP_OFFSET:
+	case UNWIND_USER_CFA_RULE_SP_OFFSET_DEREF:
+		cfa = state->sp;
+		break;
+	case UNWIND_USER_CFA_RULE_FP_OFFSET:
+	case UNWIND_USER_CFA_RULE_FP_OFFSET_DEREF:
 		if (state->fp < state->sp)
 			return -EINVAL;
 		cfa = state->fp;
-	} else {
-		cfa = state->sp;
+		break;
+	case UNWIND_USER_CFA_RULE_REG_OFFSET:
+	case UNWIND_USER_CFA_RULE_REG_OFFSET_DEREF:
+		if (!state->topmost || unwind_user_get_reg(&cfa, frame->cfa.regnum))
+			return -EINVAL;
+		break;
+	default:
+		WARN_ON_ONCE(1);
+		return -EINVAL;
 	}
-	cfa += frame->cfa_off;
+	cfa += frame->cfa.offset;
+	if (frame->cfa.rule & UNWIND_USER_RULE_DEREF &&
+	    get_user_word(&cfa, cfa, 0, state->ws))
+		return -EINVAL;
 
 	/*
 	 * Make sure that stack is not going in wrong direction.  Allow SP
-- 
2.51.0

[PATCH v15 16/20] unwind_user/sframe: Add support for SFrame V3 flexible FDEs

[Jens Remus]

SFrame V3 introduces flexible FDEs in addition to the regular FDEs.
The key difference is that flexible FDEs encode the CFA, RA, and FP
tracking information using two FRE data words, a control word and an
offset, or a single padding data word of zero (e.g. to represent FP
without RA tracking information).

The control word contains the following information:
- reg_p: Whether to use the register contents (reg_p=1) specified
  by regnum or the CFA (reg_p=0) as base.
- deref_p: Whether to dereference.
- regnum: A DWARF register number.

The offset is added to the base (i.e. CFA or register contents).  Then
the resulting address may optionally be dereferenced.

This enables the following flexible CFA and FP/RA recovery rules:
- CFA = register + offset	// reg_p=1, deref_p=0
- CFA = *(register + offset)	// reg_p=1, deref_p=1
- FP/RA = *(CFA + offset)	// reg_p=0, deref_p=0
- FP/RA = register + offset	// reg_p=1, deref_p=0
- FP/RA = *(register + offset)	// reg_p=1, deref_p=1

Note that for the CFA a rule with reg_p=0 is invalid, as the value of
the CFA cannot be described using itself as base.  For FP/RA a rule with
reg_p=0 and deref_p=0 and regnum=0 is invalid, as it that is equal to
the padding data word of zero.

Reviewed-by: Indu Bhagat <ibhagatgnu@gmail.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    Changes in v15:
    - __read_flex_fde_fre_datawords(): Add comment on FRE dataword RA/FP
      location info decoding logic. (Sashiko AI)
    - Fix outermost frame (FRE without datawords) handling to not cause
      sframe_init_cfa_rule_data() and ultimately sframe_find() to fail
      with -EINVAL. (Sashiko AI)
    - sframe_init_[cfa_]rule_data(): Reject FRE control word with
      reserved_p=1. (Sashiko AI)
    - __find_fre(): Return RC of sframe_init_[cfa_]rule_data() if bad RC.
    - Normalize error code usage (.sframe is removed for all but ENOENT):
      ENOENT: No sframe or no FDE for IP found
              (FDE found but no FRE is EINVAL)
      EFAULT: Bad address
      EINVAL: Invalid input or sframe
    
    Changes in v14:
    - Rename __read_regular_fre_datawords() to
      __read_default_fre_datawords() to align to SFrame V3 specification
      (default FRE).
    - Rename SFRAME_FDE_TYPE_FLEXIBLE to SFRAME_FDE_TYPE_FLEX to match
      SFrame V3 specification and adjust to rename of SFRAME_FDE_TYPE_*.
    - Rename SFRAME_V3_FLEX_FDE_CTLWORD_*() to
      SFRAME_V3_FLEX_FDE_CTRLWORD_*() to match SFrame V3 reference
      implementation.
    - Add arch/*/include/asm/unwind_user_sframe.h to MAINTAINERS.

 MAINTAINERS            |   1 +
 kernel/unwind/sframe.c | 282 +++++++++++++++++++++++++++++++++--------
 kernel/unwind/sframe.h |   6 +
 3 files changed, 237 insertions(+), 52 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index a9b42b67a88d..25f0b933511c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27875,6 +27875,7 @@ M:	Josh Poimboeuf <jpoimboe@kernel.org>
 M:	Steven Rostedt <rostedt@goodmis.org>
 S:	Maintained
 F:	arch/*/include/asm/unwind_user.h
+F:	arch/*/include/asm/unwind_user_sframe.h
 F:	include/asm-generic/unwind_user.h
 F:	include/linux/sframe.h
 F:	include/linux/unwind*.h
diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
index 6187379750db..48709f0bafc7 100644
--- a/kernel/unwind/sframe.c
+++ b/kernel/unwind/sframe.c
@@ -12,6 +12,7 @@
 #include <linux/mm.h>
 #include <linux/string_helpers.h>
 #include <linux/sframe.h>
+#include <asm/unwind_user_sframe.h>
 #include <linux/unwind_user_types.h>
 
 #include "sframe.h"
@@ -31,8 +32,11 @@ struct sframe_fde_internal {
 struct sframe_fre_internal {
 	unsigned int	size;
 	u32		ip_off;
+	u32		cfa_ctl;
 	s32		cfa_off;
+	u32		ra_ctl;
 	s32		ra_off;
+	u32		fp_ctl;
 	s32		fp_off;
 	u8		info;
 };
@@ -200,16 +204,158 @@ static __always_inline int __find_fde(struct sframe_section *sec,
 		 s32 :	UNSAFE_GET_USER_SIGNED_INC(to, from, size, label),	\
 		 s64 :	UNSAFE_GET_USER_SIGNED_INC(to, from, size, label))
 
+static __always_inline int
+__read_default_fre_datawords(struct sframe_section *sec,
+			     struct sframe_fde_internal *fde,
+			     unsigned long cur,
+			     unsigned char dataword_count,
+			     unsigned char dataword_size,
+			     struct sframe_fre_internal *fre)
+{
+	s32 cfa_off, ra_off, fp_off;
+	unsigned int cfa_regnum;
+
+	UNSAFE_GET_USER_INC(cfa_off, cur, dataword_size, Efault);
+	dataword_count--;
+
+	ra_off = sec->ra_off;
+	if (!ra_off && dataword_count) {
+		dataword_count--;
+		UNSAFE_GET_USER_INC(ra_off, cur, dataword_size, Efault);
+	}
+
+	fp_off = sec->fp_off;
+	if (!fp_off && dataword_count) {
+		dataword_count--;
+		UNSAFE_GET_USER_INC(fp_off, cur, dataword_size, Efault);
+	}
+
+	if (dataword_count)
+		return -EINVAL;
+
+	cfa_regnum =
+		(SFRAME_V3_FRE_CFA_BASE_REG_ID(fre->info) == SFRAME_BASE_REG_FP) ?
+			SFRAME_REG_FP : SFRAME_REG_SP;
+
+	fre->cfa_ctl	= (cfa_regnum << 3) | 1; /* regnum, deref_p=0, reg_p=1 */
+	fre->cfa_off	= cfa_off;
+	fre->ra_ctl	= ra_off ? 2 : 0; /* regnum=0, deref_p=(ra_off != 0), reg_p=0 */
+	fre->ra_off	= ra_off;
+	fre->fp_ctl	= fp_off ? 2 : 0; /* regnum=0, deref_p=(fp_off != 0), reg_p=0 */
+	fre->fp_off	= fp_off;
+
+	return 0;
+
+Efault:
+	return -EFAULT;
+}
+
+static __always_inline int
+__read_flex_fde_fre_datawords(struct sframe_section *sec,
+			      struct sframe_fde_internal *fde,
+			      unsigned long cur,
+			      unsigned char dataword_count,
+			      unsigned char dataword_size,
+			      struct sframe_fre_internal *fre)
+{
+	u32 cfa_ctl, ra_ctl, fp_ctl;
+	s32 cfa_off, ra_off, fp_off;
+
+	if (dataword_count < 2)
+		return -EINVAL;
+	UNSAFE_GET_USER_INC(cfa_ctl, cur, dataword_size, Efault);
+	UNSAFE_GET_USER_INC(cfa_off, cur, dataword_size, Efault);
+	dataword_count -= 2;
+
+	/*
+	 * Each RA/FP location info consumes either two datawords
+	 * (control word + offset) or one padding word substituting
+	 * for that pair.  Padding is only valid as substitution if
+	 * followed by further non-padding location info.  Therefore
+	 * decoding only proceeds with at least two datawords.  Any
+	 * leftover trailing datawords are invalid and rejected by
+	 * the final check.
+	 */
+
+	ra_off = sec->ra_off;
+	ra_ctl = ra_off ? 2 : 0; /* regnum=0, deref_p=(ra_off != 0), reg_p=0 */
+	if (dataword_count >= 2) {
+		UNSAFE_GET_USER_INC(ra_ctl, cur, dataword_size, Efault);
+		dataword_count--;
+		if (ra_ctl) {
+			UNSAFE_GET_USER_INC(ra_off, cur, dataword_size, Efault);
+			dataword_count--;
+		} else {
+			/* Padding RA location info */
+			ra_ctl = ra_off ? 2 : 0; /* re-deduce (see above) */
+		}
+	}
+
+	fp_off = sec->fp_off;
+	fp_ctl = fp_off ? 2 : 0; /* regnum=0, deref_p=(fp_off != 0), reg_p=0 */
+	if (dataword_count >= 2) {
+		UNSAFE_GET_USER_INC(fp_ctl, cur, dataword_size, Efault);
+		dataword_count--;
+		if (fp_ctl) {
+			UNSAFE_GET_USER_INC(fp_off, cur, dataword_size, Efault);
+			dataword_count--;
+		} else {
+			/* Padding FP location info */
+			fp_ctl = fp_off ? 2 : 0; /* re-deduce (see above) */
+		}
+	}
+
+	/* Reject trailing padding or unknown extra datawords */
+	if (dataword_count)
+		return -EINVAL;
+
+	fre->cfa_ctl	= cfa_ctl;
+	fre->cfa_off	= cfa_off;
+	fre->ra_ctl	= ra_ctl;
+	fre->ra_off	= ra_off;
+	fre->fp_ctl	= fp_ctl;
+	fre->fp_off	= fp_off;
+
+	return 0;
+
+Efault:
+	return -EFAULT;
+}
+
+static __always_inline int
+__read_fre_datawords(struct sframe_section *sec,
+		     struct sframe_fde_internal *fde,
+		     unsigned long cur,
+		     unsigned char dataword_count,
+		     unsigned char dataword_size,
+		     struct sframe_fre_internal *fre)
+{
+	unsigned char fde_type = SFRAME_V3_FDE_TYPE(fde->info2);
+
+	switch (fde_type) {
+	case SFRAME_FDE_TYPE_DEFAULT:
+		return __read_default_fre_datawords(sec, fde, cur,
+						    dataword_count,
+						    dataword_size,
+						    fre);
+	case SFRAME_FDE_TYPE_FLEX:
+		return __read_flex_fde_fre_datawords(sec, fde, cur,
+						     dataword_count,
+						     dataword_size,
+						     fre);
+	default:
+		return -EINVAL;
+	}
+}
+
 static __always_inline int __read_fre(struct sframe_section *sec,
 				      struct sframe_fde_internal *fde,
 				      unsigned long fre_addr,
 				      struct sframe_fre_internal *fre)
 {
-	unsigned char fde_type = SFRAME_V3_FDE_TYPE(fde->info2);
 	unsigned char fde_pctype = SFRAME_V3_FDE_PCTYPE(fde->info);
 	unsigned char fre_type = SFRAME_V3_FDE_FRE_TYPE(fde->info);
 	unsigned char dataword_count, dataword_size;
-	s32 cfa_off, ra_off, fp_off;
 	unsigned long cur = fre_addr;
 	unsigned char addr_size;
 	u32 ip_off;
@@ -236,75 +382,101 @@ static __always_inline int __read_fre(struct sframe_section *sec,
 	if (cur + (dataword_count * dataword_size) > sec->fres_end)
 		return -EFAULT;
 
-	/* TODO: Support for flexible FDEs not implemented yet. */
-	if (fde_type != SFRAME_FDE_TYPE_DEFAULT)
-		return -EINVAL;
+	fre->size	= addr_size + 1 + (dataword_count * dataword_size);
+	fre->ip_off	= ip_off;
+	fre->info	= info;
 
 	if (!dataword_count) {
 		/*
-		 * A FRE without data words indicates RA undefined /
-		 * outermost frame.
+		 * A FRE without datawords indicates an outermost
+		 * frame.  Zero-initialize CFA, RA, and FP location
+		 * info, except for the CFA control word, so that
+		 * neither sframe_init_cfa_rule_data() nor
+		 * sframe_init_rule_data() fail.
 		 */
-		cfa_off	= 0;
-		ra_off	= 0;
-		fp_off	= 0;
-		goto done;
-	}
-
-	UNSAFE_GET_USER_INC(cfa_off, cur, dataword_size, Efault);
-	dataword_count--;
-
-	ra_off = sec->ra_off;
-	if (!ra_off && dataword_count) {
-		dataword_count--;
-		UNSAFE_GET_USER_INC(ra_off, cur, dataword_size, Efault);
-	}
+		fre->cfa_ctl	= (SFRAME_REG_SP << 3) | 1; /* regnum=SP, deref_p=0, reg_p=1 */
+		fre->cfa_off	= 0;
+		fre->ra_ctl	= 0;
+		fre->ra_off	= 0;
+		fre->fp_ctl	= 0;
+		fre->fp_off	= 0;
 
-	fp_off = sec->fp_off;
-	if (!fp_off && dataword_count) {
-		dataword_count--;
-		UNSAFE_GET_USER_INC(fp_off, cur, dataword_size, Efault);
+		return 0;
 	}
 
-	if (dataword_count)
-		return -EINVAL;
-
-done:
-	fre->size	= addr_size + 1 + (dataword_count * dataword_size);
-	fre->ip_off	= ip_off;
-	fre->cfa_off	= cfa_off;
-	fre->ra_off	= ra_off;
-	fre->fp_off	= fp_off;
-	fre->info	= info;
-
-	return 0;
+	return __read_fre_datawords(sec, fde, cur, dataword_count, dataword_size, fre);
 
 Efault:
 	return -EFAULT;
 }
 
-static __always_inline void
+static __always_inline int
 sframe_init_cfa_rule_data(struct unwind_user_cfa_rule_data *cfa_rule_data,
-			  unsigned char fre_info,
-			  s32 offset)
+			  u32 ctlword, s32 offset)
 {
-	if (SFRAME_V3_FRE_CFA_BASE_REG_ID(fre_info) == SFRAME_BASE_REG_FP)
-		cfa_rule_data->rule = UNWIND_USER_CFA_RULE_FP_OFFSET;
-	else
+	bool deref_p = SFRAME_V3_FLEX_FDE_CTRLWORD_DEREF_P(ctlword);
+	bool reg_p = SFRAME_V3_FLEX_FDE_CTRLWORD_REG_P(ctlword);
+	bool reserved_p = SFRAME_V3_FLEX_FDE_CTRLWORD_RESERVED_P(ctlword);
+	unsigned int regnum = SFRAME_V3_FLEX_FDE_CTRLWORD_REGNUM(ctlword);
+
+	if (reserved_p)
+		return -EINVAL;
+
+	/* CFA recovery rule must be register-based */
+	if (!reg_p)
+		return -EINVAL;
+
+	switch (regnum) {
+	case SFRAME_REG_SP:
 		cfa_rule_data->rule = UNWIND_USER_CFA_RULE_SP_OFFSET;
+		break;
+	case SFRAME_REG_FP:
+		cfa_rule_data->rule = UNWIND_USER_CFA_RULE_FP_OFFSET;
+		break;
+	default:
+		cfa_rule_data->rule = UNWIND_USER_CFA_RULE_REG_OFFSET;
+		cfa_rule_data->regnum = regnum;
+	}
+
+	if (deref_p)
+		cfa_rule_data->rule |= UNWIND_USER_RULE_DEREF;
+
 	cfa_rule_data->offset = offset;
+
+	return 0;
 }
 
-static __always_inline void
+static __always_inline int
 sframe_init_rule_data(struct unwind_user_rule_data *rule_data,
-		      s32 offset)
+		      u32 ctlword, s32 offset)
 {
-	if (offset) {
-		rule_data->rule = UNWIND_USER_RULE_CFA_OFFSET_DEREF;
-		rule_data->offset = offset;
-	} else {
+	bool deref_p = SFRAME_V3_FLEX_FDE_CTRLWORD_DEREF_P(ctlword);
+	bool reg_p = SFRAME_V3_FLEX_FDE_CTRLWORD_REG_P(ctlword);
+	bool reserved_p = SFRAME_V3_FLEX_FDE_CTRLWORD_RESERVED_P(ctlword);
+
+	if (!ctlword && !offset) {
 		rule_data->rule = UNWIND_USER_RULE_RETAIN;
+		return 0;
 	}
+
+	if (reserved_p)
+		return -EINVAL;
+
+	if (reg_p) {
+		unsigned int regnum = SFRAME_V3_FLEX_FDE_CTRLWORD_REGNUM(ctlword);
+
+		rule_data->rule = UNWIND_USER_RULE_REG_OFFSET;
+		rule_data->regnum = regnum;
+	} else {
+		rule_data->rule = UNWIND_USER_RULE_CFA_OFFSET;
+	}
+
+	if (deref_p)
+		rule_data->rule |= UNWIND_USER_RULE_DEREF;
+
+	rule_data->offset = offset;
+
+	return 0;
 }
 
 static __always_inline int __find_fre(struct sframe_section *sec,
@@ -356,9 +528,15 @@ static __always_inline int __find_fre(struct sframe_section *sec,
 		return -EINVAL;
 	fre = prev_fre;
 
-	sframe_init_cfa_rule_data(&frame->cfa, fre->info, fre->cfa_off);
-	sframe_init_rule_data(&frame->ra, fre->ra_off);
-	sframe_init_rule_data(&frame->fp, fre->fp_off);
+	ret = sframe_init_cfa_rule_data(&frame->cfa, fre->cfa_ctl, fre->cfa_off);
+	if (ret)
+		return ret;
+	ret = sframe_init_rule_data(&frame->ra, fre->ra_ctl, fre->ra_off);
+	if (ret)
+		return ret;
+	ret = sframe_init_rule_data(&frame->fp, fre->fp_ctl, fre->fp_off);
+	if (ret)
+		return ret;
 	frame->outermost = SFRAME_V3_FRE_RA_UNDEFINED_P(fre->info);
 
 	return 0;
diff --git a/kernel/unwind/sframe.h b/kernel/unwind/sframe.h
index ed111fd0d702..1a2528e4b149 100644
--- a/kernel/unwind/sframe.h
+++ b/kernel/unwind/sframe.h
@@ -66,6 +66,7 @@ struct sframe_fda_v3 {
 #define SFRAME_V3_AARCH64_FDE_PAUTH_KEY(info)	(((info) >> 5) & 0x1)
 
 #define SFRAME_FDE_TYPE_DEFAULT			0
+#define SFRAME_FDE_TYPE_FLEX			1
 
 #define SFRAME_V3_FDE_TYPE_MASK			0x1f
 #define SFRAME_V3_FDE_TYPE(info2)		((info2) & SFRAME_V3_FDE_TYPE_MASK)
@@ -79,4 +80,9 @@ struct sframe_fda_v3 {
 #define SFRAME_V3_AARCH64_FRE_MANGLED_RA_P(info)	(((info) >> 7) & 0x1)
 #define SFRAME_V3_FRE_RA_UNDEFINED_P(info)		(SFRAME_V3_FRE_DATAWORD_COUNT(info) == 0)
 
+#define SFRAME_V3_FLEX_FDE_CTRLWORD_REGNUM(data)	(((data) >> 3) & 0x1f)
+#define SFRAME_V3_FLEX_FDE_CTRLWORD_RESERVED_P(data)	(((data) >> 2) & 0x1)
+#define SFRAME_V3_FLEX_FDE_CTRLWORD_DEREF_P(data)	(((data) >> 1) & 0x1)
+#define SFRAME_V3_FLEX_FDE_CTRLWORD_REG_P(data)		((data) & 0x1)
+
 #endif /* _SFRAME_H */
-- 
2.51.0

[PATCH v15 17/20] unwind_user/sframe: Separate reading of FRE from reading of FRE data words

[Jens Remus]

__find_fre() performs linear search for a matching SFrame FRE for a
given IP.  For that purpose it uses __read_fre(), which reads the whole
FRE.  That is the variable-size FRE structure as well as the trailing
variable-length array of variable-size data words.  For the search logic
to skip over the FRE it would be sufficient to read the variable-size
FRE structure only, which includes the count and size of data words.

Add fields to struct sframe_fre_internal to store the FRE data word's
address, count, and size.  Change __read_fre() to read the variable-
size FRE structure only and populate those new fields.  Change
__read_fre_datawords() to use those new fields.  Change __find_fre()
to use __read_fre_datawords() to read the FRE data words only after a
matching FRE has been found.  Introduce safe_read_fre_datawords() and
use it in sframe_validate_section() to validate that the FRE data words.

Reviewed-by: Indu Bhagat <ibhagatgnu@gmail.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    Changes in v15:
    - sframe_validate_section(): Fix format specifier for number of FREs
      in debug message. (Sashiko AI)
    
    Changes in v14:
    - Adjust to rename of SFRAME_FDE_TYPE_* and
      __read_default_fre_datawords().
    - Update function name in debug message.

 kernel/unwind/sframe.c | 100 ++++++++++++++++++++++++++---------------
 1 file changed, 64 insertions(+), 36 deletions(-)

diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
index 48709f0bafc7..ec8318977a2e 100644
--- a/kernel/unwind/sframe.c
+++ b/kernel/unwind/sframe.c
@@ -39,6 +39,9 @@ struct sframe_fre_internal {
 	u32		fp_ctl;
 	s32		fp_off;
 	u8		info;
+	unsigned long	dw_addr;
+	unsigned char	dw_count;
+	unsigned char	dw_size;
 };
 
 DEFINE_STATIC_SRCU(sframe_srcu);
@@ -207,11 +210,11 @@ static __always_inline int __find_fde(struct sframe_section *sec,
 static __always_inline int
 __read_default_fre_datawords(struct sframe_section *sec,
 			     struct sframe_fde_internal *fde,
-			     unsigned long cur,
-			     unsigned char dataword_count,
-			     unsigned char dataword_size,
 			     struct sframe_fre_internal *fre)
 {
+	unsigned char dataword_count = fre->dw_count;
+	unsigned char dataword_size = fre->dw_size;
+	unsigned long cur = fre->dw_addr;
 	s32 cfa_off, ra_off, fp_off;
 	unsigned int cfa_regnum;
 
@@ -253,11 +256,11 @@ __read_default_fre_datawords(struct sframe_section *sec,
 static __always_inline int
 __read_flex_fde_fre_datawords(struct sframe_section *sec,
 			      struct sframe_fde_internal *fde,
-			      unsigned long cur,
-			      unsigned char dataword_count,
-			      unsigned char dataword_size,
 			      struct sframe_fre_internal *fre)
 {
+	unsigned char dataword_count = fre->dw_count;
+	unsigned char dataword_size = fre->dw_size;
+	unsigned long cur = fre->dw_addr;
 	u32 cfa_ctl, ra_ctl, fp_ctl;
 	s32 cfa_off, ra_off, fp_off;
 
@@ -325,24 +328,34 @@ __read_flex_fde_fre_datawords(struct sframe_section *sec,
 static __always_inline int
 __read_fre_datawords(struct sframe_section *sec,
 		     struct sframe_fde_internal *fde,
-		     unsigned long cur,
-		     unsigned char dataword_count,
-		     unsigned char dataword_size,
 		     struct sframe_fre_internal *fre)
 {
 	unsigned char fde_type = SFRAME_V3_FDE_TYPE(fde->info2);
+	unsigned char dataword_count = fre->dw_count;
+
+	if (!dataword_count) {
+		/*
+		 * A FRE without datawords indicates an outermost
+		 * frame.  Zero-initialize CFA, RA, and FP location
+		 * info, except for the CFA control word, so that
+		 * neither sframe_init_cfa_rule_data() nor
+		 * sframe_init_rule_data() fail.
+		 */
+		fre->cfa_ctl	= (SFRAME_REG_SP << 3) | 1; /* regnum=SP, deref_p=0, reg_p=1 */
+		fre->cfa_off	= 0;
+		fre->ra_ctl	= 0;
+		fre->ra_off	= 0;
+		fre->fp_ctl	= 0;
+		fre->fp_off	= 0;
+
+		return 0;
+	}
 
 	switch (fde_type) {
 	case SFRAME_FDE_TYPE_DEFAULT:
-		return __read_default_fre_datawords(sec, fde, cur,
-						    dataword_count,
-						    dataword_size,
-						    fre);
+		return __read_default_fre_datawords(sec, fde, fre);
 	case SFRAME_FDE_TYPE_FLEX:
-		return __read_flex_fde_fre_datawords(sec, fde, cur,
-						     dataword_count,
-						     dataword_size,
-						     fre);
+		return __read_flex_fde_fre_datawords(sec, fde, fre);
 	default:
 		return -EINVAL;
 	}
@@ -385,26 +398,11 @@ static __always_inline int __read_fre(struct sframe_section *sec,
 	fre->size	= addr_size + 1 + (dataword_count * dataword_size);
 	fre->ip_off	= ip_off;
 	fre->info	= info;
+	fre->dw_addr	= cur;
+	fre->dw_count	= dataword_count;
+	fre->dw_size	= dataword_size;
 
-	if (!dataword_count) {
-		/*
-		 * A FRE without datawords indicates an outermost
-		 * frame.  Zero-initialize CFA, RA, and FP location
-		 * info, except for the CFA control word, so that
-		 * neither sframe_init_cfa_rule_data() nor
-		 * sframe_init_rule_data() fail.
-		 */
-		fre->cfa_ctl	= (SFRAME_REG_SP << 3) | 1; /* regnum=SP, deref_p=0, reg_p=1 */
-		fre->cfa_off	= 0;
-		fre->ra_ctl	= 0;
-		fre->ra_off	= 0;
-		fre->fp_ctl	= 0;
-		fre->fp_off	= 0;
-
-		return 0;
-	}
-
-	return __read_fre_datawords(sec, fde, cur, dataword_count, dataword_size, fre);
+	return 0;
 
 Efault:
 	return -EFAULT;
@@ -491,6 +489,7 @@ static __always_inline int __find_fre(struct sframe_section *sec,
 	bool which = false;
 	unsigned int i;
 	u32 ip_off;
+	int ret;
 
 	ip_off = ip - fde->func_addr;
 
@@ -528,6 +527,10 @@ static __always_inline int __find_fre(struct sframe_section *sec,
 		return -EINVAL;
 	fre = prev_fre;
 
+	ret = __read_fre_datawords(sec, fde, fre);
+	if (ret)
+		return ret;
+
 	ret = sframe_init_cfa_rule_data(&frame->cfa, fre->cfa_ctl, fre->cfa_off);
 	if (ret)
 		return ret;
@@ -611,6 +614,20 @@ static int safe_read_fre(struct sframe_section *sec,
 	return ret;
 }
 
+static int safe_read_fre_datawords(struct sframe_section *sec,
+				   struct sframe_fde_internal *fde,
+				   struct sframe_fre_internal *fre)
+{
+	int ret;
+
+	if (!user_read_access_begin((void __user *)sec->sframe_start,
+				    sec->sframe_end - sec->sframe_start))
+		return -EFAULT;
+	ret = __read_fre_datawords(sec, fde, fre);
+	user_read_access_end();
+	return ret;
+}
+
 static int sframe_validate_section(struct sframe_section *sec)
 {
 	unsigned long prev_ip = 0;
@@ -656,6 +673,17 @@ static int sframe_validate_section(struct sframe_section *sec)
 					fde.rep_size);
 				return ret;
 			}
+			ret = safe_read_fre_datawords(sec, &fde, fre);
+			if (ret) {
+				dbg_sec("FDE %u: safe_read_fre_datawords(%u) failed\n", i, j);
+				dbg_sec("FDE: func_addr:%#lx func_size:%#x fda_off:%#x fres_off:%#x fres_num:%u info:%u info2:%u rep_size:%u\n",
+					fde.func_addr, fde.func_size,
+					fde.fda_off,
+					fde.fres_off, fde.fres_num,
+					fde.info, fde.info2,
+					fde.rep_size);
+				return ret;
+			}
 
 			fre_addr += fre->size;
 
-- 
2.51.0

[PATCH v15 18/20] unwind_user/sframe: Duplicate registered .sframe section data on clone/fork

[Jens Remus]

When duplicating a process' virtual memory mappings also duplicate all
of its registered .sframe sections stored in the per-mm maple tree to
enable stacktracing using sframe of the child process.

Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---
 include/linux/sframe.h       |  5 ++++
 kernel/unwind/sframe.c       | 48 ++++++++++++++++++++++++++++++++++++
 kernel/unwind/sframe_debug.h |  7 ++++++
 mm/mmap.c                    |  9 +++++++
 4 files changed, 69 insertions(+)

diff --git a/include/linux/sframe.h b/include/linux/sframe.h
index b79c5ec09229..91889b4fe3dd 100644
--- a/include/linux/sframe.h
+++ b/include/linux/sframe.h
@@ -28,6 +28,7 @@ struct sframe_section {
 };
 
 #define INIT_MM_SFRAME .sframe_mt = MTREE_INIT(sframe_mt, 0),
+extern int sframe_dup_mm(struct mm_struct *mm, struct mm_struct *oldmm);
 extern void sframe_free_mm(struct mm_struct *mm);
 
 extern int sframe_add_section(unsigned long sframe_start, unsigned long sframe_end,
@@ -45,6 +46,10 @@ static inline bool current_has_sframe(void)
 #else /* !CONFIG_HAVE_UNWIND_USER_SFRAME */
 
 #define INIT_MM_SFRAME
+static inline int sframe_dup_mm(struct mm_struct *mm, struct mm_struct *oldmm)
+{
+	return 0;
+}
 static inline void sframe_free_mm(struct mm_struct *mm) {}
 static inline int sframe_add_section(unsigned long sframe_start, unsigned long sframe_end,
 				     unsigned long text_start, unsigned long text_end)
diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
index ec8318977a2e..0661090dd5bc 100644
--- a/kernel/unwind/sframe.c
+++ b/kernel/unwind/sframe.c
@@ -886,6 +886,54 @@ int sframe_remove_section(unsigned long sframe_start)
 	return 0;
 }
 
+static void __sframe_dup_section(struct sframe_section *sec, struct sframe_section *oldsec)
+{
+	sec->sframe_start	= oldsec->sframe_start;
+	sec->sframe_end		= oldsec->sframe_end;
+	sec->text_start		= oldsec->text_start;
+	sec->text_end		= oldsec->text_end;
+
+	sec->fdes_start		= oldsec->fdes_start;
+	sec->fres_start		= oldsec->fres_start;
+	sec->fres_end		= oldsec->fres_end;
+	sec->num_fdes		= oldsec->num_fdes;
+
+	sec->ra_off		= oldsec->ra_off;
+	sec->fp_off		= oldsec->fp_off;
+
+	dbg_dup(sec, oldsec);
+}
+
+int sframe_dup_mm(struct mm_struct *mm, struct mm_struct *oldmm)
+{
+	struct sframe_section *sec, *oldsec;
+	unsigned long index = 0;
+	int ret;
+
+	guard(srcu)(&sframe_srcu);
+
+	mt_for_each(&oldmm->sframe_mt, oldsec, index, ULONG_MAX) {
+		sec = kzalloc(sizeof(*sec), GFP_KERNEL_ACCOUNT);
+		if (!sec)
+			return -ENOMEM;
+
+		__sframe_dup_section(sec, oldsec);
+
+		ret = mtree_insert_range(&mm->sframe_mt,
+					 sec->text_start,
+					 sec->text_end - 1,
+					 sec, GFP_KERNEL_ACCOUNT);
+		if (ret)
+			goto err_free;
+	}
+
+	return 0;
+
+err_free:
+	free_section(sec);
+	return ret;
+}
+
 void sframe_free_mm(struct mm_struct *mm)
 {
 	struct sframe_section *sec;
diff --git a/kernel/unwind/sframe_debug.h b/kernel/unwind/sframe_debug.h
index a63e75cccc70..2503972155e8 100644
--- a/kernel/unwind/sframe_debug.h
+++ b/kernel/unwind/sframe_debug.h
@@ -48,6 +48,12 @@ static inline void dbg_init(struct sframe_section *sec)
 		sec->filename = kstrdup("(anonymous)", GFP_KERNEL_ACCOUNT);
 }
 
+static inline void dbg_dup(struct sframe_section *sec, struct sframe_section *oldsec)
+{
+	if (oldsec->filename)
+		sec->filename = kstrdup(oldsec->filename, GFP_KERNEL_ACCOUNT);
+}
+
 static inline void dbg_free(struct sframe_section *sec)
 {
 	kfree(sec->filename);
@@ -61,6 +67,7 @@ static inline void dbg_free(struct sframe_section *sec)
 static inline void dbg_print_header(struct sframe_section *sec) {}
 
 static inline void dbg_init(struct sframe_section *sec) {}
+static inline void dbg_dup(struct sframe_section *sec, struct sframe_section *oldsec) {}
 static inline void dbg_free(struct sframe_section *sec) {}
 
 #endif /* !CONFIG_DYNAMIC_DEBUG */
diff --git a/mm/mmap.c b/mm/mmap.c
index 5754d1c36462..250e8bea73ef 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -48,6 +48,7 @@
 #include <linux/sched/mm.h>
 #include <linux/ksm.h>
 #include <linux/memfd.h>
+#include <linux/sframe.h>
 
 #include <linux/uaccess.h>
 #include <asm/cacheflush.h>
@@ -1846,6 +1847,11 @@ __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
 	}
 	/* a new mm has just been created */
 	retval = arch_dup_mmap(oldmm, mm);
+#ifdef CONFIG_HAVE_UNWIND_USER_SFRAME
+	if (!retval) {
+		retval = sframe_dup_mm(mm, oldmm);
+	}
+#endif
 loop_out:
 	vma_iter_free(&vmi);
 	if (!retval) {
@@ -1893,6 +1899,9 @@ __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
 			vm_unacct_memory(charge);
 		}
 		__mt_destroy(&mm->mm_mt);
+#ifdef CONFIG_HAVE_UNWIND_USER_SFRAME
+		sframe_free_mm(mm);
+#endif
 		/*
 		 * The mm_struct is going to exit, but the locks will be dropped
 		 * first.  Set the mm_struct as unstable is advisable as it is
-- 
2.51.0

[PATCH v15 19/20] unwind_user/sframe/x86: Enable sframe unwinding on x86

[Jens Remus]

From: Josh Poimboeuf <jpoimboe@kernel.org>

The x86 SFrame V3 implementation works fairly well, starting with
binutils 2.46.  Enable it.

[ Jens Remus: Reword commit message for SFrame V3, starting with
binutils 2.46. ]

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Reviewed-by: Indu Bhagat <ibhagatgnu@gmail.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    Changes in v15:
    - unwind_user_get_reg(): Fail if !user_64bit_mode(). (Sashiko AI)
    - unwind_user_get_reg(): Simplify guarding using CONFIG_X86_64.
    - unwind_user_get_reg(): Add pr_debug_once() if unsupported register
      number.
    
    Changes in v14:
    - Drop superfluous empty line in unwind_user_get_reg().
    
    Changes in v13:
    - Naive implementation of unwind_user_get_reg() to support SFrame V3
      flexible FDEs (e.g. used to represent DRAP pattern).
    - Define SFRAME_REG_SP and SFRAME_REG_FP to the respective x86-64
      DWARF register numbers.
    - Reword commit message for SFrame V3 and (upcoming) binutils 2.46.

 arch/x86/Kconfig                          |  1 +
 arch/x86/include/asm/unwind_user.h        | 39 +++++++++++++++++++++++
 arch/x86/include/asm/unwind_user_sframe.h | 12 +++++++
 3 files changed, 52 insertions(+)
 create mode 100644 arch/x86/include/asm/unwind_user_sframe.h

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index f3f7cb01d69d..51286dfdb5f4 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -302,6 +302,7 @@ config X86
 	select HAVE_UACCESS_VALIDATION		if HAVE_OBJTOOL
 	select HAVE_UNSTABLE_SCHED_CLOCK
 	select HAVE_UNWIND_USER_FP		if X86_64
+	select HAVE_UNWIND_USER_SFRAME		if X86_64
 	select HAVE_USER_RETURN_NOTIFIER
 	select HAVE_GENERIC_VDSO
 	select VDSO_GETRANDOM			if X86_64
diff --git a/arch/x86/include/asm/unwind_user.h b/arch/x86/include/asm/unwind_user.h
index f38f7c5ff1de..942475235431 100644
--- a/arch/x86/include/asm/unwind_user.h
+++ b/arch/x86/include/asm/unwind_user.h
@@ -15,6 +15,45 @@ static inline int unwind_user_word_size(struct pt_regs *regs)
 	return user_64bit_mode(regs) ? 8 : 4;
 }
 
+#ifdef CONFIG_X86_64
+
+static inline int unwind_user_get_reg(unsigned long *val, unsigned int regnum)
+{
+	struct pt_regs *regs = task_pt_regs(current);
+
+	/* SFrame only supports x86-64 */
+	if (!user_64bit_mode(regs))
+		return -EINVAL;
+
+	switch (regnum) {
+	/* DWARF register numbers 0..15 */
+	case  0: *val = regs->ax; break;
+	case  1: *val = regs->dx; break;
+	case  2: *val = regs->cx; break;
+	case  3: *val = regs->bx; break;
+	case  4: *val = regs->si; break;
+	case  5: *val = regs->di; break;
+	case  6: *val = regs->bp; break;
+	case  7: *val = regs->sp; break;
+	case  8: *val = regs->r8; break;
+	case  9: *val = regs->r9; break;
+	case 10: *val = regs->r10; break;
+	case 11: *val = regs->r11; break;
+	case 12: *val = regs->r12; break;
+	case 13: *val = regs->r13; break;
+	case 14: *val = regs->r14; break;
+	case 15: *val = regs->r15; break;
+	default:
+		pr_debug_once("%s (%d): unwind_user_get_reg(%u): unsupported register number\n",
+			      current->comm, current->pid, regnum);
+		return -EINVAL;
+	}
+	return 0;
+}
+#define unwind_user_get_reg unwind_user_get_reg
+
+#endif /* CONFIG_X86_64 */
+
 #endif /* CONFIG_UNWIND_USER */
 
 #ifdef CONFIG_HAVE_UNWIND_USER_FP
diff --git a/arch/x86/include/asm/unwind_user_sframe.h b/arch/x86/include/asm/unwind_user_sframe.h
new file mode 100644
index 000000000000..d828ae1a4aac
--- /dev/null
+++ b/arch/x86/include/asm/unwind_user_sframe.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_UNWIND_USER_SFRAME_H
+#define _ASM_X86_UNWIND_USER_SFRAME_H
+
+#ifdef CONFIG_X86_64
+
+#define SFRAME_REG_SP	7
+#define SFRAME_REG_FP	6
+
+#endif
+
+#endif /* _ASM_X86_UNWIND_USER_SFRAME_H */
-- 
2.51.0

[PATCH v15 20/20] unwind_user/sframe: Add prctl() interface for registering .sframe sections

[Jens Remus]

From: Josh Poimboeuf <jpoimboe@kernel.org>

The kernel doesn't have direct visibility to the ELF contents of shared
libraries.  Add some prctl() interfaces which allow glibc to tell the
kernel where to find .sframe sections.

[
  This adds an interface for prctl() for testing loading of sframes for
  libraries. But this interface should really be a system call. This patch
  is for testing purposes only and should not be applied to mainline.
]

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Reviewed-by: Indu Bhagat <ibhagatgnu@gmail.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    Changes in v15:
    - Fix rebase error (missing break). (Sashiko AI)
    
    Changes in v14:
    - Bump PR_ADD_SFRAME and PR_REMOVE_SFRAME.

 include/uapi/linux/prctl.h | 4 ++++
 kernel/sys.c               | 9 +++++++++
 2 files changed, 13 insertions(+)

diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index b6ec6f693719..bd0bf828b033 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -416,4 +416,8 @@ struct prctl_mm_map {
 # define PR_CFI_DISABLE		_BITUL(1)
 # define PR_CFI_LOCK		_BITUL(2)
 
+/* SFRAME management */
+#define PR_ADD_SFRAME			82
+#define PR_REMOVE_SFRAME		83
+
 #endif /* _LINUX_PRCTL_H */
diff --git a/kernel/sys.c b/kernel/sys.c
index 62e842055cc9..b0a9b1e3ccd7 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -65,6 +65,7 @@
 #include <linux/rcupdate.h>
 #include <linux/uidgid.h>
 #include <linux/cred.h>
+#include <linux/sframe.h>
 
 #include <linux/nospec.h>
 
@@ -2907,6 +2908,14 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 		if (arg3 & PR_CFI_LOCK && !(arg3 & PR_CFI_DISABLE))
 			error = arch_prctl_lock_branch_landing_pad_state(me);
 		break;
+	case PR_ADD_SFRAME:
+		error = sframe_add_section(arg2, arg3, arg4, arg5);
+		break;
+	case PR_REMOVE_SFRAME:
+		if (arg3 || arg4 || arg5)
+			return -EINVAL;
+		error = sframe_remove_section(arg2);
+		break;
 	default:
 		trace_task_prctl_unknown(option, arg2, arg3, arg4, arg5);
 		error = -EINVAL;
-- 
2.51.0

Re: [PATCH v15 08/20] unwind_user: Stop when reaching an outermost frame

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Uninitialized stack variable read of `outermost` field causes premature termination of user space stack unwinding.
--

commit 6e5baf1a7025f1ab5a73320fe2e522b5d83d132d
Author: Jens Remus <jremus@linux.ibm.com>

unwind_user: Stop when reaching an outermost frame

This commit adds an outermost frame indication to the user space unwind
frame structure. It checks this flag during the unwind process to stop
unwinding when an outermost frame is reached.

[ ... ]

> diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c
> index 1fb2724197332..fdb1001e37505 100644
> --- a/kernel/unwind/user.c
> +++ b/kernel/unwind/user.c
> @@ -32,6 +32,12 @@ static int unwind_user_next_common(struct unwind_user_state *state,
>  {
>  	unsigned long cfa, fp, ra;
>  
> +	/* Stop unwinding when reaching an outermost frame. */
> +	if (frame->outermost) {

[Severity: Medium]
Will this read uninitialized stack memory during sframe unwinding?

Looking at unwind_user_next_sframe(), the frame structure is allocated on the
stack without zero-initialization:

kernel/unwind/user.c:unwind_user_next_sframe() {
	struct unwind_user_frame frame;

	/* sframe expects the frame to be local storage */
	if (sframe_find(state->ip, &frame))
		return -ENOENT;
	return unwind_user_next_common(state, &frame);
}

Since the new outermost field isn't populated by sframe_find() until a later
commit ("unwind_user/sframe: Add support for outermost frame indication"),
could this uninitialized read evaluate to true and cause premature termination
of user space stack unwinding?

This would break the bisectability of the patch series.

> +		state->done = true;
> +		return 0;
> +	}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520154004.3845823-1-jremus@linux.ibm.com?part=8

Re: [PATCH v15 09/20] unwind_user/sframe: Add support for outermost frame indication

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The size calculation for SFrame FRE evaluates to just the header size, incorrectly omitting the size of the datawords because it uses the mutated `dataword_count` variable.
--

commit a8be8d2031245c57fdb06e0cafc371dc3614077c
Author: Jens Remus <jremus@linux.ibm.com>

unwind_user/sframe: Add support for outermost frame indication

This commit adds support for the SFrame outermost frame indication. It
allows an FRE without any offsets to represent an undefined return
address.

> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
> index a38f50a363633..f723c1a32f906 100644
> --- a/kernel/unwind/sframe.c
> +++ b/kernel/unwind/sframe.c

[ ... ]

> @@ -260,6 +271,7 @@ static __always_inline int __read_fre(struct sframe_section *sec,
>  	if (dataword_count)
>  		return -EINVAL;
>  
> +done:
>  	fre->size	= addr_size + 1 + (dataword_count * dataword_size);

[Severity: High]
This is a pre-existing issue, but does this calculation correctly size the FRE?

In __read_fre(), dataword_count is progressively decremented as offsets are
parsed, and it is guaranteed to be 0 at this point due to the check right
above the done label:

    if (dataword_count)
        return -EINVAL;

done:
    fre->size = addr_size + 1 + (dataword_count * dataword_size);

Because dataword_count is 0, fre->size evaluates to just the header size,
completely omitting the size of the datawords.

When __find_fre() executes fre_addr += fre->size, it advances the pointer by
this truncated amount. This would cause subsequent iterations to read
misaligned data and potentially break stack unwinding.

I see this was fixed later in the series in commit 9c045164b82cb
("unwind_user/sframe: Separate reading of FRE from reading of FRE data words"),
but noting it here for completeness.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520154004.3845823-1-jremus@linux.ibm.com?part=9

Re: [PATCH v15 02/20] unwind_user/sframe: Add support for reading .sframe headers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Integer overflow in SFrame section offset and size calculations allows malformed headers to bypass bounds checks on 32-bit architectures.
--

commit 2144f643835a311bd7c4b9df269057ea5516231a
Author: Josh Poimboeuf <jpoimboe@kernel.org>

unwind_user/sframe: Add support for reading .sframe headers

This commit adds basic compile infrastructure and support for reading
the .sframe section header in preparation for unwinding user space stacks.
The sframe_add_section() function currently reads the header and
unconditionally returns an error until subsequent patches improve it.

Link: https://lore.kernel.org/all/f27e8463783febfa0dabb0432a3dd6be8ad98412.1737511963.git.jpoimboe@kernel.org/

> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
> new file mode 100644
> index 0000000000000..d24e9d4f8bef7
> --- /dev/null
> +++ b/kernel/unwind/sframe.c

[ ... ]

> @@ -0,0 +1,136 @@
> +static int sframe_read_header(struct sframe_section *sec)
> +{
> +	unsigned long header_end, fdes_start, fdes_end, fres_start, fres_end;
> +	struct sframe_header shdr;
> +	unsigned int num_fdes;

[ ... ]

> +	header_end = sec->sframe_start + SFRAME_HEADER_SIZE(shdr);
> +	if (header_end >= sec->sframe_end) {
> +		dbg("header doesn't fit in section\n");
> +		return -EINVAL;
> +	}
> +
> +	num_fdes   = shdr.num_fdes;
> +	fdes_start = header_end + shdr.fdes_off;
> +	fdes_end   = fdes_start + (num_fdes * sizeof(struct sframe_fde_v3));
> +
> +	fres_start = header_end + shdr.fres_off;
> +	fres_end   = fres_start + shdr.fre_len;

[Severity: Medium]
On 32-bit architectures, is it possible for these offset and length
calculations to wrap around?

User-supplied 32-bit fields from the SFrame header (fdes_off, num_fdes,
fres_off, fre_len) are used to compute section boundaries using raw
arithmetic. Since unsigned long is 32 bits on those architectures,
operations like header_end + shdr.fdes_off or num_fdes * sizeof(...)
could overflow.

If a malformed header contains values that wrap the 32-bit address space,
fres_end might appear smaller than sec->sframe_end and fres_start might
appear greater than fdes_end, completely bypassing the validation check
that follows.

Would it be safer to use the <linux/overflow.h> helpers such as
size_mul() and check_add_overflow() to protect these arithmetic
operations?

> +
> +	if (fres_start < fdes_end || fres_end > sec->sframe_end) {
> +		dbg("inconsistent fde/fre offsets\n");
> +		return -EINVAL;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520154004.3845823-1-jremus@linux.ibm.com?part=2

Re: [PATCH v15 04/20] x86/uaccess: Add unsafe_copy_from_user() implementation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The macros `unsafe_copy_to_user` and `unsafe_copy_from_user` use generic local variable names (`__dst`, `__src`, `__len`), creating a variable shadowing vulnerability.
--

commit 32beb94b1b6090052c09ccb07d8ccb9ac95181a6
Author: Josh Poimboeuf <jpoimboe@kernel.org>

x86/uaccess: Add unsafe_copy_from_user() implementation

This commit adds an x86 implementation for unsafe_copy_from_user() that
matches the existing unsafe_copy_to_user() design, and refactors both 
macros to use shared looping logic.

> diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
> --- a/arch/x86/include/asm/uaccess.h
> +++ b/arch/x86/include/asm/uaccess.h

[ ... ]

> -#define unsafe_copy_to_user(_dst,_src,_len,label)			\
> -do {									\
> -	char __user *__ucu_dst = (_dst);				\
> -	const char *__ucu_src = (_src);					\
> -	size_t __ucu_len = (_len);					\
> -	unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u64, label);	\
> -	unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u32, label);	\
> -	unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u16, label);	\
> -	unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u8, label);	\
> +#define unsafe_copy_to_user(_dst, _src, _len, label)				\
> +do {										\
> +	void __user *__dst = (_dst);						\
> +	const void *__src = (_src);						\
> +	size_t __len = (_len);							\

[Severity: Medium]
Could these generic local variable names (__dst, __src, __len) cause a 
variable shadowing regression? 

If a caller passes an argument with one of these exact names (such as __len),
the macro's local declaration would initialize itself with its own uninitialized
stack value rather than the caller's value. 

The previous implementation avoided this trap by using uniquely prefixed 
variables like __ucu_len. Is it possible to use similar prefixed variable 
names for the new macros?

> +	unsafe_copy_to_user_loop(__dst, __src, __len, u64, label);		\
> +	unsafe_copy_to_user_loop(__dst, __src, __len, u32, label);		\
> +	unsafe_copy_to_user_loop(__dst, __src, __len, u16, label);		\
> +	unsafe_copy_to_user_loop(__dst, __src, __len, u8,  label);		\
> +} while (0)

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520154004.3845823-1-jremus@linux.ibm.com?part=4

Re: [PATCH v15 11/20] unwind_user/sframe: Show file name in debug output

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] Incorrect use of `vma->vm_file` instead of `!vma_is_anonymous(vma)` to classify a VMA as file-backed.

Pre-existing issues:
- [Critical] Calling `sframe_remove_section` from `sframe_find` acquires a spinlock in NMI context, leading to deadlocks, and triggers a user-controllable WARN.
--

commit 60b05617f9a9c00f287a96dda25f8e4086c6178b
Author: Josh Poimboeuf <jpoimboe@kernel.org>

unwind_user/sframe: Show file name in debug output

This commit prefixes debug output strings with the file name to make sframe
debugging easier. It looks up the VMA corresponding to the sframe section
and extracts the filename based on the VMA properties.

> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
> --- a/kernel/unwind/sframe.c
> +++ b/kernel/unwind/sframe.c
[ ... ]
> @@ -374,14 +374,17 @@ int sframe_find(unsigned long ip, struct unwind_user_frame *frame)
>  	user_read_access_end();
>  
>  end:
> -	if (ret && ret != -ENOENT)
> +	if (ret && ret != -ENOENT) {
> +		dbg_sec("removing bad .sframe section\n");
>  		WARN_ON_ONCE(sframe_remove_section(sec->sframe_start));
> +	}

[Severity: Critical]
This is a pre-existing issue, but can this lead to deadlocks in NMI context?

sframe_find() is invoked for user stack unwinding from contexts like perf,
which executes from NMI or hardirq contexts. sframe_remove_section() calls
mtree_erase(), which acquires the maple tree's non-IRQ-safe mtree_lock.

Acquiring a regular spinlock in NMI context can cause a hard CPU deadlock
if the lock was already held by the interrupted thread.

Also, could an unprivileged user trivially trigger this WARN_ON_ONCE by
unmapping the section, potentially crashing the system if panic_on_warn
is enabled?

[ ... ]
> diff --git a/kernel/unwind/sframe_debug.h b/kernel/unwind/sframe_debug.h
> --- a/kernel/unwind/sframe_debug.h
> +++ b/kernel/unwind/sframe_debug.h
[ ... ]
> +static inline void dbg_init(struct sframe_section *sec)
> +{
> +	struct mm_struct *mm = current->mm;
> +	struct vm_area_struct *vma;
> +
> +	guard(mmap_read_lock)(mm);
> +	vma = vma_lookup(mm, sec->sframe_start);
> +	if (!vma)
> +		sec->filename = kstrdup("(vma gone???)", GFP_KERNEL_ACCOUNT);
> +	else if (vma->vm_file)
> +		sec->filename = kstrdup_quotable_file(vma->vm_file, GFP_KERNEL_ACCOUNT);

[Severity: Low]
Does this code accurately classify file-backed VMAs?

Anonymous mappings, such as private mappings of /dev/zero, can also have
vm_file populated. Relying only on vm_file might misclassify these mappings
for the debug output.

Would it be more reliable to use !vma_is_anonymous(vma) to check if the
VMA is file-backed before accessing vm_file?

> +	else if (vma->vm_ops && vma->vm_ops->name)
> +		sec->filename = kstrdup(vma->vm_ops->name(vma), GFP_KERNEL_ACCOUNT);
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520154004.3845823-1-jremus@linux.ibm.com?part=11

Re: [PATCH v15 12/20] unwind_user/sframe: Add .sframe validation option

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Missing cond_resched() or fatal signal checking in the nested loops of sframe_validate_section() allows an unprivileged local attacker to trigger a CPU soft lockup.
- [Low] The validation logic in sframe_validate_section() incorrectly rejects a valid .sframe section if the first function is located at virtual address 0.

Pre-existing issues:
- [High] sframe_find() handles read failures by calling sframe_remove_section(), which acquires a lock inside NMI or IRQ context, leading to deadlocks or PREEMPT_RT panics.
--

commit 07d74be031cf9fade2b9ddd98bfdd8f52c38c2da
Author: Josh Poimboeuf <jpoimboe@kernel.org>

unwind_user/sframe: Add .sframe validation option

This commit introduces a debugging feature to optionally validate the
entire .sframe section upon loading, rather than checking on-demand.
It uses user_read_access_begin/end to read FDEs and FREs to verify their
formatting and sorting.

> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
> --- a/kernel/unwind/sframe.c
> +++ b/kernel/unwind/sframe.c
[ ... ]
> int sframe_find(unsigned long ip, struct unwind_user_frame *frame)
> {
[ ... ]
> 	ret = __find_fre(sec, &fde, ip, frame);
> end_uaccess:
> 	user_read_access_end();
> 
> end:
> 	if (ret && ret != -ENOENT) {
> 		dbg_sec("removing bad .sframe section\n");
> 		WARN_ON_ONCE(sframe_remove_section(sec->sframe_start));
> 	}
> 
> 	return ret;
> }

[Severity: High]
This isn't a bug introduced by this patch, but does calling
sframe_remove_section() here risk a deadlock or panic on PREEMPT_RT?

If a page fault occurs during user memory access, unsafe_copy_from_user()
returns -EFAULT to prevent sleeping.

However, sframe_find() handles any error other than -ENOENT by calling
sframe_remove_section(), which in turn calls mtree_erase().

Since mtree_erase() takes an internal spinlock, doing this in NMI context
(where sframe_find() is often called) can deadlock or panic a PREEMPT_RT
kernel, while also permanently deleting a valid unwind section due to
transient memory pressure.

[ ... ]
> +#ifdef CONFIG_SFRAME_VALIDATION
[ ... ]
> +static int sframe_validate_section(struct sframe_section *sec)
> +{
> +	unsigned long prev_ip = 0;
> +	unsigned int i;
> +
> +	for (i = 0; i < sec->num_fdes; i++) {
> +		struct sframe_fre_internal *fre, *prev_fre = NULL;
> +		unsigned long ip, fre_addr;
> +		struct sframe_fde_internal fde;
> +		struct sframe_fre_internal fres[2];
> +		bool which = false;
> +		unsigned int j;
> +		int ret;
> +
> +		ret = safe_read_fde(sec, i, &fde);
> +		if (ret) {
> +			dbg_sec("safe_read_fde(%u) failed\n", i);
> +			return ret;
> +		}
> +
> +		ip = fde.func_addr;
> +		if (ip <= prev_ip) {

[Severity: Low]
Does this check accidentally reject valid sections where the first
function is located at virtual address 0?

Since prev_ip is initialized to 0, validating the first FDE when ip is 0
will cause this check to evaluate to true, falsely triggering an -EINVAL.

Valid executables can map their text section at 0 if vm.mmap_min_addr is
set to 0.

> +			dbg_sec("FDE %u not sorted\n", i);
> +			return -EINVAL;
> +		}
> +		prev_ip = ip;
> +
> +		fre_addr = sec->fres_start + fde.fres_off;
> +		for (j = 0; j < fde.fres_num; j++) {
[ ... ]
> +			fre_addr += fre->size;
> +
> +			if (prev_fre && fre->ip_off <= prev_fre->ip_off) {
> +				dbg_sec("FDE %u: FRE %u not sorted\n", i, j);
> +				return -EINVAL;
> +			}
> +
> +			prev_fre = fre;
> +		}

[Severity: High]
Could a malicious user exploit these loops to trigger a CPU soft lockup?

The variables sec->num_fdes and fde.fres_num are user-controlled and
bounded only by the VMA size.

Without a cond_resched() or fatal signal check, an attacker could supply
large counts and cause these loops to run for seconds without yielding.

On non-preemptible kernels, this monopolizes the CPU and triggers the soft
lockup watchdog, resulting in a local denial of service.

> +	}
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520154004.3845823-1-jremus@linux.ibm.com?part=12

Re: [PATCH v15 13/20] unwind_user: Enable archs that pass RA in a register

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] The header file `<linux/unwind_user.h>` uses `pr_debug_once` and `current` without including the necessary headers.

Pre-existing issues:
- [High] The SFrame FRE size calculation computes an incorrect length because `dataword_count` is consumed before it is used in the size formula.
--

commit 842bbe46528fb50996a02de1f8b7f5df1d8b1026
Author: Jens Remus <jremus@linux.ibm.com>

unwind_user: Enable archs that pass RA in a register

This commit updates the user-space unwinder to support architectures that do
not mandate saving the return address (RA) on the stack, such as s390 and
arm64. It treats an RA offset of zero as an indication that the RA is in the
link register for the topmost frame, and allows the stack pointer to remain
unchanged in the topmost frame.

> diff --git a/include/linux/unwind_user.h b/include/linux/unwind_user.h
> index 64618618febde..7bf58f23aa64a 100644
> --- a/include/linux/unwind_user.h
> +++ b/include/linux/unwind_user.h
> @@ -23,6 +23,16 @@ static inline bool unwind_user_at_function_start(struct pt_regs *regs)
>  #define unwind_user_at_function_start unwind_user_at_function_start
>  #endif
>  
> +#ifndef unwind_user_get_ra_reg
> +static inline int unwind_user_get_ra_reg(unsigned long *val)
> +{
> +	pr_debug_once("%s (%d): unwind_user_get_ra_reg() not implemented\n",
> +		      current->comm, current->pid);

[Severity: Low]
Does this require including <linux/printk.h> and <linux/sched.h> or
<asm/current.h>?

Using pr_debug_once and current without these headers might cause compilation
failures for files that include <linux/unwind_user.h> without prior inclusion
of these dependencies.

> +	return -EINVAL;
> +}
> +#define unwind_user_get_ra_reg unwind_user_get_ra_reg
> +#endif

[ ... ]

> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
> index b5f984fb2df21..4af533e9f9804 100644
> --- a/kernel/unwind/sframe.c
> +++ b/kernel/unwind/sframe.c
> @@ -255,10 +255,8 @@ static __always_inline int __read_fre(struct sframe_section *sec,
>  	dataword_count--;
>  
>  	ra_off = sec->ra_off;
> -	if (!ra_off) {
> -		if (!dataword_count--)
> -			return -EINVAL;
> -
> +	if (!ra_off && dataword_count) {
> +		dataword_count--;

[Severity: High]
This isn't a bug introduced by this patch, but does the SFrame FRE size
calculation compute an incorrect length here?

Because dataword_count is decremented as each offset is read from user space,
by the time it reaches the size calculation at the end of __read_fre():

    if (dataword_count)
        return -EINVAL;

done:
    fre->size = addr_size + 1 + (dataword_count * dataword_size);

dataword_count is guaranteed to be 0.

This completely omits the size of the parsed datawords, causing fre_addr to
advance by an incorrectly small amount during iteration and breaking SFrame
unwinding for functions with multiple FREs.

This issue was introduced earlier in the series by commit "unwind_user/sframe:
Add support for reading .sframe contents", and appears to be fixed later in
the series in commit "unwind_user/sframe: Add support for SFrame V3 flexible
FDEs", where the size assignment is moved before the decrements.

>  		UNSAFE_GET_USER_INC(ra_off, cur, dataword_size, Efault);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520154004.3845823-1-jremus@linux.ibm.com?part=13

Re: [PATCH v15 15/20] unwind_user: Flexible CFA recovery rules

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The size of an SFrame Frame Row Entry (FRE) is incorrectly calculated because the `dataword_count` variable is decremented to zero before the size calculation is performed.
--

commit b8c75c92fee05bcde0ff7d5c03df68927b72d4f5
Author: Jens Remus <jremus@linux.ibm.com>

unwind_user: Flexible CFA recovery rules

To enable support for SFrame V3 flexible FDEs with a subsequent patch,
add support for the following flexible Canonical Frame Address (CFA)
recovery rules.

> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
> index e82d1dcdd471f..6187379750db5 100644
> --- a/kernel/unwind/sframe.c
> +++ b/kernel/unwind/sframe.c
> @@ -283,6 +283,18 @@ static __always_inline int __read_fre(struct sframe_section *sec,
>  	return -EFAULT;
>  }
>  

[Severity: High]
This isn't a bug introduced by this patch, but is the size of an SFrame Frame
Row Entry (FRE) incorrectly calculated here because dataword_count is
decremented to zero before the size calculation is performed?

In __read_fre(), dataword_count is decremented as offsets are parsed:

kernel/unwind/sframe.c:__read_fre() {
	...
	UNSAFE_GET_USER_INC(cfa_off, cur, dataword_size, Efault);
	dataword_count--;

	ra_off = sec->ra_off;
	if (!ra_off && dataword_count) {
		dataword_count--;
		UNSAFE_GET_USER_INC(ra_off, cur, dataword_size, Efault);
	}
	...
}

By the time the code reaches the done: label, dataword_count is guaranteed
to be 0. Does this cause the fre->size calculation to completely ignore the
size of the data words?

kernel/unwind/sframe.c:__read_fre() {
	...
done:
	fre->size = addr_size + 1 + (dataword_count * dataword_size);
	...
}

Because the computed size is too small, when the unwinder advances to the next
FRE, does it misalign the pointer and start interpreting the previous FRE's
data words as the next FRE's header, reading garbage data and failing with
-EINVAL or -EFAULT?

I notice this is fixed later in the series in commit 9c045164b82cb
("unwind_user/sframe: Separate reading of FRE from reading of FRE data words")
which properly preserves dataword_count.

> +static __always_inline void
> +sframe_init_cfa_rule_data(struct unwind_user_cfa_rule_data *cfa_rule_data,
> +			  unsigned char fre_info,
> +			  s32 offset)
> +{
> +	if (SFRAME_V3_FRE_CFA_BASE_REG_ID(fre_info) == SFRAME_BASE_REG_FP)
> +		cfa_rule_data->rule = UNWIND_USER_CFA_RULE_FP_OFFSET;
> +	else
> +		cfa_rule_data->rule = UNWIND_USER_CFA_RULE_SP_OFFSET;
> +	cfa_rule_data->offset = offset;
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520154004.3845823-1-jremus@linux.ibm.com?part=15

Re: [PATCH v15 07/20] unwind_user/sframe: Wire up unwind_user to sframe

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incorrect FRE size calculation completely breaks SFrame parsing for functions with multiple FREs.
--

commit 8adeb8aec136e0b90e11e4b9aa74e9464240b9f3
Author: Josh Poimboeuf <jpoimboe@kernel.org>

unwind_user/sframe: Wire up unwind_user to sframe

This commit hooks up the sframe infrastructure to the unwind_user
interface. It introduces unwind_user_next_sframe() to utilize
sframe_find() and wires it into the user unwinding logic.

[ ... ]

> diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c
> index 90ab3c1a205ef..1fb2724197332 100644
> --- a/kernel/unwind/user.c
> +++ b/kernel/unwind/user.c

[ ... ]

> @@ -82,6 +83,16 @@ static int unwind_user_next_fp(struct unwind_user_state *state)
>  	return unwind_user_next_common(state, &fp_frame);
>  }
>  
> +static int unwind_user_next_sframe(struct unwind_user_state *state)
> +{
> +	struct unwind_user_frame frame;
> +
> +	/* sframe expects the frame to be local storage */
> +	if (sframe_find(state->ip, &frame))

[Severity: High]
This is a pre-existing issue, but does the SFrame parsing logic calculate
the FRE size incorrectly for functions with multiple FREs?

Looking at the call chain from sframe_find() into __find_fre() and
__read_fre(), dataword_count is decremented as datawords are parsed until
it reaches 0. This is explicitly enforced by returning -EINVAL if
dataword_count is not zero.

Immediately after, the FRE size is computed using the now-zero
dataword_count:

fre->size = addr_size + 1 + (dataword_count * dataword_size);

Because dataword_count is always 0 here, the size of the datawords is
omitted from the size calculation.

When __find_fre() advances to the next FRE using fre_addr += fre->size,
will it start reading from the middle of the current FRE's datawords
instead of the next FRE header?

I note this is addressed later in the patch series by commit 9c045164b82cb
("unwind_user/sframe: Separate reading of FRE from reading of FRE data
words"), which refactors the code to avoid decrementing dataword_count.

> +		return -ENOENT;
> +	return unwind_user_next_common(state, &frame);
> +}
> +

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520154004.3845823-1-jremus@linux.ibm.com?part=7

Re: [PATCH v15 10/20] unwind_user/sframe: Remove .sframe section on detected corruption

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Concurrent stack unwinds encountering an invalid .sframe section will trigger a WARN_ON_ONCE, allowing unprivileged users to cause kernel panics.

Pre-existing issues:
- [Critical] A TOCTOU race in sframe_remove_section leads to SRCU callback list corruption and memory leaks.
--

commit db7031336962bd6fc8db7c08a1c00d847ae5869b
Author: Josh Poimboeuf <jpoimboe@kernel.org>

unwind_user/sframe: Remove .sframe section on detected corruption

This commit updates the sframe unwinder to remove an .sframe section
if corruption is detected during the unwind process. It handles access
faults and invalid entries by calling sframe_remove_section() on the
corrupt section.

> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
> index f723c1a32f906..02331956009a0 100644
> --- a/kernel/unwind/sframe.c
> +++ b/kernel/unwind/sframe.c

[ ... ]

> @@ -360,16 +360,23 @@ int sframe_find(unsigned long ip, struct unwind_user_frame *frame)
>  		return -ENOENT;
>  
>  	if (!user_read_access_begin((void __user *)sec->sframe_start,
> -				    sec->sframe_end - sec->sframe_start))
> -		return -EFAULT;
> +				    sec->sframe_end - sec->sframe_start)) {
> +		ret = -EFAULT;
> +		goto end;
> +	}
>  
>  	ret = __find_fde(sec, ip, &fde);
>  	if (ret)
> -		goto end;
> +		goto end_uaccess;
>  
>  	ret = __find_fre(sec, &fde, ip, frame);
> -end:
> +end_uaccess:
>  	user_read_access_end();
> +
> +end:
> +	if (ret && ret != -ENOENT)
> +		WARN_ON_ONCE(sframe_remove_section(sec->sframe_start));

[Severity: High]
Could this WARN_ON_ONCE be triggered concurrently by unprivileged users?

If multiple threads concurrently encounter an error while unwinding through
an invalid .sframe section, they might both jump to the end label. The
first thread will successfully remove the section and return 0. Subsequent
threads will fail to find the section since it was already removed, causing
sframe_remove_section() to return -EINVAL.

Since -EINVAL is non-zero, this would unconditionally trigger the
WARN_ON_ONCE. If panic_on_warn is enabled, could this allow a malformed
section to intentionally crash the system?

[Severity: Critical]
This is a pre-existing issue, but does sframe_remove_section() have a race
condition that can corrupt the srcu callback list?

Looking at sframe_remove_section(), it locklessly iterates mm->sframe_mt
using mt_for_each() to find the section pointer:

kernel/unwind/sframe.c:sframe_remove_section() {
    ...
    mt_for_each(&mm->sframe_mt, sec, index, ULONG_MAX) {
        if (sec->sframe_start == sframe_start) {
            found = true;
            ret |= __sframe_remove_section(mm, sec);
        }
    }
    ...
}

If a concurrent thread erases the original section and inserts a new one
at the same text_start, mtree_erase() inside __sframe_remove_section()
would remove the new section. The code would then unconditionally call
call_srcu() with the old sec pointer.

Because the old section is already pending in the srcu callback list,
would double-queuing it corrupt the list and permanently leak the newly
erased section?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520154004.3845823-1-jremus@linux.ibm.com?part=10

Re: [PATCH v15 03/20] unwind_user/sframe: Store .sframe section data in per-mm maple tree

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Missing `MT_FLAGS_USE_RCU` during maple tree initialization causes Use-After-Free of tree internal nodes.
- [High] Synchronous `kfree()` of `sframe_section` bypasses SRCU/RCU grace periods, causing Use-After-Free for concurrent readers.
- [Low] Undeclared identifier `sframe_srcu` causes build failure.
- [Low] Macro expansion of `INIT_MM_SFRAME` breaks compilation on non-x86 architectures.
--

commit ce5ec4616b79b2c0aa851c150b33143b5d3876af
Author: Josh Poimboeuf <jpoimboe@kernel.org>

unwind_user/sframe: Store .sframe section data in per-mm maple tree

Associate an .sframe section with its mm by adding it to a per-mm maple
tree which is indexed by the corresponding text address range. A single
.sframe section can be associated with multiple text ranges.

> diff --git a/include/linux/sframe.h b/include/linux/sframe.h
[ ... ]
> @@ -19,18 +21,31 @@ struct sframe_section {
>  	signed char	fp_off;
>  };
>  
> +#define INIT_MM_SFRAME .sframe_mt = MTREE_INIT(sframe_mt, 0),

[Severity: High]
Does this maple tree need to be initialized with MT_FLAGS_USE_RCU?

Without it, the tree operates in non-RCU mode, so lockless iterations
might race with in-place node modifications and synchronous node frees,
potentially causing use-after-free of the internal nodes.

> diff --git a/kernel/fork.c b/kernel/fork.c
[ ... ]
> @@ -1073,6 +1075,13 @@ static void mmap_init_lock(struct mm_struct *mm)
>  #endif
>  }
>  
> +static void mm_init_sframe(struct mm_struct *mm)
> +{
> +#ifdef CONFIG_HAVE_UNWIND_USER_SFRAME
> +	mt_init(&mm->sframe_mt);

[Severity: High]
Similar to MTREE_INIT above, does mt_init() need to use mt_init_flags()
with MT_FLAGS_USE_RCU here to prevent use-after-free during lockless reads?

> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
[ ... ]
> +static int __sframe_remove_section(struct mm_struct *mm,
> +				   struct sframe_section *sec)
> +{
> +	if (!mtree_erase(&mm->sframe_mt, sec->text_start)) {
> +		dbg("mtree_erase failed: text=%lx\n", sec->text_start);
> +		return -EINVAL;
> +	}
> +
> +	free_section(sec);

[Severity: High]
Can this synchronous free cause a use-after-free for concurrent readers?

If an unwinder or other thread is iterating the tree locklessly and has
already obtained the section pointer before it is erased, freeing it
immediately here without waiting for an RCU or SRCU grace period would
pull the structure out from under them.

> +
> +	return 0;
> +}
> +
>  int sframe_remove_section(unsigned long sframe_start)
>  {
[ ... ]
> +	struct mm_struct *mm = current->mm;
> +	struct sframe_section *sec;
> +	unsigned long index = 0;
> +	bool found = false;
> +	int ret = 0;
> +
> +	guard(srcu)(&sframe_srcu);

[Severity: Low]
Is sframe_srcu defined anywhere? It doesn't appear to be declared in this
file or its headers, which would cause a build failure.

> diff --git a/mm/init-mm.c b/mm/init-mm.c
[ ... ]
> @@ -49,6 +50,7 @@ struct mm_struct init_mm = {
>  #endif
>  	.flexible_array	= MM_STRUCT_FLEXIBLE_ARRAY_INIT,
>  	INIT_MM_CONTEXT(init_mm)
> +	INIT_MM_SFRAME
>  };

[Severity: Low]
Can this cause compilation failures on non-x86 architectures?

If architectures like PowerPC or UML define INIT_MM_CONTEXT without a
trailing comma, placing INIT_MM_SFRAME (which expands to .sframe_mt = ...)
immediately after it might result in a missing-comma syntax error.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520154004.3845823-1-jremus@linux.ibm.com?part=3

Re: [PATCH v15 05/20] unwind_user/sframe: Add support for reading .sframe contents

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Incorrect FRE size calculation due to using a mutated `dataword_count`.
- [High] Binary search incorrectly reads 64 bits for a 32-bit PC-relative offset, corrupting the function address calculation.
- [High] Unbounded linear scan of user-controlled SFrame FREs can cause a kernel hard lockup.
- [High] Usage of `guard(srcu)` invokes `srcu_read_lock()`, which is not NMI-safe and can corrupt the SRCU lock counter.
- [Medium] Mixing goto-based error handling with scope-based `guard()` cleanup.
--

commit 12c453957aeaea0a5733d7ed09d97cf7dfd380a5
Author: Josh Poimboeuf <jpoimboe@kernel.org>

unwind_user/sframe: Add support for reading .sframe contents

In preparation for using sframe to unwind user space stacks, add an
sframe_find() interface for finding the sframe information associated
with a given text address.

Link: https://lore.kernel.org/all/77c0d1ec143bf2a53d66c4ecb190e7e0a576fbfd.1737511963.git.jpoimboe@kernel.org/
Link: https://lore.kernel.org/all/b35ca3a3-8de5-4d32-8d30-d4e562f6b0de@linux.ibm.com/

> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
> index 6b3ce3f8966d0..a38f50a363633 100644
> --- a/kernel/unwind/sframe.c
> +++ b/kernel/unwind/sframe.c

[ ... ]

> @@ -15,9 +15,350 @@
[ ... ]
> +static __always_inline int __find_fde(struct sframe_section *sec,
> +				      unsigned long ip,
> +				      struct sframe_fde_internal *fde)
> +{
[ ... ]
> +	while (low <= high) {
> +		struct sframe_fde_v3 __user *mid;
> +		s64 func_off;
> +		unsigned long func_addr;
> +
> +		mid = low + ((high - low) / 2);
> +
> +		unsafe_get_user(func_off, (s64 __user *)mid, Efault);

[Severity: High]
Does this read 64 bits for a 32-bit PC-relative offset? Since SFrame V3
specifies a 32-bit signed integer for the PC-relative offset followed by
a 32-bit function size, won't reading 8 bytes merge both fields into
func_off and corrupt the function address calculation?

> +		func_addr = (unsigned long)mid + func_off;

[ ... ]

> +static __always_inline int __read_fre(struct sframe_section *sec,
> +				      struct sframe_fde_internal *fde,
> +				      unsigned long fre_addr,
> +				      struct sframe_fre_internal *fre)
> +{
[ ... ]
> +	UNSAFE_GET_USER_INC(cfa_off, cur, dataword_size, Efault);
> +	dataword_count--;
> +
> +	ra_off = sec->ra_off;
> +	if (!ra_off) {
> +		if (!dataword_count--)
> +			return -EINVAL;
> +
> +		UNSAFE_GET_USER_INC(ra_off, cur, dataword_size, Efault);
> +	}
> +
> +	fp_off = sec->fp_off;
> +	if (!fp_off && dataword_count) {
> +		dataword_count--;
> +		UNSAFE_GET_USER_INC(fp_off, cur, dataword_size, Efault);
> +	}
> +
> +	if (dataword_count)
> +		return -EINVAL;
> +
> +	fre->size	= addr_size + 1 + (dataword_count * dataword_size);

[Severity: High]
Is the FRE size calculation correct here? Since dataword_count is decremented
as datawords are read, and the previous line ensures it is exactly 0, doesn't
this entirely omit the size of the read datawords from the total FRE size?
When __find_fre() advances to the next FRE, this might cause it to point to
the middle of the current FRE's datawords and read garbage.

> +	fre->ip_off	= ip_off;

[ ... ]

> +static __always_inline int __find_fre(struct sframe_section *sec,
> +				      struct sframe_fde_internal *fde,
> +				      unsigned long ip,
> +				      struct unwind_user_frame *frame)
> +{
[ ... ]
> +	fre_addr = sec->fres_start + fde->fres_off;
> +
> +	for (i = 0; i < fde->fres_num; i++) {
> +		int ret;

[Severity: High]
Could this loop cause a kernel hard lockup? fde->fres_num is read from user
space and can be large. Since this performs a linear scan reading each FRE
sequentially from user memory, and unwinding often runs in NMI context (like
perf PMU interrupts), could a maliciously large SFrame section force the
kernel to loop without yielding and trigger the NMI watchdog?

> +
> +		/*
> +		 * Alternate between the two fre_addr[] entries for 'fre' and
> +		 * 'prev_fre'.
> +		 */

[ ... ]

> +int sframe_find(unsigned long ip, struct unwind_user_frame *frame)
> +{
> +	struct mm_struct *mm = current->mm;
> +	struct sframe_section *sec;
> +	struct sframe_fde_internal fde;
> +	int ret;
> +
> +	if (!mm)
> +		return -EINVAL;
> +
> +	guard(srcu)(&sframe_srcu);

[Severity: High]
Is this safe to use from NMI context? Stack unwinding is frequently invoked
from NMIs (like perf PMU interrupts), but srcu_read_lock() is not NMI-safe
on all architectures (like arm64, where the non-atomic read-modify-write
could corrupt the SRCU lock counter). Should this use srcu_read_lock_nmisafe()
or standard RCU instead?

[Severity: Medium]
Does this mix goto-based error handling with scope-based cleanup? The
function declares a scope-based guard here, but uses a goto-based cleanup
label (end:) below. This goes against the cleanup subsystem guidelines
which recommend either converting all resources to scope-based cleanup or
none at all, to avoid confusing ownership semantics.

> +
> +	sec = mtree_load(&mm->sframe_mt, ip);
> +	if (!sec)
> +		return -ENOENT;
> +
> +	if (!user_read_access_begin((void __user *)sec->sframe_start,
> +				    sec->sframe_end - sec->sframe_start))
> +		return -EFAULT;
> +
> +	ret = __find_fde(sec, ip, &fde);
> +	if (ret)
> +		goto end;
> +
> +	ret = __find_fre(sec, &fde, ip, frame);
> +end:
> +	user_read_access_end();
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520154004.3845823-1-jremus@linux.ibm.com?part=5

Re: [PATCH v15 17/20] unwind_user/sframe: Separate reading of FRE from reading of FRE data words

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The `sframe_mt` maple tree is initialized without `MT_FLAGS_USE_RCU`. As a result, maple tree nodes are freed synchronously via `kmem_cache_free()` instead of being deferred by RCU. Lockless concurrent readers using `mtree_load()` (or `mt_for_each()`) can encounter a Use-After-Free if another thread concurrently executes `sframe_remove_section()`.
--

commit 9c045164b82cb49d5d976dfee059454643b1da0a
Author: Jens Remus <jremus@linux.ibm.com>

unwind_user/sframe: Separate reading of FRE from reading of FRE data words

__find_fre() performs linear search for a matching SFrame FRE for a given IP.
For that purpose it uses __read_fre(), which reads the whole FRE.

> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
> index 48709f0bafc78..ec8318977a2ef 100644
> --- a/kernel/unwind/sframe.c
> +++ b/kernel/unwind/sframe.c
> @@ -39,6 +39,9 @@ struct sframe_fre_internal {
>  	u32		fp_ctl;
>  	s32		fp_off;
>  	u8		info;
> +	unsigned long	dw_addr;
> +	unsigned char	dw_count;
> +	unsigned char	dw_size;
>  };
>  
>  DEFINE_STATIC_SRCU(sframe_srcu);

[Severity: Critical]
This is a pre-existing issue, but I noticed that mm->sframe_mt is initialized
via the INIT_MM_SFRAME macro using MTREE_INIT(sframe_mt, 0), which explicitly
leaves out the MT_FLAGS_USE_RCU flag.

Since sframe_find() and sframe_remove_section() execute under the shared
sframe_srcu read lock here, there is no mutual exclusion between a concurrent
reader and writer.

Because MT_FLAGS_USE_RCU is absent, maple tree node deletions triggered by
mtree_erase() in __sframe_remove_section() synchronously free the nodes
via kmem_cache_free() without waiting for an RCU grace period.

Could this lead to a use-after-free if a lockless concurrent reader using
mtree_load() accesses internal tree nodes while a writer is concurrently
removing a section?

If a multithreaded userspace program triggers a stack unwind in one thread
while another concurrently removes a section via prctl, could this cause
memory corruption or kernel panics?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520154004.3845823-1-jremus@linux.ibm.com?part=17

Re: [PATCH v15 20/20] unwind_user/sframe: Add prctl() interface for registering .sframe sections

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The patch introduces a temporary testing UAPI into `prctl` which is explicitly noted by the author as not intended for mainline.
- [High] Concurrent prctl(PR_ADD_SFRAME) and prctl(PR_REMOVE_SFRAME) calls lack synchronization, leading to use-after-free, double-free, and memory leaks.
- [Medium] Unsupported prctl options PR_ADD_SFRAME and PR_REMOVE_SFRAME incorrectly return -ENOSYS instead of -EINVAL when the sframe feature is disabled.
--

commit a01f291288e4c84f6ad3bbf93c1ed0939390a9b8
Author: Josh Poimboeuf <jpoimboe@kernel.org>

unwind_user/sframe: Add prctl() interface for registering .sframe sections

This commit adds new prctl interfaces to allow glibc to inform the kernel
where to find .sframe sections. This provides the kernel with visibility
into the ELF contents of shared libraries.

>     [
>       This adds an interface for prctl() for testing loading of sframes for
>       libraries. But this interface should really be a system call. This patch
>       is for testing purposes only and should not be applied to mainline.
>     ]

[Severity: High]
Should this patch be merged if it creates a permanent UAPI via prctl for a
temporary testing feature? 

The commit message explicitly notes it is for testing purposes only and
should not be applied to mainline. Committing to this interface could result
in userspace relying on a temporary test mechanism.

> diff --git a/kernel/sys.c b/kernel/sys.c
> index 62e842055cc9c..b0a9b1e3ccd7c 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
[ ... ]
> @@ -2907,6 +2908,14 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
>  		if (arg3 & PR_CFI_LOCK && !(arg3 & PR_CFI_DISABLE))
>  			error = arch_prctl_lock_branch_landing_pad_state(me);
>  		break;
> +	case PR_ADD_SFRAME:
> +		error = sframe_add_section(arg2, arg3, arg4, arg5);
> +		break;
> +	case PR_REMOVE_SFRAME:
> +		if (arg3 || arg4 || arg5)
> +			return -EINVAL;
> +		error = sframe_remove_section(arg2);
> +		break;

[Severity: High]
Are concurrent calls to PR_ADD_SFRAME and PR_REMOVE_SFRAME properly
synchronized to prevent double-free or memory leaks?

If sframe_remove_section() only holds an SRCU read lock and
sframe_add_section() only holds mmap_read_lock(), could Thread A and Thread
B concurrently executing PR_REMOVE_SFRAME both find the same section and
erase it?

If Thread A calls mtree_erase() and queues the section for freeing via
call_srcu(), and Thread C concurrently inserts a new section at the same
text_start via PR_ADD_SFRAME:

Thread B might then erase the new section but queue the old section's RCU
callback a second time. Could this sequence lead to a double-free of the
old section and a memory leak of the newly inserted section?

[Severity: Medium]
When the sframe feature is disabled in the kernel configuration, will these
calls return -ENOSYS instead of -EINVAL?

The dummy functions defined in include/linux/sframe.h return -ENOSYS:

static inline int sframe_add_section(unsigned long sframe_start,
                                     unsigned long sframe_end,
                                     unsigned long text_start,
                                     unsigned long text_end)
{
        return -ENOSYS;
}

According to the prctl ABI convention, unsupported options typically fall
through to the default case and return -EINVAL. Could returning -ENOSYS
cause unexpected errors for userspace applications probing for this feature's
availability?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520154004.3845823-1-jremus@linux.ibm.com?part=20

Re: [PATCH v15 18/20] unwind_user/sframe: Duplicate registered .sframe section data on clone/fork

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Mixing goto-based error handling with guard() scope-based cleanup in sframe_dup_mm().
- [Low] Redundant #ifdef blocks around sframe functions in dup_mmap().
--

commit 06e12a491c0dc4cc93fac24f7de9fe0b02b9a9e0
Author: Jens Remus <jremus@linux.ibm.com>
Subject: unwind_user/sframe: Duplicate registered .sframe section data on clone/fork

When duplicating a process' virtual memory mappings also duplicate all
of its registered .sframe sections stored in the per-mm maple tree to
enable stacktracing using sframe of the child process.

> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
> index ec8318977a2ef..0661090dd5bcd 100644
> --- a/kernel/unwind/sframe.c
> +++ b/kernel/unwind/sframe.c
[ ... ]
> @@ -886,6 +886,54 @@ int sframe_remove_section(unsigned long sframe_start)
[ ... ]
> +int sframe_dup_mm(struct mm_struct *mm, struct mm_struct *oldmm)
> +{
> +	struct sframe_section *sec, *oldsec;
> +	unsigned long index = 0;
> +	int ret;
> +
> +	guard(srcu)(&sframe_srcu);
> +
> +	mt_for_each(&oldmm->sframe_mt, oldsec, index, ULONG_MAX) {
> +		sec = kzalloc(sizeof(*sec), GFP_KERNEL_ACCOUNT);
> +		if (!sec)
> +			return -ENOMEM;
> +
> +		__sframe_dup_section(sec, oldsec);
> +
> +		ret = mtree_insert_range(&mm->sframe_mt,
> +					 sec->text_start,
> +					 sec->text_end - 1,
> +					 sec, GFP_KERNEL_ACCOUNT);
> +		if (ret)
> +			goto err_free;

[Severity: Medium]
Is it intentional to mix goto-based error handling with scope-based cleanup
in the same function?

The cleanup subsystem guidelines recommend against mixing goto labels and
guard declarations to prevent confusing ownership semantics and lifecycle
bugs. Would it be better to drop the err_free label entirely and explicitly
call free_section(sec) followed by returning ret directly inside this block?

> +	}
> +
> +	return 0;
> +
> +err_free:
> +	free_section(sec);
> +	return ret;
> +}
[ ... ]
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 2311ae7c2ff45..d5aa8c54f8c1c 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
[ ... ]
> @@ -1846,6 +1847,11 @@ __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
>  	}
>  	/* a new mm has just been created */
>  	retval = arch_dup_mmap(oldmm, mm);
> +#ifdef CONFIG_HAVE_UNWIND_USER_SFRAME
> +	if (!retval) {
> +		retval = sframe_dup_mm(mm, oldmm);
> +	}
> +#endif

[Severity: Low]
This isn't a bug, but are these preprocessor guards necessary?

Since include/linux/sframe.h already provides static inline dummy versions
of sframe_dup_mm() and sframe_free_mm() when CONFIG_HAVE_UNWIND_USER_SFRAME
is disabled, these conditional blocks could potentially be removed to keep
the C source code cleaner.

>  loop_out:
>  	vma_iter_free(&vmi);
>  	if (!retval) {
[ ... ]
> @@ -1893,6 +1899,9 @@ __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
>  			vm_unacct_memory(charge);
>  		}
>  		__mt_destroy(&mm->mm_mt);
> +#ifdef CONFIG_HAVE_UNWIND_USER_SFRAME
> +		sframe_free_mm(mm);
> +#endif

[Severity: Low]
This isn't a bug, but similarly, could this block be dropped since the dummy
stub handles the disabled case without needing explicit conditionals?

>  		/*
>  		 * The mm_struct is going to exit, but the locks will be dropped
>  		 * first.  Set the mm_struct as unstable is advisable as it is

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520154004.3845823-1-jremus@linux.ibm.com?part=18

Re: [PATCH v15 16/20] unwind_user/sframe: Add support for SFrame V3 flexible FDEs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The patch references undefined macros `SFRAME_REG_SP` and `SFRAME_REG_FP` and includes a nonexistent architecture-specific header `<asm/unwind_user_sframe.h>`, resulting in a build failure.
- [Low] The variable `ret` is used outside of its declared scope in `__find_fre()`, resulting in a compilation error.
--

commit 725751526853387385ad4dffd9fdbe24d42e98d3
Author: Jens Remus <jremus@linux.ibm.com>

unwind_user/sframe: Add support for SFrame V3 flexible FDEs

SFrame V3 introduces flexible FDEs in addition to the regular FDEs.
The key difference is that flexible FDEs encode the CFA, RA, and FP
tracking information using two FRE data words, a control word and an
offset, or a single padding data word of zero.

> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
> index 6187379750db5..48709f0bafc78 100644
> --- a/kernel/unwind/sframe.c
> +++ b/kernel/unwind/sframe.c
> @@ -12,6 +12,7 @@
>  #include <linux/mm.h>
>  #include <linux/string_helpers.h>
>  #include <linux/sframe.h>
> +#include <asm/unwind_user_sframe.h>

[Severity: Low]
Does this new architecture-specific header exist in the tree at this point?

It appears this include, along with macros like SFRAME_REG_SP and
SFRAME_REG_FP used later in the patch, causes a build failure because they
aren't introduced until a later commit (bfea14fc9b480 "unwind_user/sframe/x86:
Enable sframe unwinding on x86").

Each patch in a series should be self-contained and compile successfully on
its own.

[ ... ]

> @@ -356,9 +528,15 @@ static __always_inline int __find_fre(struct sframe_section *sec,
>  		return -EINVAL;
>  	fre = prev_fre;
>  
> -	sframe_init_cfa_rule_data(&frame->cfa, fre->info, fre->cfa_off);
> -	sframe_init_rule_data(&frame->ra, fre->ra_off);
> -	sframe_init_rule_data(&frame->fp, fre->fp_off);
> +	ret = sframe_init_cfa_rule_data(&frame->cfa, fre->cfa_ctl, fre->cfa_off);
        ^^^

[Severity: Low]
Is the variable ret in scope here?

It looks like int ret is declared inside the for loop in __find_fre(), so
accessing it outside the loop results in a compilation error.

This is fixed in a later commit (9c045164b82cb "unwind_user/sframe: Separate
reading of FRE from reading of FRE data words") which moves the declaration to
the function scope, but it breaks the build on this commit.

> +	if (ret)
> +		return ret;
> +	ret = sframe_init_rule_data(&frame->ra, fre->ra_ctl, fre->ra_off);
> +	if (ret)
> +		return ret;
> +	ret = sframe_init_rule_data(&frame->fp, fre->fp_ctl, fre->fp_off);
> +	if (ret)
> +		return ret;
>  	frame->outermost = SFRAME_V3_FRE_RA_UNDEFINED_P(fre->info);
>  
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520154004.3845823-1-jremus@linux.ibm.com?part=16

Re: [PATCH v15 02/20] unwind_user/sframe: Add support for reading .sframe headers

[Jens Remus]

On 5/20/2026 6:02 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Integer overflow in SFrame section offset and size calculations allows malformed headers to bypass bounds checks on 32-bit architectures.
Can be ignored.  SFrame only supports 64-bit architectures.  "[PATCH v15
05/20] unwind_user/sframe: Add support for reading .sframe contents"
therefore adds the following safeguard:

	BUILD_BUG_ON(!IS_ENABLED(CONFIG_64BIT));

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/

Re: [PATCH v15 03/20] unwind_user/sframe: Store .sframe section data in per-mm maple tree

[Jens Remus]

On 5/20/2026 6:29 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> - [High] Missing `MT_FLAGS_USE_RCU` during maple tree initialization causes Use-After-Free of tree internal nodes.
> - [High] Synchronous `kfree()` of `sframe_section` bypasses SRCU/RCU grace periods, causing Use-After-Free for concurrent readers.
> - [Low] Undeclared identifier `sframe_srcu` causes build failure.
> - [Low] Macro expansion of `INIT_MM_SFRAME` breaks compilation on non-x86 architectures.
> --
> 
> commit ce5ec4616b79b2c0aa851c150b33143b5d3876af
> Author: Josh Poimboeuf <jpoimboe@kernel.org>
> 
> unwind_user/sframe: Store .sframe section data in per-mm maple tree
> 
> Associate an .sframe section with its mm by adding it to a per-mm maple
> tree which is indexed by the corresponding text address range. A single
> .sframe section can be associated with multiple text ranges.
> 
>> diff --git a/include/linux/sframe.h b/include/linux/sframe.h
> [ ... ]
>> @@ -19,18 +21,31 @@ struct sframe_section {
>>  	signed char	fp_off;
>>  };
>>  
>> +#define INIT_MM_SFRAME .sframe_mt = MTREE_INIT(sframe_mt, 0),
> 
> [Severity: High]
> Does this maple tree need to be initialized with MT_FLAGS_USE_RCU?
> 
> Without it, the tree operates in non-RCU mode, so lockless iterations
> might race with in-place node modifications and synchronous node frees,
> potentially causing use-after-free of the internal nodes.

Steven, please advise.

> 
>> diff --git a/kernel/fork.c b/kernel/fork.c
> [ ... ]
>> @@ -1073,6 +1075,13 @@ static void mmap_init_lock(struct mm_struct *mm)
>>  #endif
>>  }
>>  
>> +static void mm_init_sframe(struct mm_struct *mm)
>> +{
>> +#ifdef CONFIG_HAVE_UNWIND_USER_SFRAME
>> +	mt_init(&mm->sframe_mt);
> 
> [Severity: High]
> Similar to MTREE_INIT above, does mt_init() need to use mt_init_flags()
> with MT_FLAGS_USE_RCU here to prevent use-after-free during lockless reads?

Likewise.

> 
>> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
> [ ... ]
>> +static int __sframe_remove_section(struct mm_struct *mm,
>> +				   struct sframe_section *sec)
>> +{
>> +	if (!mtree_erase(&mm->sframe_mt, sec->text_start)) {
>> +		dbg("mtree_erase failed: text=%lx\n", sec->text_start);
>> +		return -EINVAL;
>> +	}
>> +
>> +	free_section(sec);

I'll move the SRCU logic from patch "[PATCH v15 05/20] unwind_user/
sframe: Add support for reading .sframe contents" here:

	call_srcu(&sframe_srcu, &sec->rcu, sframe_free_srcu);

> 
> [Severity: High]
> Can this synchronous free cause a use-after-free for concurrent readers?
> 
> If an unwinder or other thread is iterating the tree locklessly and has
> already obtained the section pointer before it is erased, freeing it
> immediately here without waiting for an RCU or SRCU grace period would
> pull the structure out from under them.

This will be resolved by above and below.

> 
>> +
>> +	return 0;
>> +}
>> +
>>  int sframe_remove_section(unsigned long sframe_start)
>>  {
> [ ... ]
>> +	struct mm_struct *mm = current->mm;
>> +	struct sframe_section *sec;
>> +	unsigned long index = 0;
>> +	bool found = false;
>> +	int ret = 0;
>> +
>> +	guard(srcu)(&sframe_srcu);
> 
> [Severity: Low]
> Is sframe_srcu defined anywhere? It doesn't appear to be declared in this
> file or its headers, which would cause a build failure.

My bad!  I'll move the definition of sframe_srcu and sframe_free_srcu()
from "[PATCH v15 05/20] unwind_user/sframe: Add support for reading
.sframe contents" to this patch.

> 
>> diff --git a/mm/init-mm.c b/mm/init-mm.c
> [ ... ]
>> @@ -49,6 +50,7 @@ struct mm_struct init_mm = {
>>  #endif
>>  	.flexible_array	= MM_STRUCT_FLEXIBLE_ARRAY_INIT,
>>  	INIT_MM_CONTEXT(init_mm)
>> +	INIT_MM_SFRAME
>>  };
> 
> [Severity: Low]
> Can this cause compilation failures on non-x86 architectures?
> 
> If architectures like PowerPC or UML define INIT_MM_CONTEXT without a
> trailing comma, placing INIT_MM_SFRAME (which expands to .sframe_mt = ...)
> immediately after it might result in a missing-comma syntax error.
Possibly.  This has not changed inbetween versions of this patch.  So if
that happens it will need fixing in the affected arch's definition of
INIT_MM_CONTEXT.

Thanks and regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/

Re: [PATCH v15 05/20] unwind_user/sframe: Add support for reading .sframe contents

[Jens Remus]

On 5/20/2026 6:33 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> - [High] Incorrect FRE size calculation due to using a mutated `dataword_count`.
> - [High] Binary search incorrectly reads 64 bits for a 32-bit PC-relative offset, corrupting the function address calculation.
> - [High] Unbounded linear scan of user-controlled SFrame FREs can cause a kernel hard lockup.
> - [High] Usage of `guard(srcu)` invokes `srcu_read_lock()`, which is not NMI-safe and can corrupt the SRCU lock counter.
> - [Medium] Mixing goto-based error handling with scope-based `guard()` cleanup.
> --
> 
> commit 12c453957aeaea0a5733d7ed09d97cf7dfd380a5
> Author: Josh Poimboeuf <jpoimboe@kernel.org>
> 
> unwind_user/sframe: Add support for reading .sframe contents
> 
> In preparation for using sframe to unwind user space stacks, add an
> sframe_find() interface for finding the sframe information associated
> with a given text address.
> 
> Link: https://lore.kernel.org/all/77c0d1ec143bf2a53d66c4ecb190e7e0a576fbfd.1737511963.git.jpoimboe@kernel.org/
> Link: https://lore.kernel.org/all/b35ca3a3-8de5-4d32-8d30-d4e562f6b0de@linux.ibm.com/
> 
>> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
>> index 6b3ce3f8966d0..a38f50a363633 100644
>> --- a/kernel/unwind/sframe.c
>> +++ b/kernel/unwind/sframe.c
> 
> [ ... ]
> 
>> @@ -15,9 +15,350 @@
> [ ... ]
>> +static __always_inline int __find_fde(struct sframe_section *sec,
>> +				      unsigned long ip,
>> +				      struct sframe_fde_internal *fde)
>> +{
> [ ... ]
>> +	while (low <= high) {
>> +		struct sframe_fde_v3 __user *mid;
>> +		s64 func_off;
>> +		unsigned long func_addr;
>> +
>> +		mid = low + ((high - low) / 2);
>> +
>> +		unsafe_get_user(func_off, (s64 __user *)mid, Efault);
> 
> [Severity: High]
> Does this read 64 bits for a 32-bit PC-relative offset? Since SFrame V3
> specifies a 32-bit signed integer for the PC-relative offset followed by
> a 32-bit function size, won't reading 8 bytes merge both fields into
> func_off and corrupt the function address calculation?

Ignore.  Sashiko AI is hallucinating that the SFrame FDE function start
offset would be signed 32-bit in SFrame V3, as it had been in SFrame V2.
With SFrame V3 it is always signed 64-bit.

> 
>> +		func_addr = (unsigned long)mid + func_off;
> 
> [ ... ]
> 
>> +static __always_inline int __read_fre(struct sframe_section *sec,
>> +				      struct sframe_fde_internal *fde,
>> +				      unsigned long fre_addr,
>> +				      struct sframe_fre_internal *fre)
>> +{
> [ ... ]
>> +	UNSAFE_GET_USER_INC(cfa_off, cur, dataword_size, Efault);
>> +	dataword_count--;
>> +
>> +	ra_off = sec->ra_off;
>> +	if (!ra_off) {
>> +		if (!dataword_count--)
>> +			return -EINVAL;
>> +
>> +		UNSAFE_GET_USER_INC(ra_off, cur, dataword_size, Efault);
>> +	}
>> +
>> +	fp_off = sec->fp_off;
>> +	if (!fp_off && dataword_count) {
>> +		dataword_count--;
>> +		UNSAFE_GET_USER_INC(fp_off, cur, dataword_size, Efault);
>> +	}
>> +
>> +	if (dataword_count)
>> +		return -EINVAL;
>> +
>> +	fre->size	= addr_size + 1 + (dataword_count * dataword_size);
> 
> [Severity: High]
> Is the FRE size calculation correct here? Since dataword_count is decremented
> as datawords are read, and the previous line ensures it is exactly 0, doesn't
> this entirely omit the size of the read datawords from the total FRE size?
> When __find_fre() advances to the next FRE, this might cause it to point to
> the middle of the current FRE's datawords and read garbage.

Valid, but resolved by subsequent "[PATCH v15 16/20] unwind_user/sframe:
Add support for SFrame V3 flexible FDEs".  Will nevertheless fix in next
version.

> 
>> +	fre->ip_off	= ip_off;
> 
> [ ... ]
> 
>> +static __always_inline int __find_fre(struct sframe_section *sec,
>> +				      struct sframe_fde_internal *fde,
>> +				      unsigned long ip,
>> +				      struct unwind_user_frame *frame)
>> +{
> [ ... ]
>> +	fre_addr = sec->fres_start + fde->fres_off;
>> +
>> +	for (i = 0; i < fde->fres_num; i++) {
>> +		int ret;
> 
> [Severity: High]
> Could this loop cause a kernel hard lockup? fde->fres_num is read from user
> space and can be large. Since this performs a linear scan reading each FRE
> sequentially from user memory, and unwinding often runs in NMI context (like
> perf PMU interrupts), could a maliciously large SFrame section force the
> kernel to loop without yielding and trigger the NMI watchdog?

Ignore.  Stacktracing using SFrame is only performed deferred and thus
not in NMI context.

> 
>> +
>> +		/*
>> +		 * Alternate between the two fre_addr[] entries for 'fre' and
>> +		 * 'prev_fre'.
>> +		 */
> 
> [ ... ]
> 
>> +int sframe_find(unsigned long ip, struct unwind_user_frame *frame)
>> +{
>> +	struct mm_struct *mm = current->mm;
>> +	struct sframe_section *sec;
>> +	struct sframe_fde_internal fde;
>> +	int ret;
>> +
>> +	if (!mm)
>> +		return -EINVAL;
>> +
>> +	guard(srcu)(&sframe_srcu);
> 
> [Severity: High]
> Is this safe to use from NMI context? Stack unwinding is frequently invoked
> from NMIs (like perf PMU interrupts), but srcu_read_lock() is not NMI-safe
> on all architectures (like arm64, where the non-atomic read-modify-write
> could corrupt the SRCU lock counter). Should this use srcu_read_lock_nmisafe()
> or standard RCU instead?

Ignore.  See above.

> 
> [Severity: Medium]
> Does this mix goto-based error handling with scope-based cleanup? The
> function declares a scope-based guard here, but uses a goto-based cleanup
> label (end:) below. This goes against the cleanup subsystem guidelines
> which recommend either converting all resources to scope-based cleanup or
> none at all, to avoid confusing ownership semantics.

The scope-based cleanup is used to guard the read-access to the struct
sframe_section.  The goto-based error handling is used to guard the
user read access.

> 
>> +
>> +	sec = mtree_load(&mm->sframe_mt, ip);
>> +	if (!sec)
>> +		return -ENOENT;
>> +
>> +	if (!user_read_access_begin((void __user *)sec->sframe_start,
>> +				    sec->sframe_end - sec->sframe_start))
>> +		return -EFAULT;
>> +
>> +	ret = __find_fde(sec, ip, &fde);
>> +	if (ret)
>> +		goto end;
>> +
>> +	ret = __find_fre(sec, &fde, ip, frame);
>> +end:
>> +	user_read_access_end();
>> +	return ret;
>> +}

Will convert to scoped_user_read_access_size() as follows:

diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
@@ -348,18 +348,18 @@ int sframe_find(unsigned long ip, struct unwind_user_frame *frame)
        if (!sec)
                return -ENOENT;

-       if (!user_read_access_begin((void __user *)sec->sframe_start,
-                                   sec->sframe_end - sec->sframe_start))
-               return -EFAULT;
-
-       ret = __find_fde(sec, ip, &fde);
-       if (ret)
-               goto end;
+       scoped_user_read_access_size((void __user *)sec->sframe_start,
+                                    sec->sframe_end - sec->sframe_start,
+                                    Efault) {
+               ret = __find_fde(sec, ip, &fde);
+               if (!ret)
+                       ret = __find_fre(sec, &fde, ip, frame);
+       }

-       ret = __find_fre(sec, &fde, ip, frame);
-end:
-       user_read_access_end();
        return ret;
+
+Efault:
+       return Efault;
 }

 static void free_section(struct sframe_section *sec)

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/

Re: [PATCH v15 07/20] unwind_user/sframe: Wire up unwind_user to sframe

[Jens Remus]

On 5/20/2026 6:23 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Incorrect FRE size calculation completely breaks SFrame parsing for functions with multiple FREs.

Fixed in next version of "[PATCH v15 05/20] unwind_user/sframe: Add
support for reading .sframe contents".

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/

Re: [PATCH v15 08/20] unwind_user: Stop when reaching an outermost frame

[Jens Remus]

On 5/20/2026 6:01 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Uninitialized stack variable read of `outermost` field causes premature termination of user space stack unwinding.

Ignore, as discussed in v14 review, as subsequent "[PATCH v15 09/20]
unwind_user/sframe: Add support for outermost frame indication" resolves
the issue.

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/

Re: [PATCH v15 09/20] unwind_user/sframe: Add support for outermost frame indication

[Jens Remus]

On 5/20/2026 6:01 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The size calculation for SFrame FRE evaluates to just the header size, incorrectly omitting the size of the datawords because it uses the mutated `dataword_count` variable.

Fixed in next version of "[PATCH v15 05/20] unwind_user/sframe: Add
support for reading .sframe contents".

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/

Re: [PATCH v15 11/20] unwind_user/sframe: Show file name in debug output

[Jens Remus]

On 5/20/2026 6:14 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [Low] Incorrect use of `vma->vm_file` instead of `!vma_is_anonymous(vma)` to classify a VMA as file-backed.

Steven, Josh, please advise whether to apply the changes below.

> 
> Pre-existing issues:
> - [Critical] Calling `sframe_remove_section` from `sframe_find` acquires a spinlock in NMI context, leading to deadlocks, and triggers a user-controllable WARN.

Invalid.  Stacktracing using SFrame is only performed deferred and
therefore never in NMI context.

> --
> 
> commit 60b05617f9a9c00f287a96dda25f8e4086c6178b
> Author: Josh Poimboeuf <jpoimboe@kernel.org>
> 
> unwind_user/sframe: Show file name in debug output
> 
> This commit prefixes debug output strings with the file name to make sframe
> debugging easier. It looks up the VMA corresponding to the sframe section
> and extracts the filename based on the VMA properties.

>> diff --git a/kernel/unwind/sframe_debug.h b/kernel/unwind/sframe_debug.h
>> --- a/kernel/unwind/sframe_debug.h
>> +++ b/kernel/unwind/sframe_debug.h
> [ ... ]
>> +static inline void dbg_init(struct sframe_section *sec)
>> +{
>> +	struct mm_struct *mm = current->mm;
>> +	struct vm_area_struct *vma;
>> +
>> +	guard(mmap_read_lock)(mm);
>> +	vma = vma_lookup(mm, sec->sframe_start);
>> +	if (!vma)
>> +		sec->filename = kstrdup("(vma gone???)", GFP_KERNEL_ACCOUNT);

Should vma_is_anonymous() be explicitly tested?  Currently an anonymous
VMA is assumed as last resort (in the final else clause).

	else if (vma_is_anonymous(vma))
		sec->filename = kstrdup("(anonymous)", GFP_KERNEL_ACCOUNT);

>> +	else if (vma->vm_file)
>> +		sec->filename = kstrdup_quotable_file(vma->vm_file, GFP_KERNEL_ACCOUNT);
> 
> [Severity: Low]
> Does this code accurately classify file-backed VMAs?
> 
> Anonymous mappings, such as private mappings of /dev/zero, can also have
> vm_file populated. Relying only on vm_file might misclassify these mappings
> for the debug output.
> 
> Would it be more reliable to use !vma_is_anonymous(vma) to check if the
> VMA is file-backed before accessing vm_file?
> 
>> +	else if (vma->vm_ops && vma->vm_ops->name)

Shashiko AI review of v14 brought up that vma->vm_ops->name(vma) may
return NULL.  Should this be explicitly handled as follows?

	else if (vma->vm_ops && vma->vm_ops->name && vma->vm_ops->name(vma))

>> +		sec->filename = kstrdup(vma->vm_ops->name(vma), GFP_KERNEL_ACCOUNT);
> [ ... ]

If explicitly testing for vma_is_anonymous() above, then the last resort
would be something unknown?

	else
		sec->filename = kstrdup("(unknown vma???)", GFP_KERNEL);

Thanks and regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/

Re: [PATCH v15 15/20] unwind_user: Flexible CFA recovery rules

[Jens Remus]

On 5/20/2026 6:22 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The size of an SFrame Frame Row Entry (FRE) is incorrectly calculated because the `dataword_count` variable is decremented to zero before the size calculation is performed.

Fixed in next version of "[PATCH v15 05/20] unwind_user/sframe: Add
support for reading .sframe contents".

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/

Re: [PATCH v15 16/20] unwind_user/sframe: Add support for SFrame V3 flexible FDEs

[Jens Remus]

On 5/20/2026 7:04 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Low] The patch references undefined macros `SFRAME_REG_SP` and `SFRAME_REG_FP` and includes a nonexistent architecture-specific header `<asm/unwind_user_sframe.h>`, resulting in a build failure.
> - [Low] The variable `ret` is used outside of its declared scope in `__find_fre()`, resulting in a compilation error.
> --
> 
> commit 725751526853387385ad4dffd9fdbe24d42e98d3
> Author: Jens Remus <jremus@linux.ibm.com>
> 
> unwind_user/sframe: Add support for SFrame V3 flexible FDEs
> 
> SFrame V3 introduces flexible FDEs in addition to the regular FDEs.
> The key difference is that flexible FDEs encode the CFA, RA, and FP
> tracking information using two FRE data words, a control word and an
> offset, or a single padding data word of zero.
> 
>> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
>> index 6187379750db5..48709f0bafc78 100644
>> --- a/kernel/unwind/sframe.c
>> +++ b/kernel/unwind/sframe.c
>> @@ -12,6 +12,7 @@
>>  #include <linux/mm.h>
>>  #include <linux/string_helpers.h>
>>  #include <linux/sframe.h>
>> +#include <asm/unwind_user_sframe.h>
> 
> [Severity: Low]
> Does this new architecture-specific header exist in the tree at this point?
> 
> It appears this include, along with macros like SFRAME_REG_SP and
> SFRAME_REG_FP used later in the patch, causes a build failure because they
> aren't introduced until a later commit (bfea14fc9b480 "unwind_user/sframe/x86:
> Enable sframe unwinding on x86").
> 
> Each patch in a series should be self-contained and compile successfully on
> its own.

This is intended.  kernel/unwind/sframe.o is only built if an arch
enables HAVE_UNWIND_USER_SFRAME and provides an arch-specific
asm/unwind_user_sframe.h header that defines at least SFRAME_REG_SP and
SFRAME_REG_FP.

> 
> [ ... ]
> 
>> @@ -356,9 +528,15 @@ static __always_inline int __find_fre(struct sframe_section *sec,
>>  		return -EINVAL;
>>  	fre = prev_fre;
>>  
>> -	sframe_init_cfa_rule_data(&frame->cfa, fre->info, fre->cfa_off);
>> -	sframe_init_rule_data(&frame->ra, fre->ra_off);
>> -	sframe_init_rule_data(&frame->fp, fre->fp_off);
>> +	ret = sframe_init_cfa_rule_data(&frame->cfa, fre->cfa_ctl, fre->cfa_off);
>         ^^^
> 
> [Severity: Low]
> Is the variable ret in scope here?
> 
> It looks like int ret is declared inside the for loop in __find_fre(), so
> accessing it outside the loop results in a compilation error.
> 
> This is fixed in a later commit (9c045164b82cb "unwind_user/sframe: Separate
> reading of FRE from reading of FRE data words") which moves the declaration to
> the function scope, but it breaks the build on this commit.

Will fix in next version.

> 
>> +	if (ret)
>> +		return ret;
>> +	ret = sframe_init_rule_data(&frame->ra, fre->ra_ctl, fre->ra_off);
>> +	if (ret)
>> +		return ret;
>> +	ret = sframe_init_rule_data(&frame->fp, fre->fp_ctl, fre->fp_off);
>> +	if (ret)
>> +		return ret;
>>  	frame->outermost = SFRAME_V3_FRE_RA_UNDEFINED_P(fre->info);
>>  
>>  	return 0;
> 

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/

Re: [PATCH v15 18/20] unwind_user/sframe: Duplicate registered .sframe section data on clone/fork

[Jens Remus]

On 5/20/2026 7:01 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] Mixing goto-based error handling with guard() scope-based cleanup in sframe_dup_mm().
> - [Low] Redundant #ifdef blocks around sframe functions in dup_mmap().
> --
> 
> commit 06e12a491c0dc4cc93fac24f7de9fe0b02b9a9e0
> Author: Jens Remus <jremus@linux.ibm.com>
> Subject: unwind_user/sframe: Duplicate registered .sframe section data on clone/fork
> 
> When duplicating a process' virtual memory mappings also duplicate all
> of its registered .sframe sections stored in the per-mm maple tree to
> enable stacktracing using sframe of the child process.
> 
>> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
>> index ec8318977a2ef..0661090dd5bcd 100644
>> --- a/kernel/unwind/sframe.c
>> +++ b/kernel/unwind/sframe.c
> [ ... ]
>> @@ -886,6 +886,54 @@ int sframe_remove_section(unsigned long sframe_start)
> [ ... ]
>> +int sframe_dup_mm(struct mm_struct *mm, struct mm_struct *oldmm)
>> +{
>> +	struct sframe_section *sec, *oldsec;
>> +	unsigned long index = 0;
>> +	int ret;
>> +
>> +	guard(srcu)(&sframe_srcu);
>> +
>> +	mt_for_each(&oldmm->sframe_mt, oldsec, index, ULONG_MAX) {
>> +		sec = kzalloc(sizeof(*sec), GFP_KERNEL_ACCOUNT);
>> +		if (!sec)
>> +			return -ENOMEM;
>> +
>> +		__sframe_dup_section(sec, oldsec);
>> +
>> +		ret = mtree_insert_range(&mm->sframe_mt,
>> +					 sec->text_start,
>> +					 sec->text_end - 1,
>> +					 sec, GFP_KERNEL_ACCOUNT);
>> +		if (ret)
>> +			goto err_free;
> 
> [Severity: Medium]
> Is it intentional to mix goto-based error handling with scope-based cleanup
> in the same function?
> 
> The cleanup subsystem guidelines recommend against mixing goto labels and
> guard declarations to prevent confusing ownership semantics and lifecycle
> bugs. Would it be better to drop the err_free label entirely and explicitly
> call free_section(sec) followed by returning ret directly inside this block?

No.  sec must not be unconditionally free'd if it got successfully
inserted into the maple tree.

> 
>> +	}
>> +
>> +	return 0;
>> +
>> +err_free:
>> +	free_section(sec);
>> +	return ret;
>> +}
> [ ... ]
>> diff --git a/mm/mmap.c b/mm/mmap.c
>> index 2311ae7c2ff45..d5aa8c54f8c1c 100644
>> --- a/mm/mmap.c
>> +++ b/mm/mmap.c
> [ ... ]
>> @@ -1846,6 +1847,11 @@ __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
>>  	}
>>  	/* a new mm has just been created */
>>  	retval = arch_dup_mmap(oldmm, mm);
>> +#ifdef CONFIG_HAVE_UNWIND_USER_SFRAME
>> +	if (!retval) {
>> +		retval = sframe_dup_mm(mm, oldmm);
>> +	}
>> +#endif

Will swap the order, so that arch_dup_mmap() is last.

> 
> [Severity: Low]
> This isn't a bug, but are these preprocessor guards necessary?
> 
> Since include/linux/sframe.h already provides static inline dummy versions
> of sframe_dup_mm() and sframe_free_mm() when CONFIG_HAVE_UNWIND_USER_SFRAME
> is disabled, these conditional blocks could potentially be removed to keep
> the C source code cleaner.

Valid.  Will drop the unnecessary #ifdef.

> 
>>  loop_out:
>>  	vma_iter_free(&vmi);
>>  	if (!retval) {
> [ ... ]
>> @@ -1893,6 +1899,9 @@ __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
>>  			vm_unacct_memory(charge);
>>  		}
>>  		__mt_destroy(&mm->mm_mt);
>> +#ifdef CONFIG_HAVE_UNWIND_USER_SFRAME
>> +		sframe_free_mm(mm);
>> +#endif
> 
> [Severity: Low]
> This isn't a bug, but similarly, could this block be dropped since the dummy
> stub handles the disabled case without needing explicit conditionals?
> 
>>  		/*
>>  		 * The mm_struct is going to exit, but the locks will be dropped
>>  		 * first.  Set the mm_struct as unstable is advisable as it is
> 

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/

Re: [PATCH v15 20/20] unwind_user/sframe: Add prctl() interface for registering .sframe sections

[Jens Remus]

On 5/20/2026 6:52 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] The patch introduces a temporary testing UAPI into `prctl` which is explicitly noted by the author as not intended for mainline.

Ignore, as this is for testing only.

> - [High] Concurrent prctl(PR_ADD_SFRAME) and prctl(PR_REMOVE_SFRAME) calls lack synchronization, leading to use-after-free, double-free, and memory leaks.
> - [Medium] Unsupported prctl options PR_ADD_SFRAME and PR_REMOVE_SFRAME incorrectly return -ENOSYS instead of -EINVAL when the sframe feature is disabled.

Steven, something to consider for your upcoming syscall that is supposed
to replace these temporary prctls?


>> diff --git a/kernel/sys.c b/kernel/sys.c
>> index 62e842055cc9c..b0a9b1e3ccd7c 100644
>> --- a/kernel/sys.c
>> +++ b/kernel/sys.c
> [ ... ]
>> @@ -2907,6 +2908,14 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
>>  		if (arg3 & PR_CFI_LOCK && !(arg3 & PR_CFI_DISABLE))
>>  			error = arch_prctl_lock_branch_landing_pad_state(me);
>>  		break;
>> +	case PR_ADD_SFRAME:
>> +		error = sframe_add_section(arg2, arg3, arg4, arg5);
>> +		break;
>> +	case PR_REMOVE_SFRAME:
>> +		if (arg3 || arg4 || arg5)
>> +			return -EINVAL;
>> +		error = sframe_remove_section(arg2);
>> +		break;
> 
> [Severity: High]
> Are concurrent calls to PR_ADD_SFRAME and PR_REMOVE_SFRAME properly
> synchronized to prevent double-free or memory leaks?
> 
> If sframe_remove_section() only holds an SRCU read lock and
> sframe_add_section() only holds mmap_read_lock(), could Thread A and Thread
> B concurrently executing PR_REMOVE_SFRAME both find the same section and
> erase it?
> 
> If Thread A calls mtree_erase() and queues the section for freeing via
> call_srcu(), and Thread C concurrently inserts a new section at the same
> text_start via PR_ADD_SFRAME:
> 
> Thread B might then erase the new section but queue the old section's RCU
> callback a second time. Could this sequence lead to a double-free of the
> old section and a memory leak of the newly inserted section?
> 
> [Severity: Medium]
> When the sframe feature is disabled in the kernel configuration, will these
> calls return -ENOSYS instead of -EINVAL?
> 
> The dummy functions defined in include/linux/sframe.h return -ENOSYS:
> 
> static inline int sframe_add_section(unsigned long sframe_start,
>                                      unsigned long sframe_end,
>                                      unsigned long text_start,
>                                      unsigned long text_end)
> {
>         return -ENOSYS;
> }
> 
> According to the prctl ABI convention, unsupported options typically fall
> through to the default case and return -EINVAL. Could returning -ENOSYS
> cause unexpected errors for userspace applications probing for this feature's
> availability?
> 

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/

Re: [PATCH v15 12/20] unwind_user/sframe: Add .sframe validation option

[Jens Remus]

On 5/20/2026 6:15 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> New issues:
> - [High] Missing cond_resched() or fatal signal checking in the nested loops of sframe_validate_section() allows an unprivileged local attacker to trigger a CPU soft lockup.
> - [Low] The validation logic in sframe_validate_section() incorrectly rejects a valid .sframe section if the first function is located at virtual address 0.
> 
> Pre-existing issues:
> - [High] sframe_find() handles read failures by calling sframe_remove_section(), which acquires a lock inside NMI or IRQ context, leading to deadlocks or PREEMPT_RT panics.

Ignore.  Stacktracing using SFrame is performed deferred and therefore
never in NMI context.

> --
> 
> commit 07d74be031cf9fade2b9ddd98bfdd8f52c38c2da
> Author: Josh Poimboeuf <jpoimboe@kernel.org>
> 
> unwind_user/sframe: Add .sframe validation option
> 
> This commit introduces a debugging feature to optionally validate the
> entire .sframe section upon loading, rather than checking on-demand.
> It uses user_read_access_begin/end to read FDEs and FREs to verify their
> formatting and sorting.
> 
>> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
>> --- a/kernel/unwind/sframe.c
>> +++ b/kernel/unwind/sframe.c

>> +static int sframe_validate_section(struct sframe_section *sec)
>> +{
>> +	unsigned long prev_ip = 0;
>> +	unsigned int i;
>> +
>> +	for (i = 0; i < sec->num_fdes; i++) {
>> +		struct sframe_fre_internal *fre, *prev_fre = NULL;
>> +		unsigned long ip, fre_addr;
>> +		struct sframe_fde_internal fde;
>> +		struct sframe_fre_internal fres[2];
>> +		bool which = false;
>> +		unsigned int j;
>> +		int ret;
>> +
>> +		ret = safe_read_fde(sec, i, &fde);
>> +		if (ret) {
>> +			dbg_sec("safe_read_fde(%u) failed\n", i);
>> +			return ret;
>> +		}
>> +
>> +		ip = fde.func_addr;
>> +		if (ip <= prev_ip) {
> 
> [Severity: Low]
> Does this check accidentally reject valid sections where the first
> function is located at virtual address 0?
> 
> Since prev_ip is initialized to 0, validating the first FDE when ip is 0
> will cause this check to evaluate to true, falsely triggering an -EINVAL.
> 
> Valid executables can map their text section at 0 if vm.mmap_min_addr is
> set to 0.

Valid.  Will fix in next version.

> 
>> +			dbg_sec("FDE %u not sorted\n", i);
>> +			return -EINVAL;
>> +		}
>> +		prev_ip = ip;
>> +
>> +		fre_addr = sec->fres_start + fde.fres_off;
>> +		for (j = 0; j < fde.fres_num; j++) {
> [ ... ]

Will also replace the alternation between two FREs with a simple
prev_ip_off variable:

>> +			fre_addr += fre->size;
>> +
>> +			if (prev_fre && fre->ip_off <= prev_fre->ip_off) {

			if (j && fre.ip_off <= prev_ip_off) {

>> +				dbg_sec("FDE %u: FRE %u not sorted\n", i, j);
>> +				return -EINVAL;
>> +			}
>> +
>> +			prev_fre = fre;

			prev_ip_off = fre.ip_off;

>> +		}
> 
> [Severity: High]
> Could a malicious user exploit these loops to trigger a CPU soft lockup?
> 
> The variables sec->num_fdes and fde.fres_num are user-controlled and
> bounded only by the VMA size.
> 
> Without a cond_resched() or fatal signal check, an attacker could supply
> large counts and cause these loops to run for seconds without yielding.
> 
> On non-preemptible kernels, this monopolizes the CPU and triggers the soft
> lockup watchdog, resulting in a local denial of service.

cond_resched() is not required according to v14 review feedback from
Steven.

> 
>> +	}
>> +
>> +	return 0;
>> +}
> 

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/

Re: [PATCH v15 13/20] unwind_user: Enable archs that pass RA in a register

[Jens Remus]

On 5/20/2026 6:21 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [Low] The header file `<linux/unwind_user.h>` uses `pr_debug_once` and `current` without including the necessary headers.
> 
> Pre-existing issues:
> - [High] The SFrame FRE size calculation computes an incorrect length because `dataword_count` is consumed before it is used in the size formula.

Fixed in next version of "[PATCH v15 05/20] unwind_user/sframe: Add
support for reading .sframe contents".

> --
> 
> commit 842bbe46528fb50996a02de1f8b7f5df1d8b1026
> Author: Jens Remus <jremus@linux.ibm.com>
> 
> unwind_user: Enable archs that pass RA in a register
> 
> This commit updates the user-space unwinder to support architectures that do
> not mandate saving the return address (RA) on the stack, such as s390 and
> arm64. It treats an RA offset of zero as an indication that the RA is in the
> link register for the topmost frame, and allows the stack pointer to remain
> unchanged in the topmost frame.
> 
>> diff --git a/include/linux/unwind_user.h b/include/linux/unwind_user.h
>> index 64618618febde..7bf58f23aa64a 100644
>> --- a/include/linux/unwind_user.h
>> +++ b/include/linux/unwind_user.h
>> @@ -23,6 +23,16 @@ static inline bool unwind_user_at_function_start(struct pt_regs *regs)
>>  #define unwind_user_at_function_start unwind_user_at_function_start
>>  #endif
>>  
>> +#ifndef unwind_user_get_ra_reg
>> +static inline int unwind_user_get_ra_reg(unsigned long *val)
>> +{
>> +	pr_debug_once("%s (%d): unwind_user_get_ra_reg() not implemented\n",
>> +		      current->comm, current->pid);
> 
> [Severity: Low]
> Does this require including <linux/printk.h> and <linux/sched.h> or
> <asm/current.h>?
> 
> Using pr_debug_once and current without these headers might cause compilation
> failures for files that include <linux/unwind_user.h> without prior inclusion
> of these dependencies.

Can this be ignored, given kernel/unwind/sframe_debug.h does include
neither of these?

> 
>> +	return -EINVAL;
>> +}
>> +#define unwind_user_get_ra_reg unwind_user_get_ra_reg
>> +#endif
> 
> [ ... ]


Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/

Re: [PATCH v15 03/20] unwind_user/sframe: Store .sframe section data in per-mm maple tree

[Steven Rostedt]

On Thu, 21 May 2026 11:39:44 +0200
Jens Remus <jremus@linux.ibm.com> wrote:

> On 5/20/2026 6:29 PM, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> > - [High] Missing `MT_FLAGS_USE_RCU` during maple tree initialization causes Use-After-Free of tree internal nodes.
> > - [High] Synchronous `kfree()` of `sframe_section` bypasses SRCU/RCU grace periods, causing Use-After-Free for concurrent readers.
> > - [Low] Undeclared identifier `sframe_srcu` causes build failure.
> > - [Low] Macro expansion of `INIT_MM_SFRAME` breaks compilation on non-x86 architectures.
> > --
> > 
> > commit ce5ec4616b79b2c0aa851c150b33143b5d3876af
> > Author: Josh Poimboeuf <jpoimboe@kernel.org>
> > 
> > unwind_user/sframe: Store .sframe section data in per-mm maple tree
> > 
> > Associate an .sframe section with its mm by adding it to a per-mm maple
> > tree which is indexed by the corresponding text address range. A single
> > .sframe section can be associated with multiple text ranges.
> >   
> >> diff --git a/include/linux/sframe.h b/include/linux/sframe.h  
> > [ ... ]  
> >> @@ -19,18 +21,31 @@ struct sframe_section {
> >>  	signed char	fp_off;
> >>  };
> >>  
> >> +#define INIT_MM_SFRAME .sframe_mt = MTREE_INIT(sframe_mt, 0),  
> > 
> > [Severity: High]
> > Does this maple tree need to be initialized with MT_FLAGS_USE_RCU?
> > 
> > Without it, the tree operates in non-RCU mode, so lockless iterations
> > might race with in-place node modifications and synchronous node frees,
> > potentially causing use-after-free of the internal nodes.  
> 
> Steven, please advise.
> 
> >   
> >> diff --git a/kernel/fork.c b/kernel/fork.c  
> > [ ... ]  
> >> @@ -1073,6 +1075,13 @@ static void mmap_init_lock(struct mm_struct *mm)
> >>  #endif
> >>  }
> >>  
> >> +static void mm_init_sframe(struct mm_struct *mm)
> >> +{
> >> +#ifdef CONFIG_HAVE_UNWIND_USER_SFRAME
> >> +	mt_init(&mm->sframe_mt);  
> > 
> > [Severity: High]
> > Similar to MTREE_INIT above, does mt_init() need to use mt_init_flags()
> > with MT_FLAGS_USE_RCU here to prevent use-after-free during lockless reads?  
> 
> Likewise.

Looks like the MT_FLAGS_USE_RCU is for internal use of RCU. But if we use
src to protect the tree, I think it should be OK.

> 
> >   
> >> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c  
> > [ ... ]  
> >> +static int __sframe_remove_section(struct mm_struct *mm,
> >> +				   struct sframe_section *sec)
> >> +{
> >> +	if (!mtree_erase(&mm->sframe_mt, sec->text_start)) {
> >> +		dbg("mtree_erase failed: text=%lx\n", sec->text_start);
> >> +		return -EINVAL;
> >> +	}
> >> +
> >> +	free_section(sec);  
> 
> I'll move the SRCU logic from patch "[PATCH v15 05/20] unwind_user/
> sframe: Add support for reading .sframe contents" here:
> 
> 	call_srcu(&sframe_srcu, &sec->rcu, sframe_free_srcu);
> 
> > 
> > [Severity: High]
> > Can this synchronous free cause a use-after-free for concurrent readers?
> > 
> > If an unwinder or other thread is iterating the tree locklessly and has
> > already obtained the section pointer before it is erased, freeing it
> > immediately here without waiting for an RCU or SRCU grace period would
> > pull the structure out from under them.  
> 
> This will be resolved by above and below.

Yeah, I think having this all protected by SRCU from the start will
hopefully satisfy Sashiko.

-- Steve

Re: [PATCH v15 11/20] unwind_user/sframe: Show file name in debug output

[Steven Rostedt]

On Thu, 21 May 2026 12:55:50 +0200
Jens Remus <jremus@linux.ibm.com> wrote:

> >> +static inline void dbg_init(struct sframe_section *sec)
> >> +{
> >> +	struct mm_struct *mm = current->mm;
> >> +	struct vm_area_struct *vma;
> >> +
> >> +	guard(mmap_read_lock)(mm);
> >> +	vma = vma_lookup(mm, sec->sframe_start);
> >> +	if (!vma)
> >> +		sec->filename = kstrdup("(vma gone???)", GFP_KERNEL_ACCOUNT);  
> 
> Should vma_is_anonymous() be explicitly tested?  Currently an anonymous
> VMA is assumed as last resort (in the final else clause).

Yeah, perhaps it should be first.

> 
> 	else if (vma_is_anonymous(vma))
> 		sec->filename = kstrdup("(anonymous)", GFP_KERNEL_ACCOUNT);
> 
> >> +	else if (vma->vm_file)
> >> +		sec->filename = kstrdup_quotable_file(vma->vm_file, GFP_KERNEL_ACCOUNT);  
> > 
> > [Severity: Low]
> > Does this code accurately classify file-backed VMAs?
> > 
> > Anonymous mappings, such as private mappings of /dev/zero, can also have
> > vm_file populated. Relying only on vm_file might misclassify these mappings
> > for the debug output.
> > 
> > Would it be more reliable to use !vma_is_anonymous(vma) to check if the
> > VMA is file-backed before accessing vm_file?

As stated above, we should probably test it first. Maybe even vma->vm_mm too?

	if (!vma->vm_mm)
		sec->filename = kstrdup("(vdso)", GFP_KERNEL_ACCOUNT);
	else if (vma_is_anonymous(vma))
		sec->filename = kstrdup("(anonymous)", GFP_KERNEL_ACCOUNT);
	else if (vm->vm_file)
		[..]


> >   
> >> +	else if (vma->vm_ops && vma->vm_ops->name)  
> 
> Shashiko AI review of v14 brought up that vma->vm_ops->name(vma) may
> return NULL.  Should this be explicitly handled as follows?
> 
> 	else if (vma->vm_ops && vma->vm_ops->name && vma->vm_ops->name(vma))

Maybe we could even shortcut it with:

	const char *name;
	[..]
 	else if (vma->vm_ops && vma->vm_ops->name && (name = vma->vm_ops->name(vma)))
		sec->filename = kstrdup(name, GFP_KERNEL_ACCOUNT);  


> 
> >> +		sec->filename = kstrdup(vma->vm_ops->name(vma), GFP_KERNEL_ACCOUNT);  
> > [ ... ]  
> 
> If explicitly testing for vma_is_anonymous() above, then the last resort
> would be something unknown?
> 
> 	else
> 		sec->filename = kstrdup("(unknown vma???)", GFP_KERNEL);

Sure.

-- Steve