[PATCH v2 bpf-next 0/3] riscv: bpf: Add BPF private stack support

[PATCH v2 bpf-next 0/3] riscv: bpf: Add BPF private stack support

[Chen Pei]

This series adds private-stack support to the RV64 BPF JIT.

The verifier rejects a struct_ops program that requests a private stack
with -EACCES when the JIT does not report
bpf_jit_supports_private_stack(), and sched_ext requests one for its
dispatch callback, so no sched_ext scheduler can load on riscv64 today.

Allocation, the guard value and the lifetime handling follow arm64,
x86-64 and powerpc64; the 16-byte two-copy guard layout follows arm64
and powerpc64, since x86-64 uses an 8-byte guard with a single copy.
What is specific to riscv64 is where the pointers end up:

 - Every BPF stack access goes through BPF_REG_FP (S5), so pointing S5
   at the top of the per-CPU area and leaving SP on the kernel stack is
   all the JIT has to do. S5 is callee-saved and each subprog derives
   its own, so bpf2bpf calls need no extra save and restore.
 - The outgoing stack-argument area stays in the kernel frame: callers
   write it relative to SP while callees read it relative to RV_REG_FP.
   Unlike x86-64, no register here is shared with the private-stack
   frame pointer, so no verifier-side exclusion is needed and stack
   arguments keep working together with a private stack. arm64 already
   does the same, and patch 3 measures the three ranges at run time and
   pins them in the disassembly.
 - The per-CPU lookup is emitted after the tail-call entry point. A
   program using a private stack can still be entered as a tail-call
   target, at RV_TAILCALL_OFFSET, so it has to set up its own S5 there;
   and since that offset is a constant added to the target's bpf_func,
   everything in front of it must be the same size in every program.

The base address is materialized with absolute moves, as on arm64,
x86-64 and powerpc64. Those three reach the per-CPU offset through gs:,
TPIDR or r13 and only have to move the base into a register; riscv64 has
no such register, so the address of __per_cpu_offset is loaded here too.
A PC-relative pair is not an option for the base itself: the dynamic
per-CPU area is vmalloc'd, further from the JIT region than auipc
reaches, which is why the whole setup is ten instructions on the kernel
tested here. __per_cpu_offset is a kernel-image symbol and on that
kernel lands in the sign-extended 32-bit range, so emit_imm() renders it
in two instructions through its 32-bit fast path - what an emit_addr()
pair would cost, and the form insn_is_mov_percpu_addr() already uses;
unifying the two needs an extra_pass argument and an error return in
bpf_jit_build_prologue(), which also touches bpf_jit_comp32.c, so it is
left as is. Both constants are fixed before the offset-convergence loop,
so the prologue length is stable across passes.

RV32 does not declare support and keeps the existing behaviour; the
prologue signature is unchanged, so bpf_jit_comp32.c needs no edit.

Changes since v1:
 - Patch 1 commit message: guard layout credited to arm64 and powerpc64,
   value alone to x86-64; __per_cpu_offset described as a choice.
 - Patch 3: test_private_stack_runtime() renamed to
   serial_test_private_stack_runtime(), and the stack-argument skip
   prints a reason.

Testing
=======
QEMU virt rv64, an 8 vCPU SMP kernel and a 1 vCPU UP kernel, both built
on the base commit below, source tree clean at the head of this series,
selftests built from these patches. There are no private-stack related
failures, and no run logged a BUG, WARNING, panic, unknown opcode or
guard corruption. On UP, kfunc_call/subprog, kfunc_call/subprog_lskel
and scx_simple are not run: all three fail the same way on the unpatched
base.

 - verifier_private_stack, struct_ops_private_stack,
   private_stack_runtime.
 - tailcalls, stack_arg, subprogs, fentry_fexit, kfunc_call,
   percpu_alloc, async_stack_depth, arena_atomics, arena_ldsx.
 - verifier_stack_arg, stack_arg_precision.
 - may_goto_priv_stack, may_goto_far, verifier_may_goto_1,
   verifier_may_goto_2.
 - scx_simple.
 - sched_ext runner sweep on SMP: 40 cases, 39 pass. enable_cmask fails
   because its ops.enable() takes a struct scx_cmask __arena pointer and
   riscv64 does not implement bpf_jit_supports_arena_args().

Chen Pei (3):
  riscv: bpf: Add BPF private stack support
  selftests/bpf: Enable private stack tests for riscv64
  selftests/bpf: Add a runtime test for private stack and stack
    arguments

 arch/riscv/net/bpf_jit.h                      |   1 +
 arch/riscv/net/bpf_jit_comp64.c               |  42 +++++-
 arch/riscv/net/bpf_jit_core.c                 |  84 +++++++++--
 .../selftests/bpf/private_stack_runtime.h     |  12 ++
 .../bpf/prog_tests/private_stack_runtime.c    |  84 +++++++++++
 .../bpf/prog_tests/struct_ops_private_stack.c |   3 +-
 .../bpf/progs/private_stack_runtime.c         | 140 ++++++++++++++++++
 .../bpf/progs/verifier_private_stack.c        |  41 ++++-
 8 files changed, 393 insertions(+), 14 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/private_stack_runtime.h
 create mode 100644 tools/testing/selftests/bpf/prog_tests/private_stack_runtime.c
 create mode 100644 tools/testing/selftests/bpf/progs/private_stack_runtime.c


