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

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Uninitialized variable `ref_id` used in `check_kfunc_args` for non-pointer release kfunc arguments.
- [Low] Incorrect diagnostic message for kfunc fixed-size memory arguments failing bounds checks.
--

--- Patch [11]: [PATCH bpf-next v4 11/16] bpf: Report Call Type Safety argument errors ---
commit ea19a04c0e142c86cd13b8e8524d76e024167249
Author: Kumar Kartikeya Dwivedi <[email protected]>

bpf: Report Call Type Safety argument errors

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.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 24ae3cb38f6d8..7e28f4d9f5c4d 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -12361,29 +12437,43 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me
>  
>  		t = btf_type_skip_modifiers(btf, args[i].type, NULL);
>  
> -		if (btf_type_is_ptr(t) && (bpf_register_is_null(reg) || type_may_be_null(reg->type)) &&
> +		if (btf_type_is_ptr(t)) {
> +			ref_t = btf_type_skip_modifiers(btf, t->type, &ref_id);
> +			ref_tname = btf_name_by_offset(btf, ref_t->name_off);
> +		}
> +
> +		if (btf_type_is_ptr(t) &&
> +		    (bpf_register_is_null(reg) || type_may_be_null(reg->type)) &&
>  		    !type_may_be_null(kf_arg_type)) {

[ ... ]

>  		}
>  
>  		if (regno == meta->release_regno && !is_kfunc_arg_dynptr(meta->btf, &args[i]) &&
>  		    !reg_is_referenced(env, reg) && !bpf_register_is_null(reg)) {
> +			const char *expected_type;
> +
> +			expected_type = diag_btf_type_name(env, btf, ref_id);

[Severity: Medium]
Could this result in using an uninitialized ref_id? If a module registers a
kfunc with KF_RELEASE that takes a non-pointer argument, btf_type_is_ptr(t)
would be false, skipping the initialization of ref_id.

Then, on the release argument mismatch path here, diag_btf_type_name() would
read uninitialized stack memory from ref_id, which might lead to out-of-bounds
reads in btf_type_by_id() or leak stack contents into the verifier log.

[ ... ]

> @@ -12706,8 +12863,17 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me
>  					return -EINVAL;
>  				}
>  				ret = check_mem_reg(env, reg, argno, type_size, BPF_READ | BPF_WRITE, meta);
> -				if (ret < 0)
> +				if (ret < 0) {
> +					const char *expected_type;
> +
> +					expected_type = diag_btf_type_name(env, btf, ref_id);
> +					diag_call_arg_fmt(env, insn_idx, argno, func_name,
> +							  "Pass stack, map, context, or other verifier-known memory of the expected type and size, not an integer cast to a pointer.",
> +							  "the kfunc expects %u bytes of memory for %s, but it is %s and not verifier-known memory",
> +							  type_size, expected_type,
> +							  bpf_diag_reg_type_plain(env, reg->type));

[Severity: Low]
Does this emit a misleading diagnostic when the bounds check fails? If a kfunc
takes a fixed-size memory argument and the BPF program passes a valid
verifier-known memory pointer (like PTR_TO_STACK) but the buffer is too small,
check_mem_reg() returns an error.

This diagnostic would then unconditionally claim the argument is "not
verifier-known memory" and advise not to pass an "integer cast to a pointer",
which obscures the actual bounds-check issue.

>  					return ret;
> +				}
>  			}
>  			break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11