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

["Leizhen (ThunderTown)" <thunder.leizhen@huawei.com>]

On 2023/6/17 1:37, Song Liu wrote:
> 
> 
>> On Jun 16, 2023, at 2:31 AM, 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'
>>>
>>> Make kallsyms_on_each_match_symbol() to match symbols exactly. Since
>>> livepatch is the only user of kallsyms_on_each_match_symbol(), this
>>> change is safe.
>>>
>>> Signed-off-by: Song Liu <song@kernel.org>
>>> ---
>>> kernel/kallsyms.c | 17 +++++++++--------
>>> 1 file changed, 9 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
>>> index 77747391f49b..2ab459b43084 100644
>>> --- a/kernel/kallsyms.c
>>> +++ b/kernel/kallsyms.c
>>> @@ -187,7 +187,7 @@ static bool cleanup_symbol_name(char *s)
>>> return false;
>>> }
>>>
>>> -static int compare_symbol_name(const char *name, char *namebuf)
>>> +static int compare_symbol_name(const char *name, char *namebuf, bool match_exactly)
>>> {
>>> int ret;
>>>
>>> @@ -195,7 +195,7 @@ static int compare_symbol_name(const char *name, char *namebuf)
>>> if (!ret)
>>> return ret;
>>>
>>> - if (cleanup_symbol_name(namebuf) && !strcmp(name, namebuf))
>>> + if (!match_exactly && cleanup_symbol_name(namebuf) && !strcmp(name, namebuf))
>>
>> This may affect the lookup of static functions.
> 
> I am not following why would this be a problem. Could you give an 
> example of it?

Here are the comments in cleanup_symbol_name(). If the compiler adds a suffix to the
static function, but we do not remove the suffix, will the symbol match fail?

	/*
	 * LLVM appends various suffixes for local functions and variables that
	 * must be promoted to global scope as part of LTO.  This can break
	 * hooking of static functions with kprobes. '.' is not a valid
	 * character in an identifier in C. Suffixes observed:
	 * - foo.llvm.[0-9a-f]+
	 * - foo.[0-9a-f]+
	 */

> 
> Thanks,
> Song
> 
> .
> 

-- 
Regards,
  Zhen Lei