public inbox for bpf@vger.kernel.org
 help / color / mirror / Atom feed
From: Yonghong Song <yonghong.song@linux.dev>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>, bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	"Jose E . Marchesi" <jose.marchesi@oracle.com>,
	kernel-team@fb.com, Martin KaFai Lau <martin.lau@kernel.org>,
	Puranjay Mohan <puranjay@kernel.org>
Subject: Re: [PATCH bpf-next 7/9] bpf: Prepare verifier logs for upcoming kfunc stack arguments
Date: Tue, 21 Apr 2026 16:56:35 -0700	[thread overview]
Message-ID: <b9809b44-aef2-4ffc-945e-b5cac1dbdbbd@linux.dev> (raw)
In-Reply-To: <DHZ6BHVW9CVO.1MNKDGSAXI76Z@gmail.com>



On 4/21/26 3:07 PM, Alexei Starovoitov wrote:
> On Tue Apr 21, 2026 at 10:20 AM PDT, Yonghong Song wrote:
>> This change prepares verifier log reporting for upcoming kfunc stack
>> argument support.
>>
>> Today verifier log code mostly assumes that an argument can be described
>> directly by a register number. That works for arguments passed in `R1`
>> to `R5`, but it does not work once kfunc arguments can also be
>> passed on the stack.
>>
>> Introduce an internal `argno` representation such that register-passed
>> arguments keep using their real register numbers, while stack-passed
>> arguments use an encoded value above a dedicated base.
>> `reg_arg_name()` converts this representation into either `R%d` or
>> `*(R11-off)` when emitting verifier logs. If a particular `argno`
>> is corresponding to a stack argument, print `*(R11-off)`. Otherwise,
>> print `R%d`. Here R11 presents the base of stack arguments.
>>
>> This keeps existing logs readable for register arguments and allows the
>> same log sites to handle future stack arguments without open-coding
>> special cases.
>>
>> Update selftests accordingly.
>>
>> Acked-by: Puranjay Mohan <puranjay@kernel.org>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
>>   include/linux/bpf_verifier.h                  |   1 +
>>   kernel/bpf/verifier.c                         | 640 ++++++++++--------
>>   .../testing/selftests/bpf/prog_tests/bpf_nf.c |  22 +-
>>   .../selftests/bpf/prog_tests/cb_refs.c        |   2 +-
>>   .../selftests/bpf/prog_tests/kfunc_call.c     |   2 +-
>>   .../selftests/bpf/prog_tests/linked_list.c    |   4 +-
>>   .../selftests/bpf/progs/cgrp_kfunc_failure.c  |  14 +-
>>   .../selftests/bpf/progs/cpumask_failure.c     |  10 +-
>>   .../testing/selftests/bpf/progs/dynptr_fail.c |  22 +-
>>   .../selftests/bpf/progs/file_reader_fail.c    |   4 +-
>>   tools/testing/selftests/bpf/progs/irq.c       |   4 +-
>>   tools/testing/selftests/bpf/progs/iters.c     |   6 +-
>>   .../selftests/bpf/progs/iters_state_safety.c  |  14 +-
>>   .../selftests/bpf/progs/iters_testmod.c       |   4 +-
>>   .../selftests/bpf/progs/iters_testmod_seq.c   |   4 +-
>>   .../selftests/bpf/progs/map_kptr_fail.c       |   2 +-
>>   .../selftests/bpf/progs/percpu_alloc_fail.c   |   4 +-
>>   .../testing/selftests/bpf/progs/rbtree_fail.c |   6 +-
>>   .../bpf/progs/refcounted_kptr_fail.c          |   2 +-
>>   .../testing/selftests/bpf/progs/stream_fail.c |   2 +-
>>   .../selftests/bpf/progs/task_kfunc_failure.c  |  18 +-
>>   .../selftests/bpf/progs/task_work_fail.c      |   6 +-
>>   .../selftests/bpf/progs/test_bpf_nf_fail.c    |   8 +-
>>   .../bpf/progs/test_kfunc_dynptr_param.c       |   2 +-
>>   .../bpf/progs/test_kfunc_param_nullable.c     |   2 +-
>>   .../selftests/bpf/progs/verifier_bits_iter.c  |   4 +-
>>   .../bpf/progs/verifier_ref_tracking.c         |   6 +-
>>   .../selftests/bpf/progs/verifier_vfs_reject.c |   8 +-
>>   .../testing/selftests/bpf/progs/wq_failures.c |   2 +-
>>   tools/testing/selftests/bpf/verifier/calls.c  |  14 +-
>>   30 files changed, 464 insertions(+), 375 deletions(-)
>>
>> 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 18ab92581452..82568a427211 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -1742,6 +1742,44 @@ static struct bpf_verifier_state *push_stack(struct bpf_verifier_env *env,
>>   	return &elem->st;
>>   }
>>   
>> +#define STACK_ARGNO_BASE 100
>> +
>> +static bool is_stack_argno(int argno)
>> +{
>> +	return argno > STACK_ARGNO_BASE;
>> +}
>> +
>> +/* arg starts at 1 */
>> +static u32 make_argno(u32 arg)
>> +{
>> +	if (arg <= MAX_BPF_FUNC_REG_ARGS)
>> +		return arg;
>> +	return STACK_ARGNO_BASE + arg;
>> +}
> You can remove this and simplify everything further by
>
> static bool is_stack_argno(int argno)
> {
> 	return argno > MAX_BPF_FUNC_REG_ARGS;
> }
>
>> +
>> +static u32 arg_from_argno(int argno)
>> +{
>> +	if (is_stack_argno(argno))
>> +		return argno - STACK_ARGNO_BASE;
>> +	return argno;
>> +}
> remove as well.
>
> and a comment like:
>
> /*
>   * switch (argno) {
>   * case 1: R1
>   * case 5: R5
>   * case 6: *(u64 *)(R11 +- 8)
>   * case 7: *(u64 *)(R11 +- 16)
>   */

