Re: [PATCH bpf-next v1 15/18] bpf: Classify kfunc pointer arguments from BTF, resolve type against the register

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The removal of the `bpf_type_has_unsafe_modifiers()` check allows referenced pointers with unsafe modifiers (specifically `MEM_PERCPU`) to be passed as normal pointers to regular kfuncs.
--

--- Patch [15]: [PATCH bpf-next v1 15/18] bpf: Classify kfunc pointer arguments from BTF, resolve type against the register ---
commit 23915f0b14432c05d0cd9201ab12d531e9b8df8a
Author: Amery Hung <ameryhung@gmail.com>
Subject: bpf: Classify kfunc pointer arguments from BTF, resolve type against the register

This commit refactors kfunc pointer argument classification to rely on BTF 
rather than the caller's register state. It also moves the referenced-or-trusted 
checks for BTF ID arguments into the BTF ID resolution path.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -12383,20 +12376,46 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me
>  			break;
>  		case KF_ARG_PTR_TO_BTF_ID:
>  			/* Only base_type is checked, further checks are done here */
> -			if ((base_type(reg->type) != PTR_TO_BTF_ID ||
> -			     (bpf_type_has_unsafe_modifiers(reg->type) && !is_rcu_reg(reg))) &&
> -			    !reg2btf_ids[base_type(reg->type)]) {
> -				verbose(env, "%s is %s ", reg_arg_name(env, argno),
> -					reg_type_str(env, reg->type));
> -				verbose(env, "expected %s or socket\n",
> -					reg_type_str(env, base_type(reg->type) |
> -							  (type_flag(reg->type) & BPF_REG_TRUSTED_MODIFIERS)));
> +			if (base_type(reg->type) == PTR_TO_BTF_ID ||
> +			    reg2btf_ids[base_type(reg->type)]) {
> +
> +				if (!is_trusted_reg(env, reg)) {
> +					if (!is_kfunc_rcu(meta)) {
> +						verbose(env, "%s must be referenced or trusted\n",
> +							reg_arg_name(env, argno));
> +						return -EINVAL;
> +					}
> +					if (!is_rcu_reg(reg)) {
> +						verbose(env, "%s must be a rcu pointer\n",
> +							reg_arg_name(env, argno));
> +						return -EINVAL;
> +					}
> +				}
> +
> +				ret = process_kf_arg_ptr_to_btf_id(env, reg, ref_t, ref_tname, ref_id, meta, i, argno);

[Severity: Critical]
Does this new structure allow referenced pointers with unsafe modifiers (like
MEM_PERCPU) to bypass validation and be passed to regular kfuncs?

The original code explicitly checked for unsafe modifiers:
bpf_type_has_unsafe_modifiers(reg->type) && !is_rcu_reg(reg)

However, is_trusted_reg() unconditionally returns true for all referenced
pointers. If a BPF program allocates a per-CPU object using
bpf_percpu_obj_new(), it receives a referenced pointer with the
PTR_TO_BTF_ID | MEM_ALLOC | MEM_PERCPU type.

Since is_trusted_reg() will trust this referenced pointer, the unsafe
modifier check appears to be bypassed. If this pointer is then passed to a
generic kfunc expecting a standard kernel pointer, could dereferencing the
raw per-CPU pointer (which acts as an offset rather than a virtual address)
trigger a page fault and crash the kernel?

> +				if (ret < 0)
> +					return ret;
> +				break;
> +			}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723050806.1158442-1-ameryhung@gmail.com?part=15