Re: [RFC PATCH bpf-next v3 1/2] libbpf: support selective kernel module BTF loading via bpf_object_open_opts

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Premature loop termination in load_module_btfs when kernel contains multiple BTF objects for the same module.
- [Low] API design prevents users from explicitly skipping all kernel module BTFs without providing a dummy name.
--

--- Patch [1]: [RFC PATCH bpf-next v3 1/2] libbpf: support selective kernel module BTF loading via bpf_object_open_opts ---
commit 627a7f9dbb55055d6c4d1331859084b6d2f56792
Author: Fuyu Zhao <zhaofuyu@vivo.com>

libbpf: support selective kernel module BTF loading via bpf_object_open_opts

Add kmod_btf_names and kmod_btf_names_cnt fields to bpf_object_open_opts
to support selective kernel module BTF loading. This allows libbpf to
load only specified module BTFs and skip others.

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index b749c01742ee0..957415341caf1 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c

[ ... ]

> @@ -5851,6 +5858,121 @@ int bpf_core_add_cands(struct bpf_core_cand *local_cand,
>  	return 0;
>  }

[ ... ]

> +static int bpf_object__init_kmod_btfs(struct bpf_object *obj,
> +				      const struct bpf_object_open_opts *opts)
> +{
> +	const char **kmod_btf_names;
> +	size_t i, kmod_btf_names_cnt;
> +	int err;
> +
> +	kmod_btf_names = OPTS_GET(opts, kmod_btf_names, NULL);
> +	if (!kmod_btf_names)
> +		return 0;
> +
> +	kmod_btf_names_cnt = OPTS_GET(opts, kmod_btf_names_cnt, 0);
> +	if (!kmod_btf_names_cnt) {
> +		pr_warn("kmod_btf_names_cnt must be set when kmod_btf_names is provided\n");
> +		return -EINVAL;
> +	}

[Severity: Low]
If a user wants to optimize BPF object loading by skipping all module BTFs
(for instance, when only vmlinux BTF is needed), setting kmod_btf_names to NULL
falls back to the default behavior of loading all modules, but providing a
valid pointer with kmod_btf_names_cnt = 0 returns -EINVAL.

Does this API design force users to allocate and pass a dummy module name just
to skip loading module BTFs entirely?

> +
> +	obj->kmod_btfs = calloc(1, sizeof(*obj->kmod_btfs));

[ ... ]

> +static bool all_needed_kmod_btfs_loaded(const struct bpf_object *obj)
> +{
> +	return obj->kmod_btfs &&
> +	       obj->kmod_btfs->nr_names > 0 &&
> +	       obj->kmod_btfs->nr_names == obj->btf_module_cnt;
> +}

[Severity: Medium]
If a kernel module is unloaded but its BTF object is kept alive by an open file
descriptor, and the module is then reloaded, the kernel will contain multiple
BTF objects with the same module name.

Because btf_module_cnt increments for every loaded BTF object, but nr_names
tracks the number of unique requested module names, can duplicate BTF objects
inflate btf_module_cnt?

If a user requests BTFs for multiple modules via kmod_btf_names, would this
condition evaluate to true prematurely and skip the remaining requested modules?

> +
>  static int load_module_btfs(struct bpf_object *obj)
>  {

[ ... ]

> @@ -5939,6 +6066,9 @@ static int load_module_btfs(struct bpf_object *obj)
>  			break;
>  		}
>  		obj->btf_module_cnt++;
> +
> +		if (all_needed_kmod_btfs_loaded(obj))
> +			break;
>  	}
>  
>  	if (err) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819090426.267-1-zhaofuyu@vivo.com?part=1