Re: [PATCH bpf-next v4 07/12] bpf: Unify referenced object tracking in verifier

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The verifier uses `verifier_bug()` to handle ambiguous referenced object arguments, allowing user input to potentially trigger kernel warnings.
--

commit e4dc51a4e8f54da451d3c9dcfb864ea9abe239c6
Author: Amery Hung <ameryhung@gmail.com>

    bpf: Unify referenced object tracking in verifier

This commit unifies the reference tracking logic used by helpers and kfuncs
into a common ref_obj_desc structure. It updates tracking mechanisms and 
pushes validation of ambiguous referenced objects out to their use locations.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 792e61e07b065..542912c7983f7 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -713,6 +700,13 @@ static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_
>  				return ref_obj_id;
>  		}
>  		/* Track parent's id if the parent is a referenced object */
> +		if (ref_obj && ref_obj->ref_obj_id) {
> +			if (ref_obj->cnt > 1) {
> +				verifier_bug(env, "function expects only one referenced object but got %d\n", ref_obj->cnt);
> +				return -EFAULT;
> +			}
> +			parent_id = ref_obj->id;
> +		}

Since ref_obj->cnt is determined by the number of referenced arguments
provided by the BPF program, is it possible for a user to intentionally
trigger this verifier_bug() by passing multiple referenced objects?

Since verifier_bug() expands to BPF_WARN_ONCE(), triggering a warning
via unprivileged or user-controlled input could be problematic if
panic_on_warn is enabled.

Would it be safer to gracefully reject the program using verbose() and
returning -EINVAL, similar to how it is handled for release kfuncs earlier
in this patch?

This same pattern appears in a few other places introduced by this patch:

[ ... ]
> @@ -10165,15 +10160,14 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
>  	if (type_may_be_null(regs[BPF_REG_0].type))
>  		regs[BPF_REG_0].id = ++env->id_gen;
>  
> -	if (helper_multiple_ref_obj_use(func_id, meta.map.ptr)) {
> -		verifier_bug(env, "func %s#%d sets ref_obj_id more than once",
> -			     func_id_name(func_id), func_id);
> -		return -EFAULT;
> -	}
> -
>  	if (is_ptr_cast_function(func_id)) {
>  		/* For release_reference() */
> -		regs[BPF_REG_0].ref_obj_id = meta.ref_obj_id;
> +		if (meta.ref_obj.cnt > 1) {
> +			verifier_bug(env, "function expects only one referenced object but got %d\n",
> +				     meta.ref_obj.cnt);
> +			return -EFAULT;
> +		}
> +		regs[BPF_REG_0].ref_obj_id = meta.ref_obj.ref_obj_id;

[ ... ]
> @@ -12740,8 +12733,14 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>  				regs[BPF_REG_0].type |= MEM_RDONLY;
>  
>  			/* Ensures we don't access the memory after a release_reference() */
> -			if (meta.ref_obj_id)
> -				regs[BPF_REG_0].parent_id = meta.ref_obj_id;
> +			if (meta.ref_obj.ref_obj_id) {
> +				if (meta.ref_obj.cnt > 1) {
> +					verifier_bug(env, "function expects only one referenced object but got %d\n",
> +						     meta.ref_obj.cnt);
> +					return -EFAULT;
> +				}
> +				regs[BPF_REG_0].parent_id = meta.ref_obj.ref_obj_id;
> +			}

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