[PATCH bpf-next] bpf, riscv: Register extable entries at probe insn call sites

[PATCH bpf-next] bpf, riscv: Register extable entries at probe insn call sites

[Chen Pei]

add_exception_handler() filters insns by matching BPF_MODE() against
the probe mode values. These values occupy unused opcode slots and
collide with legitimate encodings: BPF_PROBE_MEM32SX (0xc0) shares
its mode value with BPF_ATOMIC. Relying on the mode alone therefore
risks registering exception table entries for the wrong instructions,
as would happen for plain atomics without the LDX class check.

Make the decision explicit at the call sites instead: register an
entry only when emitting a probe load, a PROBE_MEM32 store, or a
PROBE_ATOMIC atomic, and drop the fragile mode gate from
add_exception_handler() along with the now unused insn argument. No
functional change intended.

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

 arch/riscv/net/bpf_jit_comp64.c | 50 ++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 22 deletions(-)

diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
index e7378be171a9..726c0169fc60 100644
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -731,9 +731,8 @@ bool ex_handler_bpf(const struct exception_table_entry *ex,
 	return true;
 }
 
-/* For accesses to BTF pointers, add an entry to the exception table */
-static int add_exception_handler(const struct bpf_insn *insn, int dst_reg,
-				 struct rv_jit_context *ctx)
+/* Add an entry to the exception table for a faulting probe insn */
+static int add_exception_handler(int dst_reg, struct rv_jit_context *ctx)
 {
 	struct exception_table_entry *ex;
 	unsigned long pc;
@@ -744,14 +743,6 @@ static int add_exception_handler(const struct bpf_insn *insn, int dst_reg,
 	    ctx->ex_insn_off <= 0 || ctx->ex_jmp_off <= 0)
 		return 0;
 
-	if (BPF_MODE(insn->code) != BPF_PROBE_MEM &&
-	    BPF_MODE(insn->code) != BPF_PROBE_MEMSX &&
-	    BPF_MODE(insn->code) != BPF_PROBE_MEM32 &&
-	    !(BPF_MODE(insn->code) == BPF_PROBE_MEM32SX &&
-	      BPF_CLASS(insn->code) == BPF_LDX) &&
-	    BPF_MODE(insn->code) != BPF_PROBE_ATOMIC)
-		return 0;
-
 	if (WARN_ON_ONCE(ctx->nexentries >= ctx->prog->aux->num_exentries))
 		return -EINVAL;
 
@@ -1360,6 +1351,15 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *ro_image,
 	return ret < 0 ? ret : size;
 }
 
