BPF List
 help / color / mirror / Atom feed
From: Yonghong Song <yonghong.song@linux.dev>
To: "Eduard Zingerman" <eddyz87@gmail.com>,
	"Jérémy Jean" <Jeremy.Jean@oss.cyber.gouv.fr>,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Andrii Nakryiko" <andrii@kernel.org>,
	"Kumar Kartikeya Dwivedi" <memxor@gmail.com>
Cc: bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v2 1/2] bpf: reject stack-argument callback subprograms
Date: Sat, 22 Aug 2026 11:27:38 -0700	[thread overview]
Message-ID: <e2a7e092-df3c-4f92-8f12-5eb371f004df@linux.dev> (raw)
In-Reply-To: <9f50810d45db0b60248f106d21b4a278f3ec2156.camel@gmail.com>



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@oss.cyber.gouv.fr>
>>> ---
>>>    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@linux.dev/
>>
>>>    
>>>    	/* 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?



  reply	other threads:[~2026-08-22 18:27 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-17 20:48 [PATCH v2 0/2] bpf: reject stack-argument callback subprograms Jérémy Jean
2026-08-17 20:48 ` [PATCH v2 1/2] " Jérémy Jean
2026-08-17 21:11   ` sashiko-bot
2026-08-17 21:37   ` bot+bpf-ci
2026-08-18 15:23   ` Yonghong Song
2026-08-20 22:32     ` Eduard Zingerman
2026-08-22 18:27       ` Yonghong Song [this message]
2026-08-17 20:48 ` [PATCH v2 2/2] selftests/bpf: add callback stack-argument rejection test Jérémy Jean
2026-08-17 21:12   ` sashiko-bot
2026-08-17 21:24   ` bot+bpf-ci

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e2a7e092-df3c-4f92-8f12-5eb371f004df@linux.dev \
    --to=yonghong.song@linux.dev \
    --cc=Jeremy.Jean@oss.cyber.gouv.fr \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=memxor@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox