Re: [PATCH bpf-next v2 03/15] bpf: Add bpf_struct_ops accessor helpers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The newly added bpf_struct_ops_kdata_map_id function is an exact duplicate of the pre-existing bpf_struct_ops_id function.
--

commit d1f80ae7e459b35d14e3b364a031f8d29bf9da8e
Author: Martin KaFai Lau <martin.lau@kernel.org>

bpf: Add bpf_struct_ops accessor helpers

This commit adds several helper functions in bpf_struct_ops.c to be used
for creating a struct_ops to cgroup attachment link. It also refactors
link update validation into a reusable function.

> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
> index c422ce41873ee..1ca44584ed17c 100644
> --- a/kernel/bpf/bpf_struct_ops.c
> +++ b/kernel/bpf/bpf_struct_ops.c

[ ... ]

> @@ -1236,7 +1236,33 @@ int bpf_struct_ops_for_each_prog(const void *kdata,
>  }
>  EXPORT_SYMBOL_GPL(bpf_struct_ops_for_each_prog);
>  
> -static bool bpf_struct_ops_valid_to_reg(struct bpf_map *map)
> +void *bpf_struct_ops_map_kdata(struct bpf_map *map)
> +{
> +	struct bpf_struct_ops_map *st_map;
> +
> +	st_map = container_of(map, struct bpf_struct_ops_map, map);
> +	return st_map->kvalue.data;
> +}
> +
> +u32 bpf_struct_ops_kdata_map_id(void *kdata)
> +{
> +	struct bpf_struct_ops_value *kvalue =
> +		container_of(kdata, struct bpf_struct_ops_value, data);
> +	struct bpf_struct_ops_map *st_map =
> +		container_of(kvalue, struct bpf_struct_ops_map, kvalue);
> +
> +	return st_map->map.id;
> +}

[Severity: Low]
This isn't a bug, but does bpf_struct_ops_kdata_map_id() duplicate the
pre-existing bpf_struct_ops_id() function?

Looking at bpf_struct_ops_id() which is already exported in the same file:

u32 bpf_struct_ops_id(const void *kdata)
{
	struct bpf_struct_ops_value *kvalue;
	struct bpf_struct_ops_map *st_map;

	kvalue = container_of(kdata, struct bpf_struct_ops_value, data);
	st_map = container_of(kvalue, struct bpf_struct_ops_map, kvalue);

	return st_map->map.id;
}

Both functions perform the exact same series of container_of macros to retrieve
the id from the map field of a bpf_struct_ops_map. The only difference is the
parameter type (void * vs const void *). Could the external caller directly
invoke the already exported bpf_struct_ops_id() instead of adding this new
helper?

> +
> +void *bpf_struct_ops_map_cfi_stubs(struct bpf_map *map)
> +{
> +	struct bpf_struct_ops_map *st_map;
> +
> +	st_map = container_of(map, struct bpf_struct_ops_map, map);
> +	return st_map->st_ops_desc->st_ops->cfi_stubs;
> +}

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