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

[sashiko-bot@kernel.org]

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 <yonghong.song@linux.dev>

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?

[ ... ]
> @@ -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?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260419163316.731019-1-yonghong.song@linux.dev?part=12