This doesn't work. Let us see the following example:

check_kfunc_args
   process_dynptr_func (argno)
     check_mem_access (argno, 4th argument)
       check_packet_access (argno)
         check_mem_region_access (argno)
           __check_mem_access (argno)
             <== verbose log with argno

do_check
   do_check_insn (env)
     check_load_mem (insn)
       check_mem_access (insn->src_reg, 4th argument)
         check_packet_access (...)
           check_mem_region_access (...)
             __check_mem_access (insn->src_reg or argno)


In the above case, function __check_mem_access() intends to issue
an verbose log with the 'argno' argument. The possible values are
    - R0 to R11 with do_check() call stack
    - R1 to R5 or stack arguments

In such cases, print will be
    R0 to R11 if argno <= 100 (STACK_ARGNO_BASE)
    *(R11-%u) if argno > 100

Does this make sense?

>> +static const char *reg_arg_name(struct bpf_verifier_env *env, int argno)
>> +{
>> +	char *buf = env->tmp_arg_name;
>> +	int len = sizeof(env->tmp_arg_name);
>> +	u32 arg;
>> +
>> +	if (!is_stack_argno(argno)) {
>> +		snprintf(buf, len, "R%d", argno);
>> +		return buf;
>> +	}
>> +
>> +	arg = arg_from_argno(argno);
> gone
>
>> +	snprintf(buf, len, "*(R11-%u)", (arg - MAX_BPF_FUNC_REG_ARGS) * BPF_REG_SIZE);
>> +	return buf;
>> +}


  reply	other threads:[~2026-04-21 23:56 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-21 17:19 [PATCH bpf-next 0/9] bpf: Prepare to support stack arguments Yonghong Song
2026-04-21 17:19 ` [PATCH bpf-next 1/9] bpf: Remove unused parameter from check_map_kptr_access() Yonghong Song
2026-04-21 17:19 ` [PATCH bpf-next 2/9] bpf: Fix tail_call_reachable leak Yonghong Song
2026-04-21 18:06   ` bot+bpf-ci
2026-04-22  0:29     ` Yonghong Song
2026-04-21 17:19 ` [PATCH bpf-next 3/9] bpf: Remove WARN_ON_ONCE in check_kfunc_mem_size_reg() Yonghong Song
2026-04-21 17:19 ` [PATCH bpf-next 4/9] bpf: Refactor to avoid redundant calculation of bpf_reg_state Yonghong Song
2026-04-21 21:40   ` Amery Hung
2026-04-21 23:42     ` Yonghong Song
2026-04-21 17:19 ` [PATCH bpf-next 5/9] bpf: Refactor to handle memory and size together Yonghong Song
2026-04-21 17:19 ` [PATCH bpf-next 6/9] bpf: Rename existing argno to arg Yonghong Song
2026-04-21 17:20 ` [PATCH bpf-next 7/9] bpf: Prepare verifier logs for upcoming kfunc stack arguments Yonghong Song
2026-04-21 22:07   ` Alexei Starovoitov
2026-04-21 23:56     ` Yonghong Song [this message]
2026-04-22  0:37       ` Alexei Starovoitov
2026-04-22  1:20         ` Yonghong Song
2026-04-22  1:52           ` Alexei Starovoitov
2026-04-21 17:20 ` [PATCH bpf-next 8/9] bpf: Introduce bpf register BPF_REG_PARAMS Yonghong Song
2026-04-21 22:10   ` Alexei Starovoitov
2026-04-22  0:09     ` Yonghong Song
2026-04-22  0:42       ` Alexei Starovoitov
2026-04-22  1:10         ` Yonghong Song
2026-04-21 17:20 ` [PATCH bpf-next 9/9] bpf: Reuse MAX_BPF_FUNC_ARGS for maximum number of arguments Yonghong Song
2026-04-21 17:52   ` bot+bpf-ci
2026-04-21 19:13 ` [PATCH bpf-next 0/9] bpf: Prepare to support stack arguments Kumar Kartikeya Dwivedi

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=b9809b44-aef2-4ffc-945e-b5cac1dbdbbd@linux.dev \
    --to=yonghong.song@linux.dev \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=jose.marchesi@oracle.com \
    --cc=kernel-team@fb.com \
    --cc=martin.lau@kernel.org \
    --cc=puranjay@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