Re: [PATCH bpf-next v4 06/18] bpf: Use argument index instead of register index in kfunc verifier logs

Alexei Starovoitov
2026-04-12T22:01:46Z

On Sat, Apr 11, 2026 at 9:59 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>
> For kfunc argument checking, use the argument index (arg#0, arg#1, ...)
> instead of the register index (R1, R2, ...) in verifier log messages.
> This is a preparation for future stack-based arguments where kfuncs can
> accept more than 5 arguments. Stack arguments won't have a corresponding
> register, so using argument index is more appropriate.
>
> Since some functions like check_mem_access(), check_stack_read_var_off(),
> and check_stack_range_initialized() are shared between kfunc argument
> checking (check_kfunc_args) and other paths (check_func_arg, do_check_insn, ...),
> introduce a `reg_or_arg` encoding: a non-negative value represents a register
> index, while a negative value encodes an argument index as -(argno + 1).
> The helper reg_arg_name() decodes this to produce either "R%d" or
> "arg#%d" for log messages.
>
> For check_func_arg() callers, in certain cases, the register index is
> preserved so existing helper function logs remain unchanged (e.g., "R1", "R2").
>
> Update selftests to expect the new "arg#N" format in kfunc error
> messages.
>
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---
> include/linux/bpf_verifier.h | 1 +
> kernel/bpf/verifier.c | 466 +++++++++---------
> .../selftests/bpf/prog_tests/cb_refs.c | 2 +-
> .../selftests/bpf/prog_tests/linked_list.c | 4 +-
> .../selftests/bpf/progs/cpumask_failure.c | 4 +-
> .../testing/selftests/bpf/progs/dynptr_fail.c | 6 +-
> .../selftests/bpf/progs/iters_testmod.c | 6 +-
> .../bpf/progs/local_kptr_stash_fail.c | 2 +-
> .../selftests/bpf/progs/map_kptr_fail.c | 4 +-
> .../bpf/progs/mem_rdonly_untrusted.c | 2 +-
> .../bpf/progs/nested_trust_failure.c | 2 +-
> .../selftests/bpf/progs/res_spin_lock_fail.c | 2 +-
> .../testing/selftests/bpf/progs/stream_fail.c | 2 +-
> .../selftests/bpf/progs/task_kfunc_failure.c | 4 +-
> .../bpf/progs/verifier_cgroup_storage.c | 4 +-
> .../selftests/bpf/progs/verifier_ctx.c | 2 +-
> .../bpf/progs/verifier_ref_tracking.c | 2 +-
> .../selftests/bpf/progs/verifier_sock.c | 6 +-
> .../selftests/bpf/progs/verifier_unpriv.c | 4 +-
> .../selftests/bpf/progs/verifier_vfs_reject.c | 8 +-
> .../testing/selftests/bpf/progs/wq_failures.c | 4 +-
> tools/testing/selftests/bpf/verifier/calls.c | 6 +-
> .../testing/selftests/bpf/verifier/map_kptr.c | 10 +-
> 23 files changed, 286 insertions(+), 267 deletions(-)
>
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index 05b9fe98b8f8..291f11ddd176 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -910,6 +910,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[16];
> 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 54296d818d35..01df990f841a 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2179,6 +2179,18 @@ static struct bpf_verifier_state *push_stack(struct bpf_verifier_env *env,
> return &elem->st;
> }
>
> +static const char *reg_arg_name(struct bpf_verifier_env *env, int reg_or_arg)
> +{
> + char *buf = env->tmp_reg_arg_name_buf;
> + int len = sizeof(env->tmp_reg_arg_name_buf);
> +
> + if (reg_or_arg >= 0)
> + snprintf(buf, len, "R%d", reg_or_arg);
> + else
> + snprintf(buf, len, "arg#%d", -(reg_or_arg + 1));
> + return buf;
> +}

The patches 1-4 make sense, but 5, 6 are too hacky.

- { "incorrect_head_var_off1", "R1 doesn't have constant offset" },
+ { "incorrect_head_var_off1", "arg#0 doesn't have constant offset" },

This just sucks.
It degrades output for no good reason.

Instead of inband negative vs positive signalling rename all
'regno' to 'argno' and always pass whatever argno you need 1,2,..5,6, etc

Pass ptr_reg and size_reg as bpf_reg_state the way patches 1-4 are doing.
If argno <= 5 keep 'R%d' output, so all selftest don't change.
For argno >= 6 print '*(R12-xx)' where xx is where that arg lives.
Printing arg# is too cryptic. Humans/agents need to do mental
gymnastics to understand what it means.
The output must be easy to consume by agents.

I was also thinking whether we can get rid of this 'argno' too.
cur_regs - reg is that number for <= 5 and
some spilled_ptr - reg for >= 6.
Technically we can

u32 argno = cur_regs - reg;
if (argno <= 5) use it
else
argno = spilled_ptr - reg.

Feels a bit hacky. Need to sleep on it.

pw-bot: cr