public inbox for bpf@vger.kernel.org
 help / color / mirror / Atom feed
From: Yonghong Song <yonghong.song@linux.dev>
To: 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 07:57:19 -0700	[thread overview]
Message-ID: <715fcb76-c1ba-44e2-9b6f-e3133f2a5610@linux.dev> (raw)
In-Reply-To: <25c10a6b1a6f9703f2b2c943ab42245cb9de30d2004eb301d85d9de0ca6ca7b3@mail.kernel.org>



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.

>
>>   static int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
>> -			 u32 regno, u32 mem_size)
>> +			 argno_t argno, u32 mem_size)
>>   {
> [ ... ]
>
>> @@ -7157,7 +7199,7 @@ enum {
>>    * env->cur_state->active_locks remembers which map value element or allocated
>>    * object got locked and clears it after bpf_spin_unlock.
>>    */
>> -static int process_spin_lock(struct bpf_verifier_env *env, struct bpf_reg_state *reg, int regno, int flags)
>> +static int process_spin_lock(struct bpf_verifier_env *env, struct bpf_reg_state *reg, argno_t argno, int flags)
>>   {
> [ ... ]
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/24762480405


  reply	other threads:[~2026-04-22 14:57 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 [this message]
2026-04-22 15:37       ` Yonghong Song
2026-04-22 21:58         ` Alexei Starovoitov
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=715fcb76-c1ba-44e2-9b6f-e3133f2a5610@linux.dev \
    --to=yonghong.song@linux.dev \
    --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