Re: [PATCH v4] btf: reject to register duplicated kfunc

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Kfunc registration incorrectly fails if the kfunc name matches ANY internal or static function in vmlinux or loaded modules, introducing load-order dependencies.
--

commit 8a8189baf0633350e5ca2b793c2340e48f4787df
Author: Song Chen <chensong_2000@126.com>

btf: reject to register duplicated kfunc

This commit attempts to prevent duplicate BPF kfunc registrations by searching
for existing kfunc names in vmlinux and other modules' BTF data during kfunc
registration.

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index a62d785812076..ad6b925125dee 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -8684,6 +8684,41 @@ static int btf_check_iter_kfuncs(struct btf *btf, const char *func_name,
>  	return 0;
>  }
>  
> +static int btf_check_kfunc_name(struct btf *btf, const char *func_name, u32 kind)
> +{
> +#ifdef CONFIG_DEBUG_INFO_BTF_MODULES
> +	struct btf_module *btf_mod, *tmp;
> +#endif
> +	s32 id;
> +
> +	if (!btf_is_module(btf))
> +		return 0;

[Severity: High]
Does this bypass collision checks for built-in vmlinux kfuncs? It looks like
this might fail to catch intra-kernel name collisions.

> +
> +	id = btf_find_by_name_kind(bpf_get_btf_vmlinux(), func_name, kind);

[Severity: High]
Since modern kernels generate BTF for all global and static functions, does
btf_find_by_name_kind() with BTF_KIND_FUNC search the entire BTF data rather
than just registered kfuncs?

Will this incorrectly reject a valid kfunc registration if its name happens
to match any unrelated static or internal function anywhere in vmlinux?

> +	if (id >= 0) {
> +		pr_err("kfunc %s (id: %d) is already present in vmlinux.\n",
> +				func_name, id);
> +		return -EINVAL;
> +	}
> +
> +#ifdef CONFIG_DEBUG_INFO_BTF_MODULES
> +	mutex_lock(&btf_module_mutex);
> +	list_for_each_entry_safe(btf_mod, tmp, &btf_modules, list) {
> +		if (btf_mod->btf == btf)
> +			continue;

[Severity: High]
Are we skipping collision detection within the module's own BTF here?

> +		id = btf_find_by_name_kind(btf_mod->btf, func_name, kind);

[Severity: High]
Can this create unpredictable load-order dependencies between unrelated
modules?

If module A has a static function named "foo", and module B registers a
kfunc named "foo", it appears B will fail to load if A is already loaded,
but will succeed if B is loaded first. Should this code check against actually
registered kfunc sets rather than all functions in every module's BTF?

> +		if (id >= 0) {
> +			pr_err("kfunc %s (id: %d) is already present in module %s.\n",
> +					func_name, id, btf_mod->module->name);
> +			mutex_unlock(&btf_module_mutex);
> +			return -EINVAL;
> +		}
> +	}
> +	mutex_unlock(&btf_module_mutex);
> +#endif
> +	return 0;
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602110710.19642-1-chensong_2000@126.com?part=1