[PATCH bpf-next v5 1/2] bpf: Add sleepable version of bpf_get_stack in raw_tp_prog_func_proto and tp_prog_func_proto

[PATCH bpf-next v5 1/2] bpf: Add sleepable version of bpf_get_stack in raw_tp_prog_func_proto and tp_prog_func_proto

[Feng Yang]

From: Feng Yang <yangfeng@kylinos.cn>

bpf_get_stack() has two implementations.  bpf_get_stack() passes
may_fault == false and resolves build IDs with build_id_parse_nofault(),
so an IP whose backing file page is not resident falls back to
BPF_STACK_BUILD_ID_IP.  bpf_get_stack_sleepable() passes may_fault == true
and uses build_id_parse_file(), which faults the page in and returns a real
build ID.

Only kprobe/uprobe programs are routed to the sleepable implementation
today, so the other program types that can be sleepable keep losing
build IDs for non-resident pages:

 - tracing_prog_func_proto() serves BPF_TRACE_ITER, fentry/fexit/fmod_ret
   and BPF_TRACE_RAW_TP programs, and falls back to
   raw_tp_prog_func_proto() for BPF_FUNC_get_stack, which is also the
   get_func_proto() of BPF_PROG_TYPE_RAW_TRACEPOINT and is reached by
   BPF_PROG_TYPE_LSM and BPF_PROG_TYPE_SYSCALL through the default cases
   of bpf_lsm_func_proto() and syscall_prog_func_proto().

 - tp_prog_func_proto() serves BPF_PROG_TYPE_TRACEPOINT programs.

Route them to bpf_get_stack_sleepable() as well and mark the new
prototypes might_sleep, as
commit 9d02927fdf4e ("bpf: Mark faultable stack helpers as sleepable")
did for the faultable stack helper prototypes.

The verifier then rejects bpf_get_stack() from these program types
marked sleepable whenever in_sleepable_context() is false:
inside bpf_rcu_read_lock(), bpf_preempt_disable(), and so on. This also
covers calls that do not pass BPF_F_USER_BUILD_ID and would not sleep;
such programs load today. This is consistent with the existing constraints.

Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
---
v5: selftest: Remove fork-related code, use getpgid. Thanks, Andrii
Nakryiko.

v4: Update commit messages. Test modification: limit capture to one hit, add s390 flags.
https://lore.kernel.org/all/20261105093212.826479-1-yangfeng59949@163.com/

v3: Add selftest. Update commit messages.
https://lore.kernel.org/all/20261104083833.971911-1-yangfeng59949@163.com/

v2: Move regs onto the stack.
https://lore.kernel.org/all/20261102073714.1103088-1-yangfeng59949@163.com/

v1: https://lore.kernel.org/all/20261028093548.790977-1-yangfeng59949@163.com/
---
 kernel/trace/bpf_trace.c | 54 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 2aec56d7f3d3..1229ee06dd71 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -84,6 +84,7 @@ static struct bpf_raw_event_map *bpf_get_raw_tracepoint_module(const char *name)
 
 u64 bpf_get_stackid(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
 u64 bpf_get_stack(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
+u64 bpf_get_stack_sleepable(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
 
 static int bpf_btf_printf_prepare(struct btf_ptr *ptr, u32 btf_ptr_size,
 				  u64 flags, const struct btf **btf,
@@ -1477,6 +1478,26 @@ static const struct bpf_func_proto bpf_get_stack_proto_tp = {
 	.arg4_type	= ARG_ANYTHING,
 };
 
+BPF_CALL_4(bpf_get_stack_tp_sleepable, void *, tp_buff, void *, buf, u32, size,
+	   u64, flags)
+{
+	struct pt_regs *regs = *(struct pt_regs **)tp_buff;
+
+	return bpf_get_stack_sleepable((unsigned long) regs, (unsigned long) buf,
+				       (unsigned long) size, flags, 0);
+}
+
+static const struct bpf_func_proto bpf_get_stack_proto_tp_sleepable = {
+	.func		= bpf_get_stack_tp_sleepable,
+	.gpl_only	= true,
+	.might_sleep	= true,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_CTX,
+	.arg2_type	= ARG_PTR_TO_UNINIT_MEM,
+	.arg3_type	= ARG_MEM_SIZE_OR_ZERO,
+	.arg4_type	= ARG_ANYTHING,
+};
+
 static const struct bpf_func_proto *
 tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 {
@@ -1486,7 +1507,8 @@ tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 	case BPF_FUNC_get_stackid:
 		return &bpf_get_stackid_proto_tp;
 	case BPF_FUNC_get_stack:
-		return &bpf_get_stack_proto_tp;
+		return prog->sleepable ? &bpf_get_stack_proto_tp_sleepable
+				       : &bpf_get_stack_proto_tp;
 	case BPF_FUNC_get_attach_cookie:
 		return &bpf_get_attach_cookie_proto_trace;
 	default:
@@ -1716,6 +1738,33 @@ static const struct bpf_func_proto bpf_get_stack_proto_raw_tp = {
 	.arg4_type	= ARG_ANYTHING,
 };
 
+BPF_CALL_4(bpf_get_stack_raw_tp_sleepable, struct bpf_raw_tracepoint_args *, args,
+	   void *, buf, u32, size, u64, flags)
+{
+	/*
+	 * Don't take a per-CPU bpf_raw_tp_regs slot here: those slots are a
+	 * LIFO nest counter and are vulnerable to slot overwrite and
+	 * corruption across sleep.  Sleepable programs never run in
+	 * IRQ/NMI context, so a local pt_regs is sufficient.
+	 */
+	struct pt_regs regs = {};
+
+	perf_fetch_caller_regs(&regs);
+	return bpf_get_stack_sleepable((unsigned long) &regs, (unsigned long) buf,
+				       (unsigned long) size, flags, 0);
+}
+
+static const struct bpf_func_proto bpf_get_stack_proto_raw_tp_sleepable = {
+	.func		= bpf_get_stack_raw_tp_sleepable,
+	.gpl_only	= true,
+	.might_sleep	= true,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_CTX,
+	.arg2_type	= ARG_PTR_TO_UNINIT_MEM,
+	.arg3_type	= ARG_MEM_SIZE_OR_ZERO,
+	.arg4_type	= ARG_ANYTHING,
+};
+
 static const struct bpf_func_proto *
 raw_tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 {
@@ -1725,7 +1774,8 @@ raw_tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 	case BPF_FUNC_get_stackid:
 		return &bpf_get_stackid_proto_raw_tp;
 	case BPF_FUNC_get_stack:
-		return &bpf_get_stack_proto_raw_tp;
+		return prog->sleepable ? &bpf_get_stack_proto_raw_tp_sleepable
+				       : &bpf_get_stack_proto_raw_tp;
 	case BPF_FUNC_get_attach_cookie:
 		return &bpf_get_attach_cookie_proto_tracing;
 	default:
-- 
2.25.1

[PATCH bpf-next v5 2/2] selftests/bpf: Test faulting build IDs from a sleepable raw tracepoint

[Feng Yang]

From: Feng Yang <yangfeng@kylinos.cn>

Extend the build_id test with a sleepable raw tracepoint on sys_enter.
A helper process pages out the build ID page and issues a getpgid()
syscall with an invalid argument; the BPF program filters on the syscall
id and that argument, records only the first hit, and captures the user
stack with BPF_F_USER_BUILD_ID.  The test asserts that one of the frames
carries uprobe_multi's build ID.

Frame 0 is libc's syscall wrapper, so reaching uprobe_multi needs user
stack unwinding; build uprobe_multi with -mbackchain on s390, where the
unwinder follows backchains.  The non-sleepable control requires at least
one BPF_STACK_BUILD_ID_IP frame. If MADV_PAGEOUT cannot evict the page,
the affected subtests are skipped rather than failing.

Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
---
 tools/testing/selftests/bpf/Makefile          |  4 +-
 tools/testing/selftests/bpf/build_id.h        | 13 +++
 .../selftests/bpf/prog_tests/build_id.c       | 88 +++++++++++++++++++
 .../selftests/bpf/progs/test_build_id.c       | 52 +++++++++++
 tools/testing/selftests/bpf/uprobe_multi.c    | 39 +++++++-
 5 files changed, 191 insertions(+), 5 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/build_id.h

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 77682bf28c6e..367166383246 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -650,7 +650,9 @@ $(OUTPUT)/veristat: $(OUTPUT)/veristat.o
 	$(Q)$(CC) $(CFLAGS) $(LDFLAGS) $(filter %.a %.o,$^) $(LDLIBS) -o $@
 
 # Linking uprobe_multi can fail due to relocation overflows on mips.
-$(OUTPUT)/uprobe_multi: CFLAGS += $(if $(filter mips, $(ARCH)),-mxgot)
+# s390's user unwinder follows backchains, keep them for the build_id test.
+$(OUTPUT)/uprobe_multi: CFLAGS += $(if $(filter mips, $(ARCH)),-mxgot) \
+				$(if $(filter s390 s390x, $(ARCH)),-mbackchain)
 $(OUTPUT)/uprobe_multi: uprobe_multi.c uprobe_multi.ld
 	$(call msg,BINARY,,$@)
 	$(Q)$(CC) $(CFLAGS) -Wl,-T,uprobe_multi.ld -O0 $(LDFLAGS) 	\
diff --git a/tools/testing/selftests/bpf/build_id.h b/tools/testing/selftests/bpf/build_id.h
new file mode 100644
index 000000000000..f5edc3670be2
--- /dev/null
+++ b/tools/testing/selftests/bpf/build_id.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __BUILD_ID_H
+#define __BUILD_ID_H
+
+/*
+ * The helper issues getpgid() with this (nonexistent) pid after paging out the
+ * build ID page.  The BPF program filters on the syscall id and this argument,
+ * so getpid()/read() etc. issued before the page-out cannot be mistaken for it.
+ */
+#define TARGET_SYSCALL_ARG	0x7ffffffeUL
+
+#endif /* __BUILD_ID_H */
+
diff --git a/tools/testing/selftests/bpf/prog_tests/build_id.c b/tools/testing/selftests/bpf/prog_tests/build_id.c
index aec9c8d6bc96..e97a651b5aa7 100644
--- a/tools/testing/selftests/bpf/prog_tests/build_id.c
+++ b/tools/testing/selftests/bpf/prog_tests/build_id.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
+#include <sys/syscall.h>
 #include <test_progs.h>
 
 #include "test_build_id.skel.h"
@@ -104,6 +105,89 @@ static void subtest_sleepable(void)
 	test_build_id__destroy(skel);
 }
 
+static bool stack_has_build_id(const struct bpf_stack_build_id *stack, int frame_cnt,
+			       const char *id, int id_sz)
+{
+	int i;
+
+	for (i = 0; i < frame_cnt; i++)
+		if (stack[i].status == BPF_STACK_BUILD_ID_VALID &&
+		    !memcmp(stack[i].build_id, id, id_sz))
+			return true;
+
+	return false;
+}
+
+static bool stack_has_ip(const struct bpf_stack_build_id *stack, int frame_cnt)
+{
+	int i;
+
+	for (i = 0; i < frame_cnt; i++)
+		if (stack[i].status == BPF_STACK_BUILD_ID_IP)
+			return true;
+
+	return false;
+}
+
+static void subtest_raw_tp(bool sleepable)
+{
+	struct test_build_id *skel;
+	struct bpf_stack_build_id *stack;
+	int frame_cnt, status, res;
+
+	skel = test_build_id__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel_open"))
+		return;
+
+	if (sleepable) {
+		skel->links.rawtp_sleepable = bpf_program__attach(skel->progs.rawtp_sleepable);
+		if (!ASSERT_OK_PTR(skel->links.rawtp_sleepable, "attach_rawtp_sleepable"))
+			goto cleanup;
+	} else {
+		skel->links.rawtp_nofault = bpf_program__attach(skel->progs.rawtp_nofault);
+		if (!ASSERT_OK_PTR(skel->links.rawtp_nofault, "attach_rawtp_nofault"))
+			goto cleanup;
+	}
+
+	skel->bss->target_syscall = SYS_getpgid;
+	status = system("./uprobe_multi paged-out-syscall");
+	if (!ASSERT_NEQ(status, -1, "trigger_uprobe_multi"))
+		goto cleanup;
+	if (WIFEXITED(status) && WEXITSTATUS(status) == 2) {
+		/* could not evict the build ID page, nothing to test */
+		test__skip();
+		goto cleanup;
+	}
+	if (!ASSERT_OK(status, "trigger_uprobe_multi"))
+		goto cleanup;
+
+	if (sleepable) {
+		res = skel->bss->res_rawtp_sleepable;
+		stack = skel->bss->stack_rawtp_sleepable;
+	} else {
+		res = skel->bss->res_rawtp_nofault;
+		stack = skel->bss->stack_rawtp_nofault;
+	}
+	if (!ASSERT_GT(res, 0, "res"))
+		goto cleanup;
+
+	frame_cnt = res / sizeof(struct bpf_stack_build_id);
+	if (env.verbosity >= VERBOSE_NORMAL)
+		print_stack(stack, frame_cnt);
+
+	if (sleepable) {
+		ASSERT_TRUE(stack_has_build_id(stack, frame_cnt, build_id, build_id_sz),
+			    "sleepable_build_id");
+	} else {
+		ASSERT_TRUE(stack_has_ip(stack, frame_cnt), "nofault_has_ip");
+		ASSERT_FALSE(stack_has_build_id(stack, frame_cnt, build_id, build_id_sz),
+			     "nofault_has_build_id");
+	}
+
+cleanup:
+	test_build_id__destroy(skel);
+}
+
 void serial_test_build_id(void)
 {
 	build_id_sz = read_build_id("uprobe_multi", build_id, sizeof(build_id));
@@ -115,4 +199,8 @@ void serial_test_build_id(void)
 		subtest_nofault(true /* resident */);
 	if (test__start_subtest("sleepable"))
 		subtest_sleepable();
+	if (test__start_subtest("raw-tp-nofault-paged-out"))
+		subtest_raw_tp(false /* non-sleepable */);
+	if (test__start_subtest("raw-tp-sleepable-paged-out"))
+		subtest_raw_tp(true /* sleepable */);
 }
diff --git a/tools/testing/selftests/bpf/progs/test_build_id.c b/tools/testing/selftests/bpf/progs/test_build_id.c
index 32ce59f9aa27..53e88a99d214 100644
--- a/tools/testing/selftests/bpf/progs/test_build_id.c
+++ b/tools/testing/selftests/bpf/progs/test_build_id.c
@@ -2,7 +2,11 @@
 /* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
 
 #include "vmlinux.h"
+#include <bpf/bpf_core_read.h>
 #include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+#include "build_id.h"
 
 struct bpf_stack_build_id stack_sleepable[128];
 int res_sleepable;
@@ -28,4 +32,52 @@ int uprobe_sleepable(struct pt_regs *ctx)
 	return 0;
 }
 
+struct bpf_stack_build_id stack_rawtp_sleepable[128];
+int res_rawtp_sleepable;
+
+struct bpf_stack_build_id stack_rawtp_nofault[128];
+int res_rawtp_nofault;
+
+int target_syscall;
+
+static __always_inline bool is_target_syscall(struct pt_regs *regs, long id)
+{
+	return id == target_syscall &&
+	       (unsigned long)PT_REGS_PARM1_CORE_SYSCALL(regs) == TARGET_SYSCALL_ARG;
+}
+
+SEC("raw_tp/sys_enter")
+int BPF_PROG(rawtp_nofault, struct pt_regs *regs, long id)
+{
+	if (!is_target_syscall(regs, id))
+		return 0;
+
+	/* Record only the first hit, a later match must not overwrite it. */
+	if (res_rawtp_nofault)
+		return 0;
+
+	res_rawtp_nofault = bpf_get_stack(ctx, stack_rawtp_nofault,
+					  sizeof(stack_rawtp_nofault),
+					  BPF_F_USER_STACK | BPF_F_USER_BUILD_ID);
+
+	return 0;
+}
+
+SEC("raw_tp.s/sys_enter")
+int BPF_PROG(rawtp_sleepable, struct pt_regs *regs, long id)
+{
+	if (!is_target_syscall(regs, id))
+		return 0;
+
+	/* Record only the first hit, a later match must not overwrite it. */
+	if (res_rawtp_sleepable)
+		return 0;
+
+	res_rawtp_sleepable = bpf_get_stack(ctx, stack_rawtp_sleepable,
+					    sizeof(stack_rawtp_sleepable),
+					    BPF_F_USER_STACK | BPF_F_USER_BUILD_ID);
+
+	return 0;
+}
+
 char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/uprobe_multi.c b/tools/testing/selftests/bpf/uprobe_multi.c
index 0af330b6c364..c6ec45875574 100644
--- a/tools/testing/selftests/bpf/uprobe_multi.c
+++ b/tools/testing/selftests/bpf/uprobe_multi.c
@@ -5,9 +5,12 @@
 #include <stdbool.h>
 #include <stdint.h>
 #include <sys/mman.h>
+#include <sys/syscall.h>
 #include <unistd.h>
 #include <sdt.h>
 
+#include "build_id.h"
+
 #ifndef MADV_POPULATE_READ
 #define MADV_POPULATE_READ 22
 #endif
@@ -95,12 +98,12 @@ static int usdt(void)
 extern char build_id_start[];
 extern char build_id_end[];
 
-int __attribute__((weak)) trigger_uprobe(bool build_id_resident)
+/* Returns 2 if the build ID page could not be evicted. */
+static int prep_build_id_page(bool build_id_resident)
 {
 	int page_sz = sysconf(_SC_PAGESIZE);
-	void *addr;
-
 	unsigned char vec[1];
+	void *addr;
 	int poll = 0;
 
 	/* page-align build ID start */
@@ -123,12 +126,38 @@ int __attribute__((weak)) trigger_uprobe(bool build_id_resident)
 			poll++;
 			usleep(100);
 		} while (poll < 500);
+		if (vec[0] & 1)
+			return 2;
 	}
+	return 0;
+}
+
+int __attribute__((weak)) trigger_uprobe(bool build_id_resident)
+{
+	(void)prep_build_id_page(build_id_resident);
+
 	(void)uprobe();
 
 	return 0;
 }
 
+int __attribute__((weak)) trigger_syscall(void)
+{
+	int err = prep_build_id_page(false /* page-out build ID */);
+
+	if (err)
+		return err;
+
+	/*
+	 * The BPF program under test filters on this syscall and its argument
+	 * and records only the first hit, so no other syscall can overwrite
+	 * the captured stack.
+	 */
+	(void)syscall(SYS_getpgid, TARGET_SYSCALL_ARG);
+
+	return 0;
+}
+
 int main(int argc, char **argv)
 {
 	if (argc != 2)
@@ -142,10 +171,12 @@ int main(int argc, char **argv)
 		return trigger_uprobe(false /* page-out build ID */);
 	if (!strcmp("uprobe-paged-in", argv[1]))
 		return trigger_uprobe(true /* page-in build ID */);
+	if (!strcmp("paged-out-syscall", argv[1]))
+		return trigger_syscall();
 
 error:
 	fprintf(stderr,
-		"usage: %s <bench|usdt|uprobe-paged-out|uprobe-paged-in>\n",
+		"usage: %s <bench|usdt|uprobe-paged-{in,out}|paged-out-syscall>\n",
 		argv[0]);
 	return -1;
 }
-- 
2.25.1

Re: [PATCH bpf-next v5 1/2] bpf: Add sleepable version of bpf_get_stack in raw_tp_prog_func_proto and tp_prog_func_proto

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 3206 bytes --]

> bpf: Add sleepable version of bpf_get_stack in raw_tp_prog_func_proto and tp_prog_func_proto

TL;DR: Marking the new sleepable bpf_get_stack prototypes might_sleep may make
the verifier reject sleepable programs that load today, e.g. calls inside
rcu_read_lock regions, even without BPF_F_USER_BUILD_ID.

> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c

[ ... ]

> @@ -1725,7 +1774,8 @@ raw_tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>  	case BPF_FUNC_get_stackid:
>  		return &bpf_get_stackid_proto_raw_tp;
>  	case BPF_FUNC_get_stack:
> -		return &bpf_get_stack_proto_raw_tp;
> +		return prog->sleepable ? &bpf_get_stack_proto_raw_tp_sleepable
> +				       : &bpf_get_stack_proto_raw_tp;
>  	case BPF_FUNC_get_attach_cookie:
>  		return &bpf_get_attach_cookie_proto_tracing;
>  	default:

Can this make sleepable programs that load today fail to load?

The new prototypes set .might_sleep = true, and check_helper_call() rejects
such a helper whenever in_sleepable_context() is false:

    if (fn->might_sleep && !in_sleepable_context(env)) {
            verbose(env, "sleepable helper %s#%d in %s\n", ...);
            ...
            return -EINVAL;
    }

in_sleepable_context() is false inside bpf_rcu_read_lock(),
bpf_preempt_disable(), bpf_local_irq_save() and lock regions, and inside
rcu callbacks such as bpf_for_each_map_elem() on hash maps and
bpf_find_vma().

Before this patch the same call resolved to bpf_get_stack_proto_raw_tp or
bpf_get_stack_proto_tp, which do not set might_sleep and never fault.  For
example:

    SEC("lsm.s/file_open")
    int prog(void *ctx) {
            bpf_rcu_read_lock();
            bpf_get_stack(ctx, buf, sizeof(buf), 0);
            bpf_rcu_read_unlock();
            return 0;
    }

loads today and would be rejected with "sleepable helper bpf_get_stack#67 in
rcu_read_lock region".

The call chain is bpf_lsm_func_proto() -> tracing_prog_func_proto() ->
raw_tp_prog_func_proto().  The same applies to sleepable raw_tp,
tracepoint, fentry/fexit/fmod_ret, fsession and iter programs.  It also
applies to every BPF_PROG_TYPE_SYSCALL program, since check_attach_btf_id()
forces those to be sleepable.

The rejection happens even when BPF_F_USER_BUILD_ID is not passed, so the
helper could not sleep in those calls.

The commit message acknowledges this ("such programs load today") and cites
commit 9d02927fdf4e as precedent.  That commit fixed prototypes that could
actually sleep in non-sleepable regions, whereas here the old prototypes
were correct.

The verifier already records per-insn non-sleepable call sites in
env->insn_aux_data[insn_idx].non_sleepable, and specialize_kfunc() uses
that to pick bpf_arena_alloc_pages_non_sleepable().  Could the non-faulting
implementation be selected the same way at fixup time for call sites that
are not always sleepable, instead of rejecting them?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/42188184517

Re: [PATCH bpf-next v5 1/2] bpf: Add sleepable version of bpf_get_stack in raw_tp_prog_func_proto and tp_prog_func_proto

[Feng Yang]

On Fri,  6 Nov 2026 08:50:02 +0000 bot+bpf-ci@kernel.org wroted:

[ ... ]

> > @@ -1725,7 +1774,8 @@ raw_tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> >  	case BPF_FUNC_get_stackid:
> >  		return &bpf_get_stackid_proto_raw_tp;
> >  	case BPF_FUNC_get_stack:
> > -		return &bpf_get_stack_proto_raw_tp;
> > +		return prog->sleepable ? &bpf_get_stack_proto_raw_tp_sleepable
> > +				       : &bpf_get_stack_proto_raw_tp;
> >  	case BPF_FUNC_get_attach_cookie:
> >  		return &bpf_get_attach_cookie_proto_tracing;
> >  	default:
> 
> Can this make sleepable programs that load today fail to load?
> 
> The new prototypes set .might_sleep = true, and check_helper_call() rejects
> such a helper whenever in_sleepable_context() is false:
> 
>     if (fn->might_sleep && !in_sleepable_context(env)) {
>             verbose(env, "sleepable helper %s#%d in %s\n", ...);
>             ...
>             return -EINVAL;
>     }
> 
> in_sleepable_context() is false inside bpf_rcu_read_lock(),
> bpf_preempt_disable(), bpf_local_irq_save() and lock regions, and inside
> rcu callbacks such as bpf_for_each_map_elem() on hash maps and
> bpf_find_vma().
> 
> Before this patch the same call resolved to bpf_get_stack_proto_raw_tp or
> bpf_get_stack_proto_tp, which do not set might_sleep and never fault.  For
> example:
> 
>     SEC("lsm.s/file_open")
>     int prog(void *ctx) {
>             bpf_rcu_read_lock();
>             bpf_get_stack(ctx, buf, sizeof(buf), 0);
>             bpf_rcu_read_unlock();
>             return 0;
>     }

This patch is effectively d4dd9775ec24 + 9d02927fdf4e applied to the remaining
program types in one step: the resulting behavior matches uprobe.
If the rejection is acceptable for uprobe, we would prefer to keep the new types consistent with it.

> loads today and would be rejected with "sleepable helper bpf_get_stack#67 in
> rcu_read_lock region".
> 
> The call chain is bpf_lsm_func_proto() -> tracing_prog_func_proto() ->
> raw_tp_prog_func_proto().  The same applies to sleepable raw_tp,
> tracepoint, fentry/fexit/fmod_ret, fsession and iter programs.  It also
> applies to every BPF_PROG_TYPE_SYSCALL program, since check_attach_btf_id()
> forces those to be sleepable.
> 
> The rejection happens even when BPF_F_USER_BUILD_ID is not passed, so the
> helper could not sleep in those calls.
> 
> The commit message acknowledges this ("such programs load today") and cites
> commit 9d02927fdf4e as precedent.  That commit fixed prototypes that could
> actually sleep in non-sleepable regions, whereas here the old prototypes
> were correct.
> 
> The verifier already records per-insn non-sleepable call sites in
> env->insn_aux_data[insn_idx].non_sleepable, and specialize_kfunc() uses
> that to pick bpf_arena_alloc_pages_non_sleepable().  Could the non-faulting
> implementation be selected the same way at fixup time for call sites that
> are not always sleepable, instead of rejecting them?

If we do need to keep the existing unaffected behavior, would this change be similar to
the operation below?

diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
index 1d33a78d9373..d91211b15770 100644
--- a/kernel/bpf/fixups.c
+++ b/kernel/bpf/fixups.c
@@ -3416,6 +3416,9 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env)
                                     func_id_name(insn->imm), insn->imm);
                        return -EFAULT;
                }
