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

[Pu Lehui <pulehui@huawei.com>]

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;
>   		}