Re: [PATCH bpf-next v5 11/16] bpf: Support stack arguments for kfunc calls

On 4/16/26 9:40 PM, [email protected] wrote:
> Sashiko AI review found 3 potential issue(s):
> - [Critical] Precision tracking (`mark_chain_precision`) is incorrectly bypassed or misapplied for kfunc scalar arguments passed via the stack, allowing malicious BPF programs to bypass verifier safety checks through state pruning.
> - [High] `get_kfunc_arg_reg()` fetches lookahead stack arguments without bounds checking, leading to an out-of-bounds memory read or NULL pointer dereference when resolving memory sizes.
> - [High] Overwriting `insn->off` in `bpf_fixup_kfunc_call` destroys the BTF file descriptor index, breaking module kfunc calls on architectures that use JIT far calls.
> --
>
> commit 15b2d48bd7c1080e972b83211cc9794ac2ba763f
> Author: Yonghong Song <[email protected]>
>
> bpf: Support stack arguments for kfunc calls
>
> Extend the stack argument mechanism to kfunc calls, allowing kfuncs
> with more than 5 parameters to receive additional arguments via the
> r11-based stack arg area.
>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 739c3127520c3..a3f307909e401 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
> [ ... ]
>
>> @@ -11653,6 +11659,19 @@ bool bpf_is_kfunc_pkt_changing(struct bpf_kfunc_call_arg_meta *meta)
>>   	return meta->func_id == special_kfunc_list[KF_bpf_xdp_pull_data];
>>   }
>>   
>> +static struct bpf_reg_state *get_kfunc_arg_reg(struct bpf_verifier_env *env, int argno)
>> +{
>> +	struct bpf_func_state *caller;
>> +	int spi;
>> +
>> +	if (argno < MAX_BPF_FUNC_REG_ARGS)
>> +		return &cur_regs(env)[argno + 1];
>> +
>> +	caller = cur_func(env);
>> +	spi = out_arg_spi(caller, argno - MAX_BPF_FUNC_REG_ARGS);
>> +	return &caller->stack_arg_regs[spi];
>> +}
> Does get_kfunc_arg_reg() need to verify that spi is within bounds and the
> stack slot is initialized?

The checking is below:

static bool is_stack_arg_slot_initialized(struct bpf_func_state *state, int spi)
{
         if (spi >= (int)(state->stack_arg_depth / BPF_REG_SIZE))
                 return false;
         return state->stack_arg_regs[spi].type != NOT_INIT;
}

                 if (i < MAX_BPF_FUNC_REG_ARGS) {
                         reg = &regs[i + 1];
                 } else {
                         /* Retrieve the reg state from the outgoing stack arg slot. */
                         struct bpf_func_state *caller = cur_func(env);
                         int spi = out_arg_spi(caller, i - MAX_BPF_FUNC_REG_ARGS);

                         if (!is_stack_arg_slot_initialized(caller, spi)) {
                                 verbose(env, "stack %s not properly initialized\n",
                                         reg_arg_name(env, argno));
                                 return -EINVAL;
                         }

                         tmp_reg = caller->stack_arg_regs[spi];
                         reg = &tmp_reg;
                         regno = -1;
                 }

spi has to be valid. Otherwise, verification will fail.

>
> When verifying pointer arguments that have a trailing size argument
> (KF_ARG_PTR_TO_MEM_SIZE), the verifier looks ahead at argno + 1. If a BPF
> program maliciously omits the stack size argument, could spi exceed
> caller->stack_arg_depth and lead to an out-of-bounds memory read?

If the bpf program maliciously omits the stack size argument,
btf_check_func_arg_match() should catch this and reject the program.

>
> [ ... ]
>
>> @@ -12394,9 +12435,11 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
>>   						reg_arg_name(env, argno));
>>   					return -EINVAL;
>>   				}
>> -				ret = mark_chain_precision(env, regno);
>> -				if (ret < 0)
>> -					return ret;
>> +				if (regno > 0) {
>> +					ret = mark_chain_precision(env, regno);
>> +					if (ret < 0)
>> +						return ret;
>> +				}
> Stack arguments use regno = -1, so it seems mark_chain_precision() is bypassed
> for scalar constants and size arguments passed on the stack.
>
> If precision tracking is bypassed, could the verifier's state equivalence logic
> consider states with different scalar values in these stack slots as equivalent
> and incorrectly prune them?

In this particular case, we know reg is a known constant, we can do
reg->precise = true.
Will fix.

>
> [ ... ]
>
>> @@ -20145,6 +20209,16 @@ int bpf_fixup_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>>   	if (!bpf_jit_supports_far_kfunc_call())
>>   		insn->imm = BPF_CALL_IMM(desc->addr);
>>   
>> +	/*
>> +	 * After resolving the kfunc address, insn->off is no longer needed
>> +	 * for BTF fd index. Repurpose it to store the number of stack args
>> +	 * so the JIT can marshal them.
>> +	 */
>> +	if (desc->func_model.nr_args > MAX_BPF_FUNC_REG_ARGS)
>> +		insn->off = desc->func_model.nr_args - MAX_BPF_FUNC_REG_ARGS;
>> +	else
>> +		insn->off = 0;
> For architectures where bpf_jit_supports_far_kfunc_call() returns true
> (such as s390x, loongarch, and powerpc), they resolve the address later
> using bpf_get_kfunc_addr(), which requires the BTF file descriptor index
> stored in insn->off.
>
> If insn->off is overwritten here, could it destroy the BTF file descriptor
> index and break module kfunc calls during JIT compilation?
>
The JIT implementation has changed and the above is dead code. Will remove.