linux-kernel - Re: [PATCH] kallsyms, livepatch: Fix livepatch with CONFIG_LTO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zn70rQE1HkJ_2h6r@bombadil.infradead.org>
Date: Fri, 28 Jun 2024 10:36:45 -0700
From: Luis Chamberlain <mcgrof@...nel.org>
To: Miroslav Benes <mbenes@...e.cz>,
	Sami Tolvanen <samitolvanen@...gle.com>
Cc: Song Liu <song@...nel.org>, 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,
	thunder.leizhen@...wei.com, kees@...nel.org, kernel-team@...a.com
Subject: Re: [PATCH] kallsyms, livepatch: Fix livepatch with CONFIG_LTO_CLANG

On Fri, Jun 28, 2024 at 02:23:49PM +0200, Miroslav Benes wrote:
> 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?
> 
> If I am not mistaken, there was a patch set to address this. Luis might 
> remember more.

Yeah this is a real issue outside of CONFIG_LTO_CLANG, Rust modules is
another example where instead of symbol names they want to use full
hashes. So, as I hinted to you Sami, can we knock two birds with one stone
here and move CONFIG_LTO_CLANG to use the same strategy as Rust so we
have two users instead of just one? Then we resolve this. In fact
what I suggested was even to allow even non-Rust, and in this case
even with gcc to enable this world. This gives much more wider scope
of testing / review / impact of these sorts of changes and world view
and it would resolve the Rust case, the live patch CONFIG_LTO_CLANG world too.

Thoughts?

  Luis