base-commit: 8fb43ffc3327d12d57d3fbe6f4820b5a522de0d1
-- 
2.50.1

[PATCH v2 bpf-next 1/3] riscv: bpf: Add BPF private stack support

[Chen Pei]

A program on a private stack keeps its BPF frames in a per-CPU area of
its own rather than on the kernel stack of whichever context entered it.
bpf_enable_priv_stack() grants that mode only once the JIT reports
bpf_jit_supports_private_stack(), which riscv64 did not: programs there
always ran on the kernel stack, and a struct_ops program that requires
one was rejected with -EACCES.

Allocate a per-CPU area of the verifier calculated depth plus a 16 byte
guard region at each end, and point BPF_REG_FP (S5) at the top of it
while leaving SP on the kernel stack. Every BPF stack access already
goes through S5, and S5 is callee-saved with each subprog deriving its
own, so nothing else in the JIT has to know about the private stack.

The outgoing stack argument area stays in the kernel frame: callers
write it relative to SP, callees relative to RV_REG_FP, which the
prologue sets to the caller's SP.

The per-CPU lookup is emitted after the tail call entry point: a program
using a private stack can still be entered as a tail call target, and
RV_TAILCALL_OFFSET is a constant added to the target's bpf_func, so what
is in front of it must be the same size in every program.

The base is materialized with absolute moves, as on arm64, x86-64 and
powerpc64: the vmalloc'd per-CPU area is beyond an auipc pair from the
JIT region, and riscv64 has no register holding the per-CPU offset, so
the address of __per_cpu_offset is loaded too. emit_imm() is a choice
there, not a requirement; emit_addr() as used by
insn_is_mov_percpu_addr() would need an extra_pass argument and an error
return in bpf_jit_build_prologue(), touching bpf_jit_comp32.c too. Both
constants are fixed before the offset convergence loop, so the prologue
length is stable across passes.

The guard value follows arm64, x86-64 and powerpc64, and the 16 byte
two-copy region at each end follows arm64 and powerpc64, since x86-64
uses an 8 byte guard with a single copy. A jited program has them
checked when it is freed, reporting the first corrupted CPU only;
failure paths release the area unchecked since the image never ran,
which also clears priv_stack_ptr for the later bpf_jit_free().

RV32 does not declare support and keeps the existing kernel stack
behaviour.

Signed-off-by: Chen Pei <cp0613@linux.alibaba.com>
---
 arch/riscv/net/bpf_jit.h        |  1 +
 arch/riscv/net/bpf_jit_comp64.c | 42 +++++++++++++++--
 arch/riscv/net/bpf_jit_core.c   | 84 +++++++++++++++++++++++++++++----
 3 files changed, 115 insertions(+), 12 deletions(-)

diff --git a/arch/riscv/net/bpf_jit.h b/arch/riscv/net/bpf_jit.h
index 21a190a6a6d9..efab2b6803e4 100644
--- a/arch/riscv/net/bpf_jit.h
+++ b/arch/riscv/net/bpf_jit.h
@@ -14,6 +14,7 @@
 
 #define RV_MAX_REG_ARGS		8
 #define RV_EXTRA_STK_ARGS	(RV_MAX_REG_ARGS - MAX_BPF_FUNC_REG_ARGS)
+#define PRIV_STACK_GUARD_SZ	16
 
 /* verify runtime detection extension status */
 #define rv_ext_enabled(ext) \
diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
index 371405598faa..a7a7d2f9da85 100644
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -2134,12 +2134,34 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
 	return 0;
 }
 
