kallsyms: Fix kallsyms_selftest failure

JIRA: https://issues.redhat.com/browse/RHEL-28063

commit 33f0467fe06934d5e4ea6e24ce2b9c65ce618e26
Author: Yonghong Song <yonghong.song@linux.dev>
Date:   Thu Aug 24 20:46:59 2023 -0700

    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 8cc32a9bbf29 ("kallsyms: strip LTO-only suffixes
    from promoted global functions") caused the failure. Commit 8cc32a9bbf29
    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: 8cc32a9bbf29 ("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>

Signed-off-by: Donald Dutile <ddutile@redhat.com>
This commit is contained in:
Donald Dutile 2024-05-09 01:54:09 -04:00
parent 233e65a792
commit 37831f8d36
2 changed files with 8 additions and 32 deletions

View File

@ -186,16 +186,13 @@ static inline bool cleanup_symbol_name(char *s) { return false; }
static int compare_symbol_name(const char *name, char *namebuf)
{
int ret;
ret = strcmp(name, namebuf);
if (!ret)
return ret;
if (cleanup_symbol_name(namebuf) && !strcmp(name, namebuf))
return 0;
return ret;
/* The kallsyms_seqs_of_names is sorted based on names after
* cleanup_symbol_name() (see scripts/kallsyms.c) if clang lto is enabled.
* To ensure correct bisection in kallsyms_lookup_names(), do
* cleanup_symbol_name(namebuf) before comparing name and namebuf.
*/
cleanup_symbol_name(namebuf);
return strcmp(name, namebuf);
}
static unsigned int get_symbol_seq(int index)

View File

@ -196,7 +196,7 @@ static bool match_cleanup_name(const char *s, const char *name)
if (!IS_ENABLED(CONFIG_LTO_CLANG))
return false;
p = strchr(s, '.');
p = strstr(s, ".llvm.");
if (!p)
return false;
@ -345,27 +345,6 @@ static int test_kallsyms_basic_function(void)
goto failed;
}
/*
* The first '.' may be the initial letter, in which case the
* entire symbol name will be truncated to an empty string in
* cleanup_symbol_name(). Do not test these symbols.
*
* For example:
* cat /proc/kallsyms | awk '{print $3}' | grep -E "^\." | head
* .E_read_words
* .E_leading_bytes
* .E_trailing_bytes
* .E_write_words
* .E_copy
* .str.292.llvm.12122243386960820698
* .str.24.llvm.12122243386960820698
* .str.29.llvm.12122243386960820698
* .str.75.llvm.12122243386960820698
* .str.99.llvm.12122243386960820698
*/
if (IS_ENABLED(CONFIG_LTO_CLANG) && !namebuf[0])
continue;
lookup_addr = kallsyms_lookup_name(namebuf);
memset(stat, 0, sizeof(*stat));