Re: [PATCH v2 1/2] bpf: reject stack-argument callback subprograms

On 8/20/26 3:32 PM, Eduard Zingerman wrote:
> On Tue, 2026-08-18 at 08:23 -0700, Yonghong Song wrote:
>> On 8/17/26 1:48 PM, Jérémy Jean wrote:
>>> Helper callbacks enter BPF subprograms through bpf_callback_t, whose
>>> runtime ABI supplies five arguments. BTF validation nevertheless permits
>>> static callback subprograms to declare more than five arguments when JIT
>>> stack arguments are supported.
>>>
>>> This lets verifier state for a callback use outgoing stack argument slots
>>> prepared at the helper call site. The helper does not pass those slots. On
>>> x86-64, callback loads of arguments seven and later therefore read the
>>> helper native frame instead of the synthetic values checked by the
>>> verifier. KASAN reports a slab OOB write.
>>>
>>> Reject callback subprograms with incoming stack arguments when processing
>>> callback calls.
>>>
>>> Fixes: 0f6bd5e7a804 ("bpf: Support stack arguments for bpf functions")
>>> Assisted-by: Codex:gpt-5
>>> Signed-off-by: Jérémy Jean <[email protected]>
>>> ---
>>>    kernel/bpf/verifier.c | 2 ++
>>>    1 file changed, 2 insertions(+)
>>>
>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>>> index fdc5fbb1f78c..5fcefc0eaba0 100644
>>> --- a/kernel/bpf/verifier.c
>>> +++ b/kernel/bpf/verifier.c
>>> @@ -9285,6 +9285,8 @@ static int push_callback_call(struct bpf_verifier_env *env, struct bpf_insn *ins
>>>    	err = btf_check_subprog_call(env, subprog, caller->regs);
>>>    	if (err == -EFAULT)
>>>    		return err;
>>> +	if (bpf_in_stack_arg_cnt(&env->subprog_info[subprog]))
>>> +		return -EINVAL;
>> This is not good as user will not know why it failed. Your v1 does have an error message.
>>
>> But this is not needed. Without above verifer.c change, user will get an error message:
>>      func#0 writes 4 stack arg slots, but calls only require 0
>>
>> NACK, see my v1 comment: https://lore.kernel.org/bpf/[email protected]/
>>
>>>    
>>>    	/* set_callee_state is used for direct subprog calls, but we are
>>>    	 * interested in validating only BPF helpers that can call subprogs as
> Yonghong,
>
> this is a real bug. Here is an example of a program that exposes
> unsafe behavior:
>
>      unsigned long arr[10];
>      
>      __noinline __used
>      static int callback_9args(__u32 index, void *ctx, long a3, long a4,
>                                long a5, long a6, long a7, long a8, long a9)
>      {
>              return arr[a9] % 2;    // verifier sees a9 as 0 and allows this memory access
>      }
>
>      SEC("tc")
>      __description("stack_arg: callback with incoming stack args")
>      __failure
>      __naked void stack_arg_callback_many_args(void)
>      {
>              asm volatile (
>                      "r6 = 0;"
>                      "*(u64 *)(r11 - 32) = 0;"
>                      "*(u64 *)(r11 - 24) = 0;"
>                      "*(u64 *)(r11 - 16) = 0;"
>                      "*(u64 *)(r11 - 8) = 0;"
>                      "r1 = 1;"
>                      "r2 = %[callback_9args];"
>                      "r3 = 0;"
>                      "r4 = 0;"
>                      "call %[bpf_loop];"
>                      "r1 = 1;"
>                      "r2 = 2;"
>                      "r3 = 3;"
>                      "r4 = 4;"
>                      "r5 = 5;"
>                      "*(u64 *)(r11 - 32) = 0;"
>                      "*(u64 *)(r11 - 24) = 0;"
>                      "*(u64 *)(r11 - 16) = 0;"
>                      "*(u64 *)(r11 - 8) = 0;"
>                      "call callback_9args;"   // this hides the callback call from the check in bpf_fixup_call_args()
>                      "r0 = 0;"
>                      "exit;"
>                      :
>                      : __imm_ptr(callback_9args),
>                        __imm(bpf_loop)
>                      : __clobber_common, "r6"
>              );
>      }
>
> Jérémy,
>
> Please update the test case as above, as your test case does not
> really expose the bug. Also, I think that a better fix would be to
> make stack arguments not-init in the callback frame. This way the
> verifier would produce a proper error message.

Okay, I see. The key thing is the below:
    "call callback_9args;"   // this hides the callback call from the check in bpf_fixup_call_args()

So callback_9args appears twice, and bpf_fixup_call_args() only checks the second callback_9args(), right?