On Sun, Jun 18, 2023 at 8:32 PM Leizhen (ThunderTown)
<thunder.leizhen@xxxxxxxxxx> wrote:
>
>
>
> On 2023/6/17 1:37, Song Liu wrote:
> >
> >
> >> On Jun 16, 2023, at 2:31 AM, Leizhen (ThunderTown) <thunder.leizhen@xxxxxxxxxx> 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@xxxxxxxxxx>
> >>> ---
> >>> 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]+
> */
I think livepatch will not fail, as the tool chain should already match the
suffix for the function being patched. If the tool chain failed to do so,
livepatch can fail for other reasons (missing symbols, etc.)
Thanks,
Song