+/* Probe loads fault on unmapped memory and need an exception table entry */
+static bool insn_is_probe_ldx(const struct bpf_insn *insn)
+{
+	return BPF_MODE(insn->code) == BPF_PROBE_MEM ||
+	       BPF_MODE(insn->code) == BPF_PROBE_MEMSX ||
+	       BPF_MODE(insn->code) == BPF_PROBE_MEM32 ||
+	       BPF_MODE(insn->code) == BPF_PROBE_MEM32SX;
+}
+
 int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
 		      bool extra_pass)
 {
@@ -1929,9 +1929,11 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
 
 		emit_ldx(rd, off, rs, BPF_SIZE(code), sign_ext, ctx);
 
-		ret = add_exception_handler(insn, rd, ctx);
-		if (ret)
-			return ret;
+		if (insn_is_probe_ldx(insn)) {
+			ret = add_exception_handler(rd, ctx);
+			if (ret)
+				return ret;
+		}
 
 		if (BPF_SIZE(code) != BPF_DW && insn_is_zext(&insn[1]))
 			return 1;
@@ -1959,9 +1961,11 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
 
 		emit_st(rd, off, imm, BPF_SIZE(code), ctx);
 
-		ret = add_exception_handler(insn, REG_DONT_CLEAR_MARKER, ctx);
-		if (ret)
-			return ret;
+		if (BPF_MODE(insn->code) == BPF_PROBE_MEM32) {
+			ret = add_exception_handler(REG_DONT_CLEAR_MARKER, ctx);
+			if (ret)
+				return ret;
+		}
 		break;
 
 	/* STX: *(size *)(dst + off) = src */
@@ -1981,9 +1985,11 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
 
 		emit_stx(rd, off, rs, BPF_SIZE(code), ctx);
 
-		ret = add_exception_handler(insn, REG_DONT_CLEAR_MARKER, ctx);
-		if (ret)
-			return ret;
+		if (BPF_MODE(insn->code) == BPF_PROBE_MEM32) {
+			ret = add_exception_handler(REG_DONT_CLEAR_MARKER, ctx);
+			if (ret)
+				return ret;
+		}
 		break;
 
 	/* Atomics */
@@ -2001,7 +2007,7 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
 			ret = emit_atomic_rmw(rd, rs, insn, ctx);
 
 		/* ret can be 1 (skip-zext); extable entry still needs to be added */
-		if (ret >= 0) {
+		if (ret >= 0 && BPF_MODE(insn->code) == BPF_PROBE_ATOMIC) {
 			/*
 			 * A load-acquire reads into dst_reg, and a read-modify-write
 			 * carrying BPF_FETCH reads the old value into src_reg, or into
@@ -2010,7 +2016,7 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
 			 */
 			int load_reg = bpf_atomic_load_reg(insn);
 
-			ret = add_exception_handler(insn, load_reg < 0 ?
+			ret = add_exception_handler(load_reg < 0 ?
 					REG_DONT_CLEAR_MARKER : regmap[load_reg],
 					ctx) ?: ret;
 		}
-- 
2.50.1

Re: [PATCH bpf-next] bpf, riscv: Register extable entries at probe insn call sites

[sashiko-bot]

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

Pre-existing issues:
- [High] BPF_PROBE_ATOMIC cmpxchg on RISC-V without ZACAS lacks exception table entries, leading to an unhandled page fault and kernel panic for unmapped arena accesses.
--

commit 77ab37f77b219155ec1dc7c551ef131f6c642039
Author: Chen Pei <cp0613@linux.alibaba.com>
bpf, riscv: Register extable entries at probe insn call sites

This commit removes the mode gate from add_exception_handler() and explicitly
registers exception handlers only at probe instruction call sites. It aims
to prevent collisions between probe mode values and legitimate instruction
encodings.

> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> index e7378be171a9f..726c0169fc60f 100644
> --- a/arch/riscv/net/bpf_jit_comp64.c
> +++ b/arch/riscv/net/bpf_jit_comp64.c
[ ... ]
> @@ -2001,7 +2007,7 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
>  			ret = emit_atomic_rmw(rd, rs, insn, ctx);
>  
>  		/* ret can be 1 (skip-zext); extable entry still needs to be added */
> -		if (ret >= 0) {
> +		if (ret >= 0 && BPF_MODE(insn->code) == BPF_PROBE_ATOMIC) {
>  			/*
>  			 * A load-acquire reads into dst_reg, and a read-modify-write
>  			 * carrying BPF_FETCH reads the old value into src_reg, or into
> @@ -2010,7 +2016,7 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
>  			 */
>  			int load_reg = bpf_atomic_load_reg(insn);
>  
> -			ret = add_exception_handler(insn, load_reg < 0 ?
> +			ret = add_exception_handler(load_reg < 0 ?
>  					REG_DONT_CLEAR_MARKER : regmap[load_reg],
>  					ctx) ?: ret;
>  		}

[Severity: High]
This isn't a bug introduced by this patch, but does emit_cmpxchg() correctly
establish an exception handler boundary when ZACAS is not enabled?

If a BPF program uses BPF_PROBE_ATOMIC with BPF_CMPXCHG on an unmapped memory
address on a RISC-V CPU without the ZACAS extension, emit_cmpxchg() falls
back to a load-reserved/store-conditional loop:

arch/riscv/net/bpf_jit.h:emit_cmpxchg() {
    ...
	if (rv_ext_enabled(ZACAS)) {
		ctx->ex_insn_off = ctx->ninsns;
		emit(is64 ? rvzacas_amocas_d(r0, rs, rd, 1, 1) :
		     rvzacas_amocas_w(r0, rs, rd, 1, 1), ctx);
		ctx->ex_jmp_off = ctx->ninsns;
        ...
		return;
	}

	if (is64)
		emit_mv(RV_REG_T2, r0, ctx);
	else
		emit_addiw(RV_REG_T2, r0, 0, ctx);
	emit(is64 ? rv_lr_d(r0, 0, rd, 0, 0) :
	     rv_lr_w(r0, 0, rd, 0, 0), ctx);
    ...
}

In the non-ZACAS path, ctx->ex_insn_off and ctx->ex_jmp_off are not updated
before emitting the faulting load-reserved instruction.

When add_exception_handler() is subsequently called here in
bpf_jit_emit_insn(), it will use stale offsets. Can this cause an unhandled
page fault and a kernel panic when accessing unmapped arena memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901120013.16104-1-cp0613@linux.alibaba.com?part=1

Re: [PATCH bpf-next] bpf, riscv: Register extable entries at probe insn call sites

[bot+bpf-ci]

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

> bpf, riscv: Register extable entries at probe insn call sites
>
> add_exception_handler() filters insns by matching BPF_MODE() against
> the probe mode values. These values occupy unused opcode slots and
> collide with legitimate encodings: BPF_PROBE_MEM32SX (0xc0) shares
> its mode value with BPF_ATOMIC. Relying on the mode alone therefore
> risks registering exception table entries for the wrong instructions,
> as would happen for plain atomics without the LDX class check.
>
> Make the decision explicit at the call sites instead: register an
> entry only when emitting a probe load, a PROBE_MEM32 store, or a
> PROBE_ATOMIC atomic, and drop the fragile mode gate from
> add_exception_handler() along with the now unused insn argument. No
> functional change intended.
>
> Signed-off-by: Chen Pei <cp0613@linux.alibaba.com>

This looks like a bug fix for the fragile mode-based filtering in
add_exception_handler(). Should this include:

  Fixes: b4757714cc63 ("bpf, riscv: Add support for signed arena loads")


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

Re: [PATCH bpf-next] bpf, riscv: Register extable entries at probe insn call sites

[Chen Pei]

> This looks like a bug fix for the fragile mode-based filtering in
> add_exception_handler(). Should this include:
>
>   Fixes: b4757714cc63 ("bpf, riscv: Add support for signed arena loads")

No Fixes tag, because the mode/class collision has never been a live
bug in any released tree. b4757714cc63 introduced both the
BPF_PROBE_MEM32SX support and the LDX class guard in the mode gate, in
the same commit, precisely to keep plain atomics (which share the 0xc0
mode value) out of the exception table. Its commit message says so
explicitly:

  "Since BPF_PROBE_MEM32SX shares its mode value (0xc0) with
   BPF_ATOMIC, the gate accepts it only for LDX class instructions so
   that plain atomic instructions do not register exception table
   entries."

For context: I noticed this fragile shared-state gate while working on
v1 of "bpf, riscv: Add support for signed arena loads", where the class
guard was added as a preventive measure. This patch now follows up with
the actual cleanup, making the registration decision explicit at each
call site so future probe modes cannot silently collide with legitimate
encodings.

Also, regarding sashiko-bot's separate concern about BPF_PROBE_ATOMIC
cmpxchg without ZACAS: that path is unreachable, as arena BPF_CMPXCHG
is already gated on ZACAS by bpf_jit_supports_insn(), and
add_exception_handler() resets its offsets after every registration,
so stale offsets cannot be consumed. A separate fix I have prepared,
"bpf, riscv: Make arena support depend on ZACAS", further rejects
arena map creation on non-ZACAS systems altogether.

Best regards,
Pei

Re: [PATCH bpf-next] bpf, riscv: Register extable entries at probe insn call sites

[Pu Lehui]

On 2026/9/1 20:00, Chen Pei wrote:
> add_exception_handler() filters insns by matching BPF_MODE() against
> the probe mode values. These values occupy unused opcode slots and
> collide with legitimate encodings: BPF_PROBE_MEM32SX (0xc0) shares
> its mode value with BPF_ATOMIC. Relying on the mode alone therefore
> risks registering exception table entries for the wrong instructions,
> as would happen for plain atomics without the LDX class check.
> 
> Make the decision explicit at the call sites instead: register an
> entry only when emitting a probe load, a PROBE_MEM32 store, or a
> PROBE_ATOMIC atomic, and drop the fragile mode gate from
> add_exception_handler() along with the now unused insn argument. No
> functional change intended.
> 
> Signed-off-by: Chen Pei <cp0613@linux.alibaba.com>
> ---
> base-commit: d761934c9483ecde93fe99d8705282f716dfee50
> 
>   arch/riscv/net/bpf_jit_comp64.c | 50 ++++++++++++++++++---------------
>   1 file changed, 28 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> index e7378be171a9..726c0169fc60 100644
> --- a/arch/riscv/net/bpf_jit_comp64.c
> +++ b/arch/riscv/net/bpf_jit_comp64.c
> @@ -731,9 +731,8 @@ bool ex_handler_bpf(const struct exception_table_entry *ex,
>   	return true;
>   }
>   
> -/* For accesses to BTF pointers, add an entry to the exception table */
> -static int add_exception_handler(const struct bpf_insn *insn, int dst_reg,
> -				 struct rv_jit_context *ctx)
> +/* Add an entry to the exception table for a faulting probe insn */
> +static int add_exception_handler(int dst_reg, struct rv_jit_context *ctx)
>   {
>   	struct exception_table_entry *ex;
>   	unsigned long pc;
> @@ -744,14 +743,6 @@ static int add_exception_handler(const struct bpf_insn *insn, int dst_reg,
>   	    ctx->ex_insn_off <= 0 || ctx->ex_jmp_off <= 0)
>   		return 0;
>   
> -	if (BPF_MODE(insn->code) != BPF_PROBE_MEM &&
> -	    BPF_MODE(insn->code) != BPF_PROBE_MEMSX &&
> -	    BPF_MODE(insn->code) != BPF_PROBE_MEM32 &&
> -	    !(BPF_MODE(insn->code) == BPF_PROBE_MEM32SX &&
> -	      BPF_CLASS(insn->code) == BPF_LDX) &&
> -	    BPF_MODE(insn->code) != BPF_PROBE_ATOMIC)
> -		return 0;

Hi Pei,

This patch looks fine, but I still tend not to introduce these changes. 
I think the function itself should handle its own defensive checks, and 
we should also avoid "shotgun surgery".

> -
>   	if (WARN_ON_ONCE(ctx->nexentries >= ctx->prog->aux->num_exentries))
>   		return -EINVAL;
>   
> @@ -1360,6 +1351,15 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *ro_image,
>   	return ret < 0 ? ret : size;
>   }
>   
> +/* Probe loads fault on unmapped memory and need an exception table entry */
> +static bool insn_is_probe_ldx(const struct bpf_insn *insn)
> +{
> +	return BPF_MODE(insn->code) == BPF_PROBE_MEM ||
> +	       BPF_MODE(insn->code) == BPF_PROBE_MEMSX ||
> +	       BPF_MODE(insn->code) == BPF_PROBE_MEM32 ||
> +	       BPF_MODE(insn->code) == BPF_PROBE_MEM32SX;
> +}
> +
>   int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
>   		      bool extra_pass)
>   {
> @@ -1929,9 +1929,11 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
>   
>   		emit_ldx(rd, off, rs, BPF_SIZE(code), sign_ext, ctx);
>   
> -		ret = add_exception_handler(insn, rd, ctx);
> -		if (ret)
> -			return ret;
> +		if (insn_is_probe_ldx(insn)) {
> +			ret = add_exception_handler(rd, ctx);
> +			if (ret)
> +				return ret;
> +		}
>   
>   		if (BPF_SIZE(code) != BPF_DW && insn_is_zext(&insn[1]))
>   			return 1;
> @@ -1959,9 +1961,11 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
>   
>   		emit_st(rd, off, imm, BPF_SIZE(code), ctx);
>   
> -		ret = add_exception_handler(insn, REG_DONT_CLEAR_MARKER, ctx);
> -		if (ret)
> -			return ret;
> +		if (BPF_MODE(insn->code) == BPF_PROBE_MEM32) {
> +			ret = add_exception_handler(REG_DONT_CLEAR_MARKER, ctx);
> +			if (ret)
> +				return ret;
> +		}
>   		break;
>   
>   	/* STX: *(size *)(dst + off) = src */
> @@ -1981,9 +1985,11 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
>   
>   		emit_stx(rd, off, rs, BPF_SIZE(code), ctx);
>   
> -		ret = add_exception_handler(insn, REG_DONT_CLEAR_MARKER, ctx);
> -		if (ret)
> -			return ret;
> +		if (BPF_MODE(insn->code) == BPF_PROBE_MEM32) {
> +			ret = add_exception_handler(REG_DONT_CLEAR_MARKER, ctx);
> +			if (ret)
> +				return ret;
> +		}
>   		break;
>   
>   	/* Atomics */
> @@ -2001,7 +2007,7 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
>   			ret = emit_atomic_rmw(rd, rs, insn, ctx);
>   
>   		/* ret can be 1 (skip-zext); extable entry still needs to be added */
> -		if (ret >= 0) {
> +		if (ret >= 0 && BPF_MODE(insn->code) == BPF_PROBE_ATOMIC) {
>   			/*
>   			 * A load-acquire reads into dst_reg, and a read-modify-write
>   			 * carrying BPF_FETCH reads the old value into src_reg, or into
> @@ -2010,7 +2016,7 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
>   			 */
>   			int load_reg = bpf_atomic_load_reg(insn);
>   
> -			ret = add_exception_handler(insn, load_reg < 0 ?
> +			ret = add_exception_handler(load_reg < 0 ?
>   					REG_DONT_CLEAR_MARKER : regmap[load_reg],
>   					ctx) ?: ret;
>   		}

Re: [PATCH bpf-next] bpf, riscv: Register extable entries at probe insn call sites

[Chen Pei]

> This patch looks fine, but I still tend not to introduce these changes.
> I think the function itself should handle its own defensive checks, and
> we should also avoid "shotgun surgery".

Indeed, you have a point. Thanks for the review.

Best regards,
Pei