> On Jun 9, 2023, at 12:09 AM, Petr Mladek <pmladek@xxxxxxxx> wrote:
>
> On Fri 2023-06-02 16:24:01, 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'
>>
>> Fix this by using kallsyms_on_each_symbol instead, and matching symbols
>> exactly.
>>
>> --- a/kernel/livepatch/core.c
>> +++ b/kernel/livepatch/core.c
>> @@ -166,7 +159,7 @@ static int klp_find_object_symbol(const char *objname, const char *name,
>> if (objname)
>> module_kallsyms_on_each_symbol(objname, klp_find_callback, &args);
>> else
>> - kallsyms_on_each_match_symbol(klp_match_callback, name, &args);
>> + kallsyms_on_each_symbol(klp_find_callback, &args);
>
> AFAIK, you have put a lot of effort to optimize the search recently.
> The speedup was amazing, see commit 4dc533e0f2c04174e1ae
> ("kallsyms: Add helper kallsyms_on_each_match_symbol()").
>
That's not me. :) Or maybe you meant Josh?
> Do we really need to waste this effort completely?
>
> What about creating variants:
>
> + kallsyms_on_each_match_exact_symbol()
> + kallsyms_lookup_exact_names()
> + compare_exact_symbol_name()
>
> Where compare_exact_symbol_name() would not try comparing with
> cleanup_symbol_name()?
The rationale is that livepatch symbol look up is not a hot path,
and the changes (especially with kallsyms_lookup_exact_names) seem
an overkill. OTOH, this version is simpler and should work just as
well.
Thanks,
Song
>
>>
>> /*
>> * Ensure an address was found. If sympos is 0, ensure symbol is unique;
>
> Otherwise, the patch looks fine. It is acceptable for me. I just want
> to be sure that we considered the above alternative solution.
>
> Best Regards,
> Petr
