Re: [PATCH bpf-next v2 06/18] selftests/bpf: Add tests for helper and kfunc mem+size arguments

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Incorrect argument name formatting in verifier log for map pointers passed on the stack in `process_map_ptr_arg()`.
--

--- Patch [6]: [PATCH bpf-next v2 06/18] selftests/bpf: Add tests for helper and kfunc mem+size arguments ---
commit 26d0efc0fe7fbd940204152f62931d959ef53578
Author: Amery Hung <ameryhung@gmail.com>

selftests/bpf: Add tests for helper and kfunc mem+size arguments

This commit adds tests to ensure that the return of a map-of-maps lookup
cannot be passed to a nullable memory buffer argument for helpers and kfuncs.
It verifies that both mapofmaps_value_as_kfunc_mem_buf and
mapofmaps_value_as_helper_mem_buf are properly rejected by the verifier.
This guarantees that helper and kfunc arguments are checked in the same way.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 52be0a118cce0..2e56f726c12a8 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -8286,6 +8267,44 @@ static int get_constant_map_key(struct bpf_verifier_env *env,
>  
>  static bool can_elide_value_nullness(const struct bpf_map *map);
>  
> +static int process_map_ptr_arg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
> +			       argno_t argno, struct bpf_call_arg_meta *meta)
> +{
[ ... ]
> +	if (meta->map.ptr &&
> +	    (meta->map.ptr != reg->map_ptr || meta->map.uid != reg->map_uid)) {
> +		argno_t obj_argno = argno_from_reg(reg_from_argno(argno) - 1);

[Severity: Low]
If the map argument is passed on the stack, does this incorrectly calculate
the preceding argument's argno? It appears that this can result in an invalid
internal representation for obj_argno.

> +		struct btf_record *rec = meta->map.ptr->record;
> +		const char *obj_name = "workqueue";
> +
> +		if (rec->timer_off >= 0)
> +			obj_name = "timer";
> +		else if (rec->task_work_off >= 0)
> +			obj_name = "bpf_task_work";
> +
> +		verbose(env, "%s pointer in %s map_uid=%d ",
> +			obj_name, reg_arg_name(env, obj_argno), meta->map.uid);

[Severity: Low]
Does this use the incorrectly computed obj_argno, causing reg_arg_name() to
print "R2" in the verifier log instead of the actual register or stack
location?

> +		verbose(env, "doesn't match map pointer in %s map_uid=%d\n",
> +			reg_arg_name(env, argno), reg->map_uid);
> +		return -EINVAL;
> +	}
> +
> +	meta->map.ptr = reg->map_ptr;
> +	meta->map.uid = reg->map_uid;
> +	return 0;
> +}

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