Re: [PATCH bpf-next v5 03/16] bpf: Refactor to handle memory and size together

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [High] Incomplete refactoring for stack arguments breaks verifier precision tracking, marking incorrect registers.
--

commit 7bebb6c179745d67fba0c807428d095b66738135
Author: Yonghong Song <yonghong.song@linux.dev>

bpf: Refactor to handle memory and size together

This commit refactors memory and size register handling in BPF helper
verification, passing bpf_reg_state directly to prepare for stack arguments.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 2bedaa193d54d..7a7024d94cf00 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -7020,15 +7020,17 @@ static int check_helper_mem_access(struct bpf_verifier_env *env, int regno,
>  /* verify arguments to helpers or kfuncs consisting of a pointer and an access
>   * size.
>   *
> - * @regno is the register containing the access size. regno-1 is the register
> - * containing the pointer.
> + * @mem_regno is the register containing the pointer, mem_regno+1 is the register
> + * containing the access size.
>   */
>  static int check_mem_size_reg(struct bpf_verifier_env *env,
> -			      struct bpf_reg_state *reg, u32 regno,
> +			      struct bpf_reg_state *mem_reg,
> +			      struct bpf_reg_state *size_reg, u32 mem_regno,
>  			      enum bpf_access_type access_type,
>  			      bool zero_size_allowed,
>  			      struct bpf_call_arg_meta *meta)
>  {
> +	int size_regno = mem_regno + 1;
>  	int err;

Since this refactoring is intended to support stack arguments, does this
hardcoded size_regno = mem_regno + 1 break verifier precision tracking?

If the memory pointer is passed in R5, mem_regno is 5, making size_regno 6.

This would cause mark_chain_precision() to incorrectly backtrack the
callee-saved register BPF_REG_6 instead of the stack slot containing the size
argument.

Furthermore, if mem_regno is later modified to represent a 0-based argument
index, size_regno will evaluate to the wrong register entirely.

[ ... ]
> -	err = check_helper_mem_access(env, regno - 1, reg->umax_value,
> +	err = check_helper_mem_access(env, mem_reg, mem_regno, size_reg->umax_value,
>  				      access_type, zero_size_allowed, meta);
>  	if (!err)
> -		err = mark_chain_precision(env, regno);
> +		err = mark_chain_precision(env, size_regno);

Does marking the wrong register leave the actual size argument imprecise,
potentially corrupting verifier state pruning and allowing out-of-bounds
memory accesses to bypass verification?

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