From: Yonghong Song <yonghong.song@linux.dev>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
bot+bpf-ci@kernel.org, bpf@vger.kernel.org
Cc: ast@kernel.org, andrii@kernel.org, daniel@iogearbox.net,
jose.marchesi@oracle.com, kernel-team@fb.com,
martin.lau@kernel.org, eddyz87@gmail.com, clm@meta.com,
ihor.solodrai@linux.dev
Subject: Re: [PATCH bpf-next v2 7/9] bpf: Prepare verifier logs for upcoming kfunc stack arguments
Date: Wed, 22 Apr 2026 16:09:15 -0700 [thread overview]
Message-ID: <2ad13b46-a291-4dd1-bf6d-3c9ca9319eea@linux.dev> (raw)
In-Reply-To: <DI00R3E7C8FD.1U98NQCWU1BW4@gmail.com>
On 4/22/26 2:58 PM, Alexei Starovoitov wrote:
> 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.
I think the following arg_from_argno() should be okay:
static int arg_from_argno(argno_t a)
{
if (a.argno < 0)
return -a.argno;
return -1;
}
In some cases, there is a need to get an arg_idx which will be
used for an array element. e.g.,
process_iter_arg(..., argno, ...)
...
u32 arg_idx = arg_from_argno(argno) - 1;
...
btf_id = btf_check_iter_arg(meta->btf, meta->func_proto, arg_idx);
btf_check_iter_arg(..., arg_idx)
...
arg = &btf_params(func)[arg_idx];
...
next prev parent reply other threads:[~2026-04-22 23:09 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-22 5:41 [PATCH bpf-next v2 0/9] bpf: Prepare to support stack arguments Yonghong Song
2026-04-22 5:41 ` [PATCH bpf-next v2 1/9] bpf: Remove unused parameter from check_map_kptr_access() Yonghong Song
2026-04-22 5:41 ` [PATCH bpf-next v2 2/9] bpf: Fix tail_call_reachable leak Yonghong Song
2026-04-22 5:42 ` [PATCH bpf-next v2 3/9] bpf: Remove WARN_ON_ONCE in check_kfunc_mem_size_reg() Yonghong Song
2026-04-22 5:42 ` [PATCH bpf-next v2 4/9] bpf: Refactor to avoid redundant calculation of bpf_reg_state Yonghong Song
2026-04-22 5:42 ` [PATCH bpf-next v2 5/9] bpf: Refactor to handle memory and size together Yonghong Song
2026-04-22 5:42 ` [PATCH bpf-next v2 6/9] bpf: Rename existing argno to arg Yonghong Song
2026-04-22 5:42 ` [PATCH bpf-next v2 7/9] bpf: Prepare verifier logs for upcoming kfunc stack arguments Yonghong Song
2026-04-22 6:25 ` bot+bpf-ci
2026-04-22 14:57 ` Yonghong Song
2026-04-22 15:37 ` Yonghong Song
2026-04-22 21:58 ` Alexei Starovoitov
2026-04-22 23:09 ` Yonghong Song [this message]
2026-04-22 5:42 ` [PATCH bpf-next v2 8/9] bpf: Introduce bpf register BPF_REG_PARAMS Yonghong Song
2026-04-22 5:42 ` [PATCH bpf-next v2 9/9] bpf: Reuse MAX_BPF_FUNC_ARGS for maximum number of arguments Yonghong Song
2026-04-22 6:12 ` bot+bpf-ci
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2ad13b46-a291-4dd1-bf6d-3c9ca9319eea@linux.dev \
--to=yonghong.song@linux.dev \
--cc=alexei.starovoitov@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bot+bpf-ci@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=clm@meta.com \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=ihor.solodrai@linux.dev \
--cc=jose.marchesi@oracle.com \
--cc=kernel-team@fb.com \
--cc=martin.lau@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox