Skip to contentSkip to conversation

unwind, perf: sframe user space unwinding, deferred perf callchains

PATCH Superseded Josh Poimboeuf 12 participants 51 messages Sep 13, 2024 – Oct 27, 2024

Subsystems (10)
Memory management x86 VFS BPF +6 more subsystems

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Josh Poimboeuf

author maintainer
#0 lore raw

v2 cover unwind, perf: sframe user space unwinding, deferred perf callchains

This is a new version of sframe user space unwinding + perf deferred callchains. Better late than never.

Unfortunately I didn't get a chance to do any testing with this one yet as I'm rushing it out the door before GNU Cauldron starts. Conference-driven development ;-)

Plus I don't have the perf tool piece working yet so I don't have a way of doing end-to-end testing at the moment anyway.

In other words, please don't merge this yet.

Namhyung, if you're still available to write a perf tool patch which integrates with this, that would be great. Otherwise I could give it a try.

Steven, let me know if this would interface ok with your anticipated tracing usage.

v2:
- rebase on v6.11-rc7
- reorganize the patches to add sframe first
- change to sframe v2
- add new perf event type: PERF_RECORD_CALLCHAIN_DEFERRED
- add new perf attribute: defer_callchain

v1: https://lore.kernel.org/[email protected]

Some distros have started compiling frame pointers into all their packages to enable the kernel to do system-wide profiling of user space. Unfortunately that creates a runtime performance penalty across the entire system. Using DWARF (or .eh_frame) instead isn't feasible because of complexity and slowness.

For in-kernel unwinding we solved this problem with the creation of the ORC unwinder for x86_64. Similarly, for user space the GNU assembler has created the SFrame ("Simple Frame") v2 format starting with binutils 2.41.

These patches add support for unwinding user space from the kernel using SFrame with perf. It should be easy to add user unwinding support for other components like ftrace.

There were two main challenges:

1) Finding .sframe sections in shared/dlopened libraries

The kernel has no visibility to the contents of shared libraries. This was solved by adding a PR_ADD_SFRAME option to prctl() which allows the runtime linker to manually provide the in-memory address of an .sframe section to the kernel.

2) Dealing with page faults

Keeping all binaries' sframe data pinned would likely waste a lot of memory. Instead, read it from user space on demand. That can't be done from perf NMI context due to page faults, so defer the unwind to the next user exit. Since the NMI handler doesn't do exit work, self-IPI and then schedule task work to be run on exit from the IPI.

Special thanks to Indu for the original concept, and to Steven and Peter for helping a lot with the design. And to Steven for letting me do it ;-)

Josh Poimboeuf (11):
unwind: Introduce generic user space unwinding interface unwind/x86: Add HAVE_USER_UNWIND
unwind: Introduce SFrame user space unwinding unwind/x86/64: Add HAVE_USER_UNWIND_SFRAME
perf/x86: Use user_unwind interface
perf: Remove get_perf_callchain() 'init_nr' argument
perf: Remove get_perf_callchain() 'crosstask' argument
perf: Simplify get_perf_callchain() user logic
perf: Introduce deferred user callchains
perf/x86: Add HAVE_PERF_CALLCHAIN_DEFERRED
perf/x86: Enable SFrame unwinding for deferred user callchains

 arch/Kconfig                       |   9 +
 arch/x86/Kconfig                   |   3 +
 arch/x86/events/core.c             |  76 +++---
 arch/x86/include/asm/user_unwind.h |  11 +
 fs/binfmt_elf.c                    |  47 +++-
 include/linux/mm_types.h           |   3 +
 include/linux/perf_event.h         |  11 +-
 include/linux/sframe.h             |  46 ++++
 include/linux/user_unwind.h        |  32 +++
 include/uapi/linux/elf.h           |   1 +
 include/uapi/linux/perf_event.h    |  21 +-
 include/uapi/linux/prctl.h         |   3 +
 kernel/Makefile                    |   1 +
 kernel/bpf/stackmap.c              |   8 +-
 kernel/events/callchain.c          |  48 ++--
 kernel/events/core.c               |  82 +++++-
 kernel/fork.c                      |  10 +
 kernel/sys.c                       |  11 +
 kernel/unwind/Makefile             |   2 +
 kernel/unwind/sframe.c             | 420 +++++++++++++++++++++++++++++
 kernel/unwind/sframe.h             | 215 +++++++++++++++
 kernel/unwind/user.c               |  95 +++++++
 mm/init-mm.c                       |   4 +-
 23 files changed, 1086 insertions(+), 73 deletions(-)
 create mode 100644 arch/x86/include/asm/user_unwind.h
 create mode 100644 include/linux/sframe.h
 create mode 100644 include/linux/user_unwind.h
 create mode 100644 kernel/unwind/Makefile
 create mode 100644 kernel/unwind/sframe.c
 create mode 100644 kernel/unwind/sframe.h
 create mode 100644 kernel/unwind/user.c
Signature · 1 line
2.46.0
author maintainer
#1 lore raw

PATCH v2 1/11 unwind: Introduce generic user space unwinding interface

Introduce a user space unwinder interface which will provide a generic way for architectures to unwind different user space stack frame types.

---
 arch/Kconfig                |  3 ++
 include/linux/user_unwind.h | 31 ++++++++++++++
 kernel/Makefile             |  1 +
 kernel/unwind/Makefile      |  1 +
 kernel/unwind/user.c        | 81 +++++++++++++++++++++++++++++++++++++
 5 files changed, 117 insertions(+)
 create mode 100644 include/linux/user_unwind.h
 create mode 100644 kernel/unwind/Makefile
 create mode 100644 kernel/unwind/user.c
arch/Kconfig +3 -0
index 975dd22a2dbd..b1002b2da331 100644
@@ -425,6 +425,9 @@ config HAVE_HARDLOCKUP_DETECTOR_ARCH
 	  It uses the same command line parameters, and sysctl interface,
 	  as the generic hardlockup detectors.
 
+config HAVE_USER_UNWIND
+	bool
+
 config HAVE_PERF_REGS
 	bool
 	help
include/linux/user_unwind.h new file +31 -0
new file mode 100644
index 000000000000..0a19ac6c92b2
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_USER_UNWIND_H
+#define _LINUX_USER_UNWIND_H
+
+#include <linux/types.h>
+
+enum user_unwind_type {
+	USER_UNWIND_TYPE_AUTO,
+	USER_UNWIND_TYPE_FP,
+};
+
+struct user_unwind_frame {
+	s32 cfa_off;
+	s32 ra_off;
+	s32 fp_off;
+	bool use_fp;
+};
+
+struct user_unwind_state {
+	unsigned long ip, sp, fp;
+	enum user_unwind_type type;
+	bool done;
+};
+
+extern int user_unwind_start(struct user_unwind_state *state, enum user_unwind_type);
+extern int user_unwind_next(struct user_unwind_state *state);
+
+#define for_each_user_frame(state, type) \
+	for (user_unwind_start(state, type); !state.done; user_unwind_next(state))
+
+#endif /* _LINUX_USER_UNWIND_H */
kernel/Makefile +1 -0
index 3c13240dfc9f..259581df82dd 100644
@@ -50,6 +50,7 @@ obj-y += rcu/
 obj-y += livepatch/
 obj-y += dma/
 obj-y += entry/
+obj-y += unwind/
 obj-$(CONFIG_MODULES) += module/
 
 obj-$(CONFIG_KCMP) += kcmp.o
kernel/unwind/Makefile new file +1 -0
new file mode 100644
index 000000000000..eb466d6a3295
@@ -0,0 +1 @@
+obj-$(CONFIG_HAVE_USER_UNWIND) += user.o
kernel/unwind/user.c new file +81 -0

81 lines not on this page · raw message

Signature · 1 line
2.46.0
author maintainer
#2 lore raw

PATCH v2 2/11 unwind/x86: Add HAVE_USER_UNWIND

In reply to Josh Poimboeuf

Use ARCH_INIT_USER_FP_FRAME to describe how frame pointers are unwound on x86, and enable HAVE_USER_UNWIND accordinlgy so the user unwind interfaces can be used.

---
 arch/x86/Kconfig                   |  1 +
 arch/x86/include/asm/user_unwind.h | 11 +++++++++++
 2 files changed, 12 insertions(+)
 create mode 100644 arch/x86/include/asm/user_unwind.h
arch/x86/Kconfig +1 -0
index 007bab9f2a0e..266edff59058 100644
@@ -286,6 +286,7 @@ config X86
 	select HAVE_UACCESS_VALIDATION		if HAVE_OBJTOOL
 	select HAVE_UNSTABLE_SCHED_CLOCK
 	select HAVE_USER_RETURN_NOTIFIER
+	select HAVE_USER_UNWIND
 	select HAVE_GENERIC_VDSO
 	select VDSO_GETRANDOM			if X86_64
 	select HOTPLUG_PARALLEL			if SMP && X86_64
arch/x86/include/asm/user_unwind.h new file +11 -0
new file mode 100644
index 000000000000..8c509c65cfb5
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_USER_UNWIND_H
+#define _ASM_X86_USER_UNWIND_H
+
+#define ARCH_INIT_USER_FP_FRAME							\
+	.ra_off		= (s32)sizeof(long) * -1,				\
+	.cfa_off	= (s32)sizeof(long) * 2,				\
+	.fp_off		= (s32)sizeof(long) * -2,				\
+	.use_fp		= true,
+
+#endif /* _ASM_X86_USER_UNWIND_H */
Signature · 1 line
2.46.0
#3 lore raw

Re: [PATCH v2 02/11] unwind/x86: Add HAVE_USER_UNWIND

kernel test robot noticed the following build warnings:

Hi Josh,

kernel test robot noticed the following build warnings:

[auto build test WARNING on perf-tools-next/perf-tools-next]
[also build test WARNING on tip/perf/core perf-tools/perf-tools linus/master acme/perf/core v6.11-rc7 next-20240913] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Josh-Poimboeuf/unwind-Introduce-generic-user-space-unwinding-interface/20240914-070619
base:   https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git perf-tools-next
patch link:    https://lore.kernel.org/r/82ef19a767cb75e76a985ecc0d47a39400b4fdf5.1726268190.git.jpoimboe%40kernel.org
patch subject: [PATCH v2 02/11] unwind/x86: Add HAVE_USER_UNWIND
config: x86_64-randconfig-121-20240914 (https://download.01.org/0day-ci/archive/20240914/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240914/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

sparse warnings: (new ones prefixed by >>)

>> kernel/unwind/user.c:38:30: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const volatile [noderef] __user *ptr @@     got unsigned long * @@

kernel/unwind/user.c:38:30: sparse: expected void const volatile [noderef] __user *ptr
kernel/unwind/user.c:38:30: sparse: got unsigned long *
kernel/unwind/user.c:41:30: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const volatile [noderef] __user *ptr @@ got unsigned long * @@
kernel/unwind/user.c:41:30: sparse: expected void const volatile [noderef] __user *ptr
kernel/unwind/user.c:41:30: sparse: got unsigned long *

vim +38 kernel/unwind/user.c

c970b852d500d4 Josh Poimboeuf 2024-09-14  17  
c970b852d500d4 Josh Poimboeuf 2024-09-14  18  int user_unwind_next(struct user_unwind_state *state)
c970b852d500d4 Josh Poimboeuf 2024-09-14  19  {
c970b852d500d4 Josh Poimboeuf 2024-09-14  20  	struct user_unwind_frame _frame;
c970b852d500d4 Josh Poimboeuf 2024-09-14  21  	struct user_unwind_frame *frame = &_frame;
c970b852d500d4 Josh Poimboeuf 2024-09-14  22  	unsigned long cfa, fp, ra;
c970b852d500d4 Josh Poimboeuf 2024-09-14  23  	int ret = -EINVAL;
c970b852d500d4 Josh Poimboeuf 2024-09-14  24  
c970b852d500d4 Josh Poimboeuf 2024-09-14  25  	if (state->done)
c970b852d500d4 Josh Poimboeuf 2024-09-14  26  		return -EINVAL;
c970b852d500d4 Josh Poimboeuf 2024-09-14  27  
c970b852d500d4 Josh Poimboeuf 2024-09-14  28  	switch (state->type) {
c970b852d500d4 Josh Poimboeuf 2024-09-14  29  	case USER_UNWIND_TYPE_FP:
c970b852d500d4 Josh Poimboeuf 2024-09-14  30  		frame = &fp_frame;
c970b852d500d4 Josh Poimboeuf 2024-09-14  31  		break;
c970b852d500d4 Josh Poimboeuf 2024-09-14  32  	default:
c970b852d500d4 Josh Poimboeuf 2024-09-14  33  		BUG();
c970b852d500d4 Josh Poimboeuf 2024-09-14  34  	}
c970b852d500d4 Josh Poimboeuf 2024-09-14  35  
c970b852d500d4 Josh Poimboeuf 2024-09-14  36  	cfa = (frame->use_fp ? state->fp : state->sp) + frame->cfa_off;
c970b852d500d4 Josh Poimboeuf 2024-09-14  37  
c970b852d500d4 Josh Poimboeuf 2024-09-14 @38  	if (frame->ra_off && get_user(ra, (unsigned long *)(cfa + frame->ra_off)))
c970b852d500d4 Josh Poimboeuf 2024-09-14  39  		goto the_end;
c970b852d500d4 Josh Poimboeuf 2024-09-14  40  
c970b852d500d4 Josh Poimboeuf 2024-09-14  41  	if (frame->fp_off && get_user(fp, (unsigned long *)(cfa + frame->fp_off)))
c970b852d500d4 Josh Poimboeuf 2024-09-14  42  		goto the_end;
c970b852d500d4 Josh Poimboeuf 2024-09-14  43  
c970b852d500d4 Josh Poimboeuf 2024-09-14  44  	state->sp = cfa;
c970b852d500d4 Josh Poimboeuf 2024-09-14  45  	state->ip = ra;
c970b852d500d4 Josh Poimboeuf 2024-09-14  46  	if (frame->fp_off)
c970b852d500d4 Josh Poimboeuf 2024-09-14  47  		state->fp = fp;
c970b852d500d4 Josh Poimboeuf 2024-09-14  48  
c970b852d500d4 Josh Poimboeuf 2024-09-14  49  	return 0;
c970b852d500d4 Josh Poimboeuf 2024-09-14  50  
c970b852d500d4 Josh Poimboeuf 2024-09-14  51  the_end:
c970b852d500d4 Josh Poimboeuf 2024-09-14  52  	state->done = true;
c970b852d500d4 Josh Poimboeuf 2024-09-14  53  	return ret;
c970b852d500d4 Josh Poimboeuf 2024-09-14  54  }
c970b852d500d4 Josh Poimboeuf 2024-09-14  55  
Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
#4 lore raw

Re: [PATCH v2 02/11] unwind/x86: Add HAVE_USER_UNWIND

In reply to Josh Poimboeuf

Hi Josh,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Josh-Poimboeuf/unwind-Introduce-generic-user-space-unwinding-interface/20240914-070619
base:   https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git perf-tools-next
patch link:    https://lore.kernel.org/r/82ef19a767cb75e76a985ecc0d47a39400b4fdf5.1726268190.git.jpoimboe%40kernel.org
patch subject: [PATCH v2 02/11] unwind/x86: Add HAVE_USER_UNWIND
config: x86_64-randconfig-r072-20240914 (https://download.01.org/0day-ci/archive/20240915/[email protected]/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags

| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Closes: https://lore.kernel.org/r/[email protected]/

smatch warnings:
kernel/unwind/user.c:45 user_unwind_next() error: uninitialized symbol 'ra'.

vim +/ra +45 kernel/unwind/user.c

c970b852d500d4 Josh Poimboeuf 2024-09-14  18  int user_unwind_next(struct user_unwind_state *state)
c970b852d500d4 Josh Poimboeuf 2024-09-14  19  {
c970b852d500d4 Josh Poimboeuf 2024-09-14  20  	struct user_unwind_frame _frame;
c970b852d500d4 Josh Poimboeuf 2024-09-14  21  	struct user_unwind_frame *frame = &_frame;
c970b852d500d4 Josh Poimboeuf 2024-09-14  22  	unsigned long cfa, fp, ra;
c970b852d500d4 Josh Poimboeuf 2024-09-14  23  	int ret = -EINVAL;
c970b852d500d4 Josh Poimboeuf 2024-09-14  24  
c970b852d500d4 Josh Poimboeuf 2024-09-14  25  	if (state->done)
c970b852d500d4 Josh Poimboeuf 2024-09-14  26  		return -EINVAL;
c970b852d500d4 Josh Poimboeuf 2024-09-14  27  
c970b852d500d4 Josh Poimboeuf 2024-09-14  28  	switch (state->type) {
c970b852d500d4 Josh Poimboeuf 2024-09-14  29  	case USER_UNWIND_TYPE_FP:
c970b852d500d4 Josh Poimboeuf 2024-09-14  30  		frame = &fp_frame;
c970b852d500d4 Josh Poimboeuf 2024-09-14  31  		break;
c970b852d500d4 Josh Poimboeuf 2024-09-14  32  	default:
c970b852d500d4 Josh Poimboeuf 2024-09-14  33  		BUG();
c970b852d500d4 Josh Poimboeuf 2024-09-14  34  	}
c970b852d500d4 Josh Poimboeuf 2024-09-14  35  
c970b852d500d4 Josh Poimboeuf 2024-09-14  36  	cfa = (frame->use_fp ? state->fp : state->sp) + frame->cfa_off;
c970b852d500d4 Josh Poimboeuf 2024-09-14  37  
c970b852d500d4 Josh Poimboeuf 2024-09-14  38  	if (frame->ra_off && get_user(ra, (unsigned long *)(cfa + frame->ra_off)))
c970b852d500d4 Josh Poimboeuf 2024-09-14  39  		goto the_end;

ra not initialized on else path. Just do "= 0". These days the compiler is going initialize everything to zero anyway.

c970b852d500d4 Josh Poimboeuf 2024-09-14  40  
c970b852d500d4 Josh Poimboeuf 2024-09-14  41  	if (frame->fp_off && get_user(fp, (unsigned long *)(cfa + frame->fp_off)))
c970b852d500d4 Josh Poimboeuf 2024-09-14  42  		goto the_end;
c970b852d500d4 Josh Poimboeuf 2024-09-14  43  
c970b852d500d4 Josh Poimboeuf 2024-09-14  44  	state->sp = cfa;
c970b852d500d4 Josh Poimboeuf 2024-09-14 @45  	state->ip = ra;
c970b852d500d4 Josh Poimboeuf 2024-09-14  46  	if (frame->fp_off)
c970b852d500d4 Josh Poimboeuf 2024-09-14  47  		state->fp = fp;
c970b852d500d4 Josh Poimboeuf 2024-09-14  48  
c970b852d500d4 Josh Poimboeuf 2024-09-14  49  	return 0;
c970b852d500d4 Josh Poimboeuf 2024-09-14  50  
c970b852d500d4 Josh Poimboeuf 2024-09-14  51  the_end:
c970b852d500d4 Josh Poimboeuf 2024-09-14  52  	state->done = true;
c970b852d500d4 Josh Poimboeuf 2024-09-14  53  	return ret;
c970b852d500d4 Josh Poimboeuf 2024-09-14  54  }
Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
maintainer
#5 lore raw

Re: [PATCH v2 02/11] unwind/x86: Add HAVE_USER_UNWIND

In reply to Josh Poimboeuf
Quoted 29 lines · Josh Poimboeuf

Use ARCH_INIT_USER_FP_FRAME to describe how frame pointers are unwound on x86, and enable HAVE_USER_UNWIND accordinlgy so the user unwind interfaces can be used.

---
 arch/x86/Kconfig                   |  1 +
 arch/x86/include/asm/user_unwind.h | 11 +++++++++++
 2 files changed, 12 insertions(+)
 create mode 100644 arch/x86/include/asm/user_unwind.h
arch/x86/Kconfig +1 -0
index 007bab9f2a0e..266edff59058 100644
@@ -286,6 +286,7 @@ config X86
 	select HAVE_UACCESS_VALIDATION		if HAVE_OBJTOOL
 	select HAVE_UNSTABLE_SCHED_CLOCK
 	select HAVE_USER_RETURN_NOTIFIER
+	select HAVE_USER_UNWIND
 	select HAVE_GENERIC_VDSO
 	select VDSO_GETRANDOM			if X86_64
 	select HOTPLUG_PARALLEL			if SMP && X86_64
arch/x86/include/asm/user_unwind.h new file +0 -0
new file mode 100644
index 000000000000..8c509c65cfb5

arch/x86/include/asm/user_unwind.h line 9

Quoted 9 lines
arch/x86/include/asm/user_unwind.h new file +9 -0
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_USER_UNWIND_H
+#define _ASM_X86_USER_UNWIND_H
+
+#define ARCH_INIT_USER_FP_FRAME							\
+	.ra_off		= (s32)sizeof(long) * -1,				\
+	.cfa_off	= (s32)sizeof(long) * 2,				\
+	.fp_off		= (s32)sizeof(long) * -2,				\
+	.use_fp		= true,

What about compat crap?

author maintainer
#6 lore raw

Re: [PATCH v2 02/11] unwind/x86: Add HAVE_USER_UNWIND

Quoted 7 lines · Peter Zijlstra

arch/x86/include/asm/user_unwind.h line 9

Quoted 5 lines
arch/x86/include/asm/user_unwind.h new file +5 -0
+#define ARCH_INIT_USER_FP_FRAME							\
+	.ra_off		= (s32)sizeof(long) * -1,				\
+	.cfa_off	= (s32)sizeof(long) * 2,				\
+	.fp_off		= (s32)sizeof(long) * -2,				\
+	.use_fp		= true,

What about compat crap?

Sframe doesn't support x86 compat so I guess we don't need to worry about that at least for now.

Signature · 1 line
Josh
author maintainer
#7 lore raw

PATCH v2 3/11 unwind: Introduce SFrame user space unwinding

In reply to Josh Poimboeuf

Some distros have started compiling frame pointers into all their packages to enable the kernel to do system-wide profiling of user space. Unfortunately that creates a runtime performance penalty across the entire system. Using DWARF instead isn't feasible due to the complexity it would add to the kernel.

For in-kernel unwinding we solved this problem with the creation of the ORC unwinder for x86_64. Similarly, for user space the GNU assembler has created the SFrame format starting with binutils 2.41 for SFrame v2. SFrame is a simpler version of .eh_frame which gets placed in the .sframe section.

Add support for unwinding user space using SFrame.

More information about SFrame can be found here:

- https://lwn.net/Articles/932209/
- https://lwn.net/Articles/940686/
- https://sourceware.org/binutils/docs/sframe-spec.html

---
 arch/Kconfig                |   3 +
 fs/binfmt_elf.c             |  47 +++-
 include/linux/mm_types.h    |   3 +
 include/linux/sframe.h      |  46 ++++
 include/linux/user_unwind.h |   1 +
 include/uapi/linux/elf.h    |   1 +
 include/uapi/linux/prctl.h  |   3 +
 kernel/fork.c               |  10 +
 kernel/sys.c                |  11 +
 kernel/unwind/Makefile      |   1 +
 kernel/unwind/sframe.c      | 420 ++++++++++++++++++++++++++++++++++++
 kernel/unwind/sframe.h      | 215 ++++++++++++++++++
 kernel/unwind/user.c        |  14 ++
 mm/init-mm.c                |   4 +-
 14 files changed, 774 insertions(+), 5 deletions(-)
 create mode 100644 include/linux/sframe.h
 create mode 100644 kernel/unwind/sframe.c
 create mode 100644 kernel/unwind/sframe.h
arch/Kconfig +3 -0
index b1002b2da331..ff5d5bc5f947 100644
@@ -428,6 +428,9 @@ config HAVE_HARDLOCKUP_DETECTOR_ARCH
 config HAVE_USER_UNWIND
 	bool
 
+config HAVE_USER_UNWIND_SFRAME
+	bool
+
 config HAVE_PERF_REGS
 	bool
 	help
fs/binfmt_elf.c +43 -4

100 lines not on this page · raw message

include/linux/mm_types.h +3 -0
index 485424979254..1aee78cbea33 100644
@@ -1019,6 +1019,9 @@ struct mm_struct {
 #endif
 		} lru_gen;
 #endif /* CONFIG_LRU_GEN_WALKS_MMU */
+#ifdef CONFIG_HAVE_USER_UNWIND_SFRAME
+		struct maple_tree sframe_mt;
+#endif
 	} __randomize_layout;
 
 	/*
include/linux/sframe.h new file +46 -0

46 lines not on this page · raw message

include/linux/user_unwind.h +1 -0
index 0a19ac6c92b2..8003f9d35405 100644
@@ -7,6 +7,7 @@
 enum user_unwind_type {
 	USER_UNWIND_TYPE_AUTO,
 	USER_UNWIND_TYPE_FP,
+	USER_UNWIND_TYPE_SFRAME,
 };
 
 struct user_unwind_frame {
include/uapi/linux/elf.h +1 -0
index b54b313bcf07..b2aca31e1a49 100644
@@ -39,6 +39,7 @@ typedef __s64	Elf64_Sxword;
 #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 */
include/uapi/linux/prctl.h +3 -0
index 35791791a879..69511077c910 100644
@@ -328,4 +328,7 @@ struct prctl_mm_map {
 # define PR_PPC_DEXCR_CTRL_CLEAR_ONEXEC	0x10 /* Clear the aspect on exec */
 # define PR_PPC_DEXCR_CTRL_MASK		0x1f
 
+#define PR_ADD_SFRAME			74
+#define PR_REMOVE_SFRAME		75
+
 #endif /* _LINUX_PRCTL_H */
kernel/fork.c +10 -0
index cc760491f201..a216f091edfb 100644
@@ -104,6 +104,7 @@
 #include <linux/rseq.h>
 #include <uapi/linux/pidfd.h>
 #include <linux/pidfs.h>
+#include <linux/sframe.h>
 
 #include <asm/pgalloc.h>
 #include <linux/uaccess.h>
@@ -923,6 +924,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);
 }
