Re: [PATCH bpf-next v6 12/17] bpf: Support stack arguments for kfunc calls

On 4/19/26 10:08 AM, [email protected] wrote:
> Sashiko AI review found 2 potential issue(s):
> - [Critical] Loss of precision tracking for scalar stack arguments due to modifying a local copy (`tmp_reg`).
> - [Critical] Bypassing `mark_chain_precision` for stack arguments breaks data-flow backtracking, enabling unsafe state pruning.
> --
>
> commit 96adcc4832214af51212820f2056565abaf42a32
> 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 2a346e4f28e15..13f1fd7880921 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
> [ ... ]
>> @@ -7314,8 +7322,12 @@ static int check_mem_size_reg(struct bpf_verifier_env *env,
>>   	}
>>   	err = check_helper_mem_access(env, mem_reg, mem_argno, size_reg->umax_value,
>>   				      access_type, zero_size_allowed, meta);
>> -	if (!err && !is_stack_argno(size_argno))
>> -		err = mark_chain_precision(env, size_argno);
>> +	if (!err) {
>> +		if (is_stack_argno(size_argno))
>> +			size_reg->precise = true;
>> +		else
>> +			err = mark_chain_precision(env, size_argno);
>> +	}
>>   	return err;
>>   }
> Does setting size_reg->precise = true without calling mark_chain_precision()
> break data-flow backtracking?
>
> If only the current state's flag is set, parent states and the source
> registers that populated the stack argument will not be marked precise.
> Could this lead to the verifier incorrectly pruning states if execution paths
> merge before the stack store?

You are right. Let me think how to resolve this.

>
> [ ... ]
>> @@ -12373,20 +12400,44 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
> [ ... ]
>>   		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;
>>   		}
> [ ... ]
>> @@ -12413,9 +12464,13 @@ 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) {
>> +					reg->precise = true;
>> +				} else {
>> +					ret = mark_chain_precision(env, regno);
>> +					if (ret < 0)
>> +						return ret;
>> +				}
>>   				meta->arg_constant.found = true;
>>   				meta->arg_constant.value = reg->var_off.value;
> Does setting reg->precise = true here modify a local copy instead of the actual
> verifier state?
>
> Since reg points to tmp_reg for stack arguments:
>
>          tmp_reg = caller->stack_arg_regs[spi];
>          reg = &tmp_reg;
>
> it seems that reg->precise = true will only update the block-scoped tmp_reg,
> leaving the real state in caller->stack_arg_regs[spi] untouched. Could this
> cause the verifier to lose track of precision requirements for scalar
> constraints?

Right, tmp_reg is not used. I guess my previous usage of tmp_reg due to
an early hack but I didn't remove it. Will remove tmp_reg in the next revision.

>
> Additionally, similar to check_mem_size_reg() above, bypassing
> mark_chain_precision() for stack arguments appears to skip backtracking
> entirely.

Ack.