Re: [PATCH bpf-next v2 7/9] bpf: Prepare verifier logs for upcoming kfunc stack arguments

Alexei Starovoitov
2026-04-22T21:58:47Z

On Wed Apr 22, 2026 at 8:37 AM PDT, Yonghong Song wrote:
>
>
> On 4/22/26 7:57 AM, Yonghong Song wrote:
>>
>>
>> On 4/21/26 11:25 PM, bot+bpf-ci@kernel.org wrote:
>>>> diff --git a/include/linux/bpf_verifier.h
>>>> b/include/linux/bpf_verifier.h
>>>> index b148f816f25b..d5b4303315dd 100644
>>>> --- a/include/linux/bpf_verifier.h
>>>> +++ b/include/linux/bpf_verifier.h
>>>> @@ -913,6 +913,7 @@ struct bpf_verifier_env {
>>>>        * e.g., in reg_type_str() to generate reg_type string
>>>>        */
>>>>       char tmp_str_buf[TMP_STR_BUF_LEN];
>>>> +    char tmp_arg_name[32];
>>>>       struct bpf_insn insn_buf[INSN_BUF_SIZE];
>>>>       struct bpf_insn epilogue_buf[INSN_BUF_SIZE];
>>>>       struct bpf_scc_callchain callchain_buf;
>>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>>>> index c777a294302a..e7f5f0bb499b 100644
>>>> --- a/kernel/bpf/verifier.c
>>>> +++ b/kernel/bpf/verifier.c
>>>> @@ -261,6 +261,30 @@ struct bpf_kfunc_meta {
>>>>
>>>>   struct btf *btf_vmlinux;
>>>>
>>>> +typedef struct argno {
>>>> +    int argno;
>>>> +} argno_t;
>>>> +
>>>> +static argno_t argno_from_reg(u32 regno)
>>>> +{
>>>> +    return (argno_t){ .argno = regno };
>>>> +}
>>>> +
>>>> +static argno_t argno_from_arg(u32 arg)
>>>> +{
>>>> +    return (argno_t){ .argno = -arg };
>>>> +}
>>>> +
>>>> +static int reg_from_argno(argno_t a)
>>>> +{
>>>> +    return a.argno >= 0 ? a.argno : -1;
>>>> +}
>>>> +
>>>> +static int arg_from_argno(argno_t a)
>>>> +{
>>>> +    return a.argno < 0 ? -a.argno : -1;
>>>> +}
>>> [ ... ]
>>>
>>>> @@ -7026,8 +7068,8 @@ static int check_helper_mem_access(struct
>>>> bpf_verifier_env *env, struct bpf_reg_
>>>>    */
>>>>   static int check_mem_size_reg(struct bpf_verifier_env *env,
>>>>                     struct bpf_reg_state *mem_reg,
>>>> -                  struct bpf_reg_state *size_reg, u32 mem_regno,
>>>> -                  u32 size_regno, enum bpf_access_type access_type,
>>>> +                  struct bpf_reg_state *size_reg, argno_t mem_argno,
>>>> +                  argno_t size_argno, enum bpf_access_type
>>>> access_type,
>>>>                     bool zero_size_allowed,
>>>>                     struct bpf_call_arg_meta *meta)
>>>>   {
>>> [ ... ]
>>>
>>>> @@ -7052,31 +7094,31 @@ static int check_mem_size_reg(struct
>>>> bpf_verifier_env *env,
>>>>           meta = NULL;
>>>>
>>>>       if (size_reg->smin_value < 0) {
>>>> -        verbose(env, "R%d min value is negative, either use
>>>> unsigned or 'var &= const'\n",
>>>> -            size_regno);
>>>> +        verbose(env, "%s min value is negative, either use unsigned
>>>> or 'var &= const'\n",
>>>> +            reg_arg_name(env, size_argno));
>>>>           return -EACCES;
>>>>       }
>>>>
>>>>       if (size_reg->umin_value == 0 && !zero_size_allowed) {
>>>> -        verbose(env, "R%d invalid zero-sized read: u64=[%lld,%lld]\n",
>>>> -            size_regno, size_reg->umin_value, size_reg->umax_value);
>>>> +        verbose(env, "%s invalid zero-sized read: u64=[%lld,%lld]\n",
>>>> +            reg_arg_name(env, size_argno), size_reg->umin_value,
>>>> size_reg->umax_value);
>>>>           return -EACCES;
>>>>       }
>>>>
>>>>       if (size_reg->umax_value >= BPF_MAX_VAR_SIZ) {
>>>> -        verbose(env, "R%d unbounded memory access, use 'var &=
>>>> const' or 'if (var < const)'\n",
>>>> -            size_regno);
>>>> +        verbose(env, "%s unbounded memory access, use 'var &=
>>>> const' or 'if (var < const)'\n",
>>>> +            reg_arg_name(env, size_argno));
>>>>           return -EACCES;
>>>>       }
>>>> -    err = check_helper_mem_access(env, mem_reg, mem_regno,
>>>> size_reg->umax_value,
>>>> +    err = check_helper_mem_access(env, mem_reg, mem_argno,
>>>> size_reg->umax_value,
>>>>                         access_type, zero_size_allowed, meta);
>>>>       if (!err)
>>>> -        err = mark_chain_precision(env, size_regno);
>>>> +        err = mark_chain_precision(env, reg_from_argno(size_argno));
>>>>       return err;
>>>>   }
>>>                                       ^^^^^^^
>>>
>>> Does mark_chain_precision(env, reg_from_argno(size_argno)) properly
>>> handle
>>> kfunc size arguments?
>>>
>>> Before this patch, check_mem_size_reg() was called with size_regno
>>> being a
>>> u32 register number. For kfuncs with KF_ARG_PTR_TO_MEM_SIZE, the
>>> pre-patch
>>> call at check_kfunc_args was check_kfunc_mem_size_reg(env, reg,
>>> size_reg,
>>> regno, regno+1), passing real register numbers like R2..R5.
>>>
>>> After this patch, check_kfunc_args constructs size_argno via
>>> argno_from_arg(i+2) for the size parameter of a KF_ARG_PTR_TO_MEM_SIZE
>>> pair. The argno_from_arg() helper creates a negative encoding (-.argno).
>>> When reg_from_argno(size_argno) is called, it checks a.argno >= 0 and
>>> returns -1 for negative values.
>>>
>>> mark_chain_precision(env, -1) then reaches bpf_mark_chain_precision in
>>> backtrack.c, where the "if (regno >= 0)" guard causes it to skip marking
>>> the register, and the subsequent "if (bt_empty(bt)) return 0;" silently
>>> returns success without doing any precision tracking.
>>>
>>> For helpers, this works correctly because check_func_arg calls
>>> check_mem_size_reg with argno_from_reg(regno), which produces a
>>> non-negative
>>> encoding that reg_from_argno() correctly extracts back to the register
>>> number.
>>>
>>> Can you verify whether the loss of precision tracking for kfunc size
>>> registers might cause programs that previously verified to hit the
>>> BPF_COMPLEXITY_LIMIT_INSNS threshold? Kfuncs with __sz/__szk size
>>> arguments are widely used in-tree.
>>
>> This is indeed a problem.
>> Function check_mem_size_reg() is the one which does
>>    mark_chain_precision(env, reg_from_argno(size_argno))
>>
>>
>> There are two callers for check_mem_size_reg():
>>    - check_kfunc_mem_size_reg() <- check_kfunc_args
>>      so arg_from_argno() should be used here to get the value
>>    - check_func_arg() <- check_helper_call()
>>      in this case, since helper_call is fixed at most 5 registers,
>>      so argno_from_reg() is used in check_func_arg(), so later
>>      on it should use reg_from_argno() should be used to get value.
>>
>> There are two options to fix:
>>  1. do proper check like
>>     int reg_val = reg_from_argno(size_regno);
>>     if (reg_val >= 0)
>>       mark_chain_precision(env, reg_val);
>>     else
>>       mark_chain_precision(env, arg_from_argno(size_regno));
>>
>> 2. for arguments in check_helper_call(), we also use
>>    arg_from_argno() instead of reg_from_argno().
>>    This way, we can just do
>>       mark_chain_precision(env, arg_from_argno(size_regno));
>>
>> Not sure which is preferred or there are some other better alternatives.
>
> The following should work.
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index e7f5f0bb499b..2ead411b949d 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -7112,8 +7112,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)
> - err = mark_chain_precision(env, reg_from_argno(size_argno));
> + if (!err) {
> + int regno = reg_from_argno(size_argno);
> +
> + regno = regno >= 0 ? regno : arg_from_argno(size_argno);
> + err = mark_chain_precision(env, regno);
> + }

Both options are not great and
regno = arg >= 0 ? arg : reg_from_argno(argno);
in reg_arg_name() points to the same issue.

I think it should be:
static int reg_from_argno(argno_t a)
{
if (a.argno >= 0)
return a.argno;
if (a.argno >= -MAX_BPF_FUNC_REG_ARGS)
return -a.argno;
return -1;
}

whether arg_from_argno() should do similar logic is something to think about.