+               if (fn->might_sleep && env->insn_aux_data[i].non_sleepable &&
+                   insn->imm == BPF_FUNC_get_stack)
+                       fn = bpf_get_stack_proto_nosleep(env->prog);
                insn->imm = BPF_CALL_IMM(fn->func);
 next_insn:
                if (subprogs[cur_subprog + 1].start == i + 1) {
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index bb1659ce7a5c..f73dd67f523e 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13107,6 +13107,10 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
                return err;
        }
 
+       if (fn->might_sleep && !in_sleepable_context(env) &&
+           func_id == BPF_FUNC_get_stack)
+               fn = bpf_get_stack_proto_nosleep(env->prog);
+
        /* eBPF programs must be GPL compatible to use GPL-ed functions */
        if (!env->prog->gpl_compatible && fn->gpl_only) {
                verbose(env, "cannot call GPL-restricted function from non-GPL compatible program\n");
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 1229ee06dd71..9eed63059449 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1765,6 +1765,22 @@ static const struct bpf_func_proto bpf_get_stack_proto_raw_tp_sleepable = {
        .arg4_type      = ARG_ANYTHING,
 };
 
+const struct bpf_func_proto *
+bpf_get_stack_proto_nosleep(const struct bpf_prog *prog)
+{
+       switch (resolve_prog_type(prog)) {
+       case BPF_PROG_TYPE_TRACEPOINT:
+               return &bpf_get_stack_proto_tp;
+       case BPF_PROG_TYPE_RAW_TRACEPOINT:
+       case BPF_PROG_TYPE_TRACING:
+       case BPF_PROG_TYPE_LSM:
+       case BPF_PROG_TYPE_SYSCALL:
+               return &bpf_get_stack_proto_raw_tp;
+       default:
+               return &bpf_get_stack_proto;
+       }
+}

Re: [PATCH bpf-next v5 1/2] bpf: Add sleepable version of bpf_get_stack in raw_tp_prog_func_proto and tp_prog_func_proto

[Andrii Nakryiko]

On Thu, Nov 12, 2026 at 10:57 PM Feng Yang <yangfeng59949@163.com> wrote:
>
> On Fri,  6 Nov 2026 08:50:02 +0000 bot+bpf-ci@kernel.org wroted:
>
> [ ... ]
>
> > > @@ -1725,7 +1774,8 @@ raw_tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> > >     case BPF_FUNC_get_stackid:
> > >             return &bpf_get_stackid_proto_raw_tp;
> > >     case BPF_FUNC_get_stack:
> > > -           return &bpf_get_stack_proto_raw_tp;
> > > +           return prog->sleepable ? &bpf_get_stack_proto_raw_tp_sleepable
> > > +                                  : &bpf_get_stack_proto_raw_tp;
> > >     case BPF_FUNC_get_attach_cookie:
> > >             return &bpf_get_attach_cookie_proto_tracing;
> > >     default:
> >
> > Can this make sleepable programs that load today fail to load?
> >
> > The new prototypes set .might_sleep = true, and check_helper_call() rejects
> > such a helper whenever in_sleepable_context() is false:
> >
> >     if (fn->might_sleep && !in_sleepable_context(env)) {
> >             verbose(env, "sleepable helper %s#%d in %s\n", ...);
> >             ...
> >             return -EINVAL;
> >     }
> >
> > in_sleepable_context() is false inside bpf_rcu_read_lock(),
> > bpf_preempt_disable(), bpf_local_irq_save() and lock regions, and inside
> > rcu callbacks such as bpf_for_each_map_elem() on hash maps and
> > bpf_find_vma().
> >
> > Before this patch the same call resolved to bpf_get_stack_proto_raw_tp or
> > bpf_get_stack_proto_tp, which do not set might_sleep and never fault.  For
> > example:
> >
> >     SEC("lsm.s/file_open")
> >     int prog(void *ctx) {
> >             bpf_rcu_read_lock();
> >             bpf_get_stack(ctx, buf, sizeof(buf), 0);
> >             bpf_rcu_read_unlock();
> >             return 0;
> >     }
>
> This patch is effectively d4dd9775ec24 + 9d02927fdf4e applied to the remaining
> program types in one step: the resulting behavior matches uprobe.
> If the rejection is acceptable for uprobe, we would prefer to keep the new types consistent with it.
>
> > loads today and would be rejected with "sleepable helper bpf_get_stack#67 in
> > rcu_read_lock region".
> >
> > The call chain is bpf_lsm_func_proto() -> tracing_prog_func_proto() ->
> > raw_tp_prog_func_proto().  The same applies to sleepable raw_tp,
> > tracepoint, fentry/fexit/fmod_ret, fsession and iter programs.  It also
> > applies to every BPF_PROG_TYPE_SYSCALL program, since check_attach_btf_id()
> > forces those to be sleepable.
> >
> > The rejection happens even when BPF_F_USER_BUILD_ID is not passed, so the
> > helper could not sleep in those calls.
> >
> > The commit message acknowledges this ("such programs load today") and cites
> > commit 9d02927fdf4e as precedent.  That commit fixed prototypes that could
> > actually sleep in non-sleepable regions, whereas here the old prototypes
> > were correct.
> >
> > The verifier already records per-insn non-sleepable call sites in
> > env->insn_aux_data[insn_idx].non_sleepable, and specialize_kfunc() uses
> > that to pick bpf_arena_alloc_pages_non_sleepable().  Could the non-faulting
> > implementation be selected the same way at fixup time for call sites that
> > are not always sleepable, instead of rejecting them?
>
> If we do need to keep the existing unaffected behavior, would this change be similar to
> the operation below?

I think it's reasonable to have sleepable implementation, and if
someone for whatever reason did call bpf_get_stack() under explicit
RCU lock, it's an easy fix.

We do have a bit more general problem to solve eventually where
non-sleepable program can have async callback in sleepable context
(like with task_work) and vice versa, perhaps, and we should have a
generic mechanism to keep track of that and substituting correct
helper/kfunc implementation based on such context. But that's a
separate and more general issue.

I think the change is good as is.

>
> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
> index 1d33a78d9373..d91211b15770 100644
> --- a/kernel/bpf/fixups.c
> +++ b/kernel/bpf/fixups.c
> @@ -3416,6 +3416,9 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env)
>                                      func_id_name(insn->imm), insn->imm);
>                         return -EFAULT;
>                 }
> +               if (fn->might_sleep && env->insn_aux_data[i].non_sleepable &&
> +                   insn->imm == BPF_FUNC_get_stack)
> +                       fn = bpf_get_stack_proto_nosleep(env->prog);
>                 insn->imm = BPF_CALL_IMM(fn->func);
>  next_insn:
>                 if (subprogs[cur_subprog + 1].start == i + 1) {
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index bb1659ce7a5c..f73dd67f523e 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -13107,6 +13107,10 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
>                 return err;
>         }
>
> +       if (fn->might_sleep && !in_sleepable_context(env) &&
> +           func_id == BPF_FUNC_get_stack)
> +               fn = bpf_get_stack_proto_nosleep(env->prog);
> +
>         /* eBPF programs must be GPL compatible to use GPL-ed functions */
>         if (!env->prog->gpl_compatible && fn->gpl_only) {
>                 verbose(env, "cannot call GPL-restricted function from non-GPL compatible program\n");
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 1229ee06dd71..9eed63059449 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -1765,6 +1765,22 @@ static const struct bpf_func_proto bpf_get_stack_proto_raw_tp_sleepable = {
>         .arg4_type      = ARG_ANYTHING,
>  };
>
> +const struct bpf_func_proto *
> +bpf_get_stack_proto_nosleep(const struct bpf_prog *prog)
> +{
> +       switch (resolve_prog_type(prog)) {
> +       case BPF_PROG_TYPE_TRACEPOINT:
> +               return &bpf_get_stack_proto_tp;
> +       case BPF_PROG_TYPE_RAW_TRACEPOINT:
> +       case BPF_PROG_TYPE_TRACING:
> +       case BPF_PROG_TYPE_LSM:
> +       case BPF_PROG_TYPE_SYSCALL:
> +               return &bpf_get_stack_proto_raw_tp;
> +       default:
> +               return &bpf_get_stack_proto;
> +       }
> +}
>

Re: [PATCH bpf-next v5 1/2] bpf: Add sleepable version of bpf_get_stack in raw_tp_prog_func_proto and tp_prog_func_proto

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Fri,  6 Nov 2026 16:07:45 +0800 you wrote:
> From: Feng Yang <yangfeng@kylinos.cn>
> 
> bpf_get_stack() has two implementations.  bpf_get_stack() passes
> may_fault == false and resolves build IDs with build_id_parse_nofault(),
> so an IP whose backing file page is not resident falls back to
> BPF_STACK_BUILD_ID_IP.  bpf_get_stack_sleepable() passes may_fault == true
> and uses build_id_parse_file(), which faults the page in and returns a real
> build ID.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v5,1/2] bpf: Add sleepable version of bpf_get_stack in raw_tp_prog_func_proto and tp_prog_func_proto
    https://git.kernel.org/bpf/bpf-next/c/4812fd2d2942
  - [bpf-next,v5,2/2] selftests/bpf: Test faulting build IDs from a sleepable raw tracepoint
    https://git.kernel.org/bpf/bpf-next/c/e8c1bcf8d591

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html