[PATCH bpf-next 0/3] bpf: Report arena faults to BPF streams

[PATCH bpf-next 0/3] bpf: Report arena faults to BPF streams

[Puranjay Mohan]

This set adds the support of reporting page faults inside arena to BPF
stderr stream. The reported address is the one that a user would expect
to see if they pass it to bpf_printk();

Here is an example output from a stream and bpf_printk()

ERROR: Arena WRITE access at unmapped address 0xdeaddead0000
CPU: 9 UID: 0 PID: 502 Comm: test_progs
Call trace:
bpf_stream_stage_dump_stack+0xc0/0x150
bpf_prog_report_arena_violation+0x98/0xf0
ex_handler_bpf+0x5c/0x78
fixup_exception+0xf8/0x160
__do_kernel_fault+0x40/0x188
do_bad_area+0x70/0x88
do_translation_fault+0x54/0x98
do_mem_abort+0x4c/0xa8
el1_abort+0x44/0x70
el1h_64_sync_handler+0x50/0x108
el1h_64_sync+0x6c/0x70
bpf_prog_a64a9778d31b8e88_stream_arena_write_fault+0x84/0xc8
  *(page) = 1; @ stream.c:100
bpf_prog_test_run_syscall+0x100/0x328
__sys_bpf+0x508/0xb98
__arm64_sys_bpf+0x2c/0x48
invoke_syscall+0x50/0x120
el0_svc_common.constprop.0+0x48/0xf8
do_el0_svc+0x28/0x40
el0_svc+0x48/0xf8
el0t_64_sync_handler+0xa0/0xe8
el0t_64_sync+0x198/0x1a0

Same address is seen by using bpf_printk():

1389.078831: bpf_trace_printk: Read Address: 0xdeaddead0000

To make this possible, some extra metadata has to be passed to the bpf
exception handler, so the bpf exception handling mechanism for both
x86-64 and arm64 have been improved in this set.

The streams selftest has been updated to also test this new feature.

Puranjay Mohan (3):
  bpf: arm64: simplify exception table handling
  bpf: Report arena faults to BPF stderr
  selftests/bpf: Add tests for arena fault reporting

 arch/arm64/net/bpf_jit_comp.c                 | 56 ++++++++-----
 arch/x86/net/bpf_jit_comp.c                   | 80 ++++++++++++++++++-
 include/linux/bpf.h                           |  1 +
 kernel/bpf/arena.c                            | 20 +++++
 .../testing/selftests/bpf/prog_tests/stream.c | 24 ++++++
 tools/testing/selftests/bpf/progs/stream.c    | 37 +++++++++
 6 files changed, 192 insertions(+), 26 deletions(-)

-- 
2.47.3

[PATCH bpf-next 1/3] bpf: arm64: simplify exception table handling

[Puranjay Mohan]

BPF loads with BPF_PROBE_MEM(SX) can load from unsafe pointers and the
JIT adds an exception table entry for the JITed instruction which allows
the exeption handler to set the destination register of the load to zero
and continue execution from the next instruction.

As all arm64 instructions are AARCH64_INSN_SIZE size, the exception
handler can just increment the pc by AARCH64_INSN_SIZE without needing
the exact address of the instruction following the the faulting
instruction.

Simplify the exception table usage in arm64 JIT by only saving the
destination register in ex->fixup and drop everything related to
the fixup_offset. The fault handler is modified to add AARCH64_INSN_SIZE
to the pc.

Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
---
 arch/arm64/net/bpf_jit_comp.c | 25 +++----------------------
 1 file changed, 3 insertions(+), 22 deletions(-)

diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 52ffe115a8c47..42643fd9168fc 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -1066,19 +1066,18 @@ static void build_epilogue(struct jit_ctx *ctx, bool was_classic)
 	emit(A64_RET(A64_LR), ctx);
 }
 
-#define BPF_FIXUP_OFFSET_MASK	GENMASK(26, 0)
 #define BPF_FIXUP_REG_MASK	GENMASK(31, 27)
 #define DONT_CLEAR 5 /* Unused ARM64 register from BPF's POV */
 
 bool ex_handler_bpf(const struct exception_table_entry *ex,
 		    struct pt_regs *regs)
 {
-	off_t offset = FIELD_GET(BPF_FIXUP_OFFSET_MASK, ex->fixup);
 	int dst_reg = FIELD_GET(BPF_FIXUP_REG_MASK, ex->fixup);
 
 	if (dst_reg != DONT_CLEAR)
 		regs->regs[dst_reg] = 0;
-	regs->pc = (unsigned long)&ex->fixup - offset;
+	/* Skip the faulting instruction */
+	regs->pc += AARCH64_INSN_SIZE;
 	return true;
 }
 
@@ -1088,7 +1087,6 @@ static int add_exception_handler(const struct bpf_insn *insn,
 				 int dst_reg)
 {
 	off_t ins_offset;
-	off_t fixup_offset;
 	unsigned long pc;
 	struct exception_table_entry *ex;
 
@@ -1119,22 +1117,6 @@ static int add_exception_handler(const struct bpf_insn *insn,
 	if (WARN_ON_ONCE(ins_offset >= 0 || ins_offset < INT_MIN))
 		return -ERANGE;
 
-	/*
-	 * Since the extable follows the program, the fixup offset is always
-	 * negative and limited to BPF_JIT_REGION_SIZE. Store a positive value
-	 * to keep things simple, and put the destination register in the upper
-	 * bits. We don't need to worry about buildtime or runtime sort
-	 * modifying the upper bits because the table is already sorted, and
-	 * isn't part of the main exception table.
-	 *
-	 * The fixup_offset is set to the next instruction from the instruction
-	 * that may fault. The execution will jump to this after handling the
-	 * fault.
-	 */
-	fixup_offset = (long)&ex->fixup - (pc + AARCH64_INSN_SIZE);
-	if (!FIELD_FIT(BPF_FIXUP_OFFSET_MASK, fixup_offset))
-		return -ERANGE;
-
 	/*
 	 * The offsets above have been calculated using the RO buffer but we
 	 * need to use the R/W buffer for writes.
@@ -1147,8 +1129,7 @@ static int add_exception_handler(const struct bpf_insn *insn,
 	if (BPF_CLASS(insn->code) != BPF_LDX)
 		dst_reg = DONT_CLEAR;
 
-	ex->fixup = FIELD_PREP(BPF_FIXUP_OFFSET_MASK, fixup_offset) |
-		    FIELD_PREP(BPF_FIXUP_REG_MASK, dst_reg);
+	ex->fixup = FIELD_PREP(BPF_FIXUP_REG_MASK, dst_reg);
 
 	ex->type = EX_TYPE_BPF;
 
-- 
2.47.3

[PATCH bpf-next 2/3] bpf: Report arena faults to BPF stderr

[Puranjay Mohan]

Begin reporting arena page faults and the faulting address to BPF
program's stderr, this patch adds support in the arm64 and x86-64 JITs,
support for other archs can be added later.

The fault handlers receive the 32 bit address in the arena region so
the upper 32 bits of user_vm_start is added to it before printing the
address. This is what the user would expect to see as this is what is
printed by bpf_printk() is you pass it an address returned by
bpf_arena_alloc_pages();

Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
---
 arch/arm64/net/bpf_jit_comp.c | 31 ++++++++++++++
 arch/x86/net/bpf_jit_comp.c   | 80 +++++++++++++++++++++++++++++++++--
 include/linux/bpf.h           |  1 +
 kernel/bpf/arena.c            | 20 +++++++++
 4 files changed, 128 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 42643fd9168fc..5680c7cd8932f 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -1066,6 +1066,9 @@ static void build_epilogue(struct jit_ctx *ctx, bool was_classic)
 	emit(A64_RET(A64_LR), ctx);
 }
 
+#define BPF_FIXUP_LOAD_OFFSET_MASK GENMASK(15, 0)
+#define BPF_FIXUP_ARENA_REG_MASK   GENMASK(20, 16)
+#define BPF_ARENA_ACCESS	   BIT(21)
 #define BPF_FIXUP_REG_MASK	GENMASK(31, 27)
 #define DONT_CLEAR 5 /* Unused ARM64 register from BPF's POV */
 
@@ -1073,11 +1076,22 @@ bool ex_handler_bpf(const struct exception_table_entry *ex,
 		    struct pt_regs *regs)
 {
 	int dst_reg = FIELD_GET(BPF_FIXUP_REG_MASK, ex->fixup);
+	s16 off = FIELD_GET(BPF_FIXUP_LOAD_OFFSET_MASK, ex->fixup);
+	int arena_reg = FIELD_GET(BPF_FIXUP_ARENA_REG_MASK, ex->fixup);
+	bool is_arena = !!(ex->fixup & BPF_ARENA_ACCESS);
+	bool is_write = (dst_reg == DONT_CLEAR);
+	unsigned long addr;
 
 	if (dst_reg != DONT_CLEAR)
 		regs->regs[dst_reg] = 0;
 	/* Skip the faulting instruction */
 	regs->pc += AARCH64_INSN_SIZE;
+
+	if (is_arena) {
+		addr = regs->regs[arena_reg] + off;
+		bpf_prog_report_arena_violation(is_write, addr);
+	}
+
 	return true;
 }
 
@@ -1087,6 +1101,9 @@ static int add_exception_handler(const struct bpf_insn *insn,
 				 int dst_reg)
 {
 	off_t ins_offset;
+	s16 load_off = insn->off;
+	bool is_arena;
+	int arena_reg;
 	unsigned long pc;
 	struct exception_table_entry *ex;
 
@@ -1100,6 +1117,9 @@ static int add_exception_handler(const struct bpf_insn *insn,
 				BPF_MODE(insn->code) != BPF_PROBE_ATOMIC)
 		return 0;
 
+	is_arena = (BPF_MODE(insn->code) == BPF_PROBE_MEM32) ||
+		   (BPF_MODE(insn->code) == BPF_PROBE_ATOMIC);
+
 	if (!ctx->prog->aux->extable ||
 	    WARN_ON_ONCE(ctx->exentry_idx >= ctx->prog->aux->num_exentries))
 		return -EINVAL;
@@ -1131,6 +1151,17 @@ static int add_exception_handler(const struct bpf_insn *insn,
 
 	ex->fixup = FIELD_PREP(BPF_FIXUP_REG_MASK, dst_reg);
 
+	if (is_arena) {
+		ex->fixup |= BPF_ARENA_ACCESS;
+		if (BPF_CLASS(insn->code) == BPF_LDX)
+			arena_reg = bpf2a64[insn->src_reg];
+		else
+			arena_reg = bpf2a64[insn->dst_reg];
+
+		ex->fixup |=  FIELD_PREP(BPF_FIXUP_LOAD_OFFSET_MASK, load_off) |
+			      FIELD_PREP(BPF_FIXUP_ARENA_REG_MASK, arena_reg);
+	}
+
 	ex->type = EX_TYPE_BPF;
 
 	ctx->exentry_idx++;
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 7e3fca1646203..c8d99375e6de7 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -8,6 +8,7 @@
 #include <linux/netdevice.h>
 #include <linux/filter.h>
 #include <linux/if_vlan.h>
+#include <linux/bitfield.h>
 #include <linux/bpf.h>
 #include <linux/memory.h>
 #include <linux/sort.h>
@@ -1388,16 +1389,67 @@ static int emit_atomic_ld_st_index(u8 **pprog, u32 atomic_op, u32 size,
 	return 0;
 }
 
+/*
+ * Metadata encoding for exception handling in JITed code.
+ *
+ * Format of `fixup` and `data` fields in `struct exception_table_entry`:
+ *
+ * Bit layout of `fixup` (32-bit):
+ *
+ * +-----------+--------+-----------+---------+----------+
+ * | 31        | 30-24  |   23-16   |   15-8  |    7-0   |
+ * |           |        |           |         |          |
+ * | ARENA_ACC | Unused | ARENA_REG | DST_REG | INSN_LEN |
+ * +-----------+--------+-----------+---------+----------+
+ *
+ * - INSN_LEN (8 bits): Length of faulting insn (max x86 insn = 15 bytes (fits in 8 bits)).
+ * - DST_REG  (8 bits): Offset of dst_reg from reg2pt_regs[] (max offset = 112 (fits in 8 bits)).
+ *                      This is set to DONT_CLEAR if the insn is a store.
+ * - ARENA_REG (8 bits): Offset of the register that is used to calculate the
+ *                       address for load/store when accessing the arena region.
+ * - ARENA_ACCESS (1 bit): This bit is set when the faulting instruction accessed the arena region.
+ *
+ * Bit layout of `data` (32-bit):
+ *
+ * +--------------+--------+--------------+
+ * |	31-16	  |  15-8  |     7-0      |
+ * |              |	   |              |
+ * | ARENA_OFFSET | Unused |  EX_TYPE_BPF |
+ * +--------------+--------+--------------+
+ *
+ * - ARENA_OFFSET (16 bits): Offset used to calculate the address for load/store when
+ *                           accessing the arena region.
+ */
+
 #define DONT_CLEAR 1
+#define FIXUP_OFFSET_MASK	GENMASK(7, 0)
+#define FIXUP_REG_MASK		GENMASK(15, 8)
+#define FIXUP_ARENA_REG_MASK	GENMASK(23, 16)
+#define FIXUP_ARENA_ACCESS	BIT(31)
+#define DATA_ARENA_OFFSET_MASK	GENMASK(31, 16)
 
 bool ex_handler_bpf(const struct exception_table_entry *x, struct pt_regs *regs)
 {
-	u32 reg = x->fixup >> 8;
+	u32 reg = FIELD_GET(FIXUP_REG_MASK, x->fixup);
+	off_t offset = FIELD_GET(FIXUP_OFFSET_MASK, x->fixup);
+	bool is_arena = !!(x->fixup & FIXUP_ARENA_ACCESS);
+	bool is_write = (reg == DONT_CLEAR);
+	unsigned long addr;
+	s16 arena_offset;
+	u32 arena_reg;
 
 	/* jump over faulting load and clear dest register */
 	if (reg != DONT_CLEAR)
 		*(unsigned long *)((void *)regs + reg) = 0;
-	regs->ip += x->fixup & 0xff;
+	regs->ip += offset;
+
+	if (is_arena) {
+		arena_reg = FIELD_GET(FIXUP_ARENA_REG_MASK, x->data);
+		arena_offset = FIELD_GET(DATA_ARENA_OFFSET_MASK, x->data);
+		addr = *(unsigned long *)((void *)regs + arena_reg) + arena_offset;
+		bpf_prog_report_arena_violation(is_write, addr);
+	}
+
 	return true;
 }
 
@@ -2070,6 +2122,9 @@ st:			if (is_imm8(insn->off))
 			{
 				struct exception_table_entry *ex;
 				u8 *_insn = image + proglen + (start_of_ldx - temp);
+				bool is_arena;
+				u32 fixup_reg;
+				u32 arena_reg;
 				s64 delta;
 
 				if (!bpf_prog->aux->extable)
@@ -2089,8 +2144,25 @@ st:			if (is_imm8(insn->off))
 
 				ex->data = EX_TYPE_BPF;
 
-				ex->fixup = (prog - start_of_ldx) |
-					((BPF_CLASS(insn->code) == BPF_LDX ? reg2pt_regs[dst_reg] : DONT_CLEAR) << 8);
+				is_arena = (BPF_MODE(insn->code) == BPF_PROBE_MEM32) ||
+					   (BPF_MODE(insn->code) == BPF_PROBE_ATOMIC);
+
+				fixup_reg = (BPF_CLASS(insn->code) == BPF_LDX) ?
+					    reg2pt_regs[dst_reg] : DONT_CLEAR;
+
+				ex->fixup = FIELD_PREP(FIXUP_OFFSET_MASK, prog - start_of_ldx) |
+					    FIELD_PREP(FIXUP_REG_MASK, fixup_reg);
+
+				if (is_arena) {
+					ex->fixup |= FIXUP_ARENA_ACCESS;
+					if (BPF_CLASS(insn->code) == BPF_LDX)
+						arena_reg = reg2pt_regs[src_reg];
+					else
+						arena_reg = reg2pt_regs[dst_reg];
+
+					ex->fixup |= FIELD_PREP(FIXUP_ARENA_REG_MASK, arena_reg);
+					ex->data |= FIELD_PREP(DATA_ARENA_OFFSET_MASK, insn->off);
+				}
 			}
 			break;
 
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index cc700925b802f..3e62834726a97 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -3653,6 +3653,7 @@ int bpf_stream_stage_printk(struct bpf_stream_stage *ss, const char *fmt, ...);
 int bpf_stream_stage_commit(struct bpf_stream_stage *ss, struct bpf_prog *prog,
 			    enum bpf_stream_id stream_id);
 int bpf_stream_stage_dump_stack(struct bpf_stream_stage *ss);
+void bpf_prog_report_arena_violation(bool write, unsigned long addr);
 
 #define bpf_stream_printk(ss, ...) bpf_stream_stage_printk(&ss, __VA_ARGS__)
 #define bpf_stream_dump_stack(ss) bpf_stream_stage_dump_stack(&ss)
diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
index 5b37753799d20..a1653d1c04ca5 100644
--- a/kernel/bpf/arena.c
+++ b/kernel/bpf/arena.c
@@ -633,3 +633,23 @@ static int __init kfunc_init(void)
 	return register_btf_kfunc_id_set(BPF_PROG_TYPE_UNSPEC, &common_kfunc_set);
 }
 late_initcall(kfunc_init);
+
+void bpf_prog_report_arena_violation(bool write, unsigned long addr)
+{
+	struct bpf_stream_stage ss;
+	struct bpf_prog *prog;
+	u64 user_vm_start;
+
+	prog = bpf_prog_find_from_stack();
+	if (!prog)
+		return;
+
+	user_vm_start = bpf_arena_get_user_vm_start(prog->aux->arena);
+	addr += (user_vm_start >> 32) << 32;
+
+	bpf_stream_stage(ss, prog, BPF_STDERR, ({
+		bpf_stream_printk(ss, "ERROR: Arena %s access at unmapped address 0x%lx\n",
+				  write ? "WRITE" : "READ", addr);
+		bpf_stream_dump_stack(ss);
+	}));
+}
-- 
2.47.3

[PATCH bpf-next 3/3] selftests/bpf: Add tests for arena fault reporting

[Puranjay Mohan]

Add selftests for testing the reporting of arena page faults through BPF
streams. Two new bpf programs are added that read and write to an
unmapped arena address and the fault reporting is verified in the
userspace through streams.

Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
---
 .../testing/selftests/bpf/prog_tests/stream.c | 24 ++++++++++++
 tools/testing/selftests/bpf/progs/stream.c    | 37 +++++++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/stream.c b/tools/testing/selftests/bpf/prog_tests/stream.c
index d9f0185dca61b..4bdde56de35b1 100644
--- a/tools/testing/selftests/bpf/prog_tests/stream.c
+++ b/tools/testing/selftests/bpf/prog_tests/stream.c
@@ -41,6 +41,22 @@ struct {
 		"([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
 		"|[ \t]+[^\n]+\n)*",
 	},
+	{
+		offsetof(struct stream, progs.stream_arena_read_fault),
+		"ERROR: Arena READ access at unmapped address 0x.*\n"
+		"CPU: [0-9]+ UID: 0 PID: [0-9]+ Comm: .*\n"
+		"Call trace:\n"
+		"([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
+		"|[ \t]+[^\n]+\n)*",
+	},
+	{
+		offsetof(struct stream, progs.stream_arena_write_fault),
+		"ERROR: Arena WRITE access at unmapped address 0x.*\n"
+		"CPU: [0-9]+ UID: 0 PID: [0-9]+ Comm: .*\n"
+		"Call trace:\n"
+		"([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
+		"|[ \t]+[^\n]+\n)*",
+	},
 };
 
 static int match_regex(const char *pattern, const char *string)
@@ -85,6 +101,14 @@ void test_stream_errors(void)
 			continue;
 		}
 #endif
+#if !defined(__x86_64__) && !defined(__aarch64__)
+		ASSERT_TRUE(1, "Arena fault reporting unsupported, skip.");
+		if (i == 2 || i == 3) {
+			ret = bpf_prog_stream_read(prog_fd, 2, buf, sizeof(buf), &ropts);
+			ASSERT_EQ(ret, 0, "stream read");
+			continue;
+		}
+#endif
 
 		ret = bpf_prog_stream_read(prog_fd, BPF_STREAM_STDERR, buf, sizeof(buf), &ropts);
 		ASSERT_GT(ret, 0, "stream read");
diff --git a/tools/testing/selftests/bpf/progs/stream.c b/tools/testing/selftests/bpf/progs/stream.c
index 35790897dc879..58ebff60cd96a 100644
--- a/tools/testing/selftests/bpf/progs/stream.c
+++ b/tools/testing/selftests/bpf/progs/stream.c
@@ -1,10 +1,15 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
+#define BPF_NO_KFUNC_PROTOTYPES
 #include <vmlinux.h>
 #include <bpf/bpf_tracing.h>
 #include <bpf/bpf_helpers.h>
 #include "bpf_misc.h"
 #include "bpf_experimental.h"
+#include "bpf_arena_common.h"
+
+extern int bpf_res_spin_lock(struct bpf_res_spin_lock *lock) __weak __ksym;
+extern void bpf_res_spin_unlock(struct bpf_res_spin_lock *lock) __weak __ksym;
 
 struct arr_elem {
 	struct bpf_res_spin_lock lock;
@@ -17,6 +22,12 @@ struct {
 	__type(value, struct arr_elem);
 } arrmap SEC(".maps");
 
+struct {
+	__uint(type, BPF_MAP_TYPE_ARENA);
+	__uint(map_flags, BPF_F_MMAPABLE);
+	__uint(max_entries, 1); /* number of pages */
+} arena SEC(".maps");
+
 #define ENOSPC 28
 #define _STR "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
 
@@ -76,4 +87,30 @@ int stream_syscall(void *ctx)
 	return 0;
 }
 
+SEC("syscall")
+__success __retval(0)
+int stream_arena_write_fault(void *ctx)
+{
+	unsigned char __arena *page;
+
+	page = bpf_arena_alloc_pages(&arena, NULL, 1, NUMA_NO_NODE, 0);
+	bpf_arena_free_pages(&arena, page, 1);
+
+	*(page + 0xbeef) = 1;
+
+	return 0;
+}
+
+SEC("syscall")
+__success __retval(0)
+int stream_arena_read_fault(void *ctx)
+{
+	unsigned char __arena *page;
+
+	page = bpf_arena_alloc_pages(&arena, NULL, 1, NUMA_NO_NODE, 0);
+	bpf_arena_free_pages(&arena, page, 1);
+
+	return *(page + 0xbeef);
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.47.3

Re: [PATCH bpf-next 1/3] bpf: arm64: simplify exception table handling

[Yonghong Song]

On 8/6/25 1:58 AM, Puranjay Mohan wrote:
> BPF loads with BPF_PROBE_MEM(SX) can load from unsafe pointers and the
> JIT adds an exception table entry for the JITed instruction which allows
> the exeption handler to set the destination register of the load to zero
> and continue execution from the next instruction.
>
> As all arm64 instructions are AARCH64_INSN_SIZE size, the exception
> handler can just increment the pc by AARCH64_INSN_SIZE without needing
> the exact address of the instruction following the the faulting
> instruction.
>
> Simplify the exception table usage in arm64 JIT by only saving the
> destination register in ex->fixup and drop everything related to
> the fixup_offset. The fault handler is modified to add AARCH64_INSN_SIZE
> to the pc.
>
> Signed-off-by: Puranjay Mohan <puranjay@kernel.org>

Acked-by: Yonghong Song <yonghong.song@linux.dev>

Re: [PATCH bpf-next 2/3] bpf: Report arena faults to BPF stderr

[Yonghong Song]

On 8/6/25 1:58 AM, Puranjay Mohan wrote:
> Begin reporting arena page faults and the faulting address to BPF
> program's stderr, this patch adds support in the arm64 and x86-64 JITs,
> support for other archs can be added later.
>
> The fault handlers receive the 32 bit address in the arena region so
> the upper 32 bits of user_vm_start is added to it before printing the
> address. This is what the user would expect to see as this is what is
> printed by bpf_printk() is you pass it an address returned by
> bpf_arena_alloc_pages();
>
> Signed-off-by: Puranjay Mohan <puranjay@kernel.org>

LGTM with some nits below.

Acked-by: Yonghong Song <yonghong.song@linux.dev>

> ---
>   arch/arm64/net/bpf_jit_comp.c | 31 ++++++++++++++
>   arch/x86/net/bpf_jit_comp.c   | 80 +++++++++++++++++++++++++++++++++--
>   include/linux/bpf.h           |  1 +
>   kernel/bpf/arena.c            | 20 +++++++++
>   4 files changed, 128 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
> index 42643fd9168fc..5680c7cd8932f 100644
> --- a/arch/arm64/net/bpf_jit_comp.c
> +++ b/arch/arm64/net/bpf_jit_comp.c
> @@ -1066,6 +1066,9 @@ static void build_epilogue(struct jit_ctx *ctx, bool was_classic)
>   	emit(A64_RET(A64_LR), ctx);
>   }
>   
> +#define BPF_FIXUP_LOAD_OFFSET_MASK GENMASK(15, 0)
> +#define BPF_FIXUP_ARENA_REG_MASK   GENMASK(20, 16)
> +#define BPF_ARENA_ACCESS	   BIT(21)
>   #define BPF_FIXUP_REG_MASK	GENMASK(31, 27)
>   #define DONT_CLEAR 5 /* Unused ARM64 register from BPF's POV */
>   
> @@ -1073,11 +1076,22 @@ bool ex_handler_bpf(const struct exception_table_entry *ex,
>   		    struct pt_regs *regs)
>   {
>   	int dst_reg = FIELD_GET(BPF_FIXUP_REG_MASK, ex->fixup);
> +	s16 off = FIELD_GET(BPF_FIXUP_LOAD_OFFSET_MASK, ex->fixup);
> +	int arena_reg = FIELD_GET(BPF_FIXUP_ARENA_REG_MASK, ex->fixup);
> +	bool is_arena = !!(ex->fixup & BPF_ARENA_ACCESS);
> +	bool is_write = (dst_reg == DONT_CLEAR);
> +	unsigned long addr;
>   
>   	if (dst_reg != DONT_CLEAR)
>   		regs->regs[dst_reg] = 0;
>   	/* Skip the faulting instruction */
>   	regs->pc += AARCH64_INSN_SIZE;
> +
> +	if (is_arena) {
> +		addr = regs->regs[arena_reg] + off;
> +		bpf_prog_report_arena_violation(is_write, addr);
> +	}
> +
>   	return true;
>   }
>   
> @@ -1087,6 +1101,9 @@ static int add_exception_handler(const struct bpf_insn *insn,
>   				 int dst_reg)
>   {
>   	off_t ins_offset;
> +	s16 load_off = insn->off;

Change 'load_off' to 'off' so it matches the usage in ex_handler_bpf().
Also 'off' could mean the off for a store insn, right?

> +	bool is_arena;
> +	int arena_reg;
>   	unsigned long pc;
>   	struct exception_table_entry *ex;
>   
> @@ -1100,6 +1117,9 @@ static int add_exception_handler(const struct bpf_insn *insn,
>   				BPF_MODE(insn->code) != BPF_PROBE_ATOMIC)
>   		return 0;
>   
> +	is_arena = (BPF_MODE(insn->code) == BPF_PROBE_MEM32) ||
> +		   (BPF_MODE(insn->code) == BPF_PROBE_ATOMIC);
> +
>   	if (!ctx->prog->aux->extable ||
>   	    WARN_ON_ONCE(ctx->exentry_idx >= ctx->prog->aux->num_exentries))
>   		return -EINVAL;
> @@ -1131,6 +1151,17 @@ static int add_exception_handler(const struct bpf_insn *insn,
>   
>   	ex->fixup = FIELD_PREP(BPF_FIXUP_REG_MASK, dst_reg);
>   
> +	if (is_arena) {
> +		ex->fixup |= BPF_ARENA_ACCESS;
> +		if (BPF_CLASS(insn->code) == BPF_LDX)
> +			arena_reg = bpf2a64[insn->src_reg];
> +		else
> +			arena_reg = bpf2a64[insn->dst_reg];
> +
> +		ex->fixup |=  FIELD_PREP(BPF_FIXUP_LOAD_OFFSET_MASK, load_off) |
> +			      FIELD_PREP(BPF_FIXUP_ARENA_REG_MASK, arena_reg);
> +	}
> +
>   	ex->type = EX_TYPE_BPF;
>   
>   	ctx->exentry_idx++;
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 7e3fca1646203..c8d99375e6de7 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -8,6 +8,7 @@
>   #include <linux/netdevice.h>
>   #include <linux/filter.h>
>   #include <linux/if_vlan.h>
> +#include <linux/bitfield.h>
>   #include <linux/bpf.h>
>   #include <linux/memory.h>
>   #include <linux/sort.h>
> @@ -1388,16 +1389,67 @@ static int emit_atomic_ld_st_index(u8 **pprog, u32 atomic_op, u32 size,
>   	return 0;
>   }
>   
> +/*
> + * Metadata encoding for exception handling in JITed code.
> + *
> + * Format of `fixup` and `data` fields in `struct exception_table_entry`:
> + *
> + * Bit layout of `fixup` (32-bit):
> + *
> + * +-----------+--------+-----------+---------+----------+
> + * | 31        | 30-24  |   23-16   |   15-8  |    7-0   |
> + * |           |        |           |         |          |
> + * | ARENA_ACC | Unused | ARENA_REG | DST_REG | INSN_LEN |
> + * +-----------+--------+-----------+---------+----------+
> + *
> + * - INSN_LEN (8 bits): Length of faulting insn (max x86 insn = 15 bytes (fits in 8 bits)).
> + * - DST_REG  (8 bits): Offset of dst_reg from reg2pt_regs[] (max offset = 112 (fits in 8 bits)).
> + *                      This is set to DONT_CLEAR if the insn is a store.
> + * - ARENA_REG (8 bits): Offset of the register that is used to calculate the
> + *                       address for load/store when accessing the arena region.
> + * - ARENA_ACCESS (1 bit): This bit is set when the faulting instruction accessed the arena region.
> + *
> + * Bit layout of `data` (32-bit):
> + *
> + * +--------------+--------+--------------+
> + * |	31-16	  |  15-8  |     7-0      |
> + * |              |	   |              |
> + * | ARENA_OFFSET | Unused |  EX_TYPE_BPF |
> + * +--------------+--------+--------------+
> + *
> + * - ARENA_OFFSET (16 bits): Offset used to calculate the address for load/store when
> + *                           accessing the arena region.
> + */
> +
>   #define DONT_CLEAR 1
> +#define FIXUP_OFFSET_MASK	GENMASK(7, 0)

Maybe FIXUP_INSN_LEN_MASK?

> +#define FIXUP_REG_MASK		GENMASK(15, 8)
> +#define FIXUP_ARENA_REG_MASK	GENMASK(23, 16)
> +#define FIXUP_ARENA_ACCESS	BIT(31)
> +#define DATA_ARENA_OFFSET_MASK	GENMASK(31, 16)
>   
>   bool ex_handler_bpf(const struct exception_table_entry *x, struct pt_regs *regs)
>   {
> -	u32 reg = x->fixup >> 8;
> +	u32 reg = FIELD_GET(FIXUP_REG_MASK, x->fixup);
> +	off_t offset = FIELD_GET(FIXUP_OFFSET_MASK, x->fixup);

insn_len = ...

> +	bool is_arena = !!(x->fixup & FIXUP_ARENA_ACCESS);
> +	bool is_write = (reg == DONT_CLEAR);
> +	unsigned long addr;
> +	s16 arena_offset;

This should be just 'off', right? It would be good if the terminology is consistent
between different architectures.

> +	u32 arena_reg;
>   
>   	/* jump over faulting load and clear dest register */
>   	if (reg != DONT_CLEAR)
>   		*(unsigned long *)((void *)regs + reg) = 0;
> -	regs->ip += x->fixup & 0xff;
> +	regs->ip += offset;
> +
> +	if (is_arena) {
> +		arena_reg = FIELD_GET(FIXUP_ARENA_REG_MASK, x->data);
> +		arena_offset = FIELD_GET(DATA_ARENA_OFFSET_MASK, x->data);
> +		addr = *(unsigned long *)((void *)regs + arena_reg) + arena_offset;
> +		bpf_prog_report_arena_violation(is_write, addr);
> +	}
> +
>   	return true;
>   }
>   
> @@ -2070,6 +2122,9 @@ st:			if (is_imm8(insn->off))
>   			{
>   				struct exception_table_entry *ex;
>   				u8 *_insn = image + proglen + (start_of_ldx - temp);
> +				bool is_arena;
> +				u32 fixup_reg;
> +				u32 arena_reg;

the above two variables can be in the same line and can before 'is_arena'.

>   				s64 delta;
>   
>   				if (!bpf_prog->aux->extable)
> @@ -2089,8 +2144,25 @@ st:			if (is_imm8(insn->off))
>   
>   				ex->data = EX_TYPE_BPF;
>   
> -				ex->fixup = (prog - start_of_ldx) |
> -					((BPF_CLASS(insn->code) == BPF_LDX ? reg2pt_regs[dst_reg] : DONT_CLEAR) << 8);
> +				is_arena = (BPF_MODE(insn->code) == BPF_PROBE_MEM32) ||
> +					   (BPF_MODE(insn->code) == BPF_PROBE_ATOMIC);
> +
> +				fixup_reg = (BPF_CLASS(insn->code) == BPF_LDX) ?
> +					    reg2pt_regs[dst_reg] : DONT_CLEAR;
> +
> +				ex->fixup = FIELD_PREP(FIXUP_OFFSET_MASK, prog - start_of_ldx) |
> +					    FIELD_PREP(FIXUP_REG_MASK, fixup_reg);
> +
> +				if (is_arena) {
> +					ex->fixup |= FIXUP_ARENA_ACCESS;
> +					if (BPF_CLASS(insn->code) == BPF_LDX)
> +						arena_reg = reg2pt_regs[src_reg];
> +					else
> +						arena_reg = reg2pt_regs[dst_reg];
> +
> +					ex->fixup |= FIELD_PREP(FIXUP_ARENA_REG_MASK, arena_reg);
> +					ex->data |= FIELD_PREP(DATA_ARENA_OFFSET_MASK, insn->off);
> +				}
>   			}
>   			break;
>   
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index cc700925b802f..3e62834726a97 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -3653,6 +3653,7 @@ int bpf_stream_stage_printk(struct bpf_stream_stage *ss, const char *fmt, ...);
>   int bpf_stream_stage_commit(struct bpf_stream_stage *ss, struct bpf_prog *prog,
>   			    enum bpf_stream_id stream_id);
>   int bpf_stream_stage_dump_stack(struct bpf_stream_stage *ss);
> +void bpf_prog_report_arena_violation(bool write, unsigned long addr);
>   
>   #define bpf_stream_printk(ss, ...) bpf_stream_stage_printk(&ss, __VA_ARGS__)
>   #define bpf_stream_dump_stack(ss) bpf_stream_stage_dump_stack(&ss)
> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
> index 5b37753799d20..a1653d1c04ca5 100644
> --- a/kernel/bpf/arena.c
> +++ b/kernel/bpf/arena.c
> @@ -633,3 +633,23 @@ static int __init kfunc_init(void)
>   	return register_btf_kfunc_id_set(BPF_PROG_TYPE_UNSPEC, &common_kfunc_set);
>   }
>   late_initcall(kfunc_init);
> +
> +void bpf_prog_report_arena_violation(bool write, unsigned long addr)
> +{
> +	struct bpf_stream_stage ss;
> +	struct bpf_prog *prog;
> +	u64 user_vm_start;
> +
> +	prog = bpf_prog_find_from_stack();
> +	if (!prog)
> +		return;
> +
> +	user_vm_start = bpf_arena_get_user_vm_start(prog->aux->arena);
> +	addr += (user_vm_start >> 32) << 32;
> +
> +	bpf_stream_stage(ss, prog, BPF_STDERR, ({
> +		bpf_stream_printk(ss, "ERROR: Arena %s access at unmapped address 0x%lx\n",
> +				  write ? "WRITE" : "READ", addr);
> +		bpf_stream_dump_stack(ss);
> +	}));
> +}

Re: [PATCH bpf-next 3/3] selftests/bpf: Add tests for arena fault reporting

[Yonghong Song]

On 8/6/25 1:58 AM, Puranjay Mohan wrote:
> Add selftests for testing the reporting of arena page faults through BPF
> streams. Two new bpf programs are added that read and write to an
> unmapped arena address and the fault reporting is verified in the
> userspace through streams.
>
> Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
> ---
>   .../testing/selftests/bpf/prog_tests/stream.c | 24 ++++++++++++
>   tools/testing/selftests/bpf/progs/stream.c    | 37 +++++++++++++++++++
>   2 files changed, 61 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/stream.c b/tools/testing/selftests/bpf/prog_tests/stream.c
> index d9f0185dca61b..4bdde56de35b1 100644
> --- a/tools/testing/selftests/bpf/prog_tests/stream.c
> +++ b/tools/testing/selftests/bpf/prog_tests/stream.c
> @@ -41,6 +41,22 @@ struct {
>   		"([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
>   		"|[ \t]+[^\n]+\n)*",
>   	},
> +	{
> +		offsetof(struct stream, progs.stream_arena_read_fault),
> +		"ERROR: Arena READ access at unmapped address 0x.*\n"
> +		"CPU: [0-9]+ UID: 0 PID: [0-9]+ Comm: .*\n"
> +		"Call trace:\n"
> +		"([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
> +		"|[ \t]+[^\n]+\n)*",
> +	},
> +	{
> +		offsetof(struct stream, progs.stream_arena_write_fault),
> +		"ERROR: Arena WRITE access at unmapped address 0x.*\n"
> +		"CPU: [0-9]+ UID: 0 PID: [0-9]+ Comm: .*\n"
> +		"Call trace:\n"
> +		"([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
> +		"|[ \t]+[^\n]+\n)*",
> +	},
>   };
>   
>   static int match_regex(const char *pattern, const char *string)
> @@ -85,6 +101,14 @@ void test_stream_errors(void)
>   			continue;
>   		}
>   #endif
> +#if !defined(__x86_64__) && !defined(__aarch64__)
> +		ASSERT_TRUE(1, "Arena fault reporting unsupported, skip.");
> +		if (i == 2 || i == 3) {
> +			ret = bpf_prog_stream_read(prog_fd, 2, buf, sizeof(buf), &ropts);
> +			ASSERT_EQ(ret, 0, "stream read");
> +			continue;
> +		}
> +#endif
>   
>   		ret = bpf_prog_stream_read(prog_fd, BPF_STREAM_STDERR, buf, sizeof(buf), &ropts);
>   		ASSERT_GT(ret, 0, "stream read");
> diff --git a/tools/testing/selftests/bpf/progs/stream.c b/tools/testing/selftests/bpf/progs/stream.c
> index 35790897dc879..58ebff60cd96a 100644
> --- a/tools/testing/selftests/bpf/progs/stream.c
> +++ b/tools/testing/selftests/bpf/progs/stream.c
> @@ -1,10 +1,15 @@
>   // SPDX-License-Identifier: GPL-2.0
>   /* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
> +#define BPF_NO_KFUNC_PROTOTYPES

Do we have to defineBPF_NO_KFUNC_PROTOTYPES in the above? Without the above, we do not need 
below extern bpf_res_spin_lock and bpf_res_spin_unlock.

>   #include <vmlinux.h>
>   #include <bpf/bpf_tracing.h>
>   #include <bpf/bpf_helpers.h>
>   #include "bpf_misc.h"
>   #include "bpf_experimental.h"
> +#include "bpf_arena_common.h"
> +
> +extern int bpf_res_spin_lock(struct bpf_res_spin_lock *lock) __weak __ksym;
> +extern void bpf_res_spin_unlock(struct bpf_res_spin_lock *lock) __weak __ksym;
>   
>   struct arr_elem {
>   	struct bpf_res_spin_lock lock;
> @@ -17,6 +22,12 @@ struct {
>   	__type(value, struct arr_elem);
>   } arrmap SEC(".maps");
>   
> +struct {
> +	__uint(type, BPF_MAP_TYPE_ARENA);
> +	__uint(map_flags, BPF_F_MMAPABLE);
> +	__uint(max_entries, 1); /* number of pages */
> +} arena SEC(".maps");
> +
>   #define ENOSPC 28
>   #define _STR "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
>   
> @@ -76,4 +87,30 @@ int stream_syscall(void *ctx)
>   	return 0;
>   }
>   
> +SEC("syscall")
> +__success __retval(0)
> +int stream_arena_write_fault(void *ctx)
> +{
> +	unsigned char __arena *page;
> +
> +	page = bpf_arena_alloc_pages(&arena, NULL, 1, NUMA_NO_NODE, 0);
> +	bpf_arena_free_pages(&arena, page, 1);
> +
> +	*(page + 0xbeef) = 1;
> +
> +	return 0;
> +}
> +
> +SEC("syscall")
> +__success __retval(0)
> +int stream_arena_read_fault(void *ctx)
> +{
> +	unsigned char __arena *page;
> +
> +	page = bpf_arena_alloc_pages(&arena, NULL, 1, NUMA_NO_NODE, 0);
> +	bpf_arena_free_pages(&arena, page, 1);
> +
> +	return *(page + 0xbeef);
> +}
> +
>   char _license[] SEC("license") = "GPL";

