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

[Song Chen <chensong_2000@126.com>]

Hi,

在 2026/6/3 01:13, Yonghong Song 写道:
> 
> 
> On 6/2/26 4:07 AM, Song Chen wrote:
>> I had an ebpf program which calls a kfunc defined and
>> implemented in one of my kernel modules, it was working
>> fine in 6.16, but was rejected to run by libbpf in 6.19,
>> the error message was:
>>
>> libbpf: extern (func ksym) 'bpf_strstr': func_proto [5]
>> incompatible with vmlinux [94389]
>>
>> The reason is there is a new added kfunc in kernel 6.19
>> which happens to be the same name with my kfunc. However the
>> error message is not obvious enough to address problem
>> immediately.
>>
>> Therefore, this patches searches duplicated kfunc in
>> both btf_vmlinux and btf_modules list before a kernel module
>> attempts to register kfuncs through register_btf_kfunc_id_set,
>> it prints clear error message and returns error code if same name
>> kfunc has already implemented and registered, then developer
>> knows at the first place.
>>
>> Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
>> Suggested-by: Kaitao Cheng <kaitao.cheng@linux.dev>
>> Reviewed-by: Yonghong Song <yonghong.song@linux.dev>
>> Signed-off-by: Song Chen <chensong_2000@126.com>
> 
> The subject can be [PATCH bpf v4] bpf: Reject to register duplicated kfunc
> 
>>
>> ---
>> changelog:
>> v1 --- v2:
>> libbpf has already specified which module this kfunc belongs to as
>> ebpf code onwer's expectation, then verifier uses
>> find_kallsyms_symbol_value to search kfunc's addr.
>>
>> v2 --- v3:
>> After v2, I tried a new idea of introducing a namespace in libbpf
>> to specify kfunc owner in an ebpf program suggested by Kaitao Cheng,
>> please see [1]. Alex suggested to go back to report an error during
>> kmod load on conflicting kfunc name for now. What's more, v2 only
>> searched bpf_vmlinux, v3 also traverses btf_modules list.
>>
>> v3 --- v4
>> Fixed some coding style problems suggested from Kaitao Cheng.
>> ---
>>   kernel/bpf/btf.c | 38 +++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 37 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
>> index 4872d2a6c42d..fe1612677a4a 100644
>> --- a/kernel/bpf/btf.c
>> +++ b/kernel/bpf/btf.c
>> @@ -8618,6 +8618,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;
> 
>      s32 err = 0, id;
> 
>> +
>> +    if (!btf_is_module(btf))
>> +        return 0;
>> +
>> +    id = btf_find_by_name_kind(bpf_get_btf_vmlinux(), func_name, kind);
>> +    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;
>> +        id = btf_find_by_name_kind(btf_mod->btf, func_name, kind);
>> +        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;
> 
> Let us avoid the above mutex_unlock and 'return -EINVAL', just do
>      err = -EINVAL;
>      break;
> 
>> +        }
>> +    }
>> +    mutex_unlock(&btf_module_mutex);
>> +#endif
>> +    return 0;
> 
>      return err;
> 

I will go with guard(mutex), please review my next submit and let me 
know if you're ok with it or not. Thanks.

>> +}
>> +
>>   static int btf_check_kfunc_protos(struct btf *btf, u32 func_id, u32 
>> func_flags)
>>   {
>>       const struct btf_type *func;
>> @@ -8631,7 +8666,8 @@ static int btf_check_kfunc_protos(struct btf 
>> *btf, u32 func_id, u32 func_flags)
>>       /* sanity check kfunc name */
>>       func_name = btf_name_by_offset(btf, func->name_off);
>> -    if (!func_name || !func_name[0])
>> +    if (!func_name || !func_name[0] ||
>> +        btf_check_kfunc_name(btf, func_name, BTF_INFO_KIND(func->info)))
> 
> format issue:
>      if (!func_name || !func_name[0] ||
>          btf_check_kfunc_name(btf, func_name, BTF_INFO_KIND(func->info)))
>      ...
> 

I will fix it with indentation plus space.

>>           return -EINVAL;
>>       func = btf_type_by_id(btf, func->type);
> 


many thanks.

Song