Re: [PATCH bpf-next v6 08/17] bpf: Reject stack arguments in non-JITed programs

On 4/19/26 11:21 AM, [email protected] wrote:
> Sashiko AI review found 2 potential issue(s):
> - [High] The loop in `bpf_fixup_call_args()` incorrectly rejects main BPF programs with >5 arguments in their BTF signature when JIT is disabled, even though main programs do not actually use stack arguments.
> - [High] Standalone global functions with stack arguments can incorrectly fall back to the interpreter if JIT compilation fails, leading to a kernel panic.
> --
>
> commit f56b0fedf39a845476910805e073f6b2d3a4ad82
> Author: Yonghong Song <[email protected]>
>
> bpf: Reject stack arguments in non-JITed programs
>
> The interpreter does not understand the bpf register r11 (BPF_REG_PARAMS)
> used for stack argument addressing. So reject interpreter usage if stack
> arguments are used either in the main program or any subprogram.
>
>> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
>> index ae10b9ca018df..ec8523e6e4eb4 100644
>> --- a/kernel/bpf/core.c
>> +++ b/kernel/bpf/core.c
>> @@ -2599,7 +2599,7 @@ struct bpf_prog *__bpf_prog_select_runtime(struct bpf_verifier_env *env, struct
>>   		goto finalize;
>>   
>>   	if (IS_ENABLED(CONFIG_BPF_JIT_ALWAYS_ON) ||
>> -	    bpf_prog_has_kfunc_call(fp))
>> +	    bpf_prog_has_kfunc_call(fp) || fp->aux->stack_arg_depth)
>>   		jit_needed = true;
> When loading a standalone global function (env->subprog_cnt <= 1) with >5
> arguments, bpf_jit_subprogs() sets fp->aux->stack_arg_depth to 0 (the
> outgoing_stack_arg_depth). If JIT is requested, bpf_fixup_call_args() will
> exit early, bypassing the newly added loop that checks
> incoming_stack_arg_depth.
>
> Because fp->aux->stack_arg_depth has been cleared to 0, jit_needed evaluates
> to false here.

I think the below change should fix the problem.

-       if (env->subprog_cnt <= 1)
+       if (env->subprog_cnt <= 1) {
+               env->prog->aux->stack_arg_depth = env->subprog_info[0].outgoing_stack_arg_depth
+                       + env->subprog_info[0].incoming_stack_arg_depth;
                 return 0;
+       }

In the above scenaio, env->prog->aux->stack_arg_depth will not be 0, so jit_needed
evaluates to true.


>
> If JIT compilation subsequently fails (e.g., due to memory limits or
> unsupported instructions), does the kernel silently fall back to the
> interpreter instead of rejecting the program?
>
> If executed, the interpreter encounters instructions utilizing BPF_REG_PARAMS
> (r11, mapped to regs[11]), which is initialized to 0 in the interpreter's
> stack frame. Could dereferencing this register to read stack arguments
> result in a NULL pointer dereference and cause a kernel panic?
>
>> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
>> index c4e0224ad2f2e..3c9a82823913e 100644
>> --- a/kernel/bpf/fixups.c
>> +++ b/kernel/bpf/fixups.c
>> @@ -1411,6 +1411,12 @@ int bpf_fixup_call_args(struct bpf_verifier_env *env)
>>   		verbose(env, "calling kernel functions are not allowed in non-JITed programs\n");
>>   		return -EINVAL;
>>   	}
>> +	for (i = 0; i < env->subprog_cnt; i++) {
>> +		if (env->subprog_info[i].incoming_stack_arg_depth) {
> When verifying the main BPF program (i = 0), if the provided BTF signature
> has >5 arguments, the verifier sets
> env->subprog_info[0].incoming_stack_arg_depth > 0.

We can ignore i = 0. The first subprog is the main prog, we can skip it.

>
> Although the verifier subsequently realizes this signature doesn't match the
> expected single ctx argument and marks the BTF info as unreliable, it never
> clears the incoming_stack_arg_depth.
>
> Because this loop starts at i = 0 and unconditionally checks
> incoming_stack_arg_depth, will it reject the entire program if JIT is
> disabled (e.g., via sysctl net.core.bpf_jit_enable=0), even though main
> programs do not actually use stack arguments?
>
> Does this break backwards compatibility for existing valid BPF programs with
> 6+ argument BTF signatures that used to load successfully?
>
>> +			verbose(env, "stack args are not supported in non-JITed programs\n");
>> +			return -EINVAL;
>> +		}
>> +	}