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

[Yonghong Song]

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 <Jeremy.Jean@xxxxxxxxxxxxxxxxx>
> > > ---
> > >    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/14a7e7c2-36f7-4aa2-9b20-cc54700a9f1b@xxxxxxxxx/
> >
> > >    
> > >    	/* 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?