Re: [PATCH] bpf: reject stack-argument callback subprograms

Yonghong Song <[email protected]>
Newsgroups org.kernel.vger.linux-kselftest,org.kernel.vger.bpf,org.kernel.vger.linux-kernel
Message-ID <[email protected]>

On 8/16/26 1:45 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. Add a verifier regression test using bpf_loop() and a
> nine-argument callback.
>
> 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                         |  4 +++
>   .../selftests/bpf/progs/verifier_stack_arg.c  | 33 +++++++++++++++++++
>   2 files changed, 37 insertions(+)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index fdc5fbb1f78c..29aa4911c7f7 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -9285,6 +9285,10 @@ 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])) {
> +		verbose(env, "callback subprog cannot have stack args\n");
> +		return -EINVAL;
> +	}
>   
>   	/* set_callee_state is used for direct subprog calls, but we are
>   	 * interested in validating only BPF helpers that can call subprogs as
> diff --git a/tools/testing/selftests/bpf/progs/verifier_stack_arg.c b/tools/testing/selftests/bpf/progs/verifier_stack_arg.c
> index 7e0ce5db28a0..5acc3b63ca84 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_stack_arg.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_stack_arg.c
> @@ -27,6 +27,13 @@ static int subprog_7args(int a, int b, int c, int d, int e, int f, int g)
>   	return a + b + c + d + e + f + g;
>   }
>   
> +__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 a9;
> +}
> +
>   __noinline __used
>   static long subprog_deref_arg6(long a, long b, long c, long d, long e, long *f)
>   {
> @@ -79,6 +86,32 @@ __naked void stack_arg_two_subprogs(void)
>   	);
>   }
>   
> +SEC("tc")
> +__description("stack_arg: callback with incoming stack args")
> +__failure
> +__msg("callback subprog cannot have stack args")
> +__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];"
> +		"r0 = 0;"
> +		"exit;"
> +		:
> +		: __imm_ptr(callback_9args),
> +		  __imm(bpf_loop)
> +		: __clobber_all
> +	);
> +}

I build with latest bpf-next and selftest with llvm23 and I got a selftest build
failure:

progs/verifier_stack_arg.c:96:3: error: inline assembly requires more registers than available
    96 |                 "r6 = 0;"
       |                 ^
   CLNG-BPF [test_progs] verifier_value_or_null.bpf.o
1 error generated.

The reason probably due to '__imm_ptr(callback_9args)'. The following change can fix the build:

index 5acc3b63ca84..f55a433b9202 100644
--- a/tools/testing/selftests/bpf/progs/verifier_stack_arg.c
+++ b/tools/testing/selftests/bpf/progs/verifier_stack_arg.c
@@ -108,7 +108,7 @@ __naked void stack_arg_callback_many_args(void)
                 :
                 : __imm_ptr(callback_9args),
                   __imm(bpf_loop)
-               : __clobber_all
+               : __clobber_common, "r6"
         );
  }

I then tested without your verifier.c change, I get the following test failure:

     All error logs:
     tester_init:PASS:tester_log_buf 0 nsec
     process_subtest:PASS:obj_open_mem 0 nsec
     process_subtest:PASS:specs_alloc 0 nsec
     run_subtest:PASS:obj_open_mem 0 nsec
     libbpf: prog 'stack_arg_callback_many_args': BPF program load failed: -EINVAL
     libbpf: prog 'stack_arg_callback_many_args': failed to load: -EINVAL
     libbpf: failed to load object 'verifier_stack_arg'
     run_subtest:PASS:unexpected_load_success 0 nsec
     validate_msgs:FAIL:979 expect_msg
     VERIFIER LOG:
     =============
     func#0 writes 4 stack arg slots, but calls only require 0
     processed 21 insns (limit 1000000) max_states_per_insn 1 total_states 2 peak_states 2 mark_read 0
     =============
     EXPECTED   SUBSTR: 'callback subprog cannot have stack args'
     Summary: 1/18 PASSED, 0 SKIPPED, 1/1 FAILED

I think the error message "func#0 writes 4 stack arg slots, but calls only require 0"
is good enough. So the extra error message "callback subprog cannot have stack args"
is not necessary in my opinion.

> +
>   SEC("tc")
>   __description("stack_arg: read from uninitialized stack arg slot")
>   __failure
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.