Re: [PATCH bpf-next v6 04/17] bpf: Prepare verifier logs for upcoming kfunc stack arguments

Alexei Starovoitov
2026-04-21T13:48:43Z

On Mon, Apr 20, 2026 at 11:07 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>
>
>
> On 4/20/26 5:03 PM, Alexei Starovoitov wrote:
> > On Sun Apr 19, 2026 at 9:33 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.
> >>
> >> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> >> ---
> >> include/linux/bpf_verifier.h | 1 +
> >> kernel/bpf/verifier.c | 649 ++++++++++--------
> >> .../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, 474 insertions(+), 374 deletions(-)
> >>
> >> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> >> index b148f816f25b..9fbbddc40d21 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_reg_arg_name_buf[32];
> > the name is too long.
> > Just tmp_arg_name ?
> >
> >> 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 3716d9688d00..6aa4dc161a56 100644
> >> --- a/kernel/bpf/verifier.c
> >> +++ b/kernel/bpf/verifier.c
> >> @@ -1751,6 +1751,55 @@ static struct bpf_verifier_state *push_stack(struct bpf_verifier_env *env,
> >> return &elem->st;
> >> }
> >>
> >> +/*
> >> + * Unified argument number encoding for verifier log messages.
> >> + * Register args (arg_idx 0-4) use their register number (R1-R5).
> >> + * Stack args (arg_idx 5+) are encoded as STACK_ARGNO_BASE + arg_idx
> >> + * to avoid collision with register numbers. reg_arg_name() decodes
> >> + * this back to a human-readable string like "*(R11-8)" for logs.
> >> + */
> >> +#define STACK_ARGNO_BASE 100
> >> +
> >> +static bool is_stack_argno(int argno)
> >> +{
> >> + return argno >= STACK_ARGNO_BASE;
> >> +}
> >> +
> >> +static u32 make_argno(u32 arg_idx)
> >> +{
> >> + if (arg_idx < MAX_BPF_FUNC_REG_ARGS)
> >> + return BPF_REG_1 + arg_idx;
> >> + return STACK_ARGNO_BASE + arg_idx;
> >> +}
> >> +
> >> +static u32 arg_idx_from_argno(int argno)
> >> +{
> >> + if (is_stack_argno(argno))
> >> + return argno - STACK_ARGNO_BASE;
> >> + return argno - BPF_REG_1;
> >> +}
> >> +
> >> +static int next_argno(int argno)
> >> +{
> >> + return make_argno(arg_idx_from_argno(argno) + 1);
> >> +}
> > I don't like this +1, -1 dance all around. Makes the whole thing
> > hard to follow.
> > Keep argno starting at 1. So old regno == argno.
>
> The above make_argno() returns an argno having regno == argno
> for regno 1-5.

No. It doesn't.

it does:

+static u32 make_argno(u32 arg_idx)
+{
+ if (arg_idx < MAX_BPF_FUNC_REG_ARGS)
+ return BPF_REG_1 + arg_idx;

so for argno == 0 it returns 1 and that's my objection.
Make argno starting at 1.