Re: [PATCH bpf-next 2/3] bpf: Report arena faults to BPF stderr

[puranjay]

Yonghong Song <yonghong.song@linux.dev> writes:

> On 8/6/25 1:58 AM, Puranjay Mohan wrote:
>> Begin reporting arena page faults and the faulting address to BPF
>> program's stderr, this patch adds support in the arm64 and x86-64 JITs,
>> support for other archs can be added later.
>>
>> The fault handlers receive the 32 bit address in the arena region so
>> the upper 32 bits of user_vm_start is added to it before printing the
>> address. This is what the user would expect to see as this is what is
>> printed by bpf_printk() is you pass it an address returned by
>> bpf_arena_alloc_pages();
>>
>> Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
>
> LGTM with some nits below.
>
> Acked-by: Yonghong Song <yonghong.song@linux.dev>
>
>> ---
>>   arch/arm64/net/bpf_jit_comp.c | 31 ++++++++++++++
>>   arch/x86/net/bpf_jit_comp.c   | 80 +++++++++++++++++++++++++++++++++--
>>   include/linux/bpf.h           |  1 +
>>   kernel/bpf/arena.c            | 20 +++++++++
>>   4 files changed, 128 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
>> index 42643fd9168fc..5680c7cd8932f 100644
>> --- a/arch/arm64/net/bpf_jit_comp.c
>> +++ b/arch/arm64/net/bpf_jit_comp.c
>> @@ -1066,6 +1066,9 @@ static void build_epilogue(struct jit_ctx *ctx, bool was_classic)
>>   	emit(A64_RET(A64_LR), ctx);
>>   }
>>   
>> +#define BPF_FIXUP_LOAD_OFFSET_MASK GENMASK(15, 0)
>> +#define BPF_FIXUP_ARENA_REG_MASK   GENMASK(20, 16)
>> +#define BPF_ARENA_ACCESS	   BIT(21)
>>   #define BPF_FIXUP_REG_MASK	GENMASK(31, 27)
>>   #define DONT_CLEAR 5 /* Unused ARM64 register from BPF's POV */
>>   
>> @@ -1073,11 +1076,22 @@ bool ex_handler_bpf(const struct exception_table_entry *ex,
>>   		    struct pt_regs *regs)
>>   {
>>   	int dst_reg = FIELD_GET(BPF_FIXUP_REG_MASK, ex->fixup);
>> +	s16 off = FIELD_GET(BPF_FIXUP_LOAD_OFFSET_MASK, ex->fixup);
>> +	int arena_reg = FIELD_GET(BPF_FIXUP_ARENA_REG_MASK, ex->fixup);
>> +	bool is_arena = !!(ex->fixup & BPF_ARENA_ACCESS);
>> +	bool is_write = (dst_reg == DONT_CLEAR);
>> +	unsigned long addr;
>>   
>>   	if (dst_reg != DONT_CLEAR)
>>   		regs->regs[dst_reg] = 0;
>>   	/* Skip the faulting instruction */
>>   	regs->pc += AARCH64_INSN_SIZE;
>> +
>> +	if (is_arena) {
>> +		addr = regs->regs[arena_reg] + off;
>> +		bpf_prog_report_arena_violation(is_write, addr);
>> +	}
>> +
>>   	return true;
>>   }
>>   
>> @@ -1087,6 +1101,9 @@ static int add_exception_handler(const struct bpf_insn *insn,
>>   				 int dst_reg)
>>   {
>>   	off_t ins_offset;
>> +	s16 load_off = insn->off;
>
> Change 'load_off' to 'off' so it matches the usage in ex_handler_bpf().
> Also 'off' could mean the off for a store insn, right?

Yes, this is for both load and store, I will fix it in next version.

>> +	bool is_arena;
>> +	int arena_reg;
>>   	unsigned long pc;
>>   	struct exception_table_entry *ex;
>>   
>> @@ -1100,6 +1117,9 @@ static int add_exception_handler(const struct bpf_insn *insn,
>>   				BPF_MODE(insn->code) != BPF_PROBE_ATOMIC)
>>   		return 0;
>>   
>> +	is_arena = (BPF_MODE(insn->code) == BPF_PROBE_MEM32) ||
>> +		   (BPF_MODE(insn->code) == BPF_PROBE_ATOMIC);
>> +
>>   	if (!ctx->prog->aux->extable ||
>>   	    WARN_ON_ONCE(ctx->exentry_idx >= ctx->prog->aux->num_exentries))
>>   		return -EINVAL;
>> @@ -1131,6 +1151,17 @@ static int add_exception_handler(const struct bpf_insn *insn,
>>   
>>   	ex->fixup = FIELD_PREP(BPF_FIXUP_REG_MASK, dst_reg);
>>   
>> +	if (is_arena) {
>> +		ex->fixup |= BPF_ARENA_ACCESS;
>> +		if (BPF_CLASS(insn->code) == BPF_LDX)
>> +			arena_reg = bpf2a64[insn->src_reg];
>> +		else
>> +			arena_reg = bpf2a64[insn->dst_reg];
>> +
>> +		ex->fixup |=  FIELD_PREP(BPF_FIXUP_LOAD_OFFSET_MASK, load_off) |
>> +			      FIELD_PREP(BPF_FIXUP_ARENA_REG_MASK, arena_reg);
>> +	}
>> +
>>   	ex->type = EX_TYPE_BPF;
>>   
>>   	ctx->exentry_idx++;
>> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
>> index 7e3fca1646203..c8d99375e6de7 100644
>> --- a/arch/x86/net/bpf_jit_comp.c
>> +++ b/arch/x86/net/bpf_jit_comp.c
>> @@ -8,6 +8,7 @@
>>   #include <linux/netdevice.h>
>>   #include <linux/filter.h>
>>   #include <linux/if_vlan.h>
>> +#include <linux/bitfield.h>
>>   #include <linux/bpf.h>
>>   #include <linux/memory.h>
>>   #include <linux/sort.h>
>> @@ -1388,16 +1389,67 @@ static int emit_atomic_ld_st_index(u8 **pprog, u32 atomic_op, u32 size,
>>   	return 0;
>>   }
>>   
>> +/*
>> + * Metadata encoding for exception handling in JITed code.
>> + *
>> + * Format of `fixup` and `data` fields in `struct exception_table_entry`:
>> + *
>> + * Bit layout of `fixup` (32-bit):
>> + *
>> + * +-----------+--------+-----------+---------+----------+
>> + * | 31        | 30-24  |   23-16   |   15-8  |    7-0   |
>> + * |           |        |           |         |          |
>> + * | ARENA_ACC | Unused | ARENA_REG | DST_REG | INSN_LEN |
>> + * +-----------+--------+-----------+---------+----------+
>> + *
>> + * - INSN_LEN (8 bits): Length of faulting insn (max x86 insn = 15 bytes (fits in 8 bits)).
>> + * - DST_REG  (8 bits): Offset of dst_reg from reg2pt_regs[] (max offset = 112 (fits in 8 bits)).
>> + *                      This is set to DONT_CLEAR if the insn is a store.
>> + * - ARENA_REG (8 bits): Offset of the register that is used to calculate the
>> + *                       address for load/store when accessing the arena region.
>> + * - ARENA_ACCESS (1 bit): This bit is set when the faulting instruction accessed the arena region.
>> + *
>> + * Bit layout of `data` (32-bit):
>> + *
>> + * +--------------+--------+--------------+
>> + * |	31-16	  |  15-8  |     7-0      |
>> + * |              |	   |              |
>> + * | ARENA_OFFSET | Unused |  EX_TYPE_BPF |
>> + * +--------------+--------+--------------+
>> + *
>> + * - ARENA_OFFSET (16 bits): Offset used to calculate the address for load/store when
>> + *                           accessing the arena region.
>> + */
>> +
>>   #define DONT_CLEAR 1
>> +#define FIXUP_OFFSET_MASK	GENMASK(7, 0)
>
> Maybe FIXUP_INSN_LEN_MASK?

Will change in next version.

>> +#define FIXUP_REG_MASK		GENMASK(15, 8)
>> +#define FIXUP_ARENA_REG_MASK	GENMASK(23, 16)
>> +#define FIXUP_ARENA_ACCESS	BIT(31)
>> +#define DATA_ARENA_OFFSET_MASK	GENMASK(31, 16)
>>   
>>   bool ex_handler_bpf(const struct exception_table_entry *x, struct pt_regs *regs)
>>   {
>> -	u32 reg = x->fixup >> 8;
>> +	u32 reg = FIELD_GET(FIXUP_REG_MASK, x->fixup);
>> +	off_t offset = FIELD_GET(FIXUP_OFFSET_MASK, x->fixup);
>
> insn_len = ...
>
>> +	bool is_arena = !!(x->fixup & FIXUP_ARENA_ACCESS);
>> +	bool is_write = (reg == DONT_CLEAR);
>> +	unsigned long addr;
>> +	s16 arena_offset;
>
> This should be just 'off', right? It would be good if the terminology is consistent
> between different architectures.

Yes,

>
>> +	u32 arena_reg;
>>   
>>   	/* jump over faulting load and clear dest register */
>>   	if (reg != DONT_CLEAR)
>>   		*(unsigned long *)((void *)regs + reg) = 0;
>> -	regs->ip += x->fixup & 0xff;
>> +	regs->ip += offset;
>> +
>> +	if (is_arena) {
>> +		arena_reg = FIELD_GET(FIXUP_ARENA_REG_MASK, x->data);
>> +		arena_offset = FIELD_GET(DATA_ARENA_OFFSET_MASK, x->data);
>> +		addr = *(unsigned long *)((void *)regs + arena_reg) + arena_offset;
>> +		bpf_prog_report_arena_violation(is_write, addr);
>> +	}
>> +
>>   	return true;
>>   }
>>   
>> @@ -2070,6 +2122,9 @@ st:			if (is_imm8(insn->off))
>>   			{
>>   				struct exception_table_entry *ex;
>>   				u8 *_insn = image + proglen + (start_of_ldx - temp);
>> +				bool is_arena;
>> +				u32 fixup_reg;
>> +				u32 arena_reg;
>
> the above two variables can be in the same line and can before 'is_arena'.
>

Will change in next version.

Thanks,
Puranjay

Re: [PATCH bpf-next 3/3] selftests/bpf: Add tests for arena fault reporting

[puranjay]

Yonghong Song <yonghong.song@linux.dev> writes:

> On 8/6/25 1:58 AM, Puranjay Mohan wrote:
>> Add selftests for testing the reporting of arena page faults through BPF
>> streams. Two new bpf programs are added that read and write to an
>> unmapped arena address and the fault reporting is verified in the
>> userspace through streams.
>>
>> Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
>> ---
>>   .../testing/selftests/bpf/prog_tests/stream.c | 24 ++++++++++++
>>   tools/testing/selftests/bpf/progs/stream.c    | 37 +++++++++++++++++++
>>   2 files changed, 61 insertions(+)
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/stream.c b/tools/testing/selftests/bpf/prog_tests/stream.c
>> index d9f0185dca61b..4bdde56de35b1 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/stream.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/stream.c
>> @@ -41,6 +41,22 @@ struct {
>>   		"([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
>>   		"|[ \t]+[^\n]+\n)*",
>>   	},
>> +	{
>> +		offsetof(struct stream, progs.stream_arena_read_fault),
>> +		"ERROR: Arena READ access at unmapped address 0x.*\n"
>> +		"CPU: [0-9]+ UID: 0 PID: [0-9]+ Comm: .*\n"
>> +		"Call trace:\n"
>> +		"([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
>> +		"|[ \t]+[^\n]+\n)*",
>> +	},
>> +	{
>> +		offsetof(struct stream, progs.stream_arena_write_fault),
>> +		"ERROR: Arena WRITE access at unmapped address 0x.*\n"
>> +		"CPU: [0-9]+ UID: 0 PID: [0-9]+ Comm: .*\n"
>> +		"Call trace:\n"
>> +		"([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
>> +		"|[ \t]+[^\n]+\n)*",
>> +	},
>>   };
>>   
>>   static int match_regex(const char *pattern, const char *string)
>> @@ -85,6 +101,14 @@ void test_stream_errors(void)
>>   			continue;
>>   		}
>>   #endif
>> +#if !defined(__x86_64__) && !defined(__aarch64__)
>> +		ASSERT_TRUE(1, "Arena fault reporting unsupported, skip.");
>> +		if (i == 2 || i == 3) {
>> +			ret = bpf_prog_stream_read(prog_fd, 2, buf, sizeof(buf), &ropts);
>> +			ASSERT_EQ(ret, 0, "stream read");
>> +			continue;
>> +		}
>> +#endif
>>   
>>   		ret = bpf_prog_stream_read(prog_fd, BPF_STREAM_STDERR, buf, sizeof(buf), &ropts);
>>   		ASSERT_GT(ret, 0, "stream read");
>> diff --git a/tools/testing/selftests/bpf/progs/stream.c b/tools/testing/selftests/bpf/progs/stream.c
>> index 35790897dc879..58ebff60cd96a 100644
>> --- a/tools/testing/selftests/bpf/progs/stream.c
>> +++ b/tools/testing/selftests/bpf/progs/stream.c
>> @@ -1,10 +1,15 @@
>>   // SPDX-License-Identifier: GPL-2.0
>>   /* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
>> +#define BPF_NO_KFUNC_PROTOTYPES
>
> Do we have to defineBPF_NO_KFUNC_PROTOTYPES in the above? Without the above, we do not need 
> below extern bpf_res_spin_lock and bpf_res_spin_unlock.
>

If we don't define BPF_NO_KFUNC_PROTOTYPES then there are build failures
for bpf_arena_alloc/free_pages() because the prototypes in vmlinux.h
lack __arena attribute.

>> +
>> +	page = bpf_arena_alloc_pages(&arena, NULL, 1, NUMA_NO_NODE, 0);
>> +	bpf_arena_free_pages(&arena, page, 1);
>> +

Thanks,
Puranjay

Re: [PATCH bpf-next 3/3] selftests/bpf: Add tests for arena fault reporting

[Kumar Kartikeya Dwivedi]

On Thu, 7 Aug 2025 at 15:25, <puranjay@kernel.org> wrote:
>
> Yonghong Song <yonghong.song@linux.dev> writes:
>
> > On 8/6/25 1:58 AM, Puranjay Mohan wrote:
> >> Add selftests for testing the reporting of arena page faults through BPF
> >> streams. Two new bpf programs are added that read and write to an
> >> unmapped arena address and the fault reporting is verified in the
> >> userspace through streams.
> >>
> >> Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
> >> ---
> >>   .../testing/selftests/bpf/prog_tests/stream.c | 24 ++++++++++++
> >>   tools/testing/selftests/bpf/progs/stream.c    | 37 +++++++++++++++++++
> >>   2 files changed, 61 insertions(+)
> >>
> >> diff --git a/tools/testing/selftests/bpf/prog_tests/stream.c b/tools/testing/selftests/bpf/prog_tests/stream.c
> >> index d9f0185dca61b..4bdde56de35b1 100644
> >> --- a/tools/testing/selftests/bpf/prog_tests/stream.c
> >> +++ b/tools/testing/selftests/bpf/prog_tests/stream.c
> >> @@ -41,6 +41,22 @@ struct {
> >>              "([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
> >>              "|[ \t]+[^\n]+\n)*",
> >>      },
> >> +    {
> >> +            offsetof(struct stream, progs.stream_arena_read_fault),
> >> +            "ERROR: Arena READ access at unmapped address 0x.*\n"
> >> +            "CPU: [0-9]+ UID: 0 PID: [0-9]+ Comm: .*\n"
> >> +            "Call trace:\n"
> >> +            "([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
> >> +            "|[ \t]+[^\n]+\n)*",
> >> +    },
> >> +    {
> >> +            offsetof(struct stream, progs.stream_arena_write_fault),
> >> +            "ERROR: Arena WRITE access at unmapped address 0x.*\n"
> >> +            "CPU: [0-9]+ UID: 0 PID: [0-9]+ Comm: .*\n"
> >> +            "Call trace:\n"
> >> +            "([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
> >> +            "|[ \t]+[^\n]+\n)*",
> >> +    },
> >>   };
> >>
> >>   static int match_regex(const char *pattern, const char *string)
> >> @@ -85,6 +101,14 @@ void test_stream_errors(void)
> >>                      continue;
> >>              }
> >>   #endif
> >> +#if !defined(__x86_64__) && !defined(__aarch64__)
> >> +            ASSERT_TRUE(1, "Arena fault reporting unsupported, skip.");
> >> +            if (i == 2 || i == 3) {
> >> +                    ret = bpf_prog_stream_read(prog_fd, 2, buf, sizeof(buf), &ropts);
> >> +                    ASSERT_EQ(ret, 0, "stream read");
> >> +                    continue;
> >> +            }
> >> +#endif
> >>
> >>              ret = bpf_prog_stream_read(prog_fd, BPF_STREAM_STDERR, buf, sizeof(buf), &ropts);
> >>              ASSERT_GT(ret, 0, "stream read");
> >> diff --git a/tools/testing/selftests/bpf/progs/stream.c b/tools/testing/selftests/bpf/progs/stream.c
> >> index 35790897dc879..58ebff60cd96a 100644
> >> --- a/tools/testing/selftests/bpf/progs/stream.c
> >> +++ b/tools/testing/selftests/bpf/progs/stream.c
> >> @@ -1,10 +1,15 @@
> >>   // SPDX-License-Identifier: GPL-2.0
> >>   /* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
> >> +#define BPF_NO_KFUNC_PROTOTYPES
> >
> > Do we have to defineBPF_NO_KFUNC_PROTOTYPES in the above? Without the above, we do not need
> > below extern bpf_res_spin_lock and bpf_res_spin_unlock.
> >
>
> If we don't define BPF_NO_KFUNC_PROTOTYPES then there are build failures
> for bpf_arena_alloc/free_pages() because the prototypes in vmlinux.h
> lack __arena attribute.

I would address this by dropping the alloc/free.
Instead to work around "addr_space_cast insn in program without arena error",
insert a dummy store "ptr = &arena" in the program, where ptr is a
global void *.

>
> >> +
> >> +    page = bpf_arena_alloc_pages(&arena, NULL, 1, NUMA_NO_NODE, 0);
> >> +    bpf_arena_free_pages(&arena, page, 1);
> >> +
>
> Thanks,
> Puranjay

Re: [PATCH bpf-next 3/3] selftests/bpf: Add tests for arena fault reporting

[Puranjay Mohan]

Kumar Kartikeya Dwivedi <memxor@gmail.com> writes:

> On Thu, 7 Aug 2025 at 15:25, <puranjay@kernel.org> wrote:
>>
>> Yonghong Song <yonghong.song@linux.dev> writes:
>>
>> > On 8/6/25 1:58 AM, Puranjay Mohan wrote:
>> >> Add selftests for testing the reporting of arena page faults through BPF
>> >> streams. Two new bpf programs are added that read and write to an
>> >> unmapped arena address and the fault reporting is verified in the
>> >> userspace through streams.
>> >>
>> >> Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
>> >> ---
>> >>   .../testing/selftests/bpf/prog_tests/stream.c | 24 ++++++++++++
>> >>   tools/testing/selftests/bpf/progs/stream.c    | 37 +++++++++++++++++++
>> >>   2 files changed, 61 insertions(+)
>> >>
>> >> diff --git a/tools/testing/selftests/bpf/prog_tests/stream.c b/tools/testing/selftests/bpf/prog_tests/stream.c
>> >> index d9f0185dca61b..4bdde56de35b1 100644
>> >> --- a/tools/testing/selftests/bpf/prog_tests/stream.c
>> >> +++ b/tools/testing/selftests/bpf/prog_tests/stream.c
>> >> @@ -41,6 +41,22 @@ struct {
>> >>              "([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
>> >>              "|[ \t]+[^\n]+\n)*",
>> >>      },
>> >> +    {
>> >> +            offsetof(struct stream, progs.stream_arena_read_fault),
>> >> +            "ERROR: Arena READ access at unmapped address 0x.*\n"
>> >> +            "CPU: [0-9]+ UID: 0 PID: [0-9]+ Comm: .*\n"
>> >> +            "Call trace:\n"
>> >> +            "([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
>> >> +            "|[ \t]+[^\n]+\n)*",
>> >> +    },
>> >> +    {
>> >> +            offsetof(struct stream, progs.stream_arena_write_fault),
>> >> +            "ERROR: Arena WRITE access at unmapped address 0x.*\n"
>> >> +            "CPU: [0-9]+ UID: 0 PID: [0-9]+ Comm: .*\n"
>> >> +            "Call trace:\n"
>> >> +            "([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
>> >> +            "|[ \t]+[^\n]+\n)*",
>> >> +    },
>> >>   };
>> >>
>> >>   static int match_regex(const char *pattern, const char *string)
>> >> @@ -85,6 +101,14 @@ void test_stream_errors(void)
>> >>                      continue;
>> >>              }
>> >>   #endif
>> >> +#if !defined(__x86_64__) && !defined(__aarch64__)
>> >> +            ASSERT_TRUE(1, "Arena fault reporting unsupported, skip.");
>> >> +            if (i == 2 || i == 3) {
>> >> +                    ret = bpf_prog_stream_read(prog_fd, 2, buf, sizeof(buf), &ropts);
>> >> +                    ASSERT_EQ(ret, 0, "stream read");
>> >> +                    continue;
>> >> +            }
>> >> +#endif
>> >>
>> >>              ret = bpf_prog_stream_read(prog_fd, BPF_STREAM_STDERR, buf, sizeof(buf), &ropts);
>> >>              ASSERT_GT(ret, 0, "stream read");
>> >> diff --git a/tools/testing/selftests/bpf/progs/stream.c b/tools/testing/selftests/bpf/progs/stream.c
>> >> index 35790897dc879..58ebff60cd96a 100644
>> >> --- a/tools/testing/selftests/bpf/progs/stream.c
>> >> +++ b/tools/testing/selftests/bpf/progs/stream.c
>> >> @@ -1,10 +1,15 @@
>> >>   // SPDX-License-Identifier: GPL-2.0
>> >>   /* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
>> >> +#define BPF_NO_KFUNC_PROTOTYPES
>> >
>> > Do we have to defineBPF_NO_KFUNC_PROTOTYPES in the above? Without the above, we do not need
>> > below extern bpf_res_spin_lock and bpf_res_spin_unlock.
>> >
>>
>> If we don't define BPF_NO_KFUNC_PROTOTYPES then there are build failures
>> for bpf_arena_alloc/free_pages() because the prototypes in vmlinux.h
>> lack __arena attribute.
>
> I would address this by dropping the alloc/free.
> Instead to work around "addr_space_cast insn in program without arena error",
> insert a dummy store "ptr = &arena" in the program, where ptr is a
> global void *.
>

I want to use alloc/free and not use a dummy address because because
arena pointers are special as they are returned by alloc() with
arena->user_vm_start added to them, and the
bpf_prog_report_arena_violation() also adds back arena->user_vm_start to
the 32 bit address received by the fault handler. If I use a random
address in the bpf program, bpf_prog_report_arena_violation() will print
a bogus address.

So, I think we should keep using alloc/free for this test because we
want to test this arena->user_vm_start addition as well.

Thanks,
Puranjay

Re: [PATCH bpf-next 3/3] selftests/bpf: Add tests for arena fault reporting

[Eduard Zingerman]

On Thu, 2025-08-07 at 13:25 +0000, puranjay@kernel.org wrote:

[...]

> If we don't define BPF_NO_KFUNC_PROTOTYPES then there are build failures
> for bpf_arena_alloc/free_pages() because the prototypes in vmlinux.h
> lack __arena attribute.

What pahole version are you using?
For me these functions are declared as:

  extern void __attribute__((address_space(1))) *
    bpf_arena_alloc_pages(void *p__map, void __attribute__((address_space(1))) *addr__ign,
    			  u32 page_cnt, int node_id, u64 flags) __weak __ksym;

  extern void bpf_arena_free_pages(void *p__map,
  	      			   void __attribute__((address_space(1))) *ptr__ign,
				   u32 page_cnt) __weak __ksym;

The __attribute__((address_space(1))) was added relatively recently:
- dwarves commit 40e82f5be9a7 ("pahole: Introduce --btf_feature=attributes")
- kernel  commit 21cb33c7e065 ("kbuild, bpf: Enable --btf_features=attributes")

[...]

Re: [PATCH bpf-next 3/3] selftests/bpf: Add tests for arena fault reporting

[Kumar Kartikeya Dwivedi]

On Mon, 11 Aug 2025 at 12:35, Puranjay Mohan <puranjay@kernel.org> wrote:
>
> Kumar Kartikeya Dwivedi <memxor@gmail.com> writes:
>
> > On Thu, 7 Aug 2025 at 15:25, <puranjay@kernel.org> wrote:
> >>
> >> Yonghong Song <yonghong.song@linux.dev> writes:
> >>
> >> > On 8/6/25 1:58 AM, Puranjay Mohan wrote:
> >> >> Add selftests for testing the reporting of arena page faults through BPF
> >> >> streams. Two new bpf programs are added that read and write to an
> >> >> unmapped arena address and the fault reporting is verified in the
> >> >> userspace through streams.
> >> >>
> >> >> Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
> >> >> ---
> >> >>   .../testing/selftests/bpf/prog_tests/stream.c | 24 ++++++++++++
> >> >>   tools/testing/selftests/bpf/progs/stream.c    | 37 +++++++++++++++++++
> >> >>   2 files changed, 61 insertions(+)
> >> >>
> >> >> diff --git a/tools/testing/selftests/bpf/prog_tests/stream.c b/tools/testing/selftests/bpf/prog_tests/stream.c
> >> >> index d9f0185dca61b..4bdde56de35b1 100644
> >> >> --- a/tools/testing/selftests/bpf/prog_tests/stream.c
> >> >> +++ b/tools/testing/selftests/bpf/prog_tests/stream.c
> >> >> @@ -41,6 +41,22 @@ struct {
> >> >>              "([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
> >> >>              "|[ \t]+[^\n]+\n)*",
> >> >>      },
> >> >> +    {
> >> >> +            offsetof(struct stream, progs.stream_arena_read_fault),
> >> >> +            "ERROR: Arena READ access at unmapped address 0x.*\n"
> >> >> +            "CPU: [0-9]+ UID: 0 PID: [0-9]+ Comm: .*\n"
> >> >> +            "Call trace:\n"
> >> >> +            "([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
> >> >> +            "|[ \t]+[^\n]+\n)*",
> >> >> +    },
> >> >> +    {
> >> >> +            offsetof(struct stream, progs.stream_arena_write_fault),
> >> >> +            "ERROR: Arena WRITE access at unmapped address 0x.*\n"
> >> >> +            "CPU: [0-9]+ UID: 0 PID: [0-9]+ Comm: .*\n"
> >> >> +            "Call trace:\n"
> >> >> +            "([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
> >> >> +            "|[ \t]+[^\n]+\n)*",
> >> >> +    },
> >> >>   };
> >> >>
> >> >>   static int match_regex(const char *pattern, const char *string)
> >> >> @@ -85,6 +101,14 @@ void test_stream_errors(void)
> >> >>                      continue;
> >> >>              }
> >> >>   #endif
> >> >> +#if !defined(__x86_64__) && !defined(__aarch64__)
> >> >> +            ASSERT_TRUE(1, "Arena fault reporting unsupported, skip.");
> >> >> +            if (i == 2 || i == 3) {
> >> >> +                    ret = bpf_prog_stream_read(prog_fd, 2, buf, sizeof(buf), &ropts);
> >> >> +                    ASSERT_EQ(ret, 0, "stream read");
> >> >> +                    continue;
> >> >> +            }
> >> >> +#endif
> >> >>
> >> >>              ret = bpf_prog_stream_read(prog_fd, BPF_STREAM_STDERR, buf, sizeof(buf), &ropts);
> >> >>              ASSERT_GT(ret, 0, "stream read");
> >> >> diff --git a/tools/testing/selftests/bpf/progs/stream.c b/tools/testing/selftests/bpf/progs/stream.c
> >> >> index 35790897dc879..58ebff60cd96a 100644
> >> >> --- a/tools/testing/selftests/bpf/progs/stream.c
> >> >> +++ b/tools/testing/selftests/bpf/progs/stream.c
> >> >> @@ -1,10 +1,15 @@
> >> >>   // SPDX-License-Identifier: GPL-2.0
> >> >>   /* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
> >> >> +#define BPF_NO_KFUNC_PROTOTYPES
> >> >
> >> > Do we have to defineBPF_NO_KFUNC_PROTOTYPES in the above? Without the above, we do not need
> >> > below extern bpf_res_spin_lock and bpf_res_spin_unlock.
> >> >
> >>
> >> If we don't define BPF_NO_KFUNC_PROTOTYPES then there are build failures
> >> for bpf_arena_alloc/free_pages() because the prototypes in vmlinux.h
> >> lack __arena attribute.
> >
> > I would address this by dropping the alloc/free.
> > Instead to work around "addr_space_cast insn in program without arena error",
> > insert a dummy store "ptr = &arena" in the program, where ptr is a
> > global void *.
> >
>
> I want to use alloc/free and not use a dummy address because because
> arena pointers are special as they are returned by alloc() with
> arena->user_vm_start added to them, and the
> bpf_prog_report_arena_violation() also adds back arena->user_vm_start to
> the 32 bit address received by the fault handler. If I use a random
> address in the bpf program, bpf_prog_report_arena_violation() will print
> a bogus address.

That is easy to address, you can simply cast &arena to struct
bpf_arena * to get the user_vm_start.
Then just deref user_vm_start + 0xdeadbeef.
Then we also have a stable address we can match in the regex.
It will also fix your problem of needing the alloc/free pair in the
first place, i.e. the lack of an arena map reference in the program.
Then we can drop this BPF_NO_KFUNC_PROTOTYPES kludge.

As Eduard pointed out, newer pahole already emits address_space tags in
kfuncs, so the vmlinux.h suppression shouldn't be needed either way.



>
> So, I think we should keep using alloc/free for this test because we
> want to test this arena->user_vm_start addition as well.
>
> Thanks,
> Puranjay