All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yonghong Song <yonghong.song@linux.dev>
To: sashiko-reviews@lists.linux.dev
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next v4 11/25] bpf: Reject stack arguments in non-JITed programs
Date: Fri, 15 May 2026 09:00:34 -0700	[thread overview]
Message-ID: <84f08f28-74bf-487c-8040-e8d2e8613d2b@linux.dev> (raw)
In-Reply-To: <20260514235918.CAA8DC2BCB3@smtp.kernel.org>



On 5/15/26 1:59 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Critical] The patch fails to prevent interpreter fallback if stack arguments are used only in nested subprograms (e.g., Main -> Subprog A -> Subprog B). This can lead to a kernel crash via an uninitialized R11 dereference.
> - [Low] Standalone global functions with stack arguments are rejected with `-ENOTSUPP` without a verifier log, instead of `-EINVAL` with a descriptive message.
> --
>
> commit 851a484e2b5b68983f11bc3764118f2bcba5acb0
> Author: Yonghong Song <yonghong.song@linux.dev>
>
> bpf: Reject stack arguments in non-JITed programs
>
> This patch attempts to reject programs that use stack arguments when running in
> the interpreter, as the interpreter does not support the BPF_REG_PARAMS (r11)
> register. It does this by checking for stack arguments in
> __bpf_prog_select_runtime() and bpf_fixup_call_args().
>
>> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
>> index ae10b9ca018df..958d86f0beac3 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) || (env && env->subprog_info[0].stack_arg_cnt))
>>   		jit_needed = true;
> Does this condition correctly catch stack arguments in nested subprograms?

I think the above condition '(env && env->subprog_info[0].stack_arg_cnt)' is
unnecessary. This check is for the *main* prog. main prog should not call kfunc,
otherwise jit_needed must be true. helper does not have stack arguments.

If bpf-to-bpf calls are used in this prog, then bpf_fixup_call_args() will
do some checking for all subprogs.

>
> env->subprog_info[0].stack_arg_cnt only tracks the main program's incoming
> and direct outgoing stack arguments. If the main program calls Subprog A with
> no stack arguments, and Subprog A calls Subprog B with more than 5 arguments,
> the main program's stack_arg_cnt remains 0.
>
> If bpf_prog_jit_compile() subsequently fails for the main program,
> jit_needed evaluates to false and it falls back to the interpreter.
>
>>   
>>   	if (!bpf_prog_select_interpreter(fp))
>> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
>> index ba86039789fdf..19056016eed8b 100644
>> --- a/kernel/bpf/fixups.c
>> +++ b/kernel/bpf/fixups.c
>> @@ -1407,6 +1407,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;
>>   	}
> Is it possible for bpf_jit_subprogs() to succeed earlier in this function
> and return 0, bypassing these new checks entirely?
>
> If jit_requested is true, bpf_jit_subprogs() is called before this
> point:
>
> kernel/bpf/fixups.c:bpf_fixup_call_args() {
>      ...
> 	if (env->prog->jit_requested &&
> 	    !bpf_prog_is_offloaded(env->prog->aux)) {
> 		err = bpf_jit_subprogs(env);
> 		if (err == 0)
> 			return 0;
>      ...
> }
>
> If it succeeds, the function returns 0 early, skipping the new loop.

But if bpf_jit_subprogs() succeed (with more than one subprog), then we will have
            prog->bpf_func = func[0]->bpf_func;

In __bpf_prog_select_runtime(), we have
	   if (fp->bpf_func)
                 goto finalize;
So interpreter will be ignored since jit has done successfully.

>
> Combined with the issue in __bpf_prog_select_runtime() above, the interpreter
> could end up executing the main program and processing BPF_PSEUDO_CALL
> instructions by jumping to the un-JITted instructions of the subprograms.
>
> When Subprog A writes to outgoing stack slots via BPF_REG_PARAMS (r11), the
> interpreter will use an uninitialized r11, potentially corrupting memory.
>
>> +	for (i = 1; i < env->subprog_cnt; i++) {
> Should this loop start at i = 0 to properly handle standalone global
> functions?

We can do this. Previously I skip 'i = 0' due to

                 if (env->prog->aux->func_info_aux) {
                         ret = btf_prepare_func_args(env, 0);
                         if (ret || sub->arg_cnt != 1 || sub->args[0].arg_type != ARG_PTR_TO_CTX) {
                                 env->prog->aux->func_info_aux[0].unreliable = true;

                 }

since the main prog may have a btf which have more than 5 arguments although
in reality it will have just one argument. If I have i = 0, this will reject
interpreter which is not correct.

Now we have the following change

                 if (env->prog->aux->func_info_aux) {
                         ret = btf_prepare_func_args(env, 0);
                         if (ret || sub->arg_cnt != 1 || sub->args[0].arg_type != ARG_PTR_TO_CTX) {
                                 env->prog->aux->func_info_aux[0].unreliable = true;
                                 sub->arg_cnt = 1;
                                 sub->stack_arg_cnt = 0;
                         }
                 }

So we can safely start from i = 0. Note that i = 0 will definitely
succeed since we have sub->arg_cnt = 1 above. So change to i = 0
won't change anything but can avoid confusion.

>
> For standalone global functions that are verified independently,
> env->subprog_cnt is 1. Starting the loop at i = 1 skips the global
> function entirely.
>
> The program is still safely rejected later in __bpf_prog_select_runtime()
> because env->subprog_info[0].stack_arg_cnt > 0 correctly sets
> jit_needed = true, leading to an -ENOTSUPP return. However, starting at
> i = 0 would emit the expected verifier log and return -EINVAL instead of
> failing silently with -ENOTSUPP.
>
>> +		if (bpf_in_stack_arg_cnt(&env->subprog_info[i])) {
>> +			verbose(env, "stack args are not supported in non-JITed programs\n");
>> +			return -EINVAL;
>> +		}
>> +	}
>>   	if (env->subprog_cnt > 1 && env->prog->aux->tail_call_reachable) {


  reply	other threads:[~2026-05-15 16:00 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-13  4:49 [PATCH bpf-next v4 00/25] bpf: Support stack arguments for BPF functions and kfuncs Yonghong Song
2026-05-13  4:49 ` [PATCH bpf-next v4 01/25] bpf: Convert bpf_get_spilled_reg macro to static inline function Yonghong Song
2026-05-13  4:50 ` [PATCH bpf-next v4 02/25] bpf: Remove copy_register_state wrapper function Yonghong Song
2026-05-13  4:50 ` [PATCH bpf-next v4 03/25] bpf: Add helper functions for r11-based stack argument insns Yonghong Song
2026-05-13  4:50 ` [PATCH bpf-next v4 04/25] bpf: Set sub->arg_cnt earlier in btf_prepare_func_args() Yonghong Song
2026-05-13  4:50 ` [PATCH bpf-next v4 05/25] bpf: Support stack arguments for bpf functions Yonghong Song
2026-05-14 10:46   ` sashiko-bot
2026-05-14 16:07     ` Yonghong Song
2026-05-13  4:50 ` [PATCH bpf-next v4 06/25] bpf: Refactor jmp history to use dedicated spi/frame fields Yonghong Song
2026-05-13  4:50 ` [PATCH bpf-next v4 07/25] bpf: Add precision marking and backtracking for stack argument slots Yonghong Song
2026-05-13  5:44   ` bot+bpf-ci
2026-05-13  4:50 ` [PATCH bpf-next v4 08/25] bpf: Refactor record_call_access() to extract per-arg logic Yonghong Song
2026-05-13  4:50 ` [PATCH bpf-next v4 09/25] bpf: Use arg_is_fp() in has_fp_args() Yonghong Song
2026-05-13  4:50 ` [PATCH bpf-next v4 10/25] bpf: Extend liveness analysis to track stack argument slots Yonghong Song
2026-05-13  5:44   ` bot+bpf-ci
2026-05-14 22:53   ` sashiko-bot
2026-05-15 15:29     ` Yonghong Song
2026-05-13  4:50 ` [PATCH bpf-next v4 11/25] bpf: Reject stack arguments in non-JITed programs Yonghong Song
2026-05-13  5:33   ` bot+bpf-ci
2026-05-14 23:59   ` sashiko-bot
2026-05-15 16:00     ` Yonghong Song [this message]
2026-05-13  4:50 ` [PATCH bpf-next v4 12/25] bpf: Prepare architecture JIT support for stack arguments Yonghong Song
2026-05-13  5:33   ` bot+bpf-ci
2026-05-15  0:30   ` sashiko-bot
2026-05-15 16:33     ` Yonghong Song
2026-05-13  4:50 ` [PATCH bpf-next v4 13/25] bpf: Enable r11 based insns Yonghong Song
2026-05-13  4:51 ` [PATCH bpf-next v4 14/25] bpf: Support stack arguments for kfunc calls Yonghong Song
2026-05-13  4:51 ` [PATCH bpf-next v4 15/25] bpf: Reject stack arguments if tail call reachable Yonghong Song
2026-05-13  5:33   ` bot+bpf-ci
2026-05-15  3:23   ` sashiko-bot
2026-05-15 16:39     ` Yonghong Song
2026-05-13  4:51 ` [PATCH bpf-next v4 16/25] bpf: Disable private stack for x86_64 if stack arguments used Yonghong Song
2026-05-13  5:33   ` bot+bpf-ci
2026-05-15  5:28   ` sashiko-bot
2026-05-15 16:41     ` Yonghong Song
2026-05-13  4:51 ` [PATCH bpf-next v4 17/25] bpf,x86: Implement JIT support for stack arguments Yonghong Song
2026-05-15  6:02   ` sashiko-bot
2026-05-13  4:51 ` [PATCH bpf-next v4 18/25] selftests/bpf: Add tests for BPF function " Yonghong Song
2026-05-15  6:16   ` sashiko-bot
2026-05-15 16:57     ` Yonghong Song
2026-05-13  4:51 ` [PATCH bpf-next v4 19/25] selftests/bpf: Add tests for stack argument validation Yonghong Song
2026-05-13  4:51 ` [PATCH bpf-next v4 20/25] selftests/bpf: Add BTF fixup for __naked subprog parameter names Yonghong Song
2026-05-13  4:51 ` [PATCH bpf-next v4 21/25] selftests/bpf: Add verifier tests for stack argument validation Yonghong Song
2026-05-13  4:51 ` [PATCH bpf-next v4 22/25] selftests/bpf: Add precision backtracking test for stack arguments Yonghong Song
2026-05-13  4:51 ` [PATCH bpf-next v4 23/25] bpf, arm64: Map BPF_REG_0 to x8 instead of x7 Yonghong Song
2026-05-13  4:51 ` [PATCH bpf-next v4 24/25] bpf, arm64: Add JIT support for stack arguments Yonghong Song
2026-05-15  8:20   ` sashiko-bot
2026-05-13  4:52 ` [PATCH bpf-next v4 25/25] selftests/bpf: Enable stack argument tests for arm64 Yonghong Song
2026-05-13 16:33 ` [PATCH bpf-next v4 00/25] bpf: Support stack arguments for BPF functions and kfuncs Alexei Starovoitov
2026-05-13 17:41   ` Yonghong Song
2026-05-13 17:51     ` Alexei Starovoitov
2026-05-13 18:11       ` Yonghong Song
2026-05-13 16:40 ` patchwork-bot+netdevbpf

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=84f08f28-74bf-487c-8040-e8d2e8613d2b@linux.dev \
    --to=yonghong.song@linux.dev \
    --cc=bpf@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.