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

[Yonghong Song <yonghong.song@linux.dev>]

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