+static void emit_priv_stack_frame(struct rv_jit_context *ctx)
+{
+	u64 fp = (__force u64)ctx->prog->aux->priv_stack_ptr + PRIV_STACK_GUARD_SZ;
+
+	fp += round_up(ctx->prog->aux->stack_depth, STACK_ALIGN);
+	emit_imm(RV_REG_S5, fp, ctx);
+#ifdef CONFIG_SMP
+	emit_lw(RV_REG_T1, offsetof(struct thread_info, cpu), RV_REG_TP, ctx);
+	emit_imm(RV_REG_T2, (u64)__per_cpu_offset, ctx);
+	emit_sh3add(RV_REG_T1, RV_REG_T1, RV_REG_T2, ctx);
+	emit_ld(RV_REG_T1, 0, RV_REG_T1, ctx);
+	emit_add(RV_REG_S5, RV_REG_S5, RV_REG_T1, ctx);
+#endif
+}
+
 void bpf_jit_build_prologue(struct rv_jit_context *ctx, bool is_subprog)
 {
+	bool priv_stack = ctx->prog->aux->priv_stack_ptr;
 	int i, stack_adjust = 0, store_offset, bpf_stack_adjust = ctx->stack_arg_sz;
 
-	bpf_stack_adjust += round_up(ctx->prog->aux->stack_depth, STACK_ALIGN);
-	if (bpf_stack_adjust)
+	/*
+	 * With a private stack only the outgoing stack argument area is left in
+	 * this frame. It has to stay: the caller writes it relative to SP while
+	 * the callee reads it relative to RV_REG_FP, the caller's SP set below.
+	 */
+	if (!priv_stack)
+		bpf_stack_adjust += round_up(ctx->prog->aux->stack_depth, STACK_ALIGN);
+	if (bpf_stack_adjust || priv_stack)
 		mark_fp(ctx);
 
 	if (seen_reg(RV_REG_RA, ctx))
@@ -2217,7 +2239,16 @@ void bpf_jit_build_prologue(struct rv_jit_context *ctx, bool is_subprog)
 
 	emit_addi(RV_REG_FP, RV_REG_SP, stack_adjust, ctx);
 
-	if (bpf_stack_adjust)
+	/*
+	 * Point BPF_REG_FP at the top of this CPU's private stack. A program
+	 * with one can still be entered as a tail call target, at
+	 * RV_TAILCALL_OFFSET, so this has to come after that entry point; the
+	 * offset is a constant added to the target's bpf_func, so everything in
+	 * front of it must also be the same size in every program.
+	 */
+	if (priv_stack)
+		emit_priv_stack_frame(ctx);
+	else if (bpf_stack_adjust)
 		emit_addi(RV_REG_S5, RV_REG_SP, bpf_stack_adjust, ctx);
 
 	ctx->stack_size = stack_adjust;
@@ -2320,3 +2351,8 @@ bool bpf_jit_supports_hmul(void)
 {
 	return true;
 }
+
+bool bpf_jit_supports_private_stack(void)
+{
+	return true;
+}
diff --git a/arch/riscv/net/bpf_jit_core.c b/arch/riscv/net/bpf_jit_core.c
index f2497ec7d378..f2021088e447 100644
--- a/arch/riscv/net/bpf_jit_core.c
+++ b/arch/riscv/net/bpf_jit_core.c
@@ -9,6 +9,7 @@
 #include <linux/bpf.h>
 #include <linux/filter.h>
 #include <linux/memory.h>
+#include <linux/percpu.h>
 #include <asm/text-patching.h>
 #include <asm/cfi.h>
 #include "bpf_jit.h"
@@ -44,6 +45,62 @@ bool bpf_jit_needs_zext(void)
 	return true;
 }
 
+#define PRIV_STACK_GUARD_VAL	0xEB9F12345678eb9fULL
+
+static u32 priv_stack_size(struct bpf_prog *prog)
+{
+	return round_up(prog->aux->stack_depth, STACK_ALIGN) +
+	       2 * PRIV_STACK_GUARD_SZ;
+}
+
+static int alloc_priv_stack(struct bpf_prog *prog)
+{
+	u32 size = priv_stack_size(prog);
+	u32 end = (size - PRIV_STACK_GUARD_SZ) / sizeof(u64);
+	void __percpu *ptr;
+	int cpu, i;
+
+	ptr = __alloc_percpu_gfp(size, STACK_ALIGN, GFP_KERNEL);
+	if (!ptr)
+		return -ENOMEM;
+
+	for_each_possible_cpu(cpu) {
+		u64 *stack = per_cpu_ptr(ptr, cpu);
+
+		for (i = 0; i < PRIV_STACK_GUARD_SZ / sizeof(u64); i++) {
+			stack[i] = PRIV_STACK_GUARD_VAL;
+			stack[end + i] = PRIV_STACK_GUARD_VAL;
+		}
+	}
+	prog->aux->priv_stack_ptr = ptr;
+	return 0;
+}
+
+static void priv_stack_check_guard(struct bpf_prog *prog)
+{
+	u32 end = (priv_stack_size(prog) - PRIV_STACK_GUARD_SZ) / sizeof(u64);
+	int cpu;
+
+	for_each_possible_cpu(cpu) {
+		u64 *stack = per_cpu_ptr(prog->aux->priv_stack_ptr, cpu);
+
+		if (stack[0] != PRIV_STACK_GUARD_VAL ||
+		    stack[1] != PRIV_STACK_GUARD_VAL ||
+		    stack[end] != PRIV_STACK_GUARD_VAL ||
+		    stack[end + 1] != PRIV_STACK_GUARD_VAL) {
+			pr_err("bpf-jit: private stack overflow/underflow guard of prog %s corrupted on CPU %d\n",
+			       bpf_jit_get_prog_name(prog), cpu);
+			return;
+		}
+	}
+}
+
+static void free_priv_stack(struct bpf_prog *prog)
+{
+	free_percpu(prog->aux->priv_stack_ptr);
+	prog->aux->priv_stack_ptr = NULL;
+}
+
 struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_prog *prog)
 {
 	unsigned int prog_size = 0, extable_size = 0;
@@ -84,12 +141,15 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_pr
 			    round_up((stack_arg_cnt - RV_EXTRA_STK_ARGS) * 8, STACK_ALIGN) : 0;
 
 	ctx->prog = prog;
+	if (prog->aux->jits_use_priv_stack && alloc_priv_stack(prog))
+		goto out_free_hdr;
+
 	ctx->offset = kvzalloc_objs(int, prog->len);
 	if (!ctx->offset)
-		goto out_offset;
+		goto out_free_hdr;
 
 	if (build_body(ctx, extra_pass, NULL))
-		goto out_offset;
+		goto out_free_hdr;
 
 	for (i = 0; i < prog->len; i++) {
 		prev_ninsns += 32;
@@ -104,7 +164,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_pr
 		ctx->prologue_len = ctx->ninsns;
 
 		if (build_body(ctx, extra_pass, ctx->offset))
-			goto out_offset;
+			goto out_free_hdr;
 
 		ctx->epilogue_offset = ctx->ninsns;
 		bpf_jit_build_epilogue(ctx);
@@ -124,7 +184,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_pr
 							  bpf_fill_ill_insns,
 							  bpf_prog_was_classic(prog));
 			if (!jit_data->ro_header)
-				goto out_offset;
+				goto out_free_hdr;
 
 			/*
 			 * Use the image(RW) for writing the JITed instructions. But also save
@@ -190,10 +250,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_pr
 			ctx->offset[i] = ctx->offset[i - 1];
 		ctx->offset[0] = ninsns_rvoff(ctx->prologue_len);
 		bpf_prog_update_insn_ptrs(prog, ctx->offset, jit_data->ro_image);
-out_offset:
-		kvfree(ctx->offset);
-		kfree(jit_data);
-		prog->aux->jit_data = NULL;
+		goto out_free_ctx;
 	}
 
 	return prog;
@@ -209,7 +266,12 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_pr
 				   sizeof(jit_data->header->size));
 		bpf_jit_binary_pack_free(jit_data->ro_header, jit_data->header);
 	}
-	goto out_offset;
+	free_priv_stack(prog);
+out_free_ctx:
+	kvfree(ctx->offset);
+	kfree(jit_data);
+	prog->aux->jit_data = NULL;
+	return prog;
 }
 
 u64 bpf_jit_alloc_exec_limit(void)
@@ -261,6 +323,10 @@ void bpf_jit_free(struct bpf_prog *prog)
 		prog->bpf_func = (void *)prog->bpf_func - cfi_get_offset();
 		hdr = bpf_jit_binary_pack_hdr(prog);
 		bpf_jit_binary_pack_free(hdr, NULL);
+		if (prog->aux->priv_stack_ptr) {
+			priv_stack_check_guard(prog);
+			free_priv_stack(prog);
+		}
 		WARN_ON_ONCE(!bpf_prog_kallsyms_verify_off(prog));
 	}
 
-- 
2.50.1

[PATCH v2 bpf-next 2/3] selftests/bpf: Enable private stack tests for riscv64

[Chen Pei]

The RV64 JIT now supports the private stack. Allow the existing
verifier and struct_ops private-stack tests to run on riscv64 and add
the expected RV64 JITed instruction patterns.

Signed-off-by: Chen Pei <cp0613@linux.alibaba.com>
---
 .../bpf/prog_tests/struct_ops_private_stack.c |  3 +-
 .../bpf/progs/verifier_private_stack.c        | 41 ++++++++++++++++++-
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/struct_ops_private_stack.c b/tools/testing/selftests/bpf/prog_tests/struct_ops_private_stack.c
index 2b3ec2b79091..103179ff8733 100644
--- a/tools/testing/selftests/bpf/prog_tests/struct_ops_private_stack.c
+++ b/tools/testing/selftests/bpf/prog_tests/struct_ops_private_stack.c
@@ -6,7 +6,8 @@
 #include "struct_ops_private_stack_recur.skel.h"
 #include "struct_ops_private_stack_large.skel.h"
 
-#if defined(__x86_64__) || defined(__aarch64__) || defined(__powerpc64__)
+#if defined(__x86_64__) || defined(__aarch64__) || defined(__powerpc64__) || \
+	(defined(__riscv) && __riscv_xlen == 64)
 static void test_private_stack(void)
 {
 	struct struct_ops_private_stack *skel;
diff --git a/tools/testing/selftests/bpf/progs/verifier_private_stack.c b/tools/testing/selftests/bpf/progs/verifier_private_stack.c
index bbc09f516d92..4df06a82dd04 100644
--- a/tools/testing/selftests/bpf/progs/verifier_private_stack.c
+++ b/tools/testing/selftests/bpf/progs/verifier_private_stack.c
@@ -8,7 +8,8 @@
 /* From include/linux/filter.h */
 #define MAX_BPF_STACK    512
 
-#if defined(__TARGET_ARCH_x86) || defined(__TARGET_ARCH_arm64)
+#if defined(__TARGET_ARCH_x86) || defined(__TARGET_ARCH_arm64) || \
+	(defined(__TARGET_ARCH_riscv) && __riscv_xlen == 64)
 
 struct elem {
 	struct bpf_timer t;
@@ -42,6 +43,14 @@ __jited("	mov	x0, #0x2a")
 __jited("	str	x0, [x27]")
 __jited("...")
 __jited("	ldp	x25, x27, [sp], {{.*}}")
+__arch_riscv64
+__jited("	addi	sp, sp, -0x20")
+__jited("...")
+__jited("	sd	s5, 0x10(sp)")
+__jited("...")
+__jited("	sd	a0, -0x100(s5)")
+__jited("...")
+__jited("	ld	s5, 0x10(sp)")
 __naked void private_stack_single_prog(void)
 {
 	asm volatile ("			\
@@ -60,6 +69,10 @@ __jited("	subq	$0x8, %rsp")
 __arch_arm64
 __jited("	mov	x25, sp")
 __jited("	sub	sp, sp, #0x10")
+__arch_riscv64
+__jited("	addi	sp, sp, -0x30")
+__jited("...")
+__jited("	addi	s5, sp, 0x10")
 __naked void no_private_stack_nested(void)
 {
 	asm volatile ("			\
@@ -113,6 +126,14 @@ __jited("	str	x0, [x27]")
 __jited("	bl	{{.*}}")
 __jited("...")
 __jited("	ldp	x25, x27, [sp], {{.*}}")
+__arch_riscv64
+__jited("	addi	sp, sp, -0x20")
+__jited("...")
+__jited("	sd	s5, 0x8(sp)")
+__jited("...")
+__jited("	sd	a0, -0x200(s5)")
+__jited("...")
+__jited("	ld	s5, 0x8(sp)")
 __naked void private_stack_nested_1(void)
 {
 	asm volatile ("				\
@@ -183,6 +204,16 @@ __jited("	bl	0x{{.*}}")
 __jited("	mov	x8, x0")
 __jited("	mov	x8, #0x0")
 __jited("	ldp	x25, x27, [sp], {{.*}}")
+__arch_riscv64
+__jited("func #1")
+__jited("...")
+__jited("	addi	sp, sp, -0x20")
+__jited("...")
+__jited("	sd	s5, 0x8(sp)")
+__jited("...")
+__jited("	sd	a0, -0x200(s5)")
+__jited("...")
+__jited("	ld	s5, 0x8(sp)")
 __naked void private_stack_callback(void)
 {
 	asm volatile ("			\
@@ -412,6 +443,10 @@ __arch_arm64
 __jited("	mrs	x10, TPIDR_EL{{[0-1]}}")
 __jited("	add	x27, x27, x10")
 __jited("	add	x25, x27, {{.*}}")
+__arch_riscv64
+__jited("	addi	sp, sp, -0x20")
+__jited("...")
+__jited("	sd	s5, 0x8(sp)")
 int private_stack_async_callback_1(void)
 {
 	struct bpf_timer *arr_timer;
@@ -441,6 +476,10 @@ __arch_x86_64
 __jited("	subq	$0x100, %rsp")
 __arch_arm64
 __jited("	sub	sp, sp, #0x100")
+__arch_riscv64
+__jited("	addi	sp, sp, -0x120")
+__jited("...")
+__jited("	addi	s5, sp, 0x100")
 int private_stack_async_callback_2(void)
 {
 	struct bpf_timer *arr_timer;
-- 
2.50.1

[PATCH v2 bpf-next 3/3] selftests/bpf: Add a runtime test for private stack and stack arguments

[Chen Pei]

No existing test reaches the combination. bpf_enable_priv_stack() grants
an adaptive private stack only to kprobe, tracepoint, perf_event and
raw_tracepoint programs, and to tracing, lsm and struct_ops ones that
request it or recurse. The stack-argument tests run from tc, socket,
syscall and fentry/fexit, all left at NO_PRIV_STACK; the private-stack
tests call subprograms of up to three arguments, which stay in
registers. Neither pairs a private stack with outgoing stack arguments,
where the caller writes the argument area relative to SP and the callee
reads it relative to the frame pointer.

Add a runtime test for that combination. A static ten-argument callee
writes through a pointer into the caller frame and keeps a 128-byte
volatile local array, so its own depth crosses the 64-byte threshold at
which the verifier grants a subprog the adaptive private stack. The same
workload runs from a raw tracepoint, which gets a private stack, and
from tc, which does not; the test pins itself to each allowed CPU in
turn and checks the CPU the program reported.

A runtime result cannot show where the three stack areas landed, so the
riscv64 __jited patterns pin the layout: the caller's kernel frame is 48
bytes against the control group's 176 and the callee's 64 against 208,
the 128 and 144 byte differences being exactly the BPF stack that moved
to the per-CPU area. Both groups write the two outgoing arguments at
0x0(sp) and 0x8(sp) and the callee reads them back at 0x0(s0) and
0x8(s0), so the outgoing area, the saved registers and the BPF stack
occupy disjoint ranges. None of the patterns assert the per-CPU lookup,
which is only emitted on CONFIG_SMP, so one set matches SMP and UP
alike.

The zero-depth struct_ops program is a control case for the kernel
stack path, not a private-stack allocation: the verifier grants the
adaptive private stack only once a subprog's stack depth, rounded up to
the JIT's 16-byte granularity, reaches 64 bytes.

Signed-off-by: Chen Pei <cp0613@linux.alibaba.com>
---

Notes:
Reachability of a private stack together with outgoing stack arguments,
per architecture, for review context only:

- x86-64 cannot reach it. The verifier denies the private stack to
  subprograms with stack arguments there because R9 doubles as the
  private stack frame pointer and argument 6, so both halves of this
  test would be kernel stack runs.
- powerpc64 has no stack arguments at all, as it does not report
  bpf_jit_supports_stack_args(), so it cannot reach it either.
- arm64 reports both bpf_jit_supports_private_stack() and
  bpf_jit_supports_stack_args(), so the combination is reachable there
  today and has no test for it. Widening the gate to arm64 needs arm64
  __jited patterns and an arm64 machine to validate them, so I would
  post that separately once the maintainers agree this runtime test is
  worth keeping.

 .../selftests/bpf/private_stack_runtime.h     |  12 ++
 .../bpf/prog_tests/private_stack_runtime.c    |  84 +++++++++++
 .../bpf/progs/private_stack_runtime.c         | 140 ++++++++++++++++++
 3 files changed, 236 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/private_stack_runtime.h
 create mode 100644 tools/testing/selftests/bpf/prog_tests/private_stack_runtime.c
 create mode 100644 tools/testing/selftests/bpf/progs/private_stack_runtime.c

diff --git a/tools/testing/selftests/bpf/private_stack_runtime.h b/tools/testing/selftests/bpf/private_stack_runtime.h
new file mode 100644
index 000000000000..8ed49ff66a6e
--- /dev/null
+++ b/tools/testing/selftests/bpf/private_stack_runtime.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __PRIVATE_STACK_RUNTIME_H
+#define __PRIVATE_STACK_RUNTIME_H
+
+struct private_stack_result {
+	long value;
+	long first;
+	long last;
+	__u32 cpu;
+};
+
+#endif
diff --git a/tools/testing/selftests/bpf/prog_tests/private_stack_runtime.c b/tools/testing/selftests/bpf/prog_tests/private_stack_runtime.c
new file mode 100644
index 000000000000..918c41d6b179
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/private_stack_runtime.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include <network_helpers.h>
+#include "private_stack_runtime.h"
+#include "private_stack_runtime.skel.h"
+
+void serial_test_private_stack_runtime(void)
+{
+#if defined(__riscv) && __riscv_xlen == 64
+	LIBBPF_OPTS(bpf_test_run_opts, opts,
+		    .data_in = &pkt_v4,
+		    .data_size_in = sizeof(pkt_v4),
+	);
+	struct private_stack_runtime *skel;
+	struct bpf_link *trace = NULL, *ops = NULL;
+	cpu_set_t saved, single;
+	int cpu, err, tested = 0;
+
+	/*
+	 * Two independent halves. RUN_TESTS() validates the __jited patterns
+	 * through the test loader, which builds its own object from the ELF
+	 * bytes. The half below loads a separate skeleton to check the runtime
+	 * values and the CPU the program reported, which disassembly patterns
+	 * cannot express.
+	 */
+	RUN_TESTS(private_stack_runtime);
+
+	if (!ASSERT_OK(sched_getaffinity(0, sizeof(saved), &saved), "getaffinity"))
+		return;
+
+	skel = private_stack_runtime__open();
+	if (!ASSERT_OK_PTR(skel, "open"))
+		return;
+	if (!skel->rodata->has_stack_args) {
+		printf("%s:SKIP: clang lacks __BPF_FEATURE_STACK_ARGUMENT\n", __func__);
+		test__skip();
+		goto out;
+	}
+	skel->rodata->target_tgid = getpid();
+	if (!ASSERT_OK(private_stack_runtime__load(skel), "load"))
+		goto out;
+
+	trace = bpf_program__attach_raw_tracepoint(skel->progs.private_args, "sys_enter");
+	if (!ASSERT_OK_PTR(trace, "attach_tracepoint"))
+		goto out;
+
+	for (cpu = 0; cpu < CPU_SETSIZE; cpu++) {
+		if (!CPU_ISSET(cpu, &saved))
+			continue;
+		CPU_ZERO(&single);
+		CPU_SET(cpu, &single);
+		if (!ASSERT_OK(sched_setaffinity(0, sizeof(single), &single), "setaffinity"))
+			break;
+
+		err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.kernel_args), &opts);
+		if (!ASSERT_OK(err, "kernel_args") || !ASSERT_EQ(opts.retval, 0, "retval"))
+			break;
+		syscall(__NR_getpgid, 0);
+		ASSERT_EQ(skel->bss->private_result.value, 324 + getpid(), "private_value");
+		ASSERT_EQ(skel->bss->private_result.first, 29, "private_first");
+		ASSERT_EQ(skel->bss->private_result.last, 324, "private_last");
+		ASSERT_EQ(skel->bss->private_result.cpu, cpu, "private_cpu");
+		ASSERT_EQ(skel->bss->kernel_result.value, 324 + getpid(), "kernel_value");
+		ASSERT_EQ(skel->bss->kernel_result.first, 29, "kernel_first");
+		ASSERT_EQ(skel->bss->kernel_result.last, 324, "kernel_last");
+		ASSERT_EQ(skel->bss->kernel_result.cpu, cpu, "kernel_cpu");
+		tested++;
+	}
+	ASSERT_GT(tested, 0, "tested_cpus");
+
+	ops = bpf_map__attach_struct_ops(skel->maps.zero_stack_ops);
+	if (!ASSERT_OK_PTR(ops, "attach_zero_stack"))
+		goto out;
+	ASSERT_OK(trigger_module_test_read(256), "trigger_zero_stack");
+	ASSERT_EQ(skel->bss->zero_calls, 1, "zero_stack_calls");
+out:
+	bpf_link__destroy(ops);
+	bpf_link__destroy(trace);
+	private_stack_runtime__destroy(skel);
+	ASSERT_OK(sched_setaffinity(0, sizeof(saved), &saved), "restore_affinity");
+#else
+	test__skip();
+#endif
+}
diff --git a/tools/testing/selftests/bpf/progs/private_stack_runtime.c b/tools/testing/selftests/bpf/progs/private_stack_runtime.c
new file mode 100644
index 000000000000..61c144fd5cbe
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/private_stack_runtime.c
@@ -0,0 +1,140 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "../test_kmods/bpf_testmod.h"
+#include "../private_stack_runtime.h"
+#include "bpf_misc.h"
+
+char _license[] SEC("license") = "GPL";
+
+const volatile __u32 target_tgid;
+const volatile long seed = 1;
+
+struct private_stack_result private_result, kernel_result;
+int zero_calls;
+
+#ifdef __BPF_FEATURE_STACK_ARGUMENT
+const volatile bool has_stack_args = true;
+
+struct frame {
+	long slots[16];
+};
+
+static __noinline long check_args(long a, long b, long c, long d, long e,
+				  long f, long g, long h, long i,
+				  struct frame *caller)
+{
+	volatile long local[16] = {};
+	long sum;
+
+	local[0] = a;
+	local[15] = i;
+	sum = a + 2 * b + 3 * c + 4 * d + 5 * e + 6 * f + 7 * g + 8 * h + 9 * i;
+	caller->slots[15] = caller->slots[0] + sum + local[0] + local[15];
+	return caller->slots[15] + (bpf_get_current_pid_tgid() >> 32);
+}
+
+static __always_inline void run_args(struct private_stack_result *result)
+{
+	struct frame caller = {};
+	long n = seed;
+
+	caller.slots[0] = 29;
+	result->value = check_args(n, n + 1, n + 2, n + 3, n + 4,
+				   n + 5, n + 6, n + 7, n + 8, &caller);
+	result->first = caller.slots[0];
+	result->last = caller.slots[15];
+	result->cpu = bpf_get_smp_processor_id();
+}
+#else
+const volatile bool has_stack_args = false;
+
+static __always_inline void run_args(struct private_stack_result *result)
+{
+}
+#endif
+
+SEC("raw_tp/sys_enter")
+#ifdef __BPF_FEATURE_STACK_ARGUMENT
+__arch_riscv64
+/*
+ * func #0, the caller. With a private stack the kernel frame holds only the
+ * saved registers and the outgoing stack argument area, so it is 48 bytes =
+ * 32 (ra, fp, s5, tcc) + 16 (two 8 byte outgoing slots). The 128 byte BPF
+ * stack is not in this frame; it lives on the per-CPU private stack reached
+ * through s5. The caller writes the two outgoing arguments relative to sp.
+ */
+__jited("	addi	sp, sp, -0x30")
+__jited("...")
+__jited("	sd	{{.*}}, 0x0(sp)")
+__jited("...")
+__jited("	sd	{{.*}}, 0x8(sp)")
+__jited("...")
+/*
+ * func #1, the ten argument callee, gets a private stack of its own, so its
+ * kernel frame is the 64 bytes of saved registers and nothing else. It reads
+ * the two incoming stack arguments relative to fp (s0), which its prologue
+ * set to the caller's sp, i.e. exactly the area the caller wrote above.
+ */
+__jited("func #1")
+__jited("...")
+__jited("	addi	sp, sp, -0x40")
+__jited("...")
+__jited("	ld	{{.*}}, 0x0(s0)")
+__jited("...")
+__jited("	ld	{{.*}}, 0x8(s0)")
+#endif
+
+int private_args(void *ctx)
+{
+	if ((bpf_get_current_pid_tgid() >> 32) == target_tgid)
+		run_args(&private_result);
+	return 0;
+}
+
+SEC("tc")
+#ifdef __BPF_FEATURE_STACK_ARGUMENT
+__arch_riscv64
+/*
+ * func #0, the caller, control group. BPF_PROG_TYPE_SCHED_CLS is
+ * NO_PRIV_STACK, so the 128 byte BPF stack stays in the kernel frame and s5
+ * is sp relative: 176 bytes = 32 saved + 128 BPF stack + 16 outgoing, i.e.
+ * 128 bytes larger than the private stack caller above. The outgoing stack
+ * argument area is written relative to sp in both cases.
+ */
+__jited("	addi	sp, sp, -0xb0")
+__jited("...")
+__jited("	addi	s5, sp, 0x90")
+__jited("...")
+__jited("	sd	{{.*}}, 0x0(sp)")
+__jited("...")
+__jited("	sd	{{.*}}, 0x8(sp)")
+__jited("...")
+/* func #1, the callee: 208 bytes = 64 saved + 144 BPF stack. */
+__jited("func #1")
+__jited("...")
+__jited("	addi	sp, sp, -0xd0")
+__jited("...")
+__jited("	ld	{{.*}}, 0x0(s0)")
+__jited("...")
+__jited("	ld	{{.*}}, 0x8(s0)")
+#endif
+
+int kernel_args(struct __sk_buff *skb)
+{
+	run_args(&kernel_result);
+	return 0;
+}
+
+SEC("struct_ops")
+int BPF_PROG(zero_stack)
+{
+	zero_calls++;
+	return 0;
+}
+
+SEC(".struct_ops")
+struct bpf_testmod_ops3 zero_stack_ops = {
+	.test_1 = (void *)zero_stack,
+};
-- 
2.50.1

Re: [PATCH v2 bpf-next 3/3] selftests/bpf: Add a runtime test for private stack and stack arguments

[bot+bpf-ci]

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

> selftests/bpf: Add a runtime test for private stack and stack arguments

TL;DR: Commit message only: it may be inaccurate in saying fentry/fexit tests
are left at NO_PRIV_STACK, since tracing programs that recurse are eligible
for a private stack; the review suggests rewording.

>     selftests/bpf: Add a runtime test for private stack and stack arguments
>
>     No existing test reaches the combination. bpf_enable_priv_stack() grants
>     an adaptive private stack only to kprobe, tracepoint, perf_event and
>     raw_tracepoint programs, and to tracing, lsm and struct_ops ones that
>     request it or recurse. The stack-argument tests run from tc, socket,
>     syscall and fentry/fexit, all left at NO_PRIV_STACK; the private-stack
>     tests call subprograms of up to three arguments, which stay in
>     registers. Neither pairs a private stack with outgoing stack arguments,
>     where the caller writes the argument area relative to SP and the callee
>     reads it relative to the frame pointer.
>
>     Add a runtime test for that combination. A static ten-argument callee
>     writes through a pointer into the caller frame and keeps a 128-byte
>     volatile local array, so its own depth crosses the 64-byte threshold at
>     which the verifier grants a subprog the adaptive private stack. The same
>     workload runs from a raw tracepoint, which gets a private stack, and
>     from tc, which does not; the test pins itself to each allowed CPU in
>     turn and checks the CPU the program reported.
>
>     A runtime result cannot show where the three stack areas landed, so the
>     riscv64 __jited patterns pin the layout: the caller's kernel frame is 48
>     bytes against the control group's 176 and the callee's 64 against 208,
>     the 128 and 144 byte differences being exactly the BPF stack that moved
>     to the per-CPU area. Both groups write the two outgoing arguments at
>     0x0(sp) and 0x8(sp) and the callee reads them back at 0x0(s0) and
>     0x8(s0), so the outgoing area, the saved registers and the BPF stack
>     occupy disjoint ranges. None of the patterns assert the per-CPU lookup,
>     which is only emitted on CONFIG_SMP, so one set matches SMP and UP
>     alike.
>
>     The zero-depth struct_ops program is a control case for the kernel
>     stack path, not a private-stack allocation: the verifier grants the
>     adaptive private stack only once a subprog's stack depth, rounded up to
>     the JIT's 16-byte granularity, reaches 64 bytes.
>
>     Signed-off-by: Chen Pei <cp0613@linux.alibaba.com>

The commit message says the stack-argument tests run from "tc, socket,
syscall and fentry/fexit, all left at NO_PRIV_STACK".  Is that accurate
for fentry/fexit?

Those are BPF_PROG_TYPE_TRACING programs, and bpf_enable_priv_stack() does:

    case BPF_PROG_TYPE_TRACING:
    case BPF_PROG_TYPE_LSM:
    case BPF_PROG_TYPE_STRUCT_OPS:
        if (prog->aux->priv_stack_requested || bpf_prog_check_recur(prog))
            return PRIV_STACK_ADAPTIVE;

bpf_prog_check_recur() returns true for TRACING unless expected_attach_type
is BPF_TRACE_ITER, so the fentry/fexit tracers in
progs/tracing_stack_arg_tailcall_tracer.c and
progs/tracing_struct_int128_tracer.c get PRIV_STACK_ADAPTIVE at the program
level.  That is the "tracing ... ones that ... recurse" case named in the
preceding sentence.

The real reasons those tracers never pair a private stack with stack
arguments seem to be different.  check_max_stack_depth_subprog() only gives
a subprog PRIV_STACK_ADAPTIVE when its rounded depth reaches
BPF_PRIV_STACK_MIN_SIZE (64), and these programs only copy a few ctx[]
slots into globals.  They also make no outgoing stack-argument calls; the
stack arguments belong to the traced syscall-program target.

Could the sentence say that the fentry/fexit tracers are eligible for a
private stack, but stay below the 64-byte threshold and make no
stack-argument calls?


---
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/41813628482