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

Yonghong Song
2026-04-21T04:04:16Z

On 4/20/26 4:58 PM, Alexei Starovoitov wrote:
> On Sun Apr 19, 2026 at 9:33 AM PDT, Yonghong Song wrote:
>> + * @mem_regno is the register containing the pointer, mem_regno+1 is the register
>> + * containing the access size.
> wrong comment. should probably say that size_reg is the register that contains size.
>
>>
>> - WARN_ON_ONCE(regno < BPF_REG_2 || regno > BPF_REG_5);
>> + WARN_ON_ONCE(mem_regno > BPF_REG_4);
> this warn is too late here. Can be removed.
>
>>
>> memset(&meta, 0, sizeof(meta));
>>
>> @@ -7129,8 +7130,8 @@ static int check_kfunc_mem_size_reg(struct bpf_verifier_env *env, struct bpf_reg
>> mark_ptr_not_null_reg(mem_reg);
>> }
>>
>> - err = check_mem_size_reg(env, reg, regno, BPF_READ, true, &meta);
>> - err = err ?: check_mem_size_reg(env, reg, regno, BPF_WRITE, true, &meta);
>> + err = check_mem_size_reg(env, mem_reg, size_reg, mem_regno, BPF_READ, true, &meta);
>> + err = err ?: check_mem_size_reg(env, mem_reg, size_reg, mem_regno, BPF_WRITE, true, &meta);
>>
>> if (may_be_null)
>> *mem_reg = saved_reg;
>> @@ -8594,7 +8595,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
>> return -EFAULT;
>> }
>> key_size = meta->map.ptr->key_size;
>> - err = check_helper_mem_access(env, regno, key_size, BPF_READ, false, NULL);
>> + err = check_helper_mem_access(env, reg, regno, key_size, BPF_READ, false, NULL);
>> if (err)
>> return err;
>> if (can_elide_value_nullness(meta->map.ptr->map_type)) {
>> @@ -8621,7 +8622,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
>> return -EFAULT;
>> }
>> meta->raw_mode = arg_type & MEM_UNINIT;
>> - err = check_helper_mem_access(env, regno, meta->map.ptr->value_size,
>> + err = check_helper_mem_access(env, reg, regno, meta->map.ptr->value_size,
>> arg_type & MEM_WRITE ? BPF_WRITE : BPF_READ,
>> false, meta);
>> break;
>> @@ -8665,7 +8666,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
>> */
>> meta->raw_mode = arg_type & MEM_UNINIT;
>> if (arg_type & MEM_FIXED_SIZE) {
>> - err = check_helper_mem_access(env, regno, fn->arg_size[arg],
>> + err = check_helper_mem_access(env, reg, regno, fn->arg_size[arg],
>> arg_type & MEM_WRITE ? BPF_WRITE : BPF_READ,
>> false, meta);
>> if (err)
>> @@ -8675,13 +8676,13 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
>> }
>> break;
>> case ARG_CONST_SIZE:
>> - err = check_mem_size_reg(env, reg, regno,
>> + err = check_mem_size_reg(env, reg_state(env, regno - 1), reg, regno - 1,
> or moved here.
> I would just remove it.

I checked logic and it seems this warn should not really happen. So removing it sound a good idea.