Re: [PATCH bpf-next v7 02/11] bpf: Disallow interpreter fallback for user BPF_ADDR_SPACE_CAST insn

[Leon Hwang <leon.hwang@linux.dev>]

On 22/6/26 23:07, sashiko-bot@kernel.org wrote:
[...]
>> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
>> index 649cce41e13f3..49398b5bd172d 100644
>> --- a/kernel/bpf/core.c
>> +++ b/kernel/bpf/core.c
>> @@ -2620,6 +2620,7 @@ struct bpf_prog *__bpf_prog_select_runtime(struct bpf_verifier_env *env, struct
>>  		goto finalize;
>>  
>>  	if (IS_ENABLED(CONFIG_BPF_JIT_ALWAYS_ON) ||
>> +	    fp->aux->has_addr_space_cast_insn ||
>>  	    bpf_prog_has_kfunc_call(fp))
>>  		jit_needed = true;
> 
> [Severity: Medium]
> If JIT is disabled or fails, returning -ENOTSUPP here will reject the
> program load silently without a verifier log message. Unlike
> bpf_prog_has_kfunc_call() which provides a helpful verbose() log in
> bpf_fixup_call_args():
> 
> 	if (has_kfunc_call) {
> 		verbose(env, "calling kernel functions are not allowed in non-JITed programs\n");
> 		return -EINVAL;
> 	}
> 
> Users encountering this for has_addr_space_cast_insn will not see an
> explanation for the EOPNOTSUPP error. Could we add a verifier log message
> explaining why JIT is required here?
> 

No verifier log support here.

The verifier log has been finalized before calling
__bpf_prog_select_runtime() in bpf_check().

>> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
>> index 3cf2cc6e3ab66..2d5958774b61b 100644
>> --- a/kernel/bpf/fixups.c
>> +++ b/kernel/bpf/fixups.c
>> @@ -1523,6 +1523,8 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env)
>>  				insn->off = 0;
>>  				insn->imm = 0;
>>  			} /* cast from as(0) to as(1) should be handled by JIT */
>> +			if (insn_is_cast_user(insn))
>> +				prog->aux->has_addr_space_cast_insn = true;
> 
> [Severity: Critical]
> This is a pre-existing issue, but the patch attempts to fix interpreter
> fallback for BPF_ADDR_SPACE_CAST and fails to address the underlying panic
> with arena programs.
> 
> If a program uses an arena and obtains a PTR_TO_ARENA without a cast_user
> instruction (for instance, using cast_kern), or if the arena map has
> BPF_F_NO_USER_CONV set, has_addr_space_cast_insn will not be set.
> 
> In the case of BPF_F_NO_USER_CONV, the instruction is rewritten to
> BPF_ALU | BPF_MOV | BPF_X right before the insn_is_cast_user() check.
> 
> Memory accesses using PTR_TO_ARENA are rewritten by bpf_do_misc_fixups()
> to use BPF_PROBE_MEM32 or BPF_PROBE_MEM32SX. If these arena programs fall
> back to the interpreter, wouldn't they hit the BUG_ON(1) in ___bpf_prog_run()
> because it lacks handlers for these opcodes?
> 
> kernel/bpf/core.c:___bpf_prog_run() {
>     ...
> 	default_label:
> 		pr_warn("BPF interpreter: unknown opcode %02x (imm: 0x%x)\n",
> 			insn->code, insn->imm);
> 		BUG_ON(1);
>     ...
> }
> 
> Should jit_needed be enforced in __bpf_prog_select_runtime() for all
> programs using fp->aux->arena instead?
> 

Sounds reasonable.

However, I think such similar issues should be fixed using separate
patches: all interpreter-unsupported insns should be marked as JIT-needed.

Like the way of "bpf: Fix unaligned interpreter panic on JIT fallback
path" [1], we can verify whether there are the following insns in the
prog by scanning prog->insnsi:

* user ADDR_SPACE_CAST
* PROBE_MEM32, PROBE_MEM32SX
* ADDR_PERCPU
* PROBE_ATOMIC
* indirect jump using insn_array map

I think these insns are not supported by interpreter.

Link:
[1]
https://lore.kernel.org/bpf/20260615025316.24429-1-yangtiezhu@loongson.cn/

Thanks,
Leon