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

On 4/16/26 9:49 PM, [email protected] wrote:
> 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 <[email protected]>
>
> 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?

We are fine here. In function get_kfunc_ptr_arg_type(), we have

         if (argno + 1 < nargs &&
             (is_kfunc_arg_mem_size(meta->btf, &args[argno + 1], &regs[regno + 1]) ||
              is_kfunc_arg_const_mem_size(meta->btf, &args[argno + 1], &regs[regno + 1])))
                 arg_mem_size = true;

This ensures mem_size is at most at R5.

>
> 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?
>