Re: [PATCH bpf-next v5 08/14] bpf: Report Register Type Safety errors

> diff --git a/kernel/bpf/diagnostics.c b/kernel/bpf/diagnostics.c

[ ... ]

> +static int diag_stack_argno(u8 slot)
> +{
> +	return MAX_BPF_FUNC_REG_ARGS + slot + 1;
> +}
> +
> +static void diag_format_stack_arg(char *buf, size_t size, u8 slot, const char *arg_name)
> +{
> +	int argno = diag_stack_argno(slot);
> +	const char *ordinal = diag_arg_ordinal(argno);
> +
> +	if (ordinal && arg_name)
> +		scnprintf(buf, size, "outgoing stack argument %u (%s argument, %s)", slot + 1,
> +			  ordinal, arg_name);

[ ... ]

> +static const char *diag_mod_target_desc(struct bpf_verifier_env *env,
> +					const struct bpf_diag_mod_target *target)
> +{
> +	switch (target->kind) {
> +	case BPF_DIAG_MOD_TARGET_REG:
> +		return bpf_diag_fmt(env, "R%u", target->regno);
> +	case BPF_DIAG_MOD_TARGET_STACK_ARG:
> +		return bpf_diag_fmt(env, "stack arg%d", diag_stack_argno(target->stack_arg));
                                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Does this numbering match the rest of the diagnostic output?

diag_stack_argno() returns MAX_BPF_FUNC_REG_ARGS + slot + 1, which for
slot 0 gives 6 (the absolute argument ordinal). But diag_format_stack_arg()
uses slot + 1 to label the same slot, and the pre-existing verbose() message
in check_outgoing_stack_args() also uses spi + 1.

For a 6-argument bpf2bpf call where the first stack argument (slot 0) was
invalidated by an intervening call, the verifier log shows three different
numbers for the same slot:

  callee expects 6 args, stack arg1 is not initialized
    (from verbose(), using slot + 1)

  outgoing stack argument 1 (sixth argument, x) is not initialized
    (from diag_format_stack_arg(), using slot + 1)

  stack arg6: call invalidated this outgoing stack argument
    (from diag_mod_target_desc() via diag_print_history(), using slot + 6)

The string "stack arg%d" appears in both the pre-existing verbose() output
and this new Causal path section. In one place it means slot number within
the stack-arg area, in the other it means absolute argument position.

Should diag_mod_target_desc() use target->stack_arg + 1 to match the
slot-numbering convention used by verbose() and diag_format_stack_arg(), or
use different wording to clarify the number is an absolute argument ordinal
rather than a stack slot number?

> +	case BPF_DIAG_MOD_TARGET_STACK_SLOT:
> +		return bpf_diag_fmt(env, "stack slot fp%d", -(target->spi + 1) * BPF_REG_SIZE);
> +	default:
> +		return "value";
> +	}
> +}

[ ... ]


---
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/31870608176