Re: [PATCH bpf-next v2 11/17] bpf: Report Call Type Safety argument errors

[Eduard Zingerman <eddyz87@gmail.com>]

On Fri, 2026-06-19 at 22:59 +0200, Kumar Kartikeya Dwivedi wrote:
> Augment selected helper and kfunc argument-contract failures with Call Type
> Safety reports. Keep the existing terse verifier messages and add reason,
> source context, causal register or stack-argument history, and targeted
> suggestions.
> 
> Cover helper register-type mismatch, helper and kfunc non-NULL pointer
> requirements, release-helper ownership requirements, scalar and constant kfunc
> arguments, trusted and RCU pointer contracts, kfunc memory arguments,
> memory/length pairs, refcounted kptrs, constant strings, and IRQ flag stack
> arguments.
> 
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

[...]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index db151e6b8949..fdbf92bffc17 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -320,6 +320,15 @@ static int arg_idx_from_argno(argno_t a)
>  	return arg_from_argno(a) - 1;
>  }
>  
> +static int bpf_diag_stack_arg_slot_from_argno(argno_t a)

Nit: drop the bpf_diag_ prefix (or just inline at the callsite?).

> +{
> +	int arg = arg_from_argno(a);
> +
> +	if (arg <= MAX_BPF_FUNC_REG_ARGS)
> +		return -1;
> +	return arg - MAX_BPF_FUNC_REG_ARGS - 1;
> +}
> +
>  static const char *btf_type_name(const struct btf *btf, u32 id)
>  {
>  	return btf_name_by_offset(btf, btf_type_by_id(btf, id)->name_off);
> @@ -8161,6 +8170,8 @@ static const char *bpf_diag_reg_type_plain(struct bpf_verifier_env *env,
>  			return "a nullable memory pointer";
>  		return "a memory pointer";
>  	case PTR_TO_BTF_ID:
> +		if (type_may_be_null(type))
> +			return "a nullable kernel object pointer";

Nit: move this hunk such that bpf_diag_reg_type_plain() is defined in
     a single patch?

>  		if (type_is_non_owning_ref(type))
>  			return "a borrowed allocated object pointer";
>  		if (type_is_ptr_alloc_obj(type))
> @@ -8173,6 +8184,60 @@ static const char *bpf_diag_reg_type_plain(struct bpf_verifier_env *env,

[...]

> +static void diag_call_arg_fmt(struct bpf_verifier_env *env,
> +			      u32 insn_idx, argno_t argno,
> +			      const char *call_name,
> +			      const char *suggestion,
> +			      const char *fmt, ...) __printf(6, 7);

Nit: Is this prototype necessary?

> +static void diag_call_arg_fmt(struct bpf_verifier_env *env,
> +			      u32 insn_idx, argno_t argno,
> +			      const char *call_name,
> +			      const char *suggestion,
> +			      const char *fmt, ...)
> +{
> +	size_t size;
> +	va_list args;
> +	char *reason;
> +
> +	reason = bpf_diag_scratch_buf(env, 0, &size);

An alternative to juggling the scratch slot numbers
might be some kind of:

  char *bpf_diag_fmt(env, fmt_string, ...)

that would do the vaprintf behind the scene and accumulate
the buffers in env->diag, to be freed after reporting.

> +	if (reason) {
> +		va_start(args, fmt);
> +		vscnprintf(reason, size, fmt, args);
> +		va_end(args);
> +	} else {
> +		reason = "";
> +	}
> +
> +	bpf_diag_report_call_arg(env, insn_idx, argno, call_name, reason,
> +				 suggestion);
> +}
> +
>  static int check_reg_type(struct bpf_verifier_env *env, struct bpf_reg_state *reg, argno_t argno,
>  			  enum bpf_arg_type arg_type,
>  			  const u32 *arg_btf_id,
> @@ -8226,6 +8291,32 @@ static int check_reg_type(struct bpf_verifier_env *env, struct bpf_reg_state *re
>  	for (j = 0; j + 1 < i; j++)
>  		verbose(env, "%s, ", reg_type_str(env, compatible->types[j]));
>  	verbose(env, "%s\n", reg_type_str(env, compatible->types[j]));
> +	{
> +		size_t expected_size;
> +		char *expected_buf;
> +		const char *call_name;
> +		int len = 0;
> +
> +		expected_buf = bpf_diag_scratch_buf(env, 1,
> +						    &expected_size);
> +		if (expected_buf) {
> +			expected_buf[0] = '\0';
> +			for (j = 0; j < i && len < expected_size; j++)
> +				len += scnprintf(expected_buf + len,
> +						 expected_size - len,
> +						 "%s%s", j ? ", " : "",
> +						 reg_type_str(env, compatible->types[j]));

Nit: this is a bit ugly, maybe hide it as a utility function?

> +		} else {
> +			expected_buf = "";
> +		}
> +
> +		call_name = meta->func_id ? func_id_name(meta->func_id) : "callee";

Nit: I'd avoid the defensive meta->func_id ? ... here.

> +		diag_call_arg_fmt(env, env->insn_idx, argno, call_name,
> +				  "Pass a value with one of the accepted pointer or scalar types for this call.",
> +				  "it has type %s, but this argument accepts %s",
> +				  bpf_diag_reg_type_plain(env, reg->type),
> +				  expected_buf);
> +	}
>  	return -EACCES;
>  
>  found:

[...]

> @@ -11808,9 +11908,20 @@ get_kfunc_ptr_arg_type(struct bpf_verifier_env *env, struct bpf_func_state *call
>  	 */
>  	if (!btf_type_is_scalar(ref_t) && !__btf_type_is_scalar_struct(env, meta->btf, ref_t, 0) &&
>  	    (arg_mem_size ? !btf_type_is_void(ref_t) : 1)) {
> +		const char *expected_type;
> +
> +		expected_type = bpf_diag_format_btf_type_scratch(env,
> +								 1,
> +								 meta->btf,
> +								 ref_id);

Nit: isn't this exactly the diag_btf_type_name()?
     (and in a few places below).

>  		verbose(env, "%s pointer type %s %s must point to %sscalar, or struct with scalar\n",
>  			reg_arg_name(env, argno),
>  			btf_type_str(ref_t), ref_tname, arg_mem_size ? "void, " : "");
> +		diag_call_arg_fmt(env, env->insn_idx, argno, meta->func_name,
> +				  "Pass a verifier-tracked pointer to the expected kernel object type, not a pointer to stack storage or another memory buffer.",
> +				  "the kfunc expects a pointer to %s, but this argument is %s and cannot be used as that kernel object pointer",
> +				  expected_type,
> +				  bpf_diag_reg_type_plain(env, reg->type));
>  		return -EINVAL;
>  	}
>  	return arg_mem_size ? KF_ARG_PTR_TO_MEM_SIZE : KF_ARG_PTR_TO_MEM;

[...]