From: "Alexei Starovoitov" <alexei.starovoitov@gmail.com>
To: "Yonghong Song" <yonghong.song@linux.dev>,
<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 14:58:45 -0700 [thread overview]
Message-ID: <DI00R3E7C8FD.1U98NQCWU1BW4@gmail.com> (raw)
In-Reply-To: <cb70fcff-2e3e-4132-ada7-7af4e8a4ab35@linux.dev>
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.
next prev parent reply other threads:[~2026-04-22 21:58 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 [this message]
2026-04-22 23:09 ` Yonghong Song
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=DI00R3E7C8FD.1U98NQCWU1BW4@gmail.com \
--to=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 \
--cc=yonghong.song@linux.dev \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.