Re: [PATCH bpf-next 04/10] bpf: Support stack arguments for kfunc calls

Yonghong Song
2026-04-02T14:45:56Z

On 4/1/26 8:18 PM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index d424fe611ef8..6579156486b8 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
> [ ... ]
>
>> @@ -13687,13 +13694,41 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
>> * verifier sees.
>> */
>> for (i = 0; i < nargs; i++) {
>> - struct bpf_reg_state *regs = cur_regs(env), *reg = &regs[i + 1];
>> + struct bpf_reg_state *regs = cur_regs(env), *reg;
>> + struct bpf_reg_state saved_reg;
> [ ... ]
>
>> + if (i < MAX_BPF_FUNC_REG_ARGS) {
>> + reg = &regs[i + 1];
>> + } else {
>> + /*
>> + * Retrieve the spilled reg state from the stack arg slot.
>> + * Reuse the existing type checking infrastructure which
>> + * reads from cur_regs(env)[regno], temporarily copy the
>> + * stack arg reg state into regs[BPF_REG_1] and restore
>> + * it after checking.
>> + */
> [ ... ]
>
>> + is_stack_arg = true;
>> + regno = BPF_REG_1;
>> + saved_reg = regs[BPF_REG_1];
>> + regs[BPF_REG_1] = caller->stack_arg_slots[spi].spilled_ptr;
>> + reg = &regs[BPF_REG_1];
>> + }
>> +
>> if (is_kfunc_arg_prog_aux(btf, &args[i])) {
> [ ... ]
>
>> - continue;
>> + goto next_arg;
>> }
>>
>> if (is_kfunc_arg_ignore(btf, &args[i]) || is_kfunc_arg_implicit(meta, i))
>> continue;
> The is_kfunc_arg_prog_aux path above was converted from continue to
> goto next_arg so that regs[BPF_REG_1] gets restored via the next_arg
> label. The scalar path and the KF_ARG_PTR_TO_NULL path were also
> converted the same way.
>
> Should this continue for is_kfunc_arg_ignore/is_kfunc_arg_implicit
> also be goto next_arg? When a stack arg (i >= MAX_BPF_FUNC_REG_ARGS)

You are right. My initial version with a early bpf-next is okay.
But after rebase on top of master, I missed the above as it is
newly introduced. Will fix in the next revision.

> hits this continue, is_stack_arg is true but the next_arg restore is
> skipped, so regs[BPF_REG_1] keeps the spilled pointer from the
> current iteration. On the next iteration, saved_reg then captures that
> stale value instead of the original register state.
>
> The comment above says "restore it after checking", which this continue
> path does not do. The impact is currently masked because each iteration
> independently overwrites regs[BPF_REG_1] and the caller_saved clearing
> in check_kfunc_call() resets R1 after the loop, but it breaks the
> save/restore invariant the rest of this patch establishes.
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/23879588767