Skip to content

Commit 33f0467

Browse files
Yonghong Songkees
authored andcommitted
kallsyms: Fix kallsyms_selftest failure
Kernel test robot reported a kallsyms_test failure when clang lto is enabled (thin or full) and CONFIG_KALLSYMS_SELFTEST is also enabled. I can reproduce in my local environment with the following error message with thin lto: [ 1.877897] kallsyms_selftest: Test for 1750th symbol failed: (tsc_cs_mark_unstable) addr=ffffffff81038090 [ 1.877901] kallsyms_selftest: abort It appears that commit 8cc32a9 ("kallsyms: strip LTO-only suffixes from promoted global functions") caused the failure. Commit 8cc32a9 changed cleanup_symbol_name() based on ".llvm." instead of '.' where ".llvm." is appended to a before-lto-optimization local symbol name. We need to propagate such knowledge in kallsyms_selftest.c as well. Further more, compare_symbol_name() in kallsyms.c needs change as well. In scripts/kallsyms.c, kallsyms_names and kallsyms_seqs_of_names are used to record symbol names themselves and index to symbol names respectively. For example: kallsyms_names: ... __amd_smn_rw._entry <== seq 1000 __amd_smn_rw._entry.5 <== seq 1001 __amd_smn_rw.llvm.<hash> <== seq 1002 ... kallsyms_seqs_of_names are sorted based on cleanup_symbol_name() through, so the order in kallsyms_seqs_of_names actually has index 1000: seq 1002 <== __amd_smn_rw.llvm.<hash> (actual symbol comparison using '__amd_smn_rw') index 1001: seq 1000 <== __amd_smn_rw._entry index 1002: seq 1001 <== __amd_smn_rw._entry.5 Let us say at a particular point, at index 1000, symbol '__amd_smn_rw.llvm.<hash>' is comparing to '__amd_smn_rw._entry' where '__amd_smn_rw._entry' is the one to search e.g., with function kallsyms_on_each_match_symbol(). The current implementation will find out '__amd_smn_rw._entry' is less than '__amd_smn_rw.llvm.<hash>' and then continue to search e.g., index 999 and never found a match although the actual index 1001 is a match. To fix this issue, let us do cleanup_symbol_name() first and then do comparison. In the above case, comparing '__amd_smn_rw' vs '__amd_smn_rw._entry' and '__amd_smn_rw._entry' being greater than '__amd_smn_rw', the next comparison will be > index 1000 and eventually index 1001 will be hit an a match is found. For any symbols not having '.llvm.' substr, there is no functionality change for compare_symbol_name(). Fixes: 8cc32a9 ("kallsyms: strip LTO-only suffixes from promoted global functions") Reported-by: kernel test robot <oliver.sang@intel.com> Closes: https://lore.kernel.org/oe-lkp/202308232200.1c932a90-oliver.sang@intel.com Signed-off-by: Yonghong Song <yonghong.song@linux.dev> Reviewed-by: Song Liu <song@kernel.org> Reviewed-by: Zhen Lei <thunder.leizhen@huawei.com> Link: https://lore.kernel.org/r/20230825034659.1037627-1-yonghong.song@linux.dev Cc: stable@vger.kernel.org Signed-off-by: Kees Cook <keescook@chromium.org>
1 parent 2ddd3ca commit 33f0467

File tree

2 files changed

+8
-32
lines changed

2 files changed

+8
-32
lines changed

kernel/kallsyms.c

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -188,16 +188,13 @@ static bool cleanup_symbol_name(char *s)
188188

189189
static int compare_symbol_name(const char *name, char *namebuf)
190190
{
191-
int ret;
192-
193-
ret = strcmp(name, namebuf);
194-
if (!ret)
195-
return ret;
196-
197-
if (cleanup_symbol_name(namebuf) && !strcmp(name, namebuf))
198-
return 0;
199-
200-
return ret;
191+
/* The kallsyms_seqs_of_names is sorted based on names after
192+
* cleanup_symbol_name() (see scripts/kallsyms.c) if clang lto is enabled.
193+
* To ensure correct bisection in kallsyms_lookup_names(), do
194+
* cleanup_symbol_name(namebuf) before comparing name and namebuf.
195+
*/
196+
cleanup_symbol_name(namebuf);
197+
return strcmp(name, namebuf);
201198
}
202199

203200
static unsigned int get_symbol_seq(int index)

kernel/kallsyms_selftest.c

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ static bool match_cleanup_name(const char *s, const char *name)
196196
if (!IS_ENABLED(CONFIG_LTO_CLANG))
197197
return false;
198198

199-
p = strchr(s, '.');
199+
p = strstr(s, ".llvm.");
200200
if (!p)
201201
return false;
202202

@@ -344,27 +344,6 @@ static int test_kallsyms_basic_function(void)
344344
goto failed;
345345
}
346346

347-
/*
348-
* The first '.' may be the initial letter, in which case the
349-
* entire symbol name will be truncated to an empty string in
350-
* cleanup_symbol_name(). Do not test these symbols.
351-
*
352-
* For example:
353-
* cat /proc/kallsyms | awk '{print $3}' | grep -E "^\." | head
354-
* .E_read_words
355-
* .E_leading_bytes
356-
* .E_trailing_bytes
357-
* .E_write_words
358-
* .E_copy
359-
* .str.292.llvm.12122243386960820698
360-
* .str.24.llvm.12122243386960820698
361-
* .str.29.llvm.12122243386960820698
362-
* .str.75.llvm.12122243386960820698
363-
* .str.99.llvm.12122243386960820698
364-
*/
365-
if (IS_ENABLED(CONFIG_LTO_CLANG) && !namebuf[0])
366-
continue;
367-
368347
lookup_addr = kallsyms_lookup_name(namebuf);
369348

370349
memset(stat, 0, sizeof(*stat));

0 commit comments

Comments
 (0)