@@ -1249,6 +1251,13 @@ static void mm_init_uprobes_state(struct mm_struct *mm)
 #endif
 }
 
+static void mm_init_sframe(struct mm_struct *mm)
+{
+#ifdef CONFIG_HAVE_USER_UNWIND_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)
 {
@@ -1280,6 +1289,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);
 
 	if (current->mm) {
kernel/sys.c +11 -0
index 3a2df1bd9f64..e4d2b64f4ae4 100644
@@ -64,6 +64,7 @@
 #include <linux/rcupdate.h>
 #include <linux/uidgid.h>
 #include <linux/cred.h>
+#include <linux/sframe.h>
 
 #include <linux/nospec.h>
 
@@ -2782,6 +2783,16 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 	case PR_RISCV_SET_ICACHE_FLUSH_CTX:
 		error = RISCV_SET_ICACHE_FLUSH_CTX(arg2, arg3);
 		break;
+	case PR_ADD_SFRAME:
+		if (arg5)
+			return -EINVAL;
+		error = sframe_add_section(arg2, arg3, arg4);
+		break;
+	case PR_REMOVE_SFRAME:
+		if (arg3 || arg4 || arg5)
+			return -EINVAL;
+		error = sframe_remove_section(arg2);
+		break;
 	default:
 		error = -EINVAL;
 		break;
kernel/unwind/Makefile +1 -0
index eb466d6a3295..6f202c5840cf 100644
@@ -1 +1,2 @@
 obj-$(CONFIG_HAVE_USER_UNWIND) += user.o
+obj-$(CONFIG_HAVE_USER_UNWIND_SFRAME) += sframe.o
kernel/unwind/sframe.c new file +420 -0

420 lines not on this page · raw message

kernel/unwind/sframe.h new file +215 -0

215 lines not on this page · raw message

kernel/unwind/user.c +14 -0
index 5d16f9604a61..3a7b14cf522b 100644
@@ -8,6 +8,7 @@
 #include <linux/sched.h>
 #include <linux/sched/task_stack.h>
 #include <linux/user_unwind.h>
+#include <linux/sframe.h>
 #include <linux/uaccess.h>
 #include <asm/user_unwind.h>
 
@@ -29,6 +30,11 @@ int user_unwind_next(struct user_unwind_state *state)
 	case USER_UNWIND_TYPE_FP:
 		frame = &fp_frame;
 		break;
+	case USER_UNWIND_TYPE_SFRAME:
+		ret = sframe_find(state->ip, frame);
+		if (ret)
+			goto the_end;
+		break;
 	default:
 		BUG();
 	}
@@ -57,6 +63,7 @@ int user_unwind_start(struct user_unwind_state *state,
 		      enum user_unwind_type type)
 {
 	struct pt_regs *regs = task_pt_regs(current);
+	bool sframe_possible = current_has_sframe();
 
 	memset(state, 0, sizeof(*state));
 
@@ -67,6 +74,13 @@ int user_unwind_start(struct user_unwind_state *state,
 
 	switch (type) {
 	case USER_UNWIND_TYPE_AUTO:
+		state->type = sframe_possible ? USER_UNWIND_TYPE_SFRAME :
+						USER_UNWIND_TYPE_FP;
+		break;
+	case USER_UNWIND_TYPE_SFRAME:
+		if (!sframe_possible)
+			return -EINVAL;
+		break;
 	case USER_UNWIND_TYPE_FP:
 		break;
 	default:
mm/init-mm.c +3 -1
index 24c809379274..c4c6af046778 100644
@@ -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
@@ -44,7 +45,8 @@ struct mm_struct init_mm = {
 #endif
 	.user_ns	= &init_user_ns,
 	.cpu_bitmap	= CPU_BITS_NONE,
-	INIT_MM_CONTEXT(init_mm)
+	INIT_MM_CONTEXT(init_mm),
+	INIT_MM_SFRAME,
 };
 
 void setup_initial_init_mm(void *start_code, void *end_code,
Signature · 1 line
2.46.0
#8 lore raw

Re: [PATCH v2 03/11] unwind: Introduce SFrame user space unwinding

kernel test robot noticed the following build errors:

Hi Josh,

kernel test robot noticed the following build errors:

[auto build test ERROR on perf-tools-next/perf-tools-next]
[also build test ERROR on tip/perf/core perf-tools/perf-tools linus/master v6.11-rc7 next-20240913] [cannot apply to acme/perf/core]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Josh-Poimboeuf/unwind-Introduce-generic-user-space-unwinding-interface/20240914-070619
base:   https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git perf-tools-next
patch link:    https://lore.kernel.org/r/ca2e603ae3dcfa3e836162ed8c301fd4d9fd4058.1726268190.git.jpoimboe%40kernel.org
patch subject: [PATCH v2 03/11] unwind: Introduce SFrame user space unwinding
config: alpha-allnoconfig (https://download.01.org/0day-ci/archive/20240914/[email protected]/config)
compiler: alpha-linux-gcc (GCC) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240914/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All error/warnings (new ones prefixed by >>):

>> mm/init-mm.c:48:33: error: expected expression before ',' token
      48 |         INIT_MM_CONTEXT(init_mm),
         |                                 ^
>> mm/init-mm.c:48:33: warning: excess elements in struct initializer

mm/init-mm.c:48:33: note: (near initialization for 'init_mm')

vim +48 mm/init-mm.c

    22	
    23	/*
    24	 * For dynamically allocated mm_structs, there is a dynamically sized cpumask
    25	 * at the end of the structure, the size of which depends on the maximum CPU
    26	 * number the system can see. That way we allocate only as much memory for
    27	 * mm_cpumask() as needed for the hundreds, or thousands of processes that
    28	 * a system typically runs.
    29	 *
    30	 * Since there is only one init_mm in the entire system, keep it simple
    31	 * and size this cpu_bitmask to NR_CPUS.
    32	 */
    33	struct mm_struct init_mm = {
    34		.mm_mt		= MTREE_INIT_EXT(mm_mt, MM_MT_FLAGS, init_mm.mmap_lock),
    35		.pgd		= swapper_pg_dir,
    36		.mm_users	= ATOMIC_INIT(2),
    37		.mm_count	= ATOMIC_INIT(1),
    38		.write_protect_seq = SEQCNT_ZERO(init_mm.write_protect_seq),
    39		MMAP_LOCK_INITIALIZER(init_mm)
    40		.page_table_lock =  __SPIN_LOCK_UNLOCKED(init_mm.page_table_lock),
    41		.arg_lock	=  __SPIN_LOCK_UNLOCKED(init_mm.arg_lock),
    42		.mmlist		= LIST_HEAD_INIT(init_mm.mmlist),
    43	#ifdef CONFIG_PER_VMA_LOCK
    44		.mm_lock_seq	= 0,
    45	#endif
    46		.user_ns	= &init_user_ns,
    47		.cpu_bitmap	= CPU_BITS_NONE,
  > 48		INIT_MM_CONTEXT(init_mm),
    49		INIT_MM_SFRAME,
    50	};
    51	
Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
#9 lore raw

Re: [PATCH v2 03/11] unwind: Introduce SFrame user space unwinding

In reply to Josh Poimboeuf
kernel test robot noticed the following build errors:

Hi Josh,

kernel test robot noticed the following build errors:

[auto build test ERROR on perf-tools-next/perf-tools-next]
[also build test ERROR on tip/perf/core perf-tools/perf-tools linus/master v6.11-rc7 next-20240913] [cannot apply to acme/perf/core]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Josh-Poimboeuf/unwind-Introduce-generic-user-space-unwinding-interface/20240914-070619
base:   https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git perf-tools-next
patch link:    https://lore.kernel.org/r/ca2e603ae3dcfa3e836162ed8c301fd4d9fd4058.1726268190.git.jpoimboe%40kernel.org
patch subject: [PATCH v2 03/11] unwind: Introduce SFrame user space unwinding
config: arm-allnoconfig (https://download.01.org/0day-ci/archive/20240914/[email protected]/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project bf684034844c660b778f0eba103582f582b710c9)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240914/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

In file included from mm/init-mm.c:8:
In file included from include/linux/mman.h:5:
In file included from include/linux/mm.h:2232:
include/linux/vmstat.h:517:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]

     517 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
>> mm/init-mm.c:48:26: error: expected expression

48 | INIT_MM_CONTEXT(init_mm),
| ^
mm/init-mm.c:49:16: error: expected expression
49 | INIT_MM_SFRAME,
| ^
1 warning and 2 errors generated.

vim +48 mm/init-mm.c

    22	
    23	/*
    24	 * For dynamically allocated mm_structs, there is a dynamically sized cpumask
    25	 * at the end of the structure, the size of which depends on the maximum CPU
    26	 * number the system can see. That way we allocate only as much memory for
    27	 * mm_cpumask() as needed for the hundreds, or thousands of processes that
    28	 * a system typically runs.
    29	 *
    30	 * Since there is only one init_mm in the entire system, keep it simple
    31	 * and size this cpu_bitmask to NR_CPUS.
    32	 */
    33	struct mm_struct init_mm = {
    34		.mm_mt		= MTREE_INIT_EXT(mm_mt, MM_MT_FLAGS, init_mm.mmap_lock),
    35		.pgd		= swapper_pg_dir,
    36		.mm_users	= ATOMIC_INIT(2),
    37		.mm_count	= ATOMIC_INIT(1),
    38		.write_protect_seq = SEQCNT_ZERO(init_mm.write_protect_seq),
    39		MMAP_LOCK_INITIALIZER(init_mm)
    40		.page_table_lock =  __SPIN_LOCK_UNLOCKED(init_mm.page_table_lock),
    41		.arg_lock	=  __SPIN_LOCK_UNLOCKED(init_mm.arg_lock),
    42		.mmlist		= LIST_HEAD_INIT(init_mm.mmlist),
    43	#ifdef CONFIG_PER_VMA_LOCK
    44		.mm_lock_seq	= 0,
    45	#endif
    46		.user_ns	= &init_user_ns,
    47		.cpu_bitmap	= CPU_BITS_NONE,
  > 48		INIT_MM_CONTEXT(init_mm),
    49		INIT_MM_SFRAME,
    50	};
    51	
Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
maintainer
#10 lore raw

Re: [PATCH v2 03/11] unwind: Introduce SFrame user space unwinding

In reply to Josh Poimboeuf
Quoted 6 lines · On Sat, 14 Sep 2024 01:02:05 +0200 Josh Poimboeuf <[email protected]> wrote:
include/linux/sframe.h new file +0 -0
new file mode 100644
index 000000000000..3a44f76929e2

include/linux/sframe.h line 8

Quoted 8 lines
include/linux/sframe.h new file +8 -0
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_SFRAME_H
+#define _LINUX_SFRAME_H
+
+#include <linux/mm_types.h>
+
+struct sframe_file {
+	unsigned long sframe_addr, text_start, text_end;

Please make each entry a separate line:

	unsigned long		sframe_addr;
	unsigned long		text_start;
	unsigned long		text_end;

It may be fine for declaring variables like this in a function, but it should not be done in a structure.

Quoted 238 lines
include/linux/sframe.h new file +38 -0
+};
+
+struct user_unwind_frame;
+
+#ifdef CONFIG_HAVE_USER_UNWIND_SFRAME
+
+#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(struct sframe_file *file);
+extern int sframe_add_section(unsigned long sframe_addr, 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 user_unwind_frame *frame);
+
+static inline bool current_has_sframe(void)
+{
+	struct mm_struct *mm = current->mm;
+
+	return mm && !mtree_empty(&mm->sframe_mt);
+}
+
+#else /* !CONFIG_HAVE_USER_UNWIND_SFRAME */
+
+#define INIT_MM_SFRAME
+
+static inline void sframe_free_mm(struct mm_struct *mm) {}
+
+static inline int __sframe_add_section(struct sframe_file *file) { return -EINVAL; }
+static inline int sframe_add_section(unsigned long sframe_addr, unsigned long text_start, unsigned long text_end) { return -EINVAL; }
+static inline int sframe_remove_section(unsigned long sframe_addr) { return -EINVAL; }
+static inline int sframe_find(unsigned long ip, struct user_unwind_frame *frame) { return -EINVAL; }
+
+static inline bool current_has_sframe(void) { return false; }
+
+#endif /* CONFIG_HAVE_USER_UNWIND_SFRAME */
+
+#endif /* _LINUX_SFRAME_H */
include/linux/user_unwind.h +1 -0
index 0a19ac6c92b2..8003f9d35405 100644
@@ -7,6 +7,7 @@
 enum user_unwind_type {
 	USER_UNWIND_TYPE_AUTO,
 	USER_UNWIND_TYPE_FP,
+	USER_UNWIND_TYPE_SFRAME,
 };
 
 struct user_unwind_frame {
include/uapi/linux/elf.h +1 -0
index b54b313bcf07..b2aca31e1a49 100644
@@ -39,6 +39,7 @@ typedef __s64	Elf64_Sxword;
 #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 */
include/uapi/linux/prctl.h +3 -0
index 35791791a879..69511077c910 100644
@@ -328,4 +328,7 @@ struct prctl_mm_map {
 # define PR_PPC_DEXCR_CTRL_CLEAR_ONEXEC	0x10 /* Clear the aspect on exec */
 # define PR_PPC_DEXCR_CTRL_MASK		0x1f
 
+#define PR_ADD_SFRAME			74
+#define PR_REMOVE_SFRAME		75
+
 #endif /* _LINUX_PRCTL_H */
kernel/fork.c +10 -0
index cc760491f201..a216f091edfb 100644
@@ -104,6 +104,7 @@
 #include <linux/rseq.h>
 #include <uapi/linux/pidfd.h>
 #include <linux/pidfs.h>
+#include <linux/sframe.h>
 
 #include <asm/pgalloc.h>
 #include <linux/uaccess.h>
@@ -923,6 +924,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);
 }
@@ -1249,6 +1251,13 @@ static void mm_init_uprobes_state(struct mm_struct *mm)
 #endif
 }
 
+static void mm_init_sframe(struct mm_struct *mm)
+{
+#ifdef CONFIG_HAVE_USER_UNWIND_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)
 {
@@ -1280,6 +1289,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);
 
 	if (current->mm) {
kernel/sys.c +11 -0
index 3a2df1bd9f64..e4d2b64f4ae4 100644
@@ -64,6 +64,7 @@
 #include <linux/rcupdate.h>
 #include <linux/uidgid.h>
 #include <linux/cred.h>
+#include <linux/sframe.h>
 
 #include <linux/nospec.h>
 
@@ -2782,6 +2783,16 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 	case PR_RISCV_SET_ICACHE_FLUSH_CTX:
 		error = RISCV_SET_ICACHE_FLUSH_CTX(arg2, arg3);
 		break;
+	case PR_ADD_SFRAME:
+		if (arg5)
+			return -EINVAL;
+		error = sframe_add_section(arg2, arg3, arg4);
+		break;
+	case PR_REMOVE_SFRAME:
+		if (arg3 || arg4 || arg5)
+			return -EINVAL;
+		error = sframe_remove_section(arg2);
+		break;
 	default:
 		error = -EINVAL;
 		break;
kernel/unwind/Makefile +1 -0
index eb466d6a3295..6f202c5840cf 100644
@@ -1 +1,2 @@
 obj-$(CONFIG_HAVE_USER_UNWIND) += user.o
+obj-$(CONFIG_HAVE_USER_UNWIND_SFRAME) += sframe.o
kernel/unwind/sframe.c new file +80 -0
new file mode 100644
index 000000000000..3e4d29e737a1
@@ -0,0 +1,420 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/srcu.h>
+#include <linux/uaccess.h>
+#include <linux/mm.h>
+#include <linux/sframe.h>
+#include <linux/user_unwind.h>
+
+#include "sframe.h"
+
+#define SFRAME_FILENAME_LEN 32
+
+struct sframe_section {
+	struct rcu_head rcu;
+
+	unsigned long sframe_addr;
+	unsigned long text_addr;
+
+	unsigned long fdes_addr;
+	unsigned long fres_addr;
+	unsigned int  fdes_nr;
+	signed char ra_off, fp_off;
+};
+
+DEFINE_STATIC_SRCU(sframe_srcu);
+
+#define __SFRAME_GET_USER(out, user_ptr, type)				\
+({									\
+	type __tmp;							\
+	if (get_user(__tmp, (type *)user_ptr))				\
+		return -EFAULT;						\
+	user_ptr += sizeof(__tmp);					\
+	out = __tmp;							\
+})
+
+#define SFRAME_GET_USER_SIGNED(out, user_ptr, size)			\
+({									\
+	switch (size) {							\
+	case 1:								\
+		__SFRAME_GET_USER(out, user_ptr, s8);			\
+		break;							\
+	case 2:								\
+		__SFRAME_GET_USER(out, user_ptr, s16);			\
+		break;							\
+	case 4:								\
+		__SFRAME_GET_USER(out, user_ptr, s32);			\
+		break;							\
+	default:							\
+		return -EINVAL;						\
+	}								\
+})
+
+#define SFRAME_GET_USER_UNSIGNED(out, user_ptr, size)			\
+({									\
+	switch (size) {							\
+	case 1:								\
+		__SFRAME_GET_USER(out, user_ptr, u8);			\
+		break;							\
+	case 2:								\
+		__SFRAME_GET_USER(out, user_ptr, u16);			\
+		break;							\
+	case 4:								\
+		__SFRAME_GET_USER(out, user_ptr, u32);			\
+		break;							\
+	default:							\
+		return -EINVAL;						\
+	}								\
+})
+
+static unsigned char fre_type_to_size(unsigned char fre_type)
+{
+	if (fre_type > 2)
+		return 0;
+	return 1 << fre_type;
+}
+
+static unsigned char offset_size_enum_to_size(unsigned char off_size)
+{

kernel/unwind/sframe.c line 90

Quoted 10 lines
kernel/unwind/sframe.c new file +10 -0
+	if (off_size > 2)
+		return 0;
+	return 1 << off_size;
+}
+
+static int find_fde(struct sframe_section *sec, unsigned long ip,
+		    struct sframe_fde *fde)
+{
+	s32 func_off, ip_off;
+	struct sframe_fde __user *first, *last, *mid, *found;

Need to initialize found = NULL;

kernel/unwind/sframe.c line 96

Quoted 6 lines
kernel/unwind/sframe.c new file +6 -0
+
+	ip_off = ip - sec->sframe_addr;
+
+	first = (void *)sec->fdes_addr;
+	last = first + sec->fdes_nr;
+	while (first <= last) {

So we trust user space to have this table sorted?

kernel/unwind/sframe.c line 101

Quoted 5 lines
kernel/unwind/sframe.c new file +5 -0
+		mid = first + ((last - first) / 2);
+		if (get_user(func_off, (s32 *)mid))
+			return -EFAULT;
+		if (ip_off >= func_off) {
+			found = mid;

If it's not sorted, this can return the wrong value.

We should have some check that has something like:

	s32 low_func_off = 0, high_func_off = 0;

			if (low_func_off && low_func_off > func_off)
				return -EINVAL;	
			low_func_off = func_off;

kernel/unwind/sframe.c line 103

Quoted 2 lines
kernel/unwind/sframe.c new file +2 -0
+			first = mid + 1;
+		} else
			{ /* Note, this needs a bracket anyway, because
			     rules are, if one if block has a bracket,
			     the other needs one too */

			if (high_func_off && high_func_off < func_off)
				return -EINVAL;

			high_func_off = func_off;

kernel/unwind/sframe.c line 104

Quoted 1 line
kernel/unwind/sframe.c new file +1 -0
+			last = mid - 1;
		}

kernel/unwind/sframe.c line 114

Quoted 10 lines
kernel/unwind/sframe.c new file +10 -0
+	}
+
+	if (!found)
+		return -EINVAL;
+
+	if (copy_from_user(fde, found, sizeof(*fde)))
+		return -EFAULT;
+
+	return 0;
+}

-- Steve

#11 lore raw

Re: [PATCH v2 03/11] unwind: Introduce SFrame user space unwinding

Quoted 278 lines · Steven Rostedt
Quoted 6 lines · On Sat, 14 Sep 2024 01:02:05 +0200 Josh Poimboeuf <[email protected]> wrote:
include/linux/sframe.h new file +0 -0
new file mode 100644
index 000000000000..3a44f76929e2

include/linux/sframe.h line 8

Quoted 8 lines
include/linux/sframe.h new file +8 -0
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_SFRAME_H
+#define _LINUX_SFRAME_H
+
+#include <linux/mm_types.h>
+
+struct sframe_file {
+	unsigned long sframe_addr, text_start, text_end;

Please make each entry a separate line:

	unsigned long		sframe_addr;
	unsigned long		text_start;
	unsigned long		text_end;

It may be fine for declaring variables like this in a function, but it should not be done in a structure.

Quoted 238 lines
include/linux/sframe.h new file +38 -0
+};
+
+struct user_unwind_frame;
+
+#ifdef CONFIG_HAVE_USER_UNWIND_SFRAME
+
+#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(struct sframe_file *file);
+extern int sframe_add_section(unsigned long sframe_addr, 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 user_unwind_frame *frame);
+
+static inline bool current_has_sframe(void)
+{
+	struct mm_struct *mm = current->mm;
+
+	return mm && !mtree_empty(&mm->sframe_mt);
+}
+
+#else /* !CONFIG_HAVE_USER_UNWIND_SFRAME */
+
+#define INIT_MM_SFRAME
+
+static inline void sframe_free_mm(struct mm_struct *mm) {}
+
+static inline int __sframe_add_section(struct sframe_file *file) { return -EINVAL; }
+static inline int sframe_add_section(unsigned long sframe_addr, unsigned long text_start, unsigned long text_end) { return -EINVAL; }
+static inline int sframe_remove_section(unsigned long sframe_addr) { return -EINVAL; }
+static inline int sframe_find(unsigned long ip, struct user_unwind_frame *frame) { return -EINVAL; }
+
+static inline bool current_has_sframe(void) { return false; }
+
+#endif /* CONFIG_HAVE_USER_UNWIND_SFRAME */
+
+#endif /* _LINUX_SFRAME_H */
include/linux/user_unwind.h +1 -0
index 0a19ac6c92b2..8003f9d35405 100644
@@ -7,6 +7,7 @@
  enum user_unwind_type {
  	USER_UNWIND_TYPE_AUTO,
  	USER_UNWIND_TYPE_FP,
+	USER_UNWIND_TYPE_SFRAME,
  };
 
  struct user_unwind_frame {
include/uapi/linux/elf.h +1 -0
index b54b313bcf07..b2aca31e1a49 100644
@@ -39,6 +39,7 @@ typedef __s64	Elf64_Sxword;
  #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 */
include/uapi/linux/prctl.h +3 -0
index 35791791a879..69511077c910 100644
@@ -328,4 +328,7 @@ struct prctl_mm_map {
  # define PR_PPC_DEXCR_CTRL_CLEAR_ONEXEC	0x10 /* Clear the aspect on exec */
  # define PR_PPC_DEXCR_CTRL_MASK		0x1f
 
+#define PR_ADD_SFRAME			74
+#define PR_REMOVE_SFRAME		75
+
  #endif /* _LINUX_PRCTL_H */
kernel/fork.c +10 -0
index cc760491f201..a216f091edfb 100644
@@ -104,6 +104,7 @@
  #include <linux/rseq.h>
  #include <uapi/linux/pidfd.h>
  #include <linux/pidfs.h>
+#include <linux/sframe.h>
 
  #include <asm/pgalloc.h>
  #include <linux/uaccess.h>
@@ -923,6 +924,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);
  }
@@ -1249,6 +1251,13 @@ static void mm_init_uprobes_state(struct mm_struct *mm)
  #endif
  }
 
+static void mm_init_sframe(struct mm_struct *mm)
+{
+#ifdef CONFIG_HAVE_USER_UNWIND_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)
  {
@@ -1280,6 +1289,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);
 
  	if (current->mm) {
kernel/sys.c +11 -0
index 3a2df1bd9f64..e4d2b64f4ae4 100644
@@ -64,6 +64,7 @@
  #include <linux/rcupdate.h>
  #include <linux/uidgid.h>
  #include <linux/cred.h>
+#include <linux/sframe.h>
 
  #include <linux/nospec.h>
 
@@ -2782,6 +2783,16 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
  	case PR_RISCV_SET_ICACHE_FLUSH_CTX:
  		error = RISCV_SET_ICACHE_FLUSH_CTX(arg2, arg3);
  		break;
+	case PR_ADD_SFRAME:
+		if (arg5)
+			return -EINVAL;
+		error = sframe_add_section(arg2, arg3, arg4);
+		break;
+	case PR_REMOVE_SFRAME:
+		if (arg3 || arg4 || arg5)
+			return -EINVAL;
+		error = sframe_remove_section(arg2);
+		break;
  	default:
  		error = -EINVAL;
  		break;
kernel/unwind/Makefile +1 -0
index eb466d6a3295..6f202c5840cf 100644
@@ -1 +1,2 @@
  obj-$(CONFIG_HAVE_USER_UNWIND) += user.o
+obj-$(CONFIG_HAVE_USER_UNWIND_SFRAME) += sframe.o
kernel/unwind/sframe.c new file +80 -0
new file mode 100644
index 000000000000..3e4d29e737a1
@@ -0,0 +1,420 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/srcu.h>
+#include <linux/uaccess.h>
+#include <linux/mm.h>
+#include <linux/sframe.h>
+#include <linux/user_unwind.h>
+
+#include "sframe.h"
+
+#define SFRAME_FILENAME_LEN 32
+
+struct sframe_section {
+	struct rcu_head rcu;
+
+	unsigned long sframe_addr;
+	unsigned long text_addr;
+
+	unsigned long fdes_addr;
+	unsigned long fres_addr;
+	unsigned int  fdes_nr;
+	signed char ra_off, fp_off;
+};
+
+DEFINE_STATIC_SRCU(sframe_srcu);
+
+#define __SFRAME_GET_USER(out, user_ptr, type)				\
+({									\
+	type __tmp;							\
+	if (get_user(__tmp, (type *)user_ptr))				\
+		return -EFAULT;						\
+	user_ptr += sizeof(__tmp);					\
+	out = __tmp;							\
+})
+
+#define SFRAME_GET_USER_SIGNED(out, user_ptr, size)			\
+({									\
+	switch (size) {							\
+	case 1:								\
+		__SFRAME_GET_USER(out, user_ptr, s8);			\
+		break;							\
+	case 2:								\
+		__SFRAME_GET_USER(out, user_ptr, s16);			\
+		break;							\
+	case 4:								\
+		__SFRAME_GET_USER(out, user_ptr, s32);			\
+		break;							\
+	default:							\
+		return -EINVAL;						\
+	}								\
+})
+
+#define SFRAME_GET_USER_UNSIGNED(out, user_ptr, size)			\
+({									\
+	switch (size) {							\
+	case 1:								\
+		__SFRAME_GET_USER(out, user_ptr, u8);			\
+		break;							\
+	case 2:								\
+		__SFRAME_GET_USER(out, user_ptr, u16);			\
+		break;							\
+	case 4:								\
+		__SFRAME_GET_USER(out, user_ptr, u32);			\
+		break;							\
+	default:							\
+		return -EINVAL;						\
+	}								\
+})
+
+static unsigned char fre_type_to_size(unsigned char fre_type)
+{
+	if (fre_type > 2)
+		return 0;
+	return 1 << fre_type;
+}
+
+static unsigned char offset_size_enum_to_size(unsigned char off_size)
+{

kernel/unwind/sframe.c line 90

Quoted 10 lines
kernel/unwind/sframe.c new file +10 -0
+	if (off_size > 2)
+		return 0;
+	return 1 << off_size;
+}
+
+static int find_fde(struct sframe_section *sec, unsigned long ip,
+		    struct sframe_fde *fde)
+{
+	s32 func_off, ip_off;
+	struct sframe_fde __user *first, *last, *mid, *found;

Need to initialize found = NULL;

Quoted 9 lines

kernel/unwind/sframe.c line 96

Quoted 6 lines
kernel/unwind/sframe.c new file +6 -0
+
+	ip_off = ip - sec->sframe_addr;
+
+	first = (void *)sec->fdes_addr;
+	last = first + sec->fdes_nr;
+	while (first <= last) {

So we trust user space to have this table sorted?

GNU ld will create this table sorted when linking .sframe sections and will set SFRAME_F_FDE_SORTED in flags in the output .sframe section. In the current patch, I see the __sframe_add_section () includes a check for SFRAME_F_FDE_SORTED for admitting SFrame sections.

So proceeding here with the assumption that the SFrame FDE list is sorted should work fine.

Quoted 42 lines

kernel/unwind/sframe.c line 101

Quoted 5 lines
kernel/unwind/sframe.c new file +5 -0
+		mid = first + ((last - first) / 2);
+		if (get_user(func_off, (s32 *)mid))
+			return -EFAULT;
+		if (ip_off >= func_off) {
+			found = mid;

If it's not sorted, this can return the wrong value.

We should have some check that has something like:

	s32 low_func_off = 0, high_func_off = 0;

			if (low_func_off && low_func_off > func_off)
				return -EINVAL;	
			low_func_off = func_off;

kernel/unwind/sframe.c line 103

Quoted 2 lines
kernel/unwind/sframe.c new file +2 -0
+			first = mid + 1;
+		} else
			{ /* Note, this needs a bracket anyway, because
			     rules are, if one if block has a bracket,
			     the other needs one too */

			if (high_func_off && high_func_off < func_off)
				return -EINVAL;

			high_func_off = func_off;

kernel/unwind/sframe.c line 104

Quoted 1 line
kernel/unwind/sframe.c new file +1 -0
+			last = mid - 1;
		}

kernel/unwind/sframe.c line 114

Quoted 10 lines
kernel/unwind/sframe.c new file +10 -0
+	}
+
+	if (!found)
+		return -EINVAL;
+
+	if (copy_from_user(fde, found, sizeof(*fde)))
+		return -EFAULT;
+
+	return 0;
+}

-- Steve

maintainer
#12 lore raw

Re: [PATCH v2 03/11] unwind: Introduce SFrame user space unwinding

Quoted 10 lines · On Tue, 1 Oct 2024 11:20:35 -0700 Indu Bhagat <[email protected]> wrote:
Quoted 2 lines

So we trust user space to have this table sorted?

GNU ld will create this table sorted when linking .sframe sections and will set SFRAME_F_FDE_SORTED in flags in the output .sframe section. In the current patch, I see the __sframe_add_section () includes a check for SFRAME_F_FDE_SORTED for admitting SFrame sections.

So proceeding here with the assumption that the SFrame FDE list is sorted should work fine.

No not at all! We *cannot trust* user space. This could lead to a security hole if we assume it's sorted. The kernel must not trust anything it receives from user space. Because an attacker will be looking for ways to confuse the kernel to exploit it.

When I look at code that reads user space, I do not look at it as if it were made by the compiler. I look at it as if it were made by someone that's trying to find ways to crack the system. Every read from user space *must* be validated *every* time it's read. It can not even validate it once and then think its immutable (unless the kernel actually made it immutable).

-- Steve

#13 lore raw

Re: [PATCH v2 03/11] unwind: Introduce SFrame user space unwinding

* Steven Rostedt:

Quoted 11 lines
Quoted 8 lines · On Tue, 1 Oct 2024 11:20:35 -0700 Indu Bhagat <[email protected]> wrote:
Quoted 2 lines

So we trust user space to have this table sorted?

GNU ld will create this table sorted when linking .sframe sections and will set SFRAME_F_FDE_SORTED in flags in the output .sframe section. In the current patch, I see the __sframe_add_section () includes a check for SFRAME_F_FDE_SORTED for admitting SFrame sections.

Quoted 7 lines
Quoted 2 lines

So proceeding here with the assumption that the SFrame FDE list is sorted should work fine.

No not at all! We *cannot trust* user space. This could lead to a security hole if we assume it's sorted. The kernel must not trust anything it receives from user space. Because an attacker will be looking for ways to confuse the kernel to exploit it.

I don't quite understand, sorry.

Doing a binary search on an unordered table fails to find some entries that could be discovered by a linear scan. But an attacker could just as well use an incomplete table from the start. So assuming an ordered table seems rather unlikely to introduce additional problems. (Given the lack of a formal threat model, it's impossible to make more precise claims in either direction.)

Thanks,
Florian

maintainer
#14 lore raw

Re: [PATCH v2 03/11] unwind: Introduce SFrame user space unwinding

Quoted 9 lines · On Wed, 02 Oct 2024 10:18:21 +0200 Florian Weimer <[email protected]> wrote:

I don't quite understand, sorry.

Doing a binary search on an unordered table fails to find some entries that could be discovered by a linear scan. But an attacker could just as well use an incomplete table from the start. So assuming an ordered table seems rather unlikely to introduce additional problems. (Given the lack of a formal threat model, it's impossible to make more precise claims in either direction.)

Basically, the idea is if anything is out of place, scrap the entire process. An unordered table can give unpredictable results, that could be used latter as a gadget. If the kernel expects a sorted table and it ends up not being sorted, it should automatically flag it as corrupt and stop all processing.

The kernel doesn't need to scan the entire table each time to see if it is sorted, that would kill the point of it being sorted in the first place. But it can check that the values merge towards a correct answer. All it needs to do is keep track of the current highest and lowest values, and if it finds something outside that range, it should abort immediately.

The effort needed to validate is very low, so it should be done.

-- Steve

#15 lore raw

Re: [PATCH v2 03/11] unwind: Introduce SFrame user space unwinding

In reply to Josh Poimboeuf

Hello Josh!

Quoted 46 lines · Josh Poimboeuf

Some distros have started compiling frame pointers into all their packages to enable the kernel to do system-wide profiling of user space. Unfortunately that creates a runtime performance penalty across the entire system. Using DWARF instead isn't feasible due to the complexity it would add to the kernel.

For in-kernel unwinding we solved this problem with the creation of the ORC unwinder for x86_64. Similarly, for user space the GNU assembler has created the SFrame format starting with binutils 2.41 for SFrame v2. SFrame is a simpler version of .eh_frame which gets placed in the .sframe section.

Add support for unwinding user space using SFrame.

More information about SFrame can be found here:

- https://lwn.net/Articles/932209/
- https://lwn.net/Articles/940686/
- https://sourceware.org/binutils/docs/sframe-spec.html

---
  arch/Kconfig                |   3 +
  fs/binfmt_elf.c             |  47 +++-
  include/linux/mm_types.h    |   3 +
  include/linux/sframe.h      |  46 ++++
  include/linux/user_unwind.h |   1 +
  include/uapi/linux/elf.h    |   1 +
  include/uapi/linux/prctl.h  |   3 +
  kernel/fork.c               |  10 +
  kernel/sys.c                |  11 +
  kernel/unwind/Makefile      |   1 +
  kernel/unwind/sframe.c      | 420 ++++++++++++++++++++++++++++++++++++
  kernel/unwind/sframe.h      | 215 ++++++++++++++++++
  kernel/unwind/user.c        |  14 ++
  mm/init-mm.c                |   4 +-
  14 files changed, 774 insertions(+), 5 deletions(-)

create mode 100644 include/linux/sframe.h
create mode 100644 kernel/unwind/sframe.c
create mode 100644 kernel/unwind/sframe.h

arch/Kconfig +0 -0
index b1002b2da331..ff5d5bc5f947 100644

arch/Kconfig config HAVE_HARDLOCKUP_DETECTOR_ARCH · line 432

Quoted 5 lines
arch/Kconfig +2 -0
@@ -428,6 +428,9 @@ config HAVE_HARDLOCKUP_DETECTOR_ARCH
  config HAVE_USER_UNWIND
  	bool
 
+config HAVE_USER_UNWIND_SFRAME
+	bool

I found this unwinder of userspace using SFrame to depend on your generic unwinder of userspace framework. Does this warrant to add:

depends on HAVE_USER_UNWIND

Quoted 413 lines
arch/Kconfig +1 -0
+
  config HAVE_PERF_REGS
  	bool
  	help
fs/binfmt_elf.c +43 -4
index 19fa49cd9907..923aed390f2e 100644
@@ -47,6 +47,7 @@
  #include <linux/dax.h>
  #include <linux/uaccess.h>
  #include <linux/rseq.h>
+#include <linux/sframe.h>
  #include <asm/param.h>
  #include <asm/page.h>
 
@@ -633,11 +634,13 @@ 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;
  	unsigned long total_size;
+	unsigned long start_code = ~0UL;
+	unsigned long end_code = 0;
  	int i;
 
  	/* First of all, some simple consistency checks */
@@ -659,7 +662,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);
@@ -688,7 +692,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
  			/*
  			 * Check to see if the section's size will overflow the
  			 * allowed task size. Note that p_filesz must always be
-			 * <= p_memsize so it's only necessary to check p_memsz.
+			 * <= p_memsz so it's only necessary to check p_memsz.
  			 */
  			k = load_addr + eppnt->p_vaddr;
  			if (BAD_ADDR(k) ||
@@ -698,7 +702,28 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
  				error = -ENOMEM;
  				goto out;
  			}
+
+			if ((eppnt->p_flags & PF_X) && k < start_code)
+				start_code = k;
+
+			if ((eppnt->p_flags & PF_X) && k + eppnt->p_filesz > end_code)
+				end_code = k + eppnt->p_filesz;
+			break;
  		}
+		case PT_GNU_SFRAME:
+			sframe_phdr = eppnt;
+			break;
+		}
+	}
+
+	if (sframe_phdr) {
+		struct sframe_file sfile = {
+			.sframe_addr	= load_addr + sframe_phdr->p_vaddr,
+			.text_start	= start_code,
+			.text_end	= end_code,
+		};
+
+		__sframe_add_section(&sfile);
  	}
 
  	error = load_addr;
@@ -823,7 +848,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;
  	int retval, i;
  	unsigned long elf_entry;
@@ -931,6 +956,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,
@@ -1316,6 +1345,16 @@ static int load_elf_binary(struct linux_binprm *bprm)
  				MAP_FIXED | MAP_PRIVATE, 0);
  	}
 
+	if (sframe_phdr) {
+		struct sframe_file sfile = {
+			.sframe_addr	= load_bias + sframe_phdr->p_vaddr,
+			.text_start	= start_code,
+			.text_end	= end_code,
+		};
+
+		__sframe_add_section(&sfile);
+	}
+
  	regs = current_pt_regs();
  #ifdef ELF_PLAT_INIT
  	/*
include/linux/mm_types.h +3 -0
index 485424979254..1aee78cbea33 100644
@@ -1019,6 +1019,9 @@ struct mm_struct {
  #endif
  		} lru_gen;
  #endif /* CONFIG_LRU_GEN_WALKS_MMU */
+#ifdef CONFIG_HAVE_USER_UNWIND_SFRAME
+		struct maple_tree sframe_mt;
+#endif
  	} __randomize_layout;
 
  	/*
include/linux/sframe.h new file +46 -0
new file mode 100644
index 000000000000..3a44f76929e2
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_SFRAME_H
+#define _LINUX_SFRAME_H
+
+#include <linux/mm_types.h>
+
+struct sframe_file {
+	unsigned long sframe_addr, text_start, text_end;
+};
+
+struct user_unwind_frame;
+
+#ifdef CONFIG_HAVE_USER_UNWIND_SFRAME
+
+#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(struct sframe_file *file);
+extern int sframe_add_section(unsigned long sframe_addr, 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 user_unwind_frame *frame);
+
+static inline bool current_has_sframe(void)
+{
+	struct mm_struct *mm = current->mm;
+
+	return mm && !mtree_empty(&mm->sframe_mt);
+}
+
+#else /* !CONFIG_HAVE_USER_UNWIND_SFRAME */
+
+#define INIT_MM_SFRAME
+
+static inline void sframe_free_mm(struct mm_struct *mm) {}
+
+static inline int __sframe_add_section(struct sframe_file *file) { return -EINVAL; }
+static inline int sframe_add_section(unsigned long sframe_addr, unsigned long text_start, unsigned long text_end) { return -EINVAL; }
+static inline int sframe_remove_section(unsigned long sframe_addr) { return -EINVAL; }
+static inline int sframe_find(unsigned long ip, struct user_unwind_frame *frame) { return -EINVAL; }
+
+static inline bool current_has_sframe(void) { return false; }
+
+#endif /* CONFIG_HAVE_USER_UNWIND_SFRAME */
+
+#endif /* _LINUX_SFRAME_H */
include/linux/user_unwind.h +1 -0
index 0a19ac6c92b2..8003f9d35405 100644
@@ -7,6 +7,7 @@
  enum user_unwind_type {
  	USER_UNWIND_TYPE_AUTO,
  	USER_UNWIND_TYPE_FP,
+	USER_UNWIND_TYPE_SFRAME,
  };
 
  struct user_unwind_frame {
include/uapi/linux/elf.h +1 -0
index b54b313bcf07..b2aca31e1a49 100644
@@ -39,6 +39,7 @@ typedef __s64	Elf64_Sxword;
  #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 */
include/uapi/linux/prctl.h +3 -0
index 35791791a879..69511077c910 100644
@@ -328,4 +328,7 @@ struct prctl_mm_map {
  # define PR_PPC_DEXCR_CTRL_CLEAR_ONEXEC	0x10 /* Clear the aspect on exec */
  # define PR_PPC_DEXCR_CTRL_MASK		0x1f
 
+#define PR_ADD_SFRAME			74
+#define PR_REMOVE_SFRAME		75
+
  #endif /* _LINUX_PRCTL_H */
kernel/fork.c +10 -0
index cc760491f201..a216f091edfb 100644
@@ -104,6 +104,7 @@
  #include <linux/rseq.h>
  #include <uapi/linux/pidfd.h>
  #include <linux/pidfs.h>
+#include <linux/sframe.h>
 
  #include <asm/pgalloc.h>
  #include <linux/uaccess.h>
@@ -923,6 +924,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);
  }
@@ -1249,6 +1251,13 @@ static void mm_init_uprobes_state(struct mm_struct *mm)
  #endif
  }
 
+static void mm_init_sframe(struct mm_struct *mm)
+{
+#ifdef CONFIG_HAVE_USER_UNWIND_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)
  {
@@ -1280,6 +1289,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);
 
  	if (current->mm) {
kernel/sys.c +11 -0
index 3a2df1bd9f64..e4d2b64f4ae4 100644
@@ -64,6 +64,7 @@
  #include <linux/rcupdate.h>
  #include <linux/uidgid.h>
  #include <linux/cred.h>
+#include <linux/sframe.h>
 
  #include <linux/nospec.h>
 
@@ -2782,6 +2783,16 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
  	case PR_RISCV_SET_ICACHE_FLUSH_CTX:
  		error = RISCV_SET_ICACHE_FLUSH_CTX(arg2, arg3);
  		break;
+	case PR_ADD_SFRAME:
+		if (arg5)
+			return -EINVAL;
+		error = sframe_add_section(arg2, arg3, arg4);
+		break;
+	case PR_REMOVE_SFRAME:
+		if (arg3 || arg4 || arg5)
+			return -EINVAL;
+		error = sframe_remove_section(arg2);
+		break;
  	default:
  		error = -EINVAL;
  		break;
kernel/unwind/Makefile +1 -0
index eb466d6a3295..6f202c5840cf 100644
@@ -1 +1,2 @@
  obj-$(CONFIG_HAVE_USER_UNWIND) += user.o
+obj-$(CONFIG_HAVE_USER_UNWIND_SFRAME) += sframe.o
kernel/unwind/sframe.c new file +111 -0
new file mode 100644
index 000000000000..3e4d29e737a1
@@ -0,0 +1,420 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/srcu.h>
+#include <linux/uaccess.h>
+#include <linux/mm.h>
+#include <linux/sframe.h>
+#include <linux/user_unwind.h>
+
+#include "sframe.h"
+
+#define SFRAME_FILENAME_LEN 32
+
+struct sframe_section {
+	struct rcu_head rcu;
+
+	unsigned long sframe_addr;
+	unsigned long text_addr;
+
+	unsigned long fdes_addr;
+	unsigned long fres_addr;
+	unsigned int  fdes_nr;
+	signed char ra_off, fp_off;
+};
+
+DEFINE_STATIC_SRCU(sframe_srcu);
+
+#define __SFRAME_GET_USER(out, user_ptr, type)				\
+({									\
+	type __tmp;							\
+	if (get_user(__tmp, (type *)user_ptr))				\
+		return -EFAULT;						\
+	user_ptr += sizeof(__tmp);					\
+	out = __tmp;							\
+})
+
+#define SFRAME_GET_USER_SIGNED(out, user_ptr, size)			\
+({									\
+	switch (size) {							\
+	case 1:								\
+		__SFRAME_GET_USER(out, user_ptr, s8);			\
+		break;							\
+	case 2:								\
+		__SFRAME_GET_USER(out, user_ptr, s16);			\
+		break;							\
+	case 4:								\
+		__SFRAME_GET_USER(out, user_ptr, s32);			\
+		break;							\
+	default:							\
+		return -EINVAL;						\
+	}								\
+})
+
+#define SFRAME_GET_USER_UNSIGNED(out, user_ptr, size)			\
+({									\
+	switch (size) {							\
+	case 1:								\
+		__SFRAME_GET_USER(out, user_ptr, u8);			\
+		break;							\
+	case 2:								\
+		__SFRAME_GET_USER(out, user_ptr, u16);			\
+		break;							\
+	case 4:								\
+		__SFRAME_GET_USER(out, user_ptr, u32);			\
+		break;							\
+	default:							\
+		return -EINVAL;						\
+	}								\
+})
+
+static unsigned char fre_type_to_size(unsigned char fre_type)
+{
+	if (fre_type > 2)
+		return 0;
+	return 1 << fre_type;
+}
+
+static unsigned char offset_size_enum_to_size(unsigned char off_size)
+{
+	if (off_size > 2)
+		return 0;
+	return 1 << off_size;
+}
+
+static int find_fde(struct sframe_section *sec, unsigned long ip,
+		    struct sframe_fde *fde)
+{
+	s32 func_off, ip_off;
+	struct sframe_fde __user *first, *last, *mid, *found;
+
+	ip_off = ip - sec->sframe_addr;
+
+	first = (void *)sec->fdes_addr;
+	last = first + sec->fdes_nr;
+	while (first <= last) {
+		mid = first + ((last - first) / 2);
+		if (get_user(func_off, (s32 *)mid))
+			return -EFAULT;
+		if (ip_off >= func_off) {
+			found = mid;
+			first = mid + 1;
+		} else
+			last = mid - 1;
+	}
+
+	if (!found)
+		return -EINVAL;
+
+	if (copy_from_user(fde, found, sizeof(*fde)))
+		return -EFAULT;

kernel/unwind/sframe.c line 121

Quoted 10 lines
kernel/unwind/sframe.c new file +10 -0
+
+	return 0;
+}
+
+static int find_fre(struct sframe_section *sec, struct sframe_fde *fde,
+		    unsigned long ip, struct user_unwind_frame *frame)
+{
+	unsigned char fde_type = SFRAME_FUNC_FDE_TYPE(fde->info);
+	unsigned char fre_type = SFRAME_FUNC_FRE_TYPE(fde->info);
+	s32 fre_ip_off, cfa_off, ra_off, fp_off, ip_off;

Doesn't fre_ip_off need to be u32 (see also below)? The SFrame format specification states the FRE sfre_start_address is either u8, u16, or u32:
https://sourceware.org/binutils/docs/sframe-spec.html#The-SFrame-FRE-Types

Quoted 7 lines
kernel/unwind/sframe.c new file +7 -0
+	unsigned char offset_count, offset_size;
+	unsigned char addr_size;
+	void __user *f, *last_f;
+	u8 fre_info;
+	int i;
+
+	addr_size = fre_type_to_size(fre_type);

kernel/unwind/sframe.c line 138

Quoted 10 lines
kernel/unwind/sframe.c new file +10 -0
+	if (!addr_size)
+		return -EINVAL;
+
+	ip_off = ip - sec->sframe_addr - fde->start_addr;
+
+	f = (void *)sec->fres_addr + fde->fres_off;
+
+	for (i = 0; i < fde->fres_num; i++) {
+
+		SFRAME_GET_USER_UNSIGNED(fre_ip_off, f, addr_size);

You already use SFRAME_GET_USER_UNSIGNED() to read it.

Quoted 18 lines
kernel/unwind/sframe.c new file +18 -0
+
+		if (fde_type == SFRAME_FDE_TYPE_PCINC) {
+			if (fre_ip_off > ip_off)
+				break;
+		} else {
+			/* SFRAME_FDE_TYPE_PCMASK */
+			if (ip_off % fde->rep_size < fre_ip_off)
+				break;
+		}
+
+		SFRAME_GET_USER_UNSIGNED(fre_info, f, 1);
+
+		offset_count = SFRAME_FRE_OFFSET_COUNT(fre_info);
+		offset_size  = offset_size_enum_to_size(SFRAME_FRE_OFFSET_SIZE(fre_info));
+
+		if (!offset_count || !offset_size)
+			return -EINVAL;
+

kernel/unwind/sframe.c line 166

Quoted 10 lines
kernel/unwind/sframe.c new file +10 -0
+		last_f = f;
+		f += offset_count * offset_size;
+	}
+
+	if (!last_f)
+		return -EINVAL;
+
+	f = last_f;
+
+	SFRAME_GET_USER_UNSIGNED(cfa_off, f, offset_size);

As far as I know the CFA offset from CFA base register is signed in the SFrame file format. See Binutils include/sframe-api.h, sframe_fre_get_cfa_offset(). Therefore use SFRAME_GET_USER_SIGNED(). Both cfa_off and struct user_unwind_frame cfa_off are already defined as s32.

kernel/unwind/sframe.c line 170

Quoted 4 lines
kernel/unwind/sframe.c new file +4 -0
+	offset_count--;
+
+	ra_off = sec->ra_off;
+	if (!ra_off) {

On s390 there is no fixed RA offset from CFA ...

kernel/unwind/sframe.c line 172

Quoted 2 lines
kernel/unwind/sframe.c new file +2 -0
+		if (!offset_count--)
+			return -EINVAL;

... and the RA must not neccessarily be saved (e.g. at function entry). But we can address this when sending patches for s390 support.

Quoted 313 lines
kernel/unwind/sframe.c new file +248 -0
+		SFRAME_GET_USER_SIGNED(ra_off, f, offset_size);
+	}
+
+	fp_off = sec->fp_off;
+	if (!fp_off && offset_count) {
+		offset_count--;
+		SFRAME_GET_USER_SIGNED(fp_off, f, offset_size);
+	}
+
+	if (offset_count)
+		return -EINVAL;
+
+	frame->cfa_off = cfa_off;
+	frame->ra_off = ra_off;
+	frame->fp_off = fp_off;
+	frame->use_fp = SFRAME_FRE_CFA_BASE_REG_ID(fre_info) == SFRAME_BASE_REG_FP;
+
+	return 0;
+}
+
+int sframe_find(unsigned long ip, struct user_unwind_frame *frame)
+{
+	struct mm_struct *mm = current->mm;
+	struct sframe_section *sec;
+	struct sframe_fde fde;
+	int srcu_idx;
+	int ret = -EINVAL;
+
+	srcu_idx = srcu_read_lock(&sframe_srcu);
+
+	sec = mtree_load(&mm->sframe_mt, ip);
+	if (!sec) {
+		srcu_read_unlock(&sframe_srcu, srcu_idx);
+		return -EINVAL;
+	}
+
+
+	ret = find_fde(sec, ip, &fde);
+	if (ret)
+		goto err_unlock;
+
+	ret = find_fre(sec, &fde, ip, frame);
+	if (ret)
+		goto err_unlock;
+
+	srcu_read_unlock(&sframe_srcu, srcu_idx);
+	return 0;
+
+err_unlock:
+	srcu_read_unlock(&sframe_srcu, srcu_idx);
+	return ret;
+}
+
+static int get_sframe_file(unsigned long sframe_addr, struct sframe_file *file)
+{
+	struct mm_struct *mm = current->mm;
+	struct vm_area_struct *sframe_vma, *text_vma, *vma;
+	VMA_ITERATOR(vmi, mm, 0);
+
+	mmap_read_lock(mm);
+
+	sframe_vma = vma_lookup(mm, sframe_addr);
+	if (!sframe_vma || !sframe_vma->vm_file)
+		goto err_unlock;
+
+	text_vma = NULL;
+
+	for_each_vma(vmi, vma) {
+		if (vma->vm_file != sframe_vma->vm_file)
+			continue;
+		if (vma->vm_flags & VM_EXEC) {
+			if (text_vma) {
+				/*
+				 * Multiple EXEC segments in a single file
+				 * aren't currently supported, is that a thing?
+				 */
+				mmap_read_unlock(mm);
+				pr_warn_once("unsupported multiple EXEC segments in task %s[%d]\n",
+					     current->comm, current->pid);
+				return -EINVAL;
+			}
+			text_vma = vma;
+		}
+	}
+
+	file->sframe_addr	= sframe_addr;
+	file->text_start	= text_vma->vm_start;
+	file->text_end		= text_vma->vm_end;
+
+	mmap_read_unlock(mm);
+	return 0;
+
+err_unlock:
+	mmap_read_unlock(mm);
+	return -EINVAL;
+}
+
+static int validate_sframe_addrs(struct sframe_file *file)
+{
+	struct mm_struct *mm = current->mm;
+	struct vm_area_struct *text_vma;
+
+	mmap_read_lock(mm);
+
+	if (!vma_lookup(mm, file->sframe_addr))
+		goto err_unlock;
+
+	text_vma = vma_lookup(mm, file->text_start);
+	if (!(text_vma->vm_flags & VM_EXEC))
+		goto err_unlock;
+
+	if (vma_lookup(mm, file->text_end-1) != text_vma)
+		goto err_unlock;
+
+	mmap_read_unlock(mm);
+	return 0;
+
+err_unlock:
+	mmap_read_unlock(mm);
+	return -EINVAL;
+}
+
+int __sframe_add_section(struct sframe_file *file)
+{
+	struct maple_tree *sframe_mt = &current->mm->sframe_mt;
+	struct sframe_section *sec;
+	struct sframe_header shdr;
+	unsigned long header_end;
+	int ret;
+
+	if (copy_from_user(&shdr, (void *)file->sframe_addr, sizeof(shdr)))
+		return -EFAULT;
+
+	if (shdr.preamble.magic != SFRAME_MAGIC ||
+	    shdr.preamble.version != SFRAME_VERSION_2 ||
+	    !(shdr.preamble.flags & SFRAME_F_FDE_SORTED) ||
+	    shdr.auxhdr_len || !shdr.num_fdes || !shdr.num_fres ||
+	    shdr.fdes_off > shdr.fres_off) {
+		/*
+		 * Either binutils < 2.41, corrupt sframe header, or
+		 * unsupported feature.
+		 * */
+		pr_warn_once("bad sframe header in task %s[%d]\n",
+			     current->comm, current->pid);
+		return -EINVAL;
+	}
+
+	header_end = file->sframe_addr + SFRAME_HDR_SIZE(shdr);
+
+	sec = kmalloc(sizeof(*sec), GFP_KERNEL);
+	if (!sec)
+		return -ENOMEM;
+
+	sec->sframe_addr	= file->sframe_addr;
+	sec->text_addr		= file->text_start;
+	sec->fdes_addr		= header_end + shdr.fdes_off;
+	sec->fres_addr		= header_end + shdr.fres_off;
+	sec->fdes_nr		= shdr.num_fdes;
+	sec->ra_off		= shdr.cfa_fixed_ra_offset;
+	sec->fp_off		= shdr.cfa_fixed_fp_offset;
+
+	ret = mtree_insert_range(sframe_mt, file->text_start, file->text_end,
+				 sec, GFP_KERNEL);
+	if (ret) {
+		kfree(sec);
+		return ret;
+	}
+
+	return 0;
+}
+
+int sframe_add_section(unsigned long sframe_addr, unsigned long text_start, unsigned long text_end)
+{
+	struct sframe_file file;
+	int ret;
+
+	if (!text_start || !text_end) {
+		ret = get_sframe_file(sframe_addr, &file);
+		if (ret)
+			return ret;
+	} else {
+		/*
+		 * This is mainly for generated code, for which the text isn't
+		 * file-backed so the user has to give the text bounds.
+		 */
+		file.sframe_addr	= sframe_addr;
+		file.text_start		= text_start;
+		file.text_end		= text_end;
+		ret = validate_sframe_addrs(&file);
+		if (ret)
+			return ret;
+	}
+
+	return __sframe_add_section(&file);
+}
+
+static void sframe_free_rcu(struct rcu_head *rcu)
+{
+	struct sframe_section *sec = container_of(rcu, struct sframe_section, rcu);
+
+	kfree(sec);
+}
+
+static int __sframe_remove_section(struct mm_struct *mm,
+				   struct sframe_section *sec)
+{
+	struct sframe_section *s;
+
+	s = mtree_erase(&mm->sframe_mt, sec->text_addr);
+	if (!s || WARN_ON_ONCE(s != sec))
+		return -EINVAL;
+
+	call_srcu(&sframe_srcu, &sec->rcu, sframe_free_rcu);
+
+	return 0;
+}
+
+int sframe_remove_section(unsigned long sframe_addr)
+{
+	struct mm_struct *mm = current->mm;
+	struct sframe_section *sec;
+	unsigned long index = 0;
+
+	sec = mtree_load(&mm->sframe_mt, sframe_addr);
+	if (!sec)
+		return -EINVAL;
+
+	mt_for_each(&mm->sframe_mt, sec, index, ULONG_MAX) {
+		if (sec->sframe_addr == sframe_addr)
+			return __sframe_remove_section(mm, sec);
+	}
+
+	return -EINVAL;
+}
+
+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)
+		kfree(sec);
+
+	mtree_destroy(&mm->sframe_mt);
+}
kernel/unwind/sframe.h new file +59 -0
new file mode 100644
index 000000000000..aa468d6f1f4a
@@ -0,0 +1,215 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2023, Oracle and/or its affiliates.
+ *
+ * This file contains definitions for the SFrame stack tracing format, which is
+ * documented at https://sourceware.org/binutils/docs
+ */
+#ifndef _SFRAME_H
+#define _SFRAME_H
+
+#include <linux/types.h>
+
+#define SFRAME_VERSION_1	1
+#define SFRAME_VERSION_2	2
+#define SFRAME_MAGIC		0xdee2
+
+/* Function Descriptor Entries are sorted on PC. */
+#define SFRAME_F_FDE_SORTED	0x1
+/* Frame-pointer based stack tracing. Defined, but not set. */
+#define SFRAME_F_FRAME_POINTER	0x2
+
+#define SFRAME_CFA_FIXED_FP_INVALID 0
+#define SFRAME_CFA_FIXED_RA_INVALID 0
+
+/* Supported ABIs/Arch. */
+#define SFRAME_ABI_AARCH64_ENDIAN_BIG	    1 /* AARCH64 big endian. */
+#define SFRAME_ABI_AARCH64_ENDIAN_LITTLE    2 /* AARCH64 little endian. */
+#define SFRAME_ABI_AMD64_ENDIAN_LITTLE	    3 /* AMD64 little endian. */
+
+/* SFrame FRE types. */
+#define SFRAME_FRE_TYPE_ADDR1	0
+#define SFRAME_FRE_TYPE_ADDR2	1
+#define SFRAME_FRE_TYPE_ADDR4	2
+
+/*
+ * SFrame Function Descriptor Entry types.
+ *
+ * The SFrame format has two possible representations for functions. The
+ * choice of which type to use is made according to the instruction patterns
+ * in the relevant program stub.
+ */
+
+/* Unwinders perform a (PC >= FRE_START_ADDR) to look up a matching FRE. */
+#define SFRAME_FDE_TYPE_PCINC	0
+/*
+ * Unwinders perform a (PC & FRE_START_ADDR_AS_MASK >= FRE_START_ADDR_AS_MASK)
+ * to look up a matching FRE. Typical usecases are pltN entries, trampolines
+ * etc.
+ */
+#define SFRAME_FDE_TYPE_PCMASK	1
+
+/**
+ * struct sframe_preamble - SFrame Preamble.
+ * @magic: Magic number (SFRAME_MAGIC).
+ * @version: Format version number (SFRAME_VERSION).
+ * @flags: Various flags.
+ */
+struct sframe_preamble {
+	u16 magic;

kernel/unwind/sframe.h line 69

Quoted 10 lines
kernel/unwind/sframe.h new file +10 -0
+	u8  version;
+	u8  flags;
+} __packed;
+
+/**
+ * struct sframe_header - SFrame Header.
+ * @preamble: SFrame preamble.
+ * @abi_arch: Identify the arch (including endianness) and ABI.
+ * @cfa_fixed_fp_offset: Offset for the Frame Pointer (FP) from CFA may be
+ *	  fixed  for some ABIs ((e.g, in AMD64 when -fno-omit-frame-pointer is

Nit: Two consecutive spaces in "fixed for".

Quoted 205 lines
kernel/unwind/sframe.h new file +146 -0
+ *	  used). When fixed, this field specifies the fixed stack frame offset
+ *	  and the individual FREs do not need to track it. When not fixed, it
+ *	  is set to SFRAME_CFA_FIXED_FP_INVALID, and the individual FREs may
+ *	  provide the applicable stack frame offset, if any.
+ * @cfa_fixed_ra_offset: Offset for the Return Address from CFA is fixed for
+ *	  some ABIs. When fixed, this field specifies the fixed stack frame
+ *	  offset and the individual FREs do not need to track it. When not
+ *	  fixed, it is set to SFRAME_CFA_FIXED_FP_INVALID.
+ * @auxhdr_len: Number of bytes making up the auxiliary header, if any.
+ *	  Some ABI/arch, in the future, may use this space for extending the
+ *	  information in SFrame header. Auxiliary header is contained in bytes
+ *	  sequentially following the sframe_header.
+ * @num_fdes: Number of SFrame FDEs in this SFrame section.
+ * @num_fres: Number of SFrame Frame Row Entries.
+ * @fre_len:  Number of bytes in the SFrame Frame Row Entry section.
+ * @fdes_off: Offset of SFrame Function Descriptor Entry section.
+ * @fres_off: Offset of SFrame Frame Row Entry section.
+ */
+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_HDR_SIZE(sframe_hdr)	\
+	((sizeof(struct sframe_header) + (sframe_hdr).auxhdr_len))
+
+/* Two possible keys for executable (instruction) pointers signing. */
+#define SFRAME_AARCH64_PAUTH_KEY_A    0 /* Key A. */
+#define SFRAME_AARCH64_PAUTH_KEY_B    1 /* Key B. */
+
+/**
+ * struct sframe_fde - SFrame Function Descriptor Entry.
+ * @start_addr: Function start address. Encoded as a signed offset,
+ *	  relative to the current FDE.
+ * @size: Size of the function in bytes.
+ * @fres_off: Offset of the first SFrame Frame Row Entry of the function,
+ *	  relative to the beginning of the SFrame Frame Row Entry sub-section.
+ * @fres_num: Number of frame row entries for the function.
+ * @info: Additional information for deciphering the stack trace
+ *	  information for the function. Contains information about SFrame FRE
+ *	  type, SFrame FDE type, PAC authorization A/B key, etc.
+ * @rep_size: Block size for SFRAME_FDE_TYPE_PCMASK
+ * @padding: Unused
+ */
+struct sframe_fde {
+	s32 start_addr;
+	u32 size;
+	u32 fres_off;
+	u32 fres_num;
+	u8  info;
+	u8  rep_size;
+	u16 padding;
+} __packed;
+
+/*
+ * 'func_info' in SFrame FDE contains additional information for deciphering
+ * the stack trace information for the function. In V1, the information is
+ * organized as follows:
+ *   - 4-bits: Identify the FRE type used for the function.
+ *   - 1-bit: Identify the FDE type of the function - mask or inc.
+ *   - 1-bit: PAC authorization A/B key (aarch64).
+ *   - 2-bits: Unused.
+ * ---------------------------------------------------------------------
+ * |  Unused  |  PAC auth A/B key (aarch64) |  FDE type |   FRE type   |
+ * |          |        Unused (amd64)       |           |              |
+ * ---------------------------------------------------------------------
+ * 8          6                             5           4              0
+ */
+
+/* Note: Set PAC auth key to SFRAME_AARCH64_PAUTH_KEY_A by default.  */
+#define SFRAME_FUNC_INFO(fde_type, fre_enc_type) \
+	(((SFRAME_AARCH64_PAUTH_KEY_A & 0x1) << 5) | \
+	 (((fde_type) & 0x1) << 4) | ((fre_enc_type) & 0xf))
+
+#define SFRAME_FUNC_FRE_TYPE(data)	  ((data) & 0xf)
+#define SFRAME_FUNC_FDE_TYPE(data)	  (((data) >> 4) & 0x1)
+#define SFRAME_FUNC_PAUTH_KEY(data)	  (((data) >> 5) & 0x1)
+
+/*
+ * Size of stack frame offsets in an SFrame Frame Row Entry. A single
+ * SFrame FRE has all offsets of the same size. Offset size may vary
+ * across frame row entries.
+ */
+#define SFRAME_FRE_OFFSET_1B	  0
+#define SFRAME_FRE_OFFSET_2B	  1
+#define SFRAME_FRE_OFFSET_4B	  2
+
+/* An SFrame Frame Row Entry can be SP or FP based.  */
+#define SFRAME_BASE_REG_FP	0
+#define SFRAME_BASE_REG_SP	1
+
+/*
+ * The index at which a specific offset is presented in the variable length
+ * bytes of an FRE.
+ */
+#define SFRAME_FRE_CFA_OFFSET_IDX   0
+/*
+ * The RA stack offset, if present, will always be at index 1 in the variable
+ * length bytes of the FRE.
+ */
+#define SFRAME_FRE_RA_OFFSET_IDX    1
+/*
+ * The FP stack offset may appear at offset 1 or 2, depending on the ABI as RA
+ * may or may not be tracked.
+ */
+#define SFRAME_FRE_FP_OFFSET_IDX    2
+
+/*
+ * 'fre_info' in SFrame FRE contains information about:
+ *   - 1 bit: base reg for CFA
+ *   - 4 bits: Number of offsets (N). A value of up to 3 is allowed to track
+ *   all three of CFA, FP and RA (fixed implicit order).
+ *   - 2 bits: information about size of the offsets (S) in bytes.
+ *     Valid values are SFRAME_FRE_OFFSET_1B, SFRAME_FRE_OFFSET_2B,
+ *     SFRAME_FRE_OFFSET_4B
+ *   - 1 bit: Mangled RA state bit (aarch64 only).
+ * ---------------------------------------------------------------
+ * | Mangled-RA (aarch64) |  Size of   |   Number of  | base_reg |
+ * |  Unused (amd64)      |  offsets   |    offsets   |          |
+ * ---------------------------------------------------------------
+ * 8                      7             5             1          0
+ */
+
+/* Note: Set mangled_ra_p to zero by default. */
+#define SFRAME_FRE_INFO(base_reg_id, offset_num, offset_size) \
+	(((0 & 0x1) << 7) | (((offset_size) & 0x3) << 5) | \
+	 (((offset_num) & 0xf) << 1) | ((base_reg_id) & 0x1))
+
+/* Set the mangled_ra_p bit as indicated. */
+#define SFRAME_FRE_INFO_UPDATE_MANGLED_RA_P(mangled_ra_p, fre_info) \
+	((((mangled_ra_p) & 0x1) << 7) | ((fre_info) & 0x7f))
+
+#define SFRAME_FRE_CFA_BASE_REG_ID(data)	  ((data) & 0x1)
+#define SFRAME_FRE_OFFSET_COUNT(data)		  (((data) >> 1) & 0xf)
+#define SFRAME_FRE_OFFSET_SIZE(data)		  (((data) >> 5) & 0x3)
+#define SFRAME_FRE_MANGLED_RA_P(data)		  (((data) >> 7) & 0x1)
+
+#endif /* _SFRAME_H */
kernel/unwind/user.c +14 -0
index 5d16f9604a61..3a7b14cf522b 100644
@@ -8,6 +8,7 @@
  #include <linux/sched.h>
  #include <linux/sched/task_stack.h>
  #include <linux/user_unwind.h>
+#include <linux/sframe.h>
  #include <linux/uaccess.h>
  #include <asm/user_unwind.h>
 
@@ -29,6 +30,11 @@ int user_unwind_next(struct user_unwind_state *state)
  	case USER_UNWIND_TYPE_FP:
  		frame = &fp_frame;
  		break;
+	case USER_UNWIND_TYPE_SFRAME:
+		ret = sframe_find(state->ip, frame);
+		if (ret)
+			goto the_end;
+		break;
  	default:
  		BUG();
  	}
@@ -57,6 +63,7 @@ int user_unwind_start(struct user_unwind_state *state,
  		      enum user_unwind_type type)
  {
  	struct pt_regs *regs = task_pt_regs(current);
+	bool sframe_possible = current_has_sframe();
 
  	memset(state, 0, sizeof(*state));
 
@@ -67,6 +74,13 @@ int user_unwind_start(struct user_unwind_state *state,
 
  	switch (type) {
  	case USER_UNWIND_TYPE_AUTO:
+		state->type = sframe_possible ? USER_UNWIND_TYPE_SFRAME :
+						USER_UNWIND_TYPE_FP;
+		break;
+	case USER_UNWIND_TYPE_SFRAME:
+		if (!sframe_possible)
+			return -EINVAL;
+		break;
  	case USER_UNWIND_TYPE_FP:
  		break;
  	default:
mm/init-mm.c +1 -0
index 24c809379274..c4c6af046778 100644
@@ -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

mm/init-mm.c struct mm_struct init_mm = { · line 49

Quoted 6 lines
mm/init-mm.c +2 -1
@@ -44,7 +45,8 @@ struct mm_struct init_mm = {
  #endif
  	.user_ns	= &init_user_ns,
  	.cpu_bitmap	= CPU_BITS_NONE,
-	INIT_MM_CONTEXT(init_mm)
+	INIT_MM_CONTEXT(init_mm),
+	INIT_MM_SFRAME,

This does not compile on s390, as INIT_MM_CONTEXT() is defined with a trailing comma, leading to two consecutive commas. Already reported by the kernel test robot for other architectures. Same if INIT_MM_SFRAME expands into nothing there would be two consecutive commas.

mm/init-mm.c struct mm_struct init_mm = { · line 52

Quoted 3 lines
mm/init-mm.c +0 -0
  };
 
  void setup_initial_init_mm(void *start_code, void *end_code,

Regards,
Jens

Signature · 9 lines
Jens Remus
Linux on Z Development (D3303) and z/VSE Support
+49-7031-16-1128 Office
[email protected]

IBM

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

Re: [PATCH v2 03/11] unwind: Introduce SFrame user space unwinding

Quoted 9 lines · Jens Remus

arch/Kconfig config HAVE_HARDLOCKUP_DETECTOR_ARCH · line 431

Quoted 6 lines

+++ b/arch/Kconfig

arch/Kconfig +2 -0
@@ -428,6 +428,9 @@ config HAVE_HARDLOCKUP_DETECTOR_ARCH
  config HAVE_USER_UNWIND
  	bool
+config HAVE_USER_UNWIND_SFRAME
+	bool

I found this unwinder of userspace using SFrame to depend on your generic unwinder of userspace framework. Does this warrant to add:

Based on your other suggestion I now have:

config UNWIND_USER
bool

config HAVE_UNWIND_USER_FP
bool
select UNWIND_USER

config HAVE_UNWIND_USER_SFRAME
bool
select UNWIND_USER

The arches set HAVE_*, which in turn sets UNWIND_USER.

Quoted 10 lines

kernel/unwind/sframe.c line 121

Quoted 6 lines
kernel/unwind/sframe.c new file +6 -0
+static int find_fre(struct sframe_section *sec, struct sframe_fde *fde,
+		    unsigned long ip, struct user_unwind_frame *frame)
+{
+	unsigned char fde_type = SFRAME_FUNC_FDE_TYPE(fde->info);
+	unsigned char fre_type = SFRAME_FUNC_FRE_TYPE(fde->info);
+	s32 fre_ip_off, cfa_off, ra_off, fp_off, ip_off;

Doesn't fre_ip_off need to be u32 (see also below)? The SFrame format specification states the FRE sfre_start_address is either u8, u16, or u32:
https://sourceware.org/binutils/docs/sframe-spec.html#The-SFrame-FRE-Types

Indeed, I also noticed that.

Quoted 6 lines

kernel/unwind/sframe.c line 166

Quoted 1 line
kernel/unwind/sframe.c new file +1 -0
+	SFRAME_GET_USER_UNSIGNED(cfa_off, f, offset_size);

As far as I know the CFA offset from CFA base register is signed in the SFrame file format. See Binutils include/sframe-api.h, sframe_fre_get_cfa_offset(). Therefore use SFRAME_GET_USER_SIGNED(). Both cfa_off and struct user_unwind_frame cfa_off are already defined as s32.

Good catch. There's no need to have separate SIGNED/UNSIGNED variants anyway, I'll simplify that into SFRAME_GET_USER().

Quoted 13 lines
Quoted 13 lines

+++ b/mm/init-mm.c

mm/init-mm.c +1 -1
@@ -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
@@ -44,7 +45,8 @@ struct mm_struct init_mm = {
  #endif
  	.user_ns	= &init_user_ns,
  	.cpu_bitmap	= CPU_BITS_NONE,
-	INIT_MM_CONTEXT(init_mm)
Quoted 8 lines

mm/init-mm.c struct mm_struct init_mm = { · line 49

Quoted 2 lines
mm/init-mm.c +2 -0
+	INIT_MM_CONTEXT(init_mm),
+	INIT_MM_SFRAME,

This does not compile on s390, as INIT_MM_CONTEXT() is defined with a trailing comma, leading to two consecutive commas. Already reported by the kernel test robot for other architectures.
Same if INIT_MM_SFRAME expands into nothing there would be two consecutive commas.

Yeah, I saw those build errors and I've got that fixed now.

Thanks for the review!

Signature · 1 line
Josh
author maintainer
#17 lore raw

PATCH v2 4/11 unwind/x86/64: Add HAVE_USER_UNWIND_SFRAME

In reply to Josh Poimboeuf

Binutils 2.41 supports generating sframe v2 for x86_64. It works well in testing so enable it.

NOTE: An out-of-tree glibc patch is still needed to enable setting PR_ADD_SFRAME for shared libraries and dlopens.

---
 arch/x86/Kconfig | 1 +
 1 file changed, 1 insertion(+)
arch/x86/Kconfig +1 -0
index 266edff59058..0b3d7c72b65b 100644
@@ -287,6 +287,7 @@ config X86
 	select HAVE_UNSTABLE_SCHED_CLOCK
 	select HAVE_USER_RETURN_NOTIFIER
 	select HAVE_USER_UNWIND
+	select HAVE_USER_UNWIND_SFRAME		if X86_64
 	select HAVE_GENERIC_VDSO
 	select VDSO_GETRANDOM			if X86_64
 	select HOTPLUG_PARALLEL			if SMP && X86_64
Signature · 1 line
2.46.0
#18 lore raw

Re: [PATCH v2 04/11] unwind/x86/64: Add HAVE_USER_UNWIND_SFRAME

kernel test robot noticed the following build warnings:

Hi Josh,

kernel test robot noticed the following build warnings:

[auto build test WARNING on perf-tools-next/perf-tools-next]
[also build test WARNING on tip/perf/core perf-tools/perf-tools linus/master v6.11-rc7 next-20240913] [cannot apply to acme/perf/core]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Josh-Poimboeuf/unwind-Introduce-generic-user-space-unwinding-interface/20240914-070619
base:   https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git perf-tools-next
patch link:    https://lore.kernel.org/r/37de872b0894b21408754ae2903b7701cc6dfab7.1726268190.git.jpoimboe%40kernel.org
patch subject: [PATCH v2 04/11] unwind/x86/64: Add HAVE_USER_UNWIND_SFRAME
config: x86_64-randconfig-121-20240914 (https://download.01.org/0day-ci/archive/20240915/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240915/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

sparse warnings: (new ones prefixed by >>)

>> kernel/unwind/sframe.c:94:15: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct sframe_fde [noderef] __user *first @@     got void * @@
   kernel/unwind/sframe.c:94:15: sparse:     expected struct sframe_fde [noderef] __user *first
   kernel/unwind/sframe.c:94:15: sparse:     got void *
>> kernel/unwind/sframe.c:98:21: sparse: sparse: cast removes address space '__user' of expression
>> kernel/unwind/sframe.c:98:21: sparse: sparse: cast removes address space '__user' of expression
>> kernel/unwind/sframe.c:98:21: sparse: sparse: cast removes address space '__user' of expression
>> kernel/unwind/sframe.c:98:21: sparse: sparse: cast removes address space '__user' of expression
>> kernel/unwind/sframe.c:98:21: sparse: sparse: cast removes address space '__user' of expression
>> kernel/unwind/sframe.c:98:21: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const volatile [noderef] __user *ptr @@     got signed int [usertype] * @@
   kernel/unwind/sframe.c:98:21: sparse:     expected void const volatile [noderef] __user *ptr
   kernel/unwind/sframe.c:98:21: sparse:     got signed int [usertype] *
>> kernel/unwind/sframe.c:98:21: sparse: sparse: cast removes address space '__user' of expression
>> kernel/unwind/sframe.c:98:21: sparse: sparse: cast removes address space '__user' of expression
>> kernel/unwind/sframe.c:98:21: sparse: sparse: cast removes address space '__user' of expression
>> kernel/unwind/sframe.c:134:11: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected void [noderef] __user *f @@     got void * @@
   kernel/unwind/sframe.c:134:11: sparse:     expected void [noderef] __user *f
   kernel/unwind/sframe.c:134:11: sparse:     got void *
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
>> kernel/unwind/sframe.c:138:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const volatile [noderef] __user *ptr @@     got unsigned char [usertype] * @@
   kernel/unwind/sframe.c:138:17: sparse:     expected void const volatile [noderef] __user *ptr
   kernel/unwind/sframe.c:138:17: sparse:     got unsigned char [usertype] *
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
>> kernel/unwind/sframe.c:138:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const volatile [noderef] __user *ptr @@     got unsigned short [usertype] * @@
   kernel/unwind/sframe.c:138:17: sparse:     expected void const volatile [noderef] __user *ptr
   kernel/unwind/sframe.c:138:17: sparse:     got unsigned short [usertype] *
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
>> kernel/unwind/sframe.c:138:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const volatile [noderef] __user *ptr @@     got unsigned int [usertype] * @@
   kernel/unwind/sframe.c:138:17: sparse:     expected void const volatile [noderef] __user *ptr
   kernel/unwind/sframe.c:138:17: sparse:     got unsigned int [usertype] *
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:138:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const volatile [noderef] __user *ptr @@     got unsigned char [usertype] * @@
   kernel/unwind/sframe.c:149:17: sparse:     expected void const volatile [noderef] __user *ptr
   kernel/unwind/sframe.c:149:17: sparse:     got unsigned char [usertype] *
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const volatile [noderef] __user *ptr @@     got unsigned short [usertype] * @@
   kernel/unwind/sframe.c:149:17: sparse:     expected void const volatile [noderef] __user *ptr
   kernel/unwind/sframe.c:149:17: sparse:     got unsigned short [usertype] *
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const volatile [noderef] __user *ptr @@     got unsigned int [usertype] * @@
   kernel/unwind/sframe.c:149:17: sparse:     expected void const volatile [noderef] __user *ptr
   kernel/unwind/sframe.c:149:17: sparse:     got unsigned int [usertype] *
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:149:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const volatile [noderef] __user *ptr @@     got unsigned char [usertype] * @@
   kernel/unwind/sframe.c:166:9: sparse:     expected void const volatile [noderef] __user *ptr
   kernel/unwind/sframe.c:166:9: sparse:     got unsigned char [usertype] *
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const volatile [noderef] __user *ptr @@     got unsigned short [usertype] * @@
   kernel/unwind/sframe.c:166:9: sparse:     expected void const volatile [noderef] __user *ptr
   kernel/unwind/sframe.c:166:9: sparse:     got unsigned short [usertype] *
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const volatile [noderef] __user *ptr @@     got unsigned int [usertype] * @@
   kernel/unwind/sframe.c:166:9: sparse:     expected void const volatile [noderef] __user *ptr
   kernel/unwind/sframe.c:166:9: sparse:     got unsigned int [usertype] *
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:166:9: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
>> kernel/unwind/sframe.c:173:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const volatile [noderef] __user *ptr @@     got signed char [usertype] * @@
   kernel/unwind/sframe.c:173:17: sparse:     expected void const volatile [noderef] __user *ptr
   kernel/unwind/sframe.c:173:17: sparse:     got signed char [usertype] *
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
>> kernel/unwind/sframe.c:173:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const volatile [noderef] __user *ptr @@     got signed short [usertype] * @@
   kernel/unwind/sframe.c:173:17: sparse:     expected void const volatile [noderef] __user *ptr
   kernel/unwind/sframe.c:173:17: sparse:     got signed short [usertype] *
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:173:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const volatile [noderef] __user *ptr @@     got signed int [usertype] * @@
   kernel/unwind/sframe.c:173:17: sparse:     expected void const volatile [noderef] __user *ptr
   kernel/unwind/sframe.c:173:17: sparse:     got signed int [usertype] *
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:173:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const volatile [noderef] __user *ptr @@     got signed char [usertype] * @@
   kernel/unwind/sframe.c:179:17: sparse:     expected void const volatile [noderef] __user *ptr
   kernel/unwind/sframe.c:179:17: sparse:     got signed char [usertype] *
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const volatile [noderef] __user *ptr @@     got signed short [usertype] * @@
   kernel/unwind/sframe.c:179:17: sparse:     expected void const volatile [noderef] __user *ptr
   kernel/unwind/sframe.c:179:17: sparse:     got signed short [usertype] *
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const volatile [noderef] __user *ptr @@     got signed int [usertype] * @@
   kernel/unwind/sframe.c:179:17: sparse:     expected void const volatile [noderef] __user *ptr
   kernel/unwind/sframe.c:179:17: sparse:     got signed int [usertype] *
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
   kernel/unwind/sframe.c:179:17: sparse: sparse: cast removes address space '__user' of expression
>> kernel/unwind/sframe.c:303:36: sparse: sparse: incorrect type in argument 2 (different address spaces) @@     expected void const [noderef] __user *from @@     got void * @@

kernel/unwind/sframe.c:303:36: sparse: expected void const [noderef] __user *from
kernel/unwind/sframe.c:303:36: sparse: got void *

vim +94 kernel/unwind/sframe.c

6ce9812d0cc74f Josh Poimboeuf 2024-09-14   85  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   86  static int find_fde(struct sframe_section *sec, unsigned long ip,
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   87  		    struct sframe_fde *fde)
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   88  {
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   89  	s32 func_off, ip_off;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   90  	struct sframe_fde __user *first, *last, *mid, *found;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   91  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   92  	ip_off = ip - sec->sframe_addr;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   93  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  @94  	first = (void *)sec->fdes_addr;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   95  	last = first + sec->fdes_nr;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   96  	while (first <= last) {
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   97  		mid = first + ((last - first) / 2);
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  @98  		if (get_user(func_off, (s32 *)mid))
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   99  			return -EFAULT;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  100  		if (ip_off >= func_off) {
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  101  			found = mid;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  102  			first = mid + 1;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  103  		} else
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  104  			last = mid - 1;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  105  	}
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  106  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  107  	if (!found)
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  108  		return -EINVAL;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  109  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  110  	if (copy_from_user(fde, found, sizeof(*fde)))
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  111  		return -EFAULT;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  112  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  113  	return 0;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  114  }
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  115  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  116  static int find_fre(struct sframe_section *sec, struct sframe_fde *fde,
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  117  		    unsigned long ip, struct user_unwind_frame *frame)
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  118  {
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  119  	unsigned char fde_type = SFRAME_FUNC_FDE_TYPE(fde->info);
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  120  	unsigned char fre_type = SFRAME_FUNC_FRE_TYPE(fde->info);
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  121  	s32 fre_ip_off, cfa_off, ra_off, fp_off, ip_off;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  122  	unsigned char offset_count, offset_size;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  123  	unsigned char addr_size;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  124  	void __user *f, *last_f;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  125  	u8 fre_info;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  126  	int i;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  127  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  128  	addr_size = fre_type_to_size(fre_type);
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  129  	if (!addr_size)
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  130  		return -EINVAL;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  131  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  132  	ip_off = ip - sec->sframe_addr - fde->start_addr;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  133  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14 @134  	f = (void *)sec->fres_addr + fde->fres_off;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  135  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  136  	for (i = 0; i < fde->fres_num; i++) {
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  137  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14 @138  		SFRAME_GET_USER_UNSIGNED(fre_ip_off, f, addr_size);
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  139  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  140  		if (fde_type == SFRAME_FDE_TYPE_PCINC) {
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  141  			if (fre_ip_off > ip_off)
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  142  				break;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  143  		} else {
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  144  			/* SFRAME_FDE_TYPE_PCMASK */
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  145  			if (ip_off % fde->rep_size < fre_ip_off)
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  146  				break;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  147  		}
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  148  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  149  		SFRAME_GET_USER_UNSIGNED(fre_info, f, 1);
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  150  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  151  		offset_count = SFRAME_FRE_OFFSET_COUNT(fre_info);
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  152  		offset_size  = offset_size_enum_to_size(SFRAME_FRE_OFFSET_SIZE(fre_info));
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  153  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  154  		if (!offset_count || !offset_size)
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  155  			return -EINVAL;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  156  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  157  		last_f = f;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  158  		f += offset_count * offset_size;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  159  	}
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  160  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  161  	if (!last_f)
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  162  		return -EINVAL;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  163  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  164  	f = last_f;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  165  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  166  	SFRAME_GET_USER_UNSIGNED(cfa_off, f, offset_size);
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  167  	offset_count--;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  168  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  169  	ra_off = sec->ra_off;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  170  	if (!ra_off) {
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  171  		if (!offset_count--)
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  172  			return -EINVAL;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14 @173  		SFRAME_GET_USER_SIGNED(ra_off, f, offset_size);
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  174  	}
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  175  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  176  	fp_off = sec->fp_off;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  177  	if (!fp_off && offset_count) {
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  178  		offset_count--;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  179  		SFRAME_GET_USER_SIGNED(fp_off, f, offset_size);
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  180  	}
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  181  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  182  	if (offset_count)
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  183  		return -EINVAL;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  184  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  185  	frame->cfa_off = cfa_off;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  186  	frame->ra_off = ra_off;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  187  	frame->fp_off = fp_off;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  188  	frame->use_fp = SFRAME_FRE_CFA_BASE_REG_ID(fre_info) == SFRAME_BASE_REG_FP;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  189  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  190  	return 0;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  191  }
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  192  
Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
#19 lore raw

Re: [PATCH v2 04/11] unwind/x86/64: Add HAVE_USER_UNWIND_SFRAME

In reply to Josh Poimboeuf

Hi Josh,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Josh-Poimboeuf/unwind-Introduce-generic-user-space-unwinding-interface/20240914-070619
base:   https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git perf-tools-next
patch link:    https://lore.kernel.org/r/37de872b0894b21408754ae2903b7701cc6dfab7.1726268190.git.jpoimboe%40kernel.org
patch subject: [PATCH v2 04/11] unwind/x86/64: Add HAVE_USER_UNWIND_SFRAME
config: x86_64-randconfig-r072-20240914 (https://download.01.org/0day-ci/archive/20240915/[email protected]/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags

| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Closes: https://lore.kernel.org/r/[email protected]/

smatch warnings:
kernel/unwind/sframe.c:107 find_fde() error: uninitialized symbol 'found'. kernel/unwind/sframe.c:161 find_fre() error: uninitialized symbol 'last_f'. kernel/unwind/sframe.c:166 find_fre() error: uninitialized symbol 'offset_size'. kernel/unwind/sframe.c:167 find_fre() error: uninitialized symbol 'offset_count'. kernel/unwind/sframe.c:188 find_fre() error: uninitialized symbol 'fre_info'.

vim +/found +107 kernel/unwind/sframe.c

6ce9812d0cc74f Josh Poimboeuf 2024-09-14   86  static int find_fde(struct sframe_section *sec, unsigned long ip,
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   87  		    struct sframe_fde *fde)
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   88  {
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   89  	s32 func_off, ip_off;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   90  	struct sframe_fde __user *first, *last, *mid, *found;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   91  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   92  	ip_off = ip - sec->sframe_addr;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   93  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   94  	first = (void *)sec->fdes_addr;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   95  	last = first + sec->fdes_nr;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   96  	while (first <= last) {
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   97  		mid = first + ((last - first) / 2);
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   98  		if (get_user(func_off, (s32 *)mid))
6ce9812d0cc74f Josh Poimboeuf 2024-09-14   99  			return -EFAULT;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  100  		if (ip_off >= func_off) {
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  101  			found = mid;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  102  			first = mid + 1;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  103  		} else
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  104  			last = mid - 1;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  105  	}
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  106  

6ce9812d0cc74f Josh Poimboeuf 2024-09-14 @107 if (!found)

found needs to be initialized to NULL at the start.

6ce9812d0cc74f Josh Poimboeuf 2024-09-14  108  		return -EINVAL;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  109  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  110  	if (copy_from_user(fde, found, sizeof(*fde)))
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  111  		return -EFAULT;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  112  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  113  	return 0;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  114  }
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  115  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  116  static int find_fre(struct sframe_section *sec, struct sframe_fde *fde,
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  117  		    unsigned long ip, struct user_unwind_frame *frame)
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  118  {
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  119  	unsigned char fde_type = SFRAME_FUNC_FDE_TYPE(fde->info);
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  120  	unsigned char fre_type = SFRAME_FUNC_FRE_TYPE(fde->info);
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  121  	s32 fre_ip_off, cfa_off, ra_off, fp_off, ip_off;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  122  	unsigned char offset_count, offset_size;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  123  	unsigned char addr_size;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  124  	void __user *f, *last_f;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  125  	u8 fre_info;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  126  	int i;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  127  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  128  	addr_size = fre_type_to_size(fre_type);
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  129  	if (!addr_size)
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  130  		return -EINVAL;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  131  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  132  	ip_off = ip - sec->sframe_addr - fde->start_addr;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  133  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  134  	f = (void *)sec->fres_addr + fde->fres_off;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  135  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  136  	for (i = 0; i < fde->fres_num; i++) {
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  137  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  138  		SFRAME_GET_USER_UNSIGNED(fre_ip_off, f, addr_size);
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  139  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  140  		if (fde_type == SFRAME_FDE_TYPE_PCINC) {
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  141  			if (fre_ip_off > ip_off)
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  142  				break;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  143  		} else {
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  144  			/* SFRAME_FDE_TYPE_PCMASK */
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  145  			if (ip_off % fde->rep_size < fre_ip_off)
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  146  				break;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  147  		}
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  148  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  149  		SFRAME_GET_USER_UNSIGNED(fre_info, f, 1);
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  150  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  151  		offset_count = SFRAME_FRE_OFFSET_COUNT(fre_info);
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  152  		offset_size  = offset_size_enum_to_size(SFRAME_FRE_OFFSET_SIZE(fre_info));
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  153  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  154  		if (!offset_count || !offset_size)
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  155  			return -EINVAL;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  156  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  157  		last_f = f;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  158  		f += offset_count * offset_size;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  159  	}
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  160  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14 @161  	if (!last_f)
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  162  		return -EINVAL;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  163  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  164  	f = last_f;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  165  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14 @166  	SFRAME_GET_USER_UNSIGNED(cfa_off, f, offset_size);
6ce9812d0cc74f Josh Poimboeuf 2024-09-14 @167  	offset_count--;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  168  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  169  	ra_off = sec->ra_off;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  170  	if (!ra_off) {
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  171  		if (!offset_count--)
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  172  			return -EINVAL;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  173  		SFRAME_GET_USER_SIGNED(ra_off, f, offset_size);
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  174  	}
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  175  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  176  	fp_off = sec->fp_off;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  177  	if (!fp_off && offset_count) {
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  178  		offset_count--;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  179  		SFRAME_GET_USER_SIGNED(fp_off, f, offset_size);
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  180  	}
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  181  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  182  	if (offset_count)
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  183  		return -EINVAL;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  184  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  185  	frame->cfa_off = cfa_off;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  186  	frame->ra_off = ra_off;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  187  	frame->fp_off = fp_off;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14 @188  	frame->use_fp = SFRAME_FRE_CFA_BASE_REG_ID(fre_info) == SFRAME_BASE_REG_FP;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  189  
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  190  	return 0;
6ce9812d0cc74f Josh Poimboeuf 2024-09-14  191  }
Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
author maintainer
#20 lore raw

PATCH v2 5/11 perf/x86: Use user_unwind interface

In reply to Josh Poimboeuf

Simplify __perf_callchain_user() and prepare to enable deferred sframe unwinding by switching to the generic user unwind interface.

---
 arch/x86/events/core.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)
arch/x86/events/core.c +5 -13
index be01823b1bb4..e82aadf99d9b 100644
@@ -29,6 +29,7 @@
 #include <linux/device.h>
 #include <linux/nospec.h>
 #include <linux/static_call.h>
+#include <linux/user_unwind.h>
 
 #include <asm/apic.h>
 #include <asm/stacktrace.h>
@@ -2862,8 +2863,7 @@ perf_callchain_user32(struct pt_regs *regs, struct perf_callchain_entry_ctx *ent
 void
 perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs)
 {
-	struct stack_frame frame;
-	const struct stack_frame __user *fp;
+	struct user_unwind_state state;
 
 	if (perf_guest_state()) {
 		/* TODO: We don't support guest os callchain now */
@@ -2876,8 +2876,6 @@ perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs
 	if (regs->flags & (X86_VM_MASK | PERF_EFLAGS_VM))
 		return;
 
-	fp = (void __user *)regs->bp;
-
 	perf_callchain_store(entry, regs->ip);
 
 	if (!nmi_uaccess_okay())
@@ -2887,18 +2885,12 @@ perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs
 		return;
 
 	pagefault_disable();
-	while (entry->nr < entry->max_stack) {
-		if (!valid_user_frame(fp, sizeof(frame)))
-			break;
 
-		if (__get_user(frame.next_frame, &fp->next_frame))
+	for_each_user_frame(&state, USER_UNWIND_TYPE_FP) {
+		if (perf_callchain_store(entry, state.ip))
 			break;
-		if (__get_user(frame.return_address, &fp->return_address))
-			break;
-
-		perf_callchain_store(entry, frame.return_address);
-		fp = (void __user *)frame.next_frame;
 	}
+
 	pagefault_enable();
 }
 
Signature · 1 line
2.46.0
#21 lore raw

Re: [PATCH v2 05/11] perf/x86: Use user_unwind interface

kernel test robot noticed "kernel_BUG_at_kernel/unwind/user.c" on:

Hello,

kernel test robot noticed "kernel_BUG_at_kernel/unwind/user.c" on:

commit: 164c5ae4072303c9eb4e263115a5e70d5a3cc052 ("[PATCH v2 05/11] perf/x86: Use user_unwind interface")
url: https://github.com/intel-lab-lkp/linux/commits/Josh-Poimboeuf/unwind-Introduce-generic-user-space-unwinding-interface/20240914-070619
base: https://git.kernel.org/cgit/linux/kernel/git/perf/perf-tools-next.git perf-tools-next
patch link: https://lore.kernel.org/all/daf3f59e0d14ee11b45ad6735b8a211a3c7534dc.1726268190.git.jpoimboe@kernel.org/
patch subject: [PATCH v2 05/11] perf/x86: Use user_unwind interface

in testcase: fsmark
version: fsmark-x86_64-2628be5-1_20240224
with following parameters:

	iterations: 1x
	nr_threads: 64t
	disk: 1BRD_48G
	fs: xfs
	filesize: 4M
	test_size: 24G
	sync_method: NoSync
	cpufreq_governor: performance

compiler: gcc-12
test machine: 96 threads 2 sockets Intel(R) Xeon(R) Platinum 8260L CPU @ 2.40GHz (Cascade Lake) with 128G memory

(please refer to attached dmesg/kmsg for entire log/backtrace)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-lkp/[email protected]

[   53.036370][    C3] ------------[ cut here ]------------
[   53.036374][    C3] kernel BUG at kernel/unwind/user.c:39!
[   53.036381][    C3] Oops: invalid opcode: 0000 [#1] SMP NOPTI
[   53.036385][    C3] CPU: 3 UID: 0 PID: 1317 Comm: sed Tainted: G S                 6.11.0-rc6-00501-g164c5ae40723 #1
[   53.036388][    C3] Tainted: [S]=CPU_OUT_OF_SPEC
[   53.036389][    C3] Hardware name: Intel Corporation S2600WFD/S2600WFD, BIOS SE5C620.86B.0D.01.0286.011120190816 01/11/2019
[ 53.036390][ C3] RIP: 0010:user_unwind_next (kernel/unwind/user.c:39) 
[ 53.036400][ C3] Code: 01 eb c1 48 01 f0 e8 a0 15 ea 00 85 c0 75 ee 48 89 73 08 4c 89 03 8b 41 08 85 c0 74 a5 48 89 53 10 eb 9f bf ea ff ff ff eb 9a <0f> 0b e8 ba b9 ea 00 66 66 2e 0f 1f 84 00 00 00 00 00 66 66 2e 0f
All code
========
   0:	01 eb                	add    %ebp,%ebx
   2:	c1 48 01 f0          	rorl   $0xf0,0x1(%rax)
   6:	e8 a0 15 ea 00       	callq  0xea15ab
   b:	85 c0                	test   %eax,%eax
   d:	75 ee                	jne    0xfffffffffffffffd
   f:	48 89 73 08          	mov    %rsi,0x8(%rbx)
  13:	4c 89 03             	mov    %r8,(%rbx)
  16:	8b 41 08             	mov    0x8(%rcx),%eax
  19:	85 c0                	test   %eax,%eax
  1b:	74 a5                	je     0xffffffffffffffc2
  1d:	48 89 53 10          	mov    %rdx,0x10(%rbx)
  21:	eb 9f                	jmp    0xffffffffffffffc2
  23:	bf ea ff ff ff       	mov    $0xffffffea,%edi
  28:	eb 9a                	jmp    0xffffffffffffffc4
  2a:*	0f 0b                	ud2    		<-- trapping instruction
  2c:	e8 ba b9 ea 00       	callq  0xeab9eb
  31:	66 66 2e 0f 1f 84 00 	data16 nopw %cs:0x0(%rax,%rax,1)
  38:	00 00 00 00 
  3c:	66                   	data16
  3d:	66                   	data16
  3e:	2e                   	cs
  3f:	0f                   	.byte 0xf

Code starting with the faulting instruction
===========================================
0: 0f 0b ud2
2: e8 ba b9 ea 00 callq 0xeab9c1
7: 66 66 2e 0f 1f 84 00 data16 nopw %cs:0x0(%rax,%rax,1)
e: 00 00 00 00

  12:	66                   	data16
  13:	66                   	data16
  14:	2e                   	cs
  15:	0f                   	.byte 0xf
[   53.036402][    C3] RSP: 0000:ffffc9000b38b938 EFLAGS: 00010093
[   53.036404][    C3] RAX: 0000000000000000 RBX: ffffc9000b38b960 RCX: ffff888230156880
[   53.036405][    C3] RDX: 000055b666c6ef3c RSI: 0000000000000001 RDI: ffffc9000b38b960
[   53.036407][    C3] RBP: ffffc9000b38bf58 R08: 0000000000000000 R09: 0000000000000000
[   53.036408][    C3] R10: ffffc9000b38bf58 R11: 0000000000000000 R12: ffff8881fcc88000
[   53.036409][    C3] R13: 000000000000007f R14: ffffc9000b38bf58 R15: 0000000000000000
[   53.036411][    C3] FS:  00007f9da3761800(0000) GS:ffff88903eb80000(0000) knlGS:0000000000000000
[   53.036412][    C3] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   53.036413][    C3] CR2: 00007f4a5dbb53d8 CR3: 0000000216b96005 CR4: 00000000007706f0
[   53.036414][    C3] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   53.036415][    C3] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[   53.036416][    C3] PKRU: 55555554
[   53.036417][    C3] Call Trace:
[   53.036419][    C3]  <TASK>
[ 53.036422][ C3] ? die (arch/x86/kernel/dumpstack.c:421 arch/x86/kernel/dumpstack.c:434 arch/x86/kernel/dumpstack.c:447) 
[ 53.036429][ C3] ? do_trap (arch/x86/kernel/traps.c:114 arch/x86/kernel/traps.c:155) 
[ 53.036433][ C3] ? user_unwind_next (kernel/unwind/user.c:39) 
[ 53.036435][ C3] ? do_error_trap (arch/x86/include/asm/traps.h:58 arch/x86/kernel/traps.c:176) 
[ 53.036437][ C3] ? user_unwind_next (kernel/unwind/user.c:39) 
[ 53.036439][ C3] ? exc_invalid_op (arch/x86/kernel/traps.c:267) 
[ 53.036446][ C3] ? user_unwind_next (kernel/unwind/user.c:39) 
[ 53.036448][ C3] ? asm_exc_invalid_op (arch/x86/include/asm/idtentry.h:621) 
[ 53.036455][ C3] ? user_unwind_next (kernel/unwind/user.c:39) 
[ 53.036457][ C3] perf_callchain_user (include/linux/uaccess.h:233 include/linux/uaccess.h:260 arch/x86/events/core.c:2894) 
[ 53.036460][ C3] get_perf_callchain (kernel/events/callchain.c:184 kernel/events/callchain.c:259) 
[ 53.036465][ C3] perf_callchain (kernel/events/core.c:7693) 
[ 53.036469][ C3] setup_pebs_fixed_sample_data (include/linux/perf_event.h:1237 arch/x86/events/intel/ds.c:1772) 
[ 53.036473][ C3] intel_pmu_drain_pebs_nhm (arch/x86/events/intel/ds.c:2201 arch/x86/events/intel/ds.c:2378) 
[ 53.036477][ C3] handle_pmi_common (arch/x86/events/intel/core.c:3066) 
[ 53.036480][ C3] ? __intel_pmu_enable_all+0x28/0xf0 
[ 53.036484][ C3] ? perf_rotate_context (kernel/events/core.c:1154 kernel/events/core.c:1150 kernel/events/core.c:4322) 
[ 53.036486][ C3] ? __pfx_perf_mux_hrtimer_handler (kernel/events/core.c:1082) 
[ 53.036488][ C3] ? ktime_get (kernel/time/timekeeping.c:195 (discriminator 4) kernel/time/timekeeping.c:395 (discriminator 4) kernel/time/timekeeping.c:403 (discriminator 4) kernel/time/timekeeping.c:850 (discriminator 4)) 
[ 53.036492][ C3] intel_pmu_handle_irq (arch/x86/include/asm/msr.h:86 arch/x86/include/asm/msr.h:133 arch/x86/events/intel/core.c:2488 arch/x86/events/intel/core.c:3186) 
[ 53.036493][ C3] perf_event_nmi_handler (arch/x86/events/core.c:1748 arch/x86/events/core.c:1734) 
[ 53.036499][ C3] nmi_handle (arch/x86/kernel/nmi.c:151 (discriminator 7)) 
[ 53.036500][ C3] default_do_nmi (arch/x86/kernel/nmi.c:352) 
[ 53.036503][ C3] exc_nmi (arch/x86/kernel/nmi.c:546) 
[ 53.036504][ C3] asm_exc_nmi (arch/x86/entry/entry_64.S:1198) 
[   53.036507][    C3] RIP: 0033:0x55b666c6ef3c
[ 53.036509][ C3] Code: 75 10 48 c7 c1 ff ff ff ff 41 b8 10 00 00 00 ba 01 00 00 00 e8 75 91 00 00 49 8b 4d 08 48 89 c7 eb 89 0f 1f 40 00 44 09 71 08 <48> 83 c4 18 5b 5d 41 5c 41 5d 41 5e 41 5f c3 4c 3b 62 10 7d 08 4c
All code
========
   0:	75 10                	jne    0x12
   2:	48 c7 c1 ff ff ff ff 	mov    $0xffffffffffffffff,%rcx
   9:	41 b8 10 00 00 00    	mov    $0x10,%r8d
   f:	ba 01 00 00 00       	mov    $0x1,%edx
  14:	e8 75 91 00 00       	callq  0x918e
  19:	49 8b 4d 08          	mov    0x8(%r13),%rcx
  1d:	48 89 c7             	mov    %rax,%rdi
  20:	eb 89                	jmp    0xffffffffffffffab
  22:	0f 1f 40 00          	nopl   0x0(%rax)
  26:	44 09 71 08          	or     %r14d,0x8(%rcx)
  2a:*	48 83 c4 18          	add    $0x18,%rsp		<-- trapping instruction
  2e:	5b                   	pop    %rbx
  2f:	5d                   	pop    %rbp
  30:	41 5c                	pop    %r12
  32:	41 5d                	pop    %r13
  34:	41 5e                	pop    %r14
  36:	41 5f                	pop    %r15
  38:	c3                   	retq   
  39:	4c 3b 62 10          	cmp    0x10(%rdx),%r12
  3d:	7d 08                	jge    0x47
  3f:	4c                   	rex.WR

Code starting with the faulting instruction
===========================================

   0:	48 83 c4 18          	add    $0x18,%rsp
   4:	5b                   	pop    %rbx
   5:	5d                   	pop    %rbp
   6:	41 5c                	pop    %r12
   8:	41 5d                	pop    %r13
   a:	41 5e                	pop    %r14
   c:	41 5f                	pop    %r15
   e:	c3                   	retq   
   f:	4c 3b 62 10          	cmp    0x10(%rdx),%r12
  13:	7d 08                	jge    0x1d
  15:	4c                   	rex.WR

The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20240916/[email protected]

Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
maintainer
#22 lore raw

Re: [PATCH v2 05/11] perf/x86: Use user_unwind interface

Hello,

Quoted 34 lines · kernel test robot

Hello,

kernel test robot noticed "kernel_BUG_at_kernel/unwind/user.c" on:

commit: 164c5ae4072303c9eb4e263115a5e70d5a3cc052 ("[PATCH v2 05/11] perf/x86: Use user_unwind interface")
url: https://github.com/intel-lab-lkp/linux/commits/Josh-Poimboeuf/unwind-Introduce-generic-user-space-unwinding-interface/20240914-070619
base: https://git.kernel.org/cgit/linux/kernel/git/perf/perf-tools-next.git perf-tools-next
patch link: https://lore.kernel.org/all/daf3f59e0d14ee11b45ad6735b8a211a3c7534dc.1726268190.git.jpoimboe@kernel.org/
patch subject: [PATCH v2 05/11] perf/x86: Use user_unwind interface

in testcase: fsmark
version: fsmark-x86_64-2628be5-1_20240224
with following parameters:

	iterations: 1x
	nr_threads: 64t
	disk: 1BRD_48G
	fs: xfs
	filesize: 4M
	test_size: 24G
	sync_method: NoSync
	cpufreq_governor: performance

compiler: gcc-12
test machine: 96 threads 2 sockets Intel(R) Xeon(R) Platinum 8260L CPU @ 2.40GHz (Cascade Lake) with 128G memory

(please refer to attached dmesg/kmsg for entire log/backtrace)

Quoted 8 lines

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-lkp/[email protected]

[   53.036370][    C3] ------------[ cut here ]------------
[   53.036374][    C3] kernel BUG at kernel/unwind/user.c:39!

I also noticed this and the following patch would fix it.

Thanks,
Namhyung

kernel/unwind/user.c +2 -0
index 3a7b14cf522b4139..3c8f1deb6d34ec37 100644
@@ -80,8 +80,10 @@ int user_unwind_start(struct user_unwind_state *state,
 	case USER_UNWIND_TYPE_SFRAME:
 		if (!sframe_possible)
 			return -EINVAL;
+		state->type = type;
 		break;
 	case USER_UNWIND_TYPE_FP:
+		state->type = type;
 		break;
 	default:
 		return -EINVAL;
author maintainer
#23 lore raw

PATCH v2 6/11 perf: Remove get_perf_callchain() 'init_nr' argument

In reply to Josh Poimboeuf

The 'init_nr' argument has double duty: it's used to initialize both the number of contexts and the number of stack entries. That's confusing and the callers always pass zero anyway. Hard code the zero.

---
 include/linux/perf_event.h |  2 +-
 kernel/bpf/stackmap.c      |  4 ++--
 kernel/events/callchain.c  | 12 ++++++------
 kernel/events/core.c       |  2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)
include/linux/perf_event.h +1 -1
index 1a8942277dda..4365bb0684a5 100644
@@ -1540,7 +1540,7 @@ DECLARE_PER_CPU(struct perf_callchain_entry, perf_callchain_entry);
 extern void perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs);
 extern void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs);
 extern struct perf_callchain_entry *
-get_perf_callchain(struct pt_regs *regs, u32 init_nr, bool kernel, bool user,
+get_perf_callchain(struct pt_regs *regs, bool kernel, bool user,
 		   u32 max_stack, bool crosstask, bool add_mark);
 extern int get_callchain_buffers(int max_stack);
 extern void put_callchain_buffers(void);
kernel/bpf/stackmap.c +2 -2
index c99f8e5234ac..0f922e43b524 100644
@@ -297,7 +297,7 @@ BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *, map,
 	if (max_depth > sysctl_perf_event_max_stack)
 		max_depth = sysctl_perf_event_max_stack;
 
-	trace = get_perf_callchain(regs, 0, kernel, user, max_depth,
+	trace = get_perf_callchain(regs, kernel, user, max_depth,
 				   false, false);
 
 	if (unlikely(!trace))
@@ -432,7 +432,7 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
 	else if (kernel && task)
 		trace = get_callchain_entry_for_task(task, max_depth);
 	else
-		trace = get_perf_callchain(regs, 0, kernel, user, max_depth,
+		trace = get_perf_callchain(regs, kernel, user, max_depth,
 					   crosstask, false);
 	if (unlikely(!trace))
 		goto err_fault;
kernel/events/callchain.c +6 -6
index 8a47e52a454f..83834203e144 100644
@@ -216,7 +216,7 @@ static void fixup_uretprobe_trampoline_entries(struct perf_callchain_entry *entr
 }
 
 struct perf_callchain_entry *
-get_perf_callchain(struct pt_regs *regs, u32 init_nr, bool kernel, bool user,
+get_perf_callchain(struct pt_regs *regs, bool kernel, bool user,
 		   u32 max_stack, bool crosstask, bool add_mark)
 {
 	struct perf_callchain_entry *entry;
@@ -227,11 +227,11 @@ get_perf_callchain(struct pt_regs *regs, u32 init_nr, bool kernel, bool user,
 	if (!entry)
 		return NULL;
 
-	ctx.entry     = entry;
-	ctx.max_stack = max_stack;
-	ctx.nr	      = entry->nr = init_nr;
-	ctx.contexts       = 0;
-	ctx.contexts_maxed = false;
+	ctx.entry		= entry;
+	ctx.max_stack		= max_stack;
+	ctx.nr			= entry->nr = 0;
+	ctx.contexts		= 0;
+	ctx.contexts_maxed	= false;
 
 	if (kernel && !user_mode(regs)) {
 		if (add_mark)
kernel/events/core.c +1 -1
index 8a6c6bbcd658..8038cd4d981b 100644
@@ -7694,7 +7694,7 @@ perf_callchain(struct perf_event *event, struct pt_regs *regs)
 	if (!kernel && !user)
 		return &__empty_callchain;
 
-	callchain = get_perf_callchain(regs, 0, kernel, user,
+	callchain = get_perf_callchain(regs, kernel, user,
 				       max_stack, crosstask, true);
 	return callchain ?: &__empty_callchain;
 }
Signature · 1 line
2.46.0
author maintainer
#24 lore raw

PATCH v2 7/11 perf: Remove get_perf_callchain() 'crosstask' argument

In reply to Josh Poimboeuf

get_perf_callchain() doesn't support cross-task unwinding, so it doesn't make much sense to have 'crosstask' as an argument. Instead, have perf_callchain() adjust 'user' accordingly.

---
 include/linux/perf_event.h | 2 +-
 kernel/bpf/stackmap.c      | 5 ++---
 kernel/events/callchain.c  | 6 +-----
 kernel/events/core.c       | 8 ++++----
 4 files changed, 8 insertions(+), 13 deletions(-)
include/linux/perf_event.h +1 -1
index 4365bb0684a5..64f9efe19553 100644
@@ -1541,7 +1541,7 @@ extern void perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct p
 extern void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs);
 extern struct perf_callchain_entry *
 get_perf_callchain(struct pt_regs *regs, bool kernel, bool user,
-		   u32 max_stack, bool crosstask, bool add_mark);
+		   u32 max_stack, bool add_mark);
 extern int get_callchain_buffers(int max_stack);
 extern void put_callchain_buffers(void);
 extern struct perf_callchain_entry *get_callchain_entry(int *rctx);
kernel/bpf/stackmap.c +2 -3
index 0f922e43b524..ff6f0ef7ba5d 100644
@@ -297,8 +297,7 @@ BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *, map,
 	if (max_depth > sysctl_perf_event_max_stack)
 		max_depth = sysctl_perf_event_max_stack;
 
-	trace = get_perf_callchain(regs, kernel, user, max_depth,
-				   false, false);
+	trace = get_perf_callchain(regs, kernel, user, max_depth, false);
 
 	if (unlikely(!trace))
 		/* couldn't fetch the stack trace */
@@ -433,7 +432,7 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
 		trace = get_callchain_entry_for_task(task, max_depth);
 	else
 		trace = get_perf_callchain(regs, kernel, user, max_depth,
-					   crosstask, false);
+					   false);
 	if (unlikely(!trace))
 		goto err_fault;
 
kernel/events/callchain.c +1 -5
index 83834203e144..655fb25a725b 100644
@@ -217,7 +217,7 @@ static void fixup_uretprobe_trampoline_entries(struct perf_callchain_entry *entr
 
 struct perf_callchain_entry *
 get_perf_callchain(struct pt_regs *regs, bool kernel, bool user,
-		   u32 max_stack, bool crosstask, bool add_mark)
+		   u32 max_stack, bool add_mark)
 {
 	struct perf_callchain_entry *entry;
 	struct perf_callchain_entry_ctx ctx;
@@ -248,9 +248,6 @@ get_perf_callchain(struct pt_regs *regs, bool kernel, bool user,
 		}
 
 		if (regs) {
-			if (crosstask)
-				goto exit_put;
-
 			if (add_mark)
 				perf_callchain_store_context(&ctx, PERF_CONTEXT_USER);
 
@@ -260,7 +257,6 @@ get_perf_callchain(struct pt_regs *regs, bool kernel, bool user,
 		}
 	}
 
-exit_put:
 	put_callchain_entry(rctx);
 
 	return entry;
kernel/events/core.c +4 -4
index 8038cd4d981b..19fd7bd38ecf 100644
@@ -7686,16 +7686,16 @@ perf_callchain(struct perf_event *event, struct pt_regs *regs)
 {
 	bool kernel = !event->attr.exclude_callchain_kernel;
 	bool user   = !event->attr.exclude_callchain_user;
-	/* Disallow cross-task user callchains. */
-	bool crosstask = event->ctx->task && event->ctx->task != current;
 	const u32 max_stack = event->attr.sample_max_stack;
 	struct perf_callchain_entry *callchain;
 
+	/* Disallow cross-task user callchains. */
+	user &= !event->ctx->task || event->ctx->task == current;
+
 	if (!kernel && !user)
 		return &__empty_callchain;
 
-	callchain = get_perf_callchain(regs, kernel, user,
-				       max_stack, crosstask, true);
+	callchain = get_perf_callchain(regs, kernel, user, max_stack, true);
 	return callchain ?: &__empty_callchain;
 }
 
Signature · 1 line
2.46.0
author maintainer
#25 lore raw

PATCH v2 8/11 perf: Simplify get_perf_callchain() user logic

In reply to Josh Poimboeuf

Simplify the get_perf_callchain() user logic a bit. task_pt_regs() should never be NULL.

---
 kernel/events/callchain.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)
kernel/events/callchain.c +9 -11
index 655fb25a725b..2278402b7ac9 100644
@@ -241,22 +241,20 @@ get_perf_callchain(struct pt_regs *regs, bool kernel, bool user,
 
 	if (user) {
 		if (!user_mode(regs)) {
-			if  (current->mm)
-				regs = task_pt_regs(current);
-			else
-				regs = NULL;
+			if (!current->mm)
+				goto exit_put;
+			regs = task_pt_regs(current);
 		}
 
-		if (regs) {
-			if (add_mark)
-				perf_callchain_store_context(&ctx, PERF_CONTEXT_USER);
+		if (add_mark)
+			perf_callchain_store_context(&ctx, PERF_CONTEXT_USER);
 
-			start_entry_idx = entry->nr;
-			perf_callchain_user(&ctx, regs);
-			fixup_uretprobe_trampoline_entries(entry, start_entry_idx);
-		}
+		start_entry_idx = entry->nr;
+		perf_callchain_user(&ctx, regs);
+		fixup_uretprobe_trampoline_entries(entry, start_entry_idx);
 	}
 
+exit_put:
 	put_callchain_entry(rctx);
 
 	return entry;
Signature · 1 line
2.46.0
author maintainer
#26 lore raw

PATCH v2 9/11 perf: Introduce deferred user callchains

In reply to Josh Poimboeuf

Instead of attempting to unwind user space from the NMI handler, defer it to run in task context by sending a self-IPI and then scheduling the unwind to run in the IRQ's exit task work before returning to user space.

This allows the user stack page to be paged in if needed, avoids duplicate unwinds for kernel-bound workloads, and prepares for SFrame unwinding (so .sframe sections can be paged in on demand).

---
 arch/Kconfig                    |  3 ++
 include/linux/perf_event.h      |  9 +++-
 include/uapi/linux/perf_event.h | 21 ++++++++-
 kernel/bpf/stackmap.c           |  5 +--
 kernel/events/callchain.c       | 12 +++++-
 kernel/events/core.c            | 76 ++++++++++++++++++++++++++++++++-
 6 files changed, 119 insertions(+), 7 deletions(-)
arch/Kconfig +3 -0
index ff5d5bc5f947..0629c1aa2a5c 100644
@@ -431,6 +431,9 @@ config HAVE_USER_UNWIND
 config HAVE_USER_UNWIND_SFRAME
 	bool
 
+config HAVE_PERF_CALLCHAIN_DEFERRED
+	bool
+
 config HAVE_PERF_REGS
 	bool
 	help
include/linux/perf_event.h +8 -1
index 64f9efe19553..a617aad2851c 100644
@@ -787,6 +787,7 @@ struct perf_event {
 	struct callback_head		pending_task;
 	unsigned int			pending_work;
 	struct rcuwait			pending_work_wait;
+	unsigned int			pending_callchain;
 
 	atomic_t			event_limit;
 
@@ -1541,12 +1542,18 @@ extern void perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct p
 extern void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs);
 extern struct perf_callchain_entry *
 get_perf_callchain(struct pt_regs *regs, bool kernel, bool user,
-		   u32 max_stack, bool add_mark);
+		   u32 max_stack, bool add_mark, bool defer_user);
 extern int get_callchain_buffers(int max_stack);
 extern void put_callchain_buffers(void);
 extern struct perf_callchain_entry *get_callchain_entry(int *rctx);
 extern void put_callchain_entry(int rctx);
 
+#ifdef CONFIG_HAVE_PERF_CALLCHAIN_DEFERRED
+extern void perf_callchain_user_deferred(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs);
+#else
+static inline void perf_callchain_user_deferred(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs) {}
+#endif
+
 extern int sysctl_perf_event_max_stack;
 extern int sysctl_perf_event_max_contexts_per_stack;
 
include/uapi/linux/perf_event.h +20 -1
index 4842c36fdf80..a7f875eb29dd 100644
@@ -460,7 +460,8 @@ struct perf_event_attr {
 				inherit_thread :  1, /* children only inherit if cloned with CLONE_THREAD */
 				remove_on_exec :  1, /* event is removed from task on exec */
 				sigtrap        :  1, /* send synchronous SIGTRAP on event */
-				__reserved_1   : 26;
+				defer_callchain:  1, /* generate PERF_RECORD_CALLCHAIN_DEFERRED records */
+				__reserved_1   : 25;
 
 	union {
 		__u32		wakeup_events;	  /* wakeup every n events */
@@ -1217,6 +1218,23 @@ enum perf_event_type {
 	 */
 	PERF_RECORD_AUX_OUTPUT_HW_ID		= 21,
 
+	/*
+	 * This user callchain capture was deferred until shortly before
+	 * returning to user space.  Previous samples would have kernel
+	 * callchains only and they need to be stitched with this to make full
+	 * callchains.
+	 *
+	 * TODO: do PERF_SAMPLE_{REGS,STACK}_USER also need deferral?
+	 *
+	 * struct {
+	 *	struct perf_event_header	header;
+	 *	u64				nr;
+	 *	u64				ips[nr];
+	 *	struct sample_id		sample_id;
+	 * };
+	 */
+	PERF_RECORD_CALLCHAIN_DEFERRED		= 22,
+
 	PERF_RECORD_MAX,			/* non-ABI */
 };
 
@@ -1247,6 +1265,7 @@ enum perf_callchain_context {
 	PERF_CONTEXT_HV			= (__u64)-32,
 	PERF_CONTEXT_KERNEL		= (__u64)-128,
 	PERF_CONTEXT_USER		= (__u64)-512,
+	PERF_CONTEXT_USER_DEFERRED	= (__u64)-640,
 
 	PERF_CONTEXT_GUEST		= (__u64)-2048,
 	PERF_CONTEXT_GUEST_KERNEL	= (__u64)-2176,
kernel/bpf/stackmap.c +2 -3
index ff6f0ef7ba5d..9d58a84e553a 100644
@@ -297,8 +297,7 @@ BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *, map,
 	if (max_depth > sysctl_perf_event_max_stack)
 		max_depth = sysctl_perf_event_max_stack;
 
-	trace = get_perf_callchain(regs, kernel, user, max_depth, false);
-
+	trace = get_perf_callchain(regs, kernel, user, max_depth, false, false);
 	if (unlikely(!trace))
 		/* couldn't fetch the stack trace */
 		return -EFAULT;
@@ -432,7 +431,7 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
 		trace = get_callchain_entry_for_task(task, max_depth);
 	else
 		trace = get_perf_callchain(regs, kernel, user, max_depth,
-					   false);
+					   false, false);
 	if (unlikely(!trace))
 		goto err_fault;
 
kernel/events/callchain.c +11 -1
index 2278402b7ac9..883f0ac9ef3a 100644
@@ -217,7 +217,7 @@ static void fixup_uretprobe_trampoline_entries(struct perf_callchain_entry *entr
 
 struct perf_callchain_entry *
 get_perf_callchain(struct pt_regs *regs, bool kernel, bool user,
-		   u32 max_stack, bool add_mark)
+		   u32 max_stack, bool add_mark, bool defer_user)
 {
 	struct perf_callchain_entry *entry;
 	struct perf_callchain_entry_ctx ctx;
@@ -246,6 +246,16 @@ get_perf_callchain(struct pt_regs *regs, bool kernel, bool user,
 			regs = task_pt_regs(current);
 		}
 
+		if (defer_user) {
+			/*
+			 * Foretell the coming of a
+			 * PERF_RECORD_CALLCHAIN_DEFERRED sample which can be
+			 * stitched to this one.
+			 */
+			perf_callchain_store_context(&ctx, PERF_CONTEXT_USER_DEFERRED);
+			goto exit_put;
+		}
+
 		if (add_mark)
 			perf_callchain_store_context(&ctx, PERF_CONTEXT_USER);
 
kernel/events/core.c +75 -1

105 lines not on this page · raw message

Signature · 1 line
2.46.0
#27 lore raw

Re: [PATCH v2 09/11] perf: Introduce deferred user callchains

kernel test robot noticed the following build warnings:

Hi Josh,

kernel test robot noticed the following build warnings:

[auto build test WARNING on perf-tools-next/perf-tools-next] [also build test WARNING on perf-tools/perf-tools linus/master v6.11-rc7] [cannot apply to tip/perf/core acme/perf/core next-20240913] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Josh-Poimboeuf/unwind-Introduce-generic-user-space-unwinding-interface/20240914-070619
base:   https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git perf-tools-next
patch link:    https://lore.kernel.org/r/5bc834b68fe14daaa1782b925ab54fc414245334.1726268190.git.jpoimboe%40kernel.org
patch subject: [PATCH v2 09/11] perf: Introduce deferred user callchains
config: sh-allnoconfig (https://download.01.org/0day-ci/archive/20240914/[email protected]/config)
compiler: sh4-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240914/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

kernel/events/core.c: In function 'perf_event_callchain_deferred':

>> kernel/events/core.c:6906:1: warning: the frame size of 1292 bytes is larger than 1024 bytes [-Wframe-larger-than=]
    6906 | }
         | ^

vim +6906 kernel/events/core.c

6861
6862 #define PERF_CALLCHAIN_DEFERRED_EVENT_SIZE \
6863 sizeof(struct perf_callchain_deferred_event) + \
6864 (sizeof(__u64) * 1) + /* PERF_CONTEXT_USER */ \
6865 (sizeof(__u64) * PERF_MAX_STACK_DEPTH)
6866
6867 static void perf_event_callchain_deferred(struct perf_event *event)
6868 {
6869 struct pt_regs *regs = task_pt_regs(current);
6870 struct perf_callchain_entry *callchain;
6871 struct perf_output_handle handle;
6872 struct perf_sample_data data;
6873 unsigned char buf[PERF_CALLCHAIN_DEFERRED_EVENT_SIZE];
6874 struct perf_callchain_entry_ctx ctx;
6875 struct perf_callchain_deferred_event *deferred_event;
6876
6877 deferred_event = (void *)&buf;
6878
6879 callchain = &deferred_event->callchain;
6880 callchain->nr = 0;
6881
6882 ctx.entry = callchain;
6883 ctx.max_stack = MIN(event->attr.sample_max_stack,
6884 PERF_MAX_STACK_DEPTH);
6885 ctx.nr = 0;
6886 ctx.contexts = 0;
6887 ctx.contexts_maxed = false;
6888
6889 perf_callchain_store_context(&ctx, PERF_CONTEXT_USER);
6890 perf_callchain_user_deferred(&ctx, regs);
6891
6892 deferred_event->header.type = PERF_RECORD_CALLCHAIN_DEFERRED;
6893 deferred_event->header.misc = 0;
6894 deferred_event->header.size = sizeof(*deferred_event) +
6895 (callchain->nr * sizeof(u64));
6896
6897 perf_event_header__init_id(&deferred_event->header, &data, event);
6898
6899 if (perf_output_begin(&handle, &data, event,
6900 deferred_event->header.size))
6901 return;
6902
6903 perf_output_copy(&handle, deferred_event, deferred_event->header.size);
6904 perf_event__output_id_sample(event, &handle, &data);
6905 perf_output_end(&handle);

> 6906	}

6907

Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
maintainer
#28 lore raw

Re: [PATCH v2 09/11] perf: Introduce deferred user callchains

In reply to Josh Poimboeuf
Quoted 12 lines · Josh Poimboeuf

Instead of attempting to unwind user space from the NMI handler, defer it to run in task context by sending a self-IPI and then scheduling the unwind to run in the IRQ's exit task work before returning to user space.

This allows the user stack page to be paged in if needed, avoids duplicate unwinds for kernel-bound workloads, and prepares for SFrame unwinding (so .sframe sections can be paged in on demand).

---

[SNIP]

Quoted 35 lines
kernel/events/core.c +27 -0
index 19fd7bd38ecf..5fc7c5156287 100644
@@ -6854,11 +6860,70 @@ static void perf_pending_irq(struct irq_work *entry)
 		perf_swevent_put_recursion_context(rctx);
 }
 
+struct perf_callchain_deferred_event {
+	struct perf_event_header	header;
+	struct perf_callchain_entry	callchain;
+};
+
+#define PERF_CALLCHAIN_DEFERRED_EVENT_SIZE				\
+	sizeof(struct perf_callchain_deferred_event) +			\
+	(sizeof(__u64) * 1) + /* PERF_CONTEXT_USER */			\
+	(sizeof(__u64) * PERF_MAX_STACK_DEPTH)
+
+static void perf_event_callchain_deferred(struct perf_event *event)
+{
+	struct pt_regs *regs = task_pt_regs(current);
+	struct perf_callchain_entry *callchain;
+	struct perf_output_handle handle;
+	struct perf_sample_data data;
+	unsigned char buf[PERF_CALLCHAIN_DEFERRED_EVENT_SIZE];
+	struct perf_callchain_entry_ctx ctx;
+	struct perf_callchain_deferred_event *deferred_event;
+
+	deferred_event = (void *)&buf;
+
+	callchain = &deferred_event->callchain;
+	callchain->nr = 0;
+
+	ctx.entry		= callchain;
+	ctx.max_stack		= MIN(event->attr.sample_max_stack,

kernel/events/core.c static void perf_pending_irq(struct irq_work *entry) · line 6899

Quoted 10 lines
kernel/events/core.c +10 -0
+				      PERF_MAX_STACK_DEPTH);
+	ctx.nr			= 0;
+	ctx.contexts		= 0;
+	ctx.contexts_maxed	= false;
+
+	perf_callchain_store_context(&ctx, PERF_CONTEXT_USER);
+	perf_callchain_user_deferred(&ctx, regs);
+
+	deferred_event->header.type = PERF_RECORD_CALLCHAIN_DEFERRED;
+	deferred_event->header.misc = 0;

I think we can use PERF_RECORD_MISC_USER here as it's about user callchains.

kernel/events/core.c static void perf_pending_irq(struct irq_work *entry) · line 6909

Quoted 10 lines
kernel/events/core.c +10 -0
+	deferred_event->header.size = sizeof(*deferred_event) +
+				      (callchain->nr * sizeof(u64));
+
+	perf_event_header__init_id(&deferred_event->header, &data, event);
+
+	if (perf_output_begin(&handle, &data, event,
+			      deferred_event->header.size))
+		return;
+
+	perf_output_copy(&handle, deferred_event, deferred_event->header.size);

You should not copy the whole event size because it also contains the id_sample parts in the below. Maybe something like this instead?

	perf_output_put(&handle, *deferred_event);
	__output_copy(&handle, callchain->ip, callchain->nr * sizeof(u64));

Thanks,
Namhyung

Quoted 48 lines
kernel/events/core.c +22 -1
+	perf_event__output_id_sample(event, &handle, &data);
+	perf_output_end(&handle);
+}
+
 static void perf_pending_task(struct callback_head *head)
 {
 	struct perf_event *event = container_of(head, struct perf_event, pending_task);
 	int rctx;
 
+	if (!is_software_event(event)) {
+		if (event->pending_callchain) {
+			perf_event_callchain_deferred(event);
+			event->pending_callchain = 0;
+		}
+		return;
+	}
+
 	/*
 	 * All accesses to the event must belong to the same implicit RCU read-side
 	 * critical section as the ->pending_work reset. See comment in
@@ -7688,6 +7753,8 @@ perf_callchain(struct perf_event *event, struct pt_regs *regs)
 	bool user   = !event->attr.exclude_callchain_user;
 	const u32 max_stack = event->attr.sample_max_stack;
 	struct perf_callchain_entry *callchain;
+	bool defer_user = IS_ENABLED(CONFIG_HAVE_PERF_CALLCHAIN_DEFERRED) &&
+			  event->attr.defer_callchain;
 
 	/* Disallow cross-task user callchains. */
 	user &= !event->ctx->task || event->ctx->task == current;
@@ -7695,7 +7762,14 @@ perf_callchain(struct perf_event *event, struct pt_regs *regs)
 	if (!kernel && !user)
 		return &__empty_callchain;
 
-	callchain = get_perf_callchain(regs, kernel, user, max_stack, true);
+	callchain = get_perf_callchain(regs, kernel, user, max_stack, true,
+				       defer_user);
+
+	if (user && defer_user && !event->pending_callchain) {
+		event->pending_callchain = 1;
+		irq_work_queue(&event->pending_irq);
+	}
+
 	return callchain ?: &__empty_callchain;
 }
 
Signature · 1 line
2.46.0
author maintainer
#29 lore raw

PATCH v2 10/11 perf/x86: Add HAVE_PERF_CALLCHAIN_DEFERRED

In reply to Josh Poimboeuf

Enable deferred user space unwinding on x86. Frame pointers are still the default for now.

---
 arch/x86/Kconfig       |  1 +
 arch/x86/events/core.c | 52 +++++++++++++++++++++++++++---------------
 2 files changed, 34 insertions(+), 19 deletions(-)
arch/x86/Kconfig +1 -0
index 0b3d7c72b65b..24d9373cc5e6 100644
@@ -265,6 +265,7 @@ config X86
 	select HAVE_PERF_EVENTS_NMI
 	select HAVE_HARDLOCKUP_DETECTOR_PERF	if PERF_EVENTS && HAVE_PERF_EVENTS_NMI
 	select HAVE_PCI
+	select HAVE_PERF_CALLCHAIN_DEFERRED
 	select HAVE_PERF_REGS
 	select HAVE_PERF_USER_STACK_DUMP
 	select MMU_GATHER_RCU_TABLE_FREE	if PARAVIRT
arch/x86/events/core.c +33 -19

87 lines not on this page · raw message

Signature · 1 line
2.46.0
author maintainer
#30 lore raw

PATCH v2 11/11 perf/x86: Enable SFrame unwinding for deferred user callchains

In reply to Josh Poimboeuf

Use SFrame for deferred user callchains, if available.

Non-deferred user callchains still need to use frame pointers, as SFrame is likely to fault when it pages in the .sframe section.

---
 arch/x86/events/core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
arch/x86/events/core.c +3 -1
index d6ea265d9aa8..d618c50865d3 100644
@@ -2861,6 +2861,7 @@ static int __perf_callchain_user32(struct pt_regs *regs,
 static void __perf_callchain_user(struct perf_callchain_entry_ctx *entry,
 				  struct pt_regs *regs, bool atomic)
 {
+	bool unwind_type = USER_UNWIND_TYPE_AUTO;
 	struct user_unwind_state state;
 
 	if (perf_guest_state()) {
@@ -2879,13 +2880,14 @@ static void __perf_callchain_user(struct perf_callchain_entry_ctx *entry,
 	if (atomic) {
 		if (!nmi_uaccess_okay())
 			return;
+		unwind_type = USER_UNWIND_TYPE_FP;
 		pagefault_disable();
 	}
 
 	if (__perf_callchain_user32(regs, entry))
 		goto done;
 
-	for_each_user_frame(&state, USER_UNWIND_TYPE_FP) {
+	for_each_user_frame(&state, unwind_type) {
 		if (perf_callchain_store(entry, state.ip))
 			goto done;
 	}
Signature · 1 line
2.46.0
maintainer
#31 lore raw
In reply to Josh Poimboeuf
Quoted 6 lines · On Sat, 14 Sep 2024 01:02:02 +0200 Josh Poimboeuf <[email protected]> wrote:

This is a new version of sframe user space unwinding + perf deferred callchains. Better late than never.

Unfortunately I didn't get a chance to do any testing with this one yet as I'm rushing it out the door before GNU Cauldron starts. Conference-driven development ;-)

Thanks for posting this!

BTW, next time, can you also Cc [email protected].

Quoted 4 lines

Plus I don't have the perf tool piece working yet so I don't have a way of doing end-to-end testing at the moment anyway.

Quoted 8 lines

In other words, please don't merge this yet.

Namhyung, if you're still available to write a perf tool patch which integrates with this, that would be great. Otherwise I could give it a try.

Steven, let me know if this would interface ok with your anticipated tracing usage.

This is a very good starting point for me. A couple of notes.

I think the unwinder should have an interface itself that provides the deferred unwinding, instead of having all the tracers to implement their own.

The user space unwinder (regardless of sframe or not) should provide a way to say "I want a user space stack trace for here, but don't do it yet. Just give me a cookie and then call my callback function with the stack and cookie before going back to user space".

That is, we should have an interface like:

typedef void (unwinder_callback_t)(struct user_space_stack *, u64 cookie);

struct unwinder_client {
	struct list_head	list;
	unwinder_callback_t	callback;
};
struct unwinder_client my_unwinder = {
	.callback = mycallback;
};
	register_unwinder(&my_unwinder);

Then a tracer could do this in NMI context:

	/* Get the current cookie of the current task */
	cookie = unwinder_get_cookie();

	/* Tell the unwinder to call the callbacks */
	unwinder_trigger();

	/* then the tracer can save off that cookie, do a kernel stack trace
	 * and save the cookie with that stack trace.
	 */

On return to user space, it will go the ptrace slow path and call all registered callbacks with the cookie, regardless if it asked for it or not.

	/* Save the stack somewhere, could even allocate memory to do so */
	list_for_each_entry(callback, &unwinder_callback, list) {
		callback->callback(userstack, cookie);
	}
	update_next_cookie();

The cookie is a unique number for every entry into the kernel by all tasks. Then it is up to the tracer's callback to know if it should save the trace or ignore it. That is, if perf and ftrace are expecting callbacks, but only perf triggered the callback, it should record the cookie, and save the stack if it matches a cookie it wants. ftrace's callback would ignore the trace, because it did not ask for a callback. That is, the tracer will need to save off the cookie to know if its callback needs to do something or not.

The cookie can be generated by this:

static u64 next_cookie;

u64 unwinder_get_cookie(void)

{
	u64 next;

	if (current->unwinder_cookie)
		return current->unwinder_cookie;

	next = next_cookie;
	while (!try_cmpxchg64(&next_cookie, &next, next + 1))
		;

	next++;

	/* Check for races with NMI */
	if (!cmpxchg(&current->unwinder_cookie, 0, next))
		next = current->unwinder_cookie;

	return next;
}

When the task goes back to user space and does all the callbacks, it would reset the task cookie.

	current->unwinder_cookie = 0;

Now the loop of callbacks should also catch if unwinder_trigger() was called again (NMI), and may need to recall all the back traces again with the new cookie.

That will need some thought about handling that race.

This is just an idea to start discussion.

-- Steve

#32 lore raw
Quoted 8 lines · Steven Rostedt

I think the unwinder should have an interface itself that provides the deferred unwinding, instead of having all the tracers to implement their own.

The user space unwinder (regardless of sframe or not) should provide a way to say "I want a user space stack trace for here, but don't do it yet. Just give me a cookie and then call my callback function with the stack and cookie before going back to user space".

We (Steven, Mathieu and I) have been discussing this at GNU Cauldron and I think we're in basic agreement on this.

I think the biggest tweak we decided on is that the context id (aka "cookie") would be percpu. Its initial value is (cpuid << 48). It gets incremented for every entry from user space.

Quoted 7 lines

That is, we should have an interface like:

typedef void (unwinder_callback_t)(struct user_space_stack *, u64 cookie);

struct unwinder_client {
	struct list_head	list;
	unwinder_callback_t	callback;

I assume we want to allow tracers to pick sframes or FP (or auto). If so we would need to add a user_unwind_type enum to this struct.

Then the question is, what to do if tracer A wants sframe and tracer B wants FP? I'm thinking it's fine to allow that. I assume the "multiple tracers unwinding user space" case isn't realistic so any extra overhead from these cases is the user's fault?

The unwinder would need two sets of callbacks and task work functions:
one for sframe, one for FP.

The tracer would need to pass its &my_unwinder struct to unwinder_trigger() so the unwinder knows which task work function to activate.

I thinking we also need a 'max_entries' field. No need to keep unwinding if we've already reached the tracer's max. If there are multiple callbacks then we'd have to get the max of the maxes, but that's easy enough.

Mathieu had requested passing an opaque void * from the NMI to the callback, but I don't think that's possible with this scheme since the unwinder wouldn't know which callback to give it to. Instead the tracer would have to keep track of its own data associated with the given cookie.

Signature · 1 line
Josh
maintainer
#33 lore raw
Quoted 10 lines · On Sun, 15 Sep 2024 13:11:11 +0200 Josh Poimboeuf <[email protected]> wrote:
Quoted 8 lines · Steven Rostedt

I think the unwinder should have an interface itself that provides the deferred unwinding, instead of having all the tracers to implement their own.

The user space unwinder (regardless of sframe or not) should provide a way to say "I want a user space stack trace for here, but don't do it yet. Just give me a cookie and then call my callback function with the stack and cookie before going back to user space".

Quoted 6 lines

We (Steven, Mathieu and I) have been discussing this at GNU Cauldron and I think we're in basic agreement on this.

I think the biggest tweak we decided on is that the context id (aka "cookie") would be percpu. Its initial value is (cpuid << 48). It gets incremented for every entry from user space.

Well, in concept it is incremented for every entry from user space, but in reality it should only be incremented the first time it is referenced when coming into user space. That is, if there's nothing asking for a stack trace, then nothing should be incremented.

Quoted 11 lines
Quoted 8 lines

That is, we should have an interface like:

typedef void (unwinder_callback_t)(struct user_space_stack *, u64 cookie);

struct unwinder_client {
	struct list_head	list;
	unwinder_callback_t	callback;  

I assume we want to allow tracers to pick sframes or FP (or auto). If so we would need to add a user_unwind_type enum to this struct.

Actually, I don't think they should care. A stack trace should just be passed to the callback. How that stack trace is created, is up to the unwinder. sframe should be used if it's avaliable, otherwise it will use the frame pointer. If neither is available, the callback could be called with NULL for the stack trace and then it can figure out what to do via pt_regs.

Quoted 14 lines

Then the question is, what to do if tracer A wants sframe and tracer B wants FP? I'm thinking it's fine to allow that. I assume the "multiple tracers unwinding user space" case isn't realistic so any extra overhead from these cases is the user's fault?

The unwinder would need two sets of callbacks and task work functions:
one for sframe, one for FP.

The tracer would need to pass its &my_unwinder struct to unwinder_trigger() so the unwinder knows which task work function to activate.

Quoted 4 lines

I thinking we also need a 'max_entries' field. No need to keep unwinding if we've already reached the tracer's max. If there are multiple callbacks then we'd have to get the max of the maxes, but that's easy enough.

We could set a max when the tracer registers a callback. It can be -1 to mean "no limit".

I was talking with Mathieu at lunch, and we talked about having the stack information pointer be part of the task_struct.

That is, the first time the unwinder is triggered and its doing the unwind (either sfarme or fp), it will allocate memory to store the stack trace information. This information is what is sent to the tracer's callbacks. Again, the tracers do not care how the stack frame was created. This memory is then saved in the task struct (freed when the task exits), and reused for future stack traces.

If a stack trace could not be created, the callback just gets NULL and that will tell the tracer that nothing is available for the user space stack trace, and it can try to do something else. But that's not some the unwinder should care about.

Quoted 8 lines

Mathieu had requested passing an opaque void * from the NMI to the callback, but I don't think that's possible with this scheme since the unwinder wouldn't know which callback to give it to. Instead the tracer would have to keep track of its own data associated with the given cookie.

Not sure what that means, but from NMI, I would see the tracer doing:

	u64 cookie;

	cookie = undwinder_trigger();

And then the tracer is responsible to do something with that cookie. This would also trigger the "task_work()" where the ptrace path is entered when going back to user space, the unwinder will try to create a stack trace (from sframe or fp), saving it in this allocated memory that is then stored via a pointer on the task_struct so it only needs to be allocated once (or if a stack trace is bigger than what was allocated).

Then all the tracer callbacks will be called with the stack trace and the cookie. The tracer will decided if it should use the stack trace from the cookie, or just ignore it. But the tracer shouldn't care about sframes or frame pointers. That's an implementation detail of the unwinder.

-- Steve

maintainer
#34 lore raw
In reply to Josh Poimboeuf
Quoted 10 lines · Josh Poimboeuf
Quoted 8 lines · Steven Rostedt

I think the unwinder should have an interface itself that provides the deferred unwinding, instead of having all the tracers to implement their own.

The user space unwinder (regardless of sframe or not) should provide a way to say "I want a user space stack trace for here, but don't do it yet. Just give me a cookie and then call my callback function with the stack and cookie before going back to user space".

Quoted 6 lines

We (Steven, Mathieu and I) have been discussing this at GNU Cauldron and I think we're in basic agreement on this.

I think the biggest tweak we decided on is that the context id (aka "cookie") would be percpu. Its initial value is (cpuid << 48). It gets incremented for every entry from user space.

Why? What's the purpose of the cookie? This scheme seems unsound, pin yourself on CPU0 and trigger 1<<48 unwinds while keeping CPU1 idle.

Quoted 3 lines
Quoted 3 lines

That is, we should have an interface like:

typedef void (unwinder_callback_t)(struct user_space_stack *, u64 cookie);

Just make it a void* and let the consumer figure it out.

author maintainer
#35 lore raw
Quoted 11 lines · Peter Zijlstra
Quoted 10 lines · Josh Poimboeuf
Quoted 8 lines · Steven Rostedt

I think the unwinder should have an interface itself that provides the deferred unwinding, instead of having all the tracers to implement their own.

The user space unwinder (regardless of sframe or not) should provide a way to say "I want a user space stack trace for here, but don't do it yet. Just give me a cookie and then call my callback function with the stack and cookie before going back to user space".

Quoted 8 lines
Quoted 6 lines

We (Steven, Mathieu and I) have been discussing this at GNU Cauldron and I think we're in basic agreement on this.

I think the biggest tweak we decided on is that the context id (aka "cookie") would be percpu. Its initial value is (cpuid << 48). It gets incremented for every entry from user space.

Why? What's the purpose of the cookie?

The cookie is incremented once per entry from userspace, when needed.

It's a unique id used by the tracers which is emitted with both kernel and user stacktrace samples so the userspace tool can stitch them back together. Even if you have multiple tracers triggering at the same time they can all share a single user trace.

Completely untested patch here that I hacked up today:

git://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git sframe-2.1

To avoid races, unwind_user_deferred() can't be called from NMI. The tracers will need to trigger irq_work to call it.

Quoted 1 line

This scheme seems unsound

It may not be communicated well in this thread. We had much discussion at Cauldron the last few days. The patch may be the best way to communicate it at this point, though I'm sure I missed some details.

Quoted 1 line

pin yourself on CPU0 and trigger 1<<48 unwinds while keeping CPU1 idle.

Hm???

Quoted 5 lines
Quoted 3 lines
Quoted 3 lines

That is, we should have an interface like:

typedef void (unwinder_callback_t)(struct user_space_stack *, u64 cookie);

Just make it a void* and let the consumer figure it out.

I think it makes sense to manage the cookie in the deferred unwinder since the need to correlate the stack traces is an inherent part of the deferral.

Signature · 1 line
Josh
maintainer
#36 lore raw
Quoted 6 lines · Josh Poimboeuf

The cookie is incremented once per entry from userspace, when needed.

It's a unique id used by the tracers which is emitted with both kernel and user stacktrace samples so the userspace tool can stitch them back together. Even if you have multiple tracers triggering at the same time they can all share a single user trace.

But perf don't need this at all, right? It knows that the next deferred trace for that tid will be the one.

Quoted 5 lines
Quoted 3 lines

This scheme seems unsound

pin yourself on CPU0 and trigger 1<<48 unwinds while keeping CPU1 idle.

Hm???

The point being that it is possible to wrap one CPU into the id space of another CPU. It is not trivial, but someone who wants to can make it happen.

Combined I don't see the need to force this into this scheme, carry an opaque (void*) pointer and let the user do whatever it wants, perf for instance can pass NULL and not do anything like this.

maintainer
#37 lore raw
Quoted 11 lines · On Mon, 16 Sep 2024 20:15:45 +0200 Peter Zijlstra <[email protected]> wrote:
Quoted 6 lines · Josh Poimboeuf

The cookie is incremented once per entry from userspace, when needed.

It's a unique id used by the tracers which is emitted with both kernel and user stacktrace samples so the userspace tool can stitch them back together. Even if you have multiple tracers triggering at the same time they can all share a single user trace.

But perf don't need this at all, right? It knows that the next deferred trace for that tid will be the one.

Is that because perf uses per task buffers? Will perf know this if it uses a global buffer? What does perf do with "-a"?

Quoted 7 lines
Quoted 6 lines
Quoted 4 lines

This scheme seems unsound

pin yourself on CPU0 and trigger 1<<48 unwinds while keeping CPU1 idle.

Hm???

Quoted 7 lines

The point being that it is possible to wrap one CPU into the id space of another CPU. It is not trivial, but someone who wants to can make it happen.

Combined I don't see the need to force this into this scheme, carry an opaque (void*) pointer and let the user do whatever it wants, perf for instance can pass NULL and not do anything like this.

That would put a huge burden on those tracers. Specifically ftrace. Just because perf doesn't need it, it doesn't mean it can't be part of the infrastructure. The cookie isn't even per-cpu. The counter is just generated per cpu to avoid races. The cookie will have the CPU as part of the bits, plus the per-cpu counter. This will give us a unique value across all CPUs without having to do any atomic operations. It's just that the cookie must be unique for every time a task enters the kernel, and stay the same while it is in the kernel.

So if the task enters the kernel on CPU 1 and a trace is requested, the tracer still needs to save that event when the request occurred (for example, the kernel stack trace). It needs a unique identifier that will be attached to the user stack trace when it is made, so that post processing can tell where to append that stack.

Then the task can migrate to several other CPUs, but it will still have the same cookie as when it received it on the first CPU. The cookie only gets updated on the task when it goes back to user space.

ftrace allows for several instances that could be filtering on the same or different tasks. One instance could filter 3 tasks, another 2 tasks, where one of those tasks is in both instance. The user stack infrastructure should give a way to easily map the request to the stack via some kind of identifier.

What would perf do on dropped events? Say a system call happened, perf requested a user stack trace, then the events were dropped, and when it started again, it was in a system call, how would perf know it is in the same system call? I guess the callback when the dropped user stack trace occurred would tell perf to reset. Still it adds a pretty big burden for the tracers to handle these corner cases whereas a simple identifier is much more simple and robust.

-- Steve

maintainer
#38 lore raw

Hello,

Quoted 7 lines · Steven Rostedt
Quoted 4 lines · On Mon, 16 Sep 2024 20:15:45 +0200 Peter Zijlstra <[email protected]> wrote:
Quoted 2 lines · Josh Poimboeuf

The cookie is incremented once per entry from userspace, when needed.

Quoted 7 lines
Quoted 7 lines
Quoted 4 lines

It's a unique id used by the tracers which is emitted with both kernel and user stacktrace samples so the userspace tool can stitch them back together. Even if you have multiple tracers triggering at the same time they can all share a single user trace.

But perf don't need this at all, right? It knows that the next deferred trace for that tid will be the one.

Well.. technically you can sample without tid. But I'm not sure how much it'd be useful if you collect callchains without tid.

Quoted 3 lines

Is that because perf uses per task buffers? Will perf know this if it uses a global buffer? What does perf do with "-a"?

Then it'd use per-cpu ring buffers. But each sample would have pid/tid pair and time so perf tools can match it with a deferred callchian.

Thanks,
Namhyung

#39 lore raw
Quoted 22 lines · Namhyung Kim

Hello,

Quoted 7 lines · Steven Rostedt
Quoted 4 lines · On Mon, 16 Sep 2024 20:15:45 +0200 Peter Zijlstra <[email protected]> wrote:
Quoted 2 lines · Josh Poimboeuf

The cookie is incremented once per entry from userspace, when needed.

Quoted 7 lines
Quoted 7 lines
Quoted 4 lines

It's a unique id used by the tracers which is emitted with both kernel and user stacktrace samples so the userspace tool can stitch them back together. Even if you have multiple tracers triggering at the same time they can all share a single user trace.

But perf don't need this at all, right? It knows that the next deferred trace for that tid will be the one.

Well.. technically you can sample without tid. But I'm not sure how much it'd be useful if you collect callchains without tid.

Quoted 1 line
Quoted 5 lines
Quoted 2 lines

Is that because perf uses per task buffers? Will perf know this if it uses a global buffer? What does perf do with "-a"?

Then it'd use per-cpu ring buffers. But each sample would have pid/tid pair and time so perf tools can match it with a deferred callchian.

That semantic correlation based on trace information should work fine if you do not miss important events in the trace. What the unique id cookie provides is robustness against confusion that can arise when events are discarded.

Discarded events happen when the event throughput is too high for the ring buffer to handle. The following type of confusion can then arise:

- If you miss a stack trace event and then a stack-sample-request event, then the post-processing tools can incorrectly infer causality between a prior stack-sample-request and a following stack trace for the same tid.

- If you miss even more information about the end/beginning of lifetime of two threads, post-processing can be confused and associate a prior stack-sample-request from tid=N with a later stack trace for tid=N where N was re-used due to an exit/clone sequence.

Saving the unique id cookie along with the stack-sample-request and with the stack trace allows more robust causality relationship between the two. Showing reliable causality information may not be super important for profiling use-cases, but it is important for event tracers.

Thanks,

Mathieu

Signature · 3 lines
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
#40 lore raw
In reply to Peter Zijlstra

On 2024-09-16 20:15, Peter Zijlstra wrote:
[...]

Quoted 3 lines

The point being that it is possible to wrap one CPU into the id space of another CPU. It is not trivial, but someone who wants to can make it happen.

I agree that the overflow of the free-running counter bleeding into the CPU numbers is something we want to prevent. We don't care if this counter overflows after day/months/years for sake of correlation within a system call, but we do care about the fact that this free-running counter could be made to overflow after a very long time while the system runs, and then we reach a state where the CPU numbers are mixed up, which leads to short-term correlation issues.

I would recommend this layout for this 64-bit value instead:

low-bits: cpu number
high-bits: free-running counter

This way, we eliminate any carry from overflow into the cpu number bits.

Thanks,

Mathieu

Signature · 3 lines
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
#41 lore raw
Quoted 15 lines · Mathieu Desnoyers

On 2024-09-16 20:15, Peter Zijlstra wrote:
[...]

Quoted 3 lines

The point being that it is possible to wrap one CPU into the id space of another CPU. It is not trivial, but someone who wants to can make it happen.

I agree that the overflow of the free-running counter bleeding into the CPU numbers is something we want to prevent. We don't care if this counter overflows after day/months/years for sake of correlation within a system call, but we do care about the fact that this free-running counter could be made to overflow after a very long time while the system runs, and then we reach a state where the CPU numbers are mixed up, which leads to short-term correlation issues.

Quoted 6 lines

I would recommend this layout for this 64-bit value instead:

low-bits: cpu number
high-bits: free-running counter

This way, we eliminate any carry from overflow into the cpu number bits.

Even better: AFAIR from the discussion I had with Steven and Josh, we intend to have the cookie stored to/read from the task struct with interrupts off, we can simply do:

struct stackwalk_cookie {
uint64_t counter; /* free running per-cpu counter value */
int cpu; /* cpu on which the counter was snapshot. */
};

So we don't have to deal with any overflow trickiness, there is no need for bit-shifting, and we get a full 64-bit free-running counter independently of the number of CPUs.

Thanks,

Mathieu

Quoted 6 lines

Thanks,

Mathieu

Signature · 3 lines
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
#42 lore raw
Quoted 27 lines · Mathieu Desnoyers
Quoted 15 lines · Mathieu Desnoyers

On 2024-09-16 20:15, Peter Zijlstra wrote:
[...]

Quoted 3 lines

The point being that it is possible to wrap one CPU into the id space of another CPU. It is not trivial, but someone who wants to can make it happen.

I agree that the overflow of the free-running counter bleeding into the CPU numbers is something we want to prevent. We don't care if this counter overflows after day/months/years for sake of correlation within a system call, but we do care about the fact that this free-running counter could be made to overflow after a very long time while the system runs, and then we reach a state where the CPU numbers are mixed up, which leads to short-term correlation issues.

Quoted 6 lines

I would recommend this layout for this 64-bit value instead:

low-bits: cpu number
high-bits: free-running counter

This way, we eliminate any carry from overflow into the cpu number bits.

Even better: AFAIR from the discussion I had with Steven and Josh, we intend to have the cookie stored to/read from the task struct with interrupts off, we can simply do:

Quoted 8 lines

struct stackwalk_cookie {
    uint64_t counter;    /* free running per-cpu counter value */ int cpu;        /* cpu on which the counter was snapshot. */
};

So we don't have to deal with any overflow trickiness, there is no need for bit-shifting, and we get a full 64-bit free-running counter independently of the number of CPUs.

Sorry my laptop had the wrong date when I sent this email. Re-sending to make sure it gets seen in the correct order time-wise.

Thanks,

Mathieu

Quoted 14 lines

Thanks,

Mathieu

Quoted 6 lines

Thanks,

Mathieu

Signature · 3 lines
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
maintainer
#43 lore raw
In reply to Josh Poimboeuf
Quoted 1 line · On Mon, 16 Sep 2024 17:39:53 +0200 Josh Poimboeuf <[email protected]> wrote:

Completely untested patch here that I hacked up today:

I can tell it wasn't tested ;-)

Quoted 5 lines

git://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git sframe-2.1

To avoid races, unwind_user_deferred() can't be called from NMI. The tracers will need to trigger irq_work to call it.

kernel/unwind/user.c +2 -1
index 7edb0833fe46..c56cf5d564df 100644
@@ -177,7 +181,8 @@ int unwind_user_deferred(struct unwind_callback *callback, u64 *ctx_cookie)
 
 		cookie = __this_cpu_read(ctx_ctr);
 		cookie &= ((1UL << 48) - 1);
-		cookie |= ((cpu << 48) + 1);
+		cookie |= cpu << 48;
+		cookie++;
 		__this_cpu_write(ctx_ctr, cookie);
 
 		current->unwind_ctx_cookie = cookie;

As the cookie never got incremented.

That was just one issue. Things are still not working but I'll debug the rest later.

-- Steve

author maintainer
#44 lore raw
Quoted 12 lines · Steven Rostedt

+++ b/kernel/unwind/user.c

kernel/unwind/user.c +2 -1
@@ -177,7 +181,8 @@ int unwind_user_deferred(struct unwind_callback *callback, u64 *ctx_cookie)
 
 		cookie = __this_cpu_read(ctx_ctr);
 		cookie &= ((1UL << 48) - 1);
-		cookie |= ((cpu << 48) + 1);
+		cookie |= cpu << 48;
+		cookie++;
 		__this_cpu_write(ctx_ctr, cookie);
 
 		current->unwind_ctx_cookie = cookie;
Quoted 1 line

As the cookie never got incremented.

Ha, that might help ;-)

Quoted 2 lines

That was just one issue. Things are still not working but I'll debug the rest later.

If you don't want to run on the bleeding edge, I'll be testing it soon myself once I get the coding done for my v3.

Signature · 1 line
Josh
maintainer
#45 lore raw
Quoted 5 lines · On Wed, 2 Oct 2024 19:37:02 -0700 Josh Poimboeuf <[email protected]> wrote:
Quoted 2 lines

That was just one issue. Things are still not working but I'll debug the rest later.

If you don't want to run on the bleeding edge, I'll be testing it soon myself once I get the coding done for my v3.

Yeah, I'll wait for that. I have some code working for ftrace on top of this, but I also promised Mathieu I would start looking at his faultable trace points, so I should do that before working more on sframe.

-- Steve

maintainer
#46 lore raw
In reply to Peter Zijlstra
Quoted 5 lines · On Mon, 16 Sep 2024 16:08:56 +0200 Peter Zijlstra <[email protected]> wrote:
Quoted 4 lines

I think the biggest tweak we decided on is that the context id (aka "cookie") would be percpu. Its initial value is (cpuid << 48). It gets incremented for every entry from user space.

Quoted 8 lines

Why? What's the purpose of the cookie? This scheme seems unsound, pin yourself on CPU0 and trigger 1<<48 unwinds while keeping CPU1 idle.

Quoted 3 lines
Quoted 3 lines

That is, we should have an interface like:

typedef void (unwinder_callback_t)(struct user_space_stack *, u64 cookie);

Just make it a void* and let the consumer figure it out.

Let me try to explain the rationale for the "cookie". It's actually a "context_cookie". The cookie is unique for every time user space enters the kernel.

Basically it's a "tag" (we could call it that too, but "cookie" seemed more appropriate as it follows what web browsers do). The idea is this:

* You're in interrupt context and want a user space back trace, and request to have a user stack trace when the task goes back to user space. Now the unwinder will return the current context cookie" and will use that same cookie when it creates the user stack on exit back to user space.

* If this happens in a long system call that does many stack traces (records the kernel stack and also wants the user stack), it does not record the user trace at the time it is requested. But we need a way to tag this event with the user stack trace that will happen when the user stack is recorded when going back to user space. We can't pin the task to the CPU when we want to profile it. The cookie/tag needs to be the same for every request that happens with a single entry into the kernel. It must be different for another entry into the kernel as the user stack will be different then.

Basically think that each cookie represents a single user stack.

* When going back to user space, the ptrace path is taken and the user stack trace is recorded. It will then call the tracers that requested it with the stack trace and the cookie that represents it.

Here's an example:

system_call() {

<interrupt> trigger user stack trace: assign cookie 123
tracer records kernel stack trace and adds cookie 123.

<interrupt> trigger user stack trace: assign cookie 123
tracer records kernel stack trace and adds cookie 123.

<interrupt> trigger user stack trace: assign cookie 123
tracer records kernel stack trace and adds cookie 123.

<return to user space, take ptrace path>

<record user stack trace>
call all the tracers with user stack trace with cookie 123.
}

system_call() {

<interrupt> trigger user stack trace: assign cookie 124
tracer records kernel stack trace and adds cookie 124.

<return to user space, take ptrace path>

<record user stack trace>
call all the tracers with user stack trace with cookie 124.
}

Then the tracers can post process the events and append the user stack trace with cookie 123 on top of the kernel stack events that had cookie 123, and append the user stack trace with cookie 124 on top of the kernel stack events that had cookie 124.

-- Steve

maintainer
#47 lore raw
In reply to Josh Poimboeuf

Hello Josh,

Quoted 6 lines · Josh Poimboeuf

This is a new version of sframe user space unwinding + perf deferred callchains. Better late than never.

Unfortunately I didn't get a chance to do any testing with this one yet as I'm rushing it out the door before GNU Cauldron starts. Conference-driven development ;-)

lol

Quoted 9 lines

Plus I don't have the perf tool piece working yet so I don't have a way of doing end-to-end testing at the moment anyway.

In other words, please don't merge this yet.

Namhyung, if you're still available to write a perf tool patch which integrates with this, that would be great. Otherwise I could give it a try.

Sure, I'll take a look and post the perf tools patches.

Thanks,
Namhyung

#48 lore raw
In reply to Josh Poimboeuf

Hello Josh!

Quoted 2 lines · Josh Poimboeuf

This is a new version of sframe user space unwinding + perf deferred callchains. Better late than never.

...

Quoted 3 lines

These patches add support for unwinding user space from the kernel using SFrame with perf. It should be easy to add user unwinding support for other components like ftrace.

...

We are looking forward to implement support for unwinding of user space using SFrame in kernel/perf on s390. One major concern is that your x86 implementation currently relies on a fallback to unwinding using frame pointer. On s390 unwinding using frame pointer is unsupported, because of lack of proper s390x ABI [1] specification and compiler support. In theory there would be a s390-specific alternative of unwinding using backchain (compiler option -mbackchain), but this has limitations and there is currently no distribution where user space is built with backchain.

How much of an issue would it be if s390 could not provide an unwinding fallback implementation? Do you see the possibility to get away without?

For s390 support of unwinding using SFrame we would need to make a few changes to your generic unwinding framework in the kernel:

- Support for architectures that do not define CFA == SP at callsite:
On s390 the CFA is defined as SP at callsite +160. The stack pointer (SP) therefore unwinds as SP = CFA - 160. For that we would introduce e.g. a sp_val_off field (SP value offset from CFA) in struct user_unwind_frame that would default to 0 on all architectures except s390.

- Support for architectures where RA is not necessarily saved on stack:
On s390 the return address (RA) is not saved (on stack) at function entry. In leaf functions it is not necessarily saved at all.

- Support for architectures were RA/FP are saved in registers in leaf functions:
On s390 the frame pointer (FP) and return address (RA) registers can be saved in other registers when in leaf functions. In the SFrame format we would encode the DWARF register number as offset using the least-significant bit as indication: offset = (regnum << 1) | 1. Therefore we would need to extend your generic unwinding framework to support FP and RA to be restored from registers.

[1]: s390x ELF ABI supplement,

     https://github.com/IBM/s390x-abi/releases

Thanks and regards,
Jens

Signature · 9 lines
Jens Remus
Linux on Z Development (D3303) and z/VSE Support
+49-7031-16-1128 Office
[email protected]

IBM

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/
maintainer
#49 lore raw
Quoted 10 lines · On Wed, 23 Oct 2024 15:22:35 +0200 Jens Remus <[email protected]> wrote:

We are looking forward to implement support for unwinding of user space using SFrame in kernel/perf on s390. One major concern is that your x86 implementation currently relies on a fallback to unwinding using frame pointer. On s390 unwinding using frame pointer is unsupported, because of lack of proper s390x ABI [1] specification and compiler support. In theory there would be a s390-specific alternative of unwinding using backchain (compiler option -mbackchain), but this has limitations and there is currently no distribution where user space is built with backchain.

Quoted 2 lines

How much of an issue would it be if s390 could not provide an unwinding fallback implementation? Do you see the possibility to get away without?

Yes. Even with x86, there's no guarantee that the applications will have frame pointers available. Basically it just returns a stack frame of one (the IP of where user space entered the kernel).

Quoted 24 lines

For s390 support of unwinding using SFrame we would need to make a few changes to your generic unwinding framework in the kernel:

- Support for architectures that do not define CFA == SP at callsite:
On s390 the CFA is defined as SP at callsite +160. The stack pointer (SP) therefore unwinds as SP = CFA - 160. For that we would introduce e.g. a sp_val_off field (SP value offset from CFA) in struct user_unwind_frame that would default to 0 on all architectures except s390.

- Support for architectures where RA is not necessarily saved on stack:
On s390 the return address (RA) is not saved (on stack) at function entry. In leaf functions it is not necessarily saved at all.

- Support for architectures were RA/FP are saved in registers in leaf functions:
On s390 the frame pointer (FP) and return address (RA) registers can be saved in other registers when in leaf functions. In the SFrame format we would encode the DWARF register number as offset using the least-significant bit as indication: offset = (regnum << 1) | 1. Therefore we would need to extend your generic unwinding framework to support FP and RA to be restored from registers.

Quoted 2 lines

[1]: s390x ELF ABI supplement,

      https://github.com/IBM/s390x-abi/releases

Note that Indu (who's on the Cc and is also the author of sframes) gave a talk at GNU Cauldron about s390 support. I'm assuming that her new sframe specification will cover all of this. Then we will have to implement it.

-- Steve

#50 lore raw
In reply to Jens Remus
Quoted 10 lines · Jens Remus

We are looking forward to implement support for unwinding of user space using SFrame in kernel/perf on s390. One major concern is that your x86 implementation currently relies on a fallback to unwinding using frame pointer. On s390 unwinding using frame pointer is unsupported, because of lack of proper s390x ABI [1] specification and compiler support. In theory there would be a s390-specific alternative of unwinding using backchain (compiler option -mbackchain), but this has limitations and there is currently no distribution where user space is built with backchain.

Quoted 2 lines

How much of an issue would it be if s390 could not provide an unwinding fallback implementation? Do you see the possibility to get away without?

No problem, I'll make the fallback dependent on CONFIG_HAVE_UNWIND_USER_FP.

Quoted 2 lines

For s390 support of unwinding using SFrame we would need to make a few changes to your generic unwinding framework in the kernel:

Also no problem, I'm sure there will need to be tweaks going forward.

Thanks for looking at it! v3 will be posted soon.

Signature · 1 line
Josh