Re: [PATCH] kallsyms: let kallsyms_on_each_match_symbol match symbols exactly

[Song Liu <songliubraving@meta.com>]

> On Jun 16, 2023, at 1:52 AM, Leizhen (ThunderTown) <thunder.leizhen@huawei.com> wrote:
> 
> 
> 
> On 2023/6/16 16:43, Leizhen (ThunderTown) wrote:
>> 
>> 
>> On 2023/6/16 16:11, Leizhen (ThunderTown) wrote:
>>> 
>>> 
>>> On 2023/6/16 13:01, Song Liu wrote:
>>>> 
>>>> 
>>>>> On Jun 15, 2023, at 7:19 PM, Leizhen (ThunderTown) <thunder.leizhen@huawei.com> wrote:
>>>>> 
>>>>> On 2023/6/16 1:00, Song Liu wrote:
>>>>>> With CONFIG_LTO_CLANG, kallsyms.c:cleanup_symbol_name() removes symbols
>>>>>> suffixes during comparison. This is problematic for livepatch, as
>>>>>> kallsyms_on_each_match_symbol may find multiple matches for the same
>>>>>> symbol, and fail with:
>>>>>> 
>>>>>> livepatch: unresolvable ambiguity for symbol 'xxx' in object 'yyy'
>>>>> 
>>>>> Did you forget to specify 'old_sympos'? When there are multiple symbols with
>>>>> the same name, we need to specify the sequence number of the symbols to be
>>>>> matched.
>>>> 
>>>> 
>>>> old_sympos is indeed 0 here. However, the issue with CONFIG_LTO_CLANG 
>>>> is different. Here is an example:
>>>> 
>>>> $ grep bpf_verifier_vlog /proc/kallsyms
>>>> ffffffff81549f60 t bpf_verifier_vlog
>>>> ffffffff8268b430 d bpf_verifier_vlog._entry
>>>> ffffffff8282a958 d bpf_verifier_vlog._entry_ptr
>>>> ffffffff82e12a1f d bpf_verifier_vlog.__already_done
>>>> 
>>>> kallsyms_on_each_match_symbol matches "bpf_verifier_vlog" to all of 
>>>> these because of cleanup_symbol_name(). IOW, we only have one 
>>>> function called bpf_verifier_vlog, but kallsyms_on_each_match_symbol() 
>>>> matches it to bpf_verifier_vlog.*. 
>>>> 
>>>> Does this make sense?
>>> 
>>> Sorry. I mistakenly thought you were operating a static function.
>>> 
>>> These suffixes are not mentioned in the comments in the function
>>> cleanup_symbol_name(). So I didn't notice it.
>> We can keep these three suffixes on the kallsyms tool end.
> 
> And modify cleanup_symbol_name() not to cleanup these three suffixes.

I think livepatch should match symbols exactly anyway, so it is not 
necessary to expose more details in cleanup_symbol_name()?

Thanks,
Song