[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.LSU.2.21.2406281420590.15826@pobox.suse.cz>
Date: Fri, 28 Jun 2024 14:23:49 +0200 (CEST)
From: Miroslav Benes <mbenes@...e.cz>
To: Song Liu <song@...nel.org>
cc: live-patching@...r.kernel.org, linux-kernel@...r.kernel.org,
jpoimboe@...nel.org, jikos@...nel.org, pmladek@...e.com,
joe.lawrence@...hat.com, nathan@...nel.org, morbo@...gle.com,
justinstitt@...gle.com, mcgrof@...nel.org, thunder.leizhen@...wei.com,
kees@...nel.org, kernel-team@...a.com
Subject: Re: [PATCH] kallsyms, livepatch: Fix livepatch with
CONFIG_LTO_CLANG
On Fri, 7 Jun 2024, Song Liu wrote:
> Hi Miroslav,
>
> Thanks for reviewing the patch!
>
> On Fri, Jun 7, 2024 at 6:06 AM Miroslav Benes <mbenes@...e.cz> wrote:
> >
> > Hi,
> >
> > On Tue, 4 Jun 2024, Song Liu wrote:
> >
> > > With CONFIG_LTO_CLANG, the compiler may postfix symbols with .llvm.<hash>
> > > to avoid symbol duplication. scripts/kallsyms.c sorted the symbols
> > > without these postfixes. The default symbol lookup also removes these
> > > postfixes before comparing symbols.
> > >
> > > On the other hand, livepatch need to look up symbols with the full names.
> > > However, calling kallsyms_on_each_match_symbol with full name (with the
> > > postfix) cannot find the symbol(s). As a result, we cannot livepatch
> > > kernel functions with .llvm.<hash> postfix or kernel functions that use
> > > relocation information to symbols with .llvm.<hash> postfixes.
> > >
> > > Fix this by calling kallsyms_on_each_match_symbol without the postfix;
> > > and then match the full name (with postfix) in klp_match_callback.
> > >
> > > Signed-off-by: Song Liu <song@...nel.org>
> > > ---
> > > include/linux/kallsyms.h | 13 +++++++++++++
> > > kernel/kallsyms.c | 21 ++++++++++++++++-----
> > > kernel/livepatch/core.c | 32 +++++++++++++++++++++++++++++++-
> > > 3 files changed, 60 insertions(+), 6 deletions(-)
> >
> > I do not like much that something which seems to be kallsyms-internal is
> > leaked out. You need to export cleanup_symbol_name() and there is now a
> > lot of code outside. I would feel much more comfortable if it is all
> > hidden from kallsyms users and kept there. Would it be possible?
>
> I think it is possible. Currently, kallsyms_on_each_match_symbol matches
> symbols without the postfix. We can add a variation or a parameter, so
> that it matches the full name with post fix.
I think it might be better.
Luis, what is your take on this?
> > Moreover, isn't there a similar problem for ftrace, kprobes, ebpf,...?
>
> Yes, there is a similar problem with tracing use cases. But the requirements
> are not the same:
>
> For livepatch, we have to point to the exact symbol we want to patch or
> relocation to. We have sympos API defined to differentiate different symbols
> with the same name.
Yes. In fact, sympos may be used to solve even this problem. The user
would disregard .llvm.<hash> suffix and they are suddenly in the same
situation which sympos aims to solve. I will not argue with you if say it
is cumbersome.
> For tracing, some discrepancy is acceptable. AFAICT, there isn't an API
> similar to sympos yet. Also, we can play some tricks with tracing. For
> example, we can use "uniq symbol + offset" to point a kprobe to one of
> the duplicated symbols.
If I am not mistaken, there was a patch set to address this. Luis might
remember more.
Regards,
Miroslav
Powered by blists - more mailing lists