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

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

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.

> 
> 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))
>  		return 0;
>  
>  	return ret;
> @@ -213,7 +213,8 @@ static unsigned int get_symbol_seq(int index)
>  
>  static int kallsyms_lookup_names(const char *name,
>  				 unsigned int *start,
> -				 unsigned int *end)
> +				 unsigned int *end,
> +				 bool match_exactly)
>  {
>  	int ret;
>  	int low, mid, high;
> @@ -228,7 +229,7 @@ static int kallsyms_lookup_names(const char *name,
>  		seq = get_symbol_seq(mid);
>  		off = get_symbol_offset(seq);
>  		kallsyms_expand_symbol(off, namebuf, ARRAY_SIZE(namebuf));
> -		ret = compare_symbol_name(name, namebuf);
> +		ret = compare_symbol_name(name, namebuf, match_exactly);
>  		if (ret > 0)
>  			low = mid + 1;
>  		else if (ret < 0)
> @@ -245,7 +246,7 @@ static int kallsyms_lookup_names(const char *name,
>  		seq = get_symbol_seq(low - 1);
>  		off = get_symbol_offset(seq);
>  		kallsyms_expand_symbol(off, namebuf, ARRAY_SIZE(namebuf));
> -		if (compare_symbol_name(name, namebuf))
> +		if (compare_symbol_name(name, namebuf, match_exactly))
>  			break;
>  		low--;
>  	}
> @@ -257,7 +258,7 @@ static int kallsyms_lookup_names(const char *name,
>  			seq = get_symbol_seq(high + 1);
>  			off = get_symbol_offset(seq);
>  			kallsyms_expand_symbol(off, namebuf, ARRAY_SIZE(namebuf));
> -			if (compare_symbol_name(name, namebuf))
> +			if (compare_symbol_name(name, namebuf, match_exactly))
>  				break;
>  			high++;
>  		}
> @@ -277,7 +278,7 @@ unsigned long kallsyms_lookup_name(const char *name)
>  	if (!*name)
>  		return 0;
>  
> -	ret = kallsyms_lookup_names(name, &i, NULL);
> +	ret = kallsyms_lookup_names(name, &i, NULL, false);
>  	if (!ret)
>  		return kallsyms_sym_address(get_symbol_seq(i));
>  
> @@ -312,7 +313,7 @@ int kallsyms_on_each_match_symbol(int (*fn)(void *, unsigned long),
>  	int ret;
>  	unsigned int i, start, end;
>  
> -	ret = kallsyms_lookup_names(name, &start, &end);
> +	ret = kallsyms_lookup_names(name, &start, &end, true);
>  	if (ret)
>  		return 0;
>  
> 

-- 
Regards,
  Zhen Lei