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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <ZoxbEEsK40ASi1cY@bombadil.infradead.org>
Date: Mon, 8 Jul 2024 14:33:04 -0700
From: Luis Chamberlain <mcgrof@...nel.org>
To: Petr Mladek <pmladek@...e.com>, Sami Tolvanen <samitolvanen@...gle.com>,
	Gary Guo <gary@...yguo.net>, Masahiro Yamada <masahiroy@...nel.org>,
	Michal Suchánek <msuchanek@...e.de>,
	Lucas De Marchi <lucas.demarchi@...el.com>,
	Andreas Hindborg <nmi@...aspace.dk>
Cc: Josh Poimboeuf <jpoimboe@...nel.org>, Miroslav Benes <mbenes@...e.cz>,
	Sami Tolvanen <samitolvanen@...gle.com>, Song Liu <song@...nel.org>,
	live-patching@...r.kernel.org, linux-kernel@...r.kernel.org,
	jikos@...nel.org, 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 Thu, Jul 04, 2024 at 11:02:18AM +0200, Petr Mladek wrote:
> On Wed 2024-07-03 08:30:33, Luis Chamberlain wrote:
> > On Tue, Jul 02, 2024 at 10:56:41PM -0700, Josh Poimboeuf wrote:
> > > On Mon, Jul 01, 2024 at 03:13:23PM +0200, Petr Mladek wrote:
> > > > So, you suggest to search the symbols by a hash. Do I get it correctly?
> > 
> > I meant, that in the Rust world the symbols go over the allowed limit,
> > and so an alternative for them is to just use a hash. What I'm
> > suggesting is for a new kconfig option where that world is the
> > new one, so that they have to also do the proper userspace tooling
> > for it. Without that, I don't see it as properly tested or scalable.
> > And if we're gonna have that option for Rust for modules, then it begs
> > the question if this can be used by other users.
> 
> I am still not sure at which level the symbol names would get hashed ;-)

Let's clarify that. The Rust world mangles symbols to be very long. In
order to allow external modules built to be usable in other systems we support
modversioning. To help with this each module has an array of crcs of each
symbol it depends on. This is added to the module.mod.c so for my built
xfs.mod.c I have for example:

static const struct modversion_info ____versions[]
__used __section("__versions") = {
{ 0x5b8239ca, "__x86_return_thunk" },
{ 0x93a6e0b2, "io_schedule" }, 
{ 0x6383b27c, "__x86_indirect_thunk_rdx" },
{ 0xc5b6f236, "queue_work_on" },
{ 0xd966a5ea, "list_lru_walk_one" },
...
{ 0xe3565824, "vfs_readlink" },
{ 0xaed0dc33, "bdev_freeze" },
{ 0x7ce18c9f, "from_kqid" },
{ 0xd36dc10c, "get_random_u32" },
{ 0xba4edeaf, "__percpu_down_read" },
{ 0xb9f07b78, "file_modified" },
{ 0x48755bd1, "module_layout"
},
};

If a module depends on a symbol from a Rust module or Rust built-in
code, it cannot fit becuase we currenlty define:

struct modversion_info {                                                         
	unsigned long crc;                                                       
	char name[MODULE_NAME_LEN];                                              
};  

Although perhaps this started off as thing for module versions only,
clearly we're using it for all symbols a module uses so MODULE_NAME_LEN
really was in the end silly and does not suffice today.

A version of the effort from Rust folks last year tried to modify the
length but its claimed that the blocker for that was that userspace
would need to change, so their new attempt tried to work around that
using hashes...

Looking at this again its not to me why Masahiro Yamada's suggestion on
that old patch series to just increase the length and put long symbols
names into its own section [0] could not be embraced with a new kconfig
option, so new kernels and new userspace could support it:

https://lore.kernel.org/lkml/CAK7LNATsuszFR7JB5ZkqVS1W=hWr9=E7bTf+MvgJ+NXT3aZNwg@mail.gmail.com/

> The symbols names are used in many situations, e.g. backtraces,
> crashdump, objdump, nm, gdb, tracing, livepatching, kprobes, ...
> 
> Would kallsyms provide some translation table between the usual
> "long" symbol name and a hash?
> 
> Would it allows to search the symbols both ways?

Let's review the scope of both issues first, certainly Rust's issue is
limitted in scope, but since a new userspace change might be required
its worth reviewing if it may help the CONFIG_LTO_CLANG case as well.

> I am a bit scared because using hashed symbol names in backtraces, gdb,
> ... would be a nightmare. Hashes are not human readable and
> they would complicate the life a lot. And using different names
> in different interfaces would complicate the life either.

All great points.

The scope of the Rust issue is self contained to modversion_info,
whereas for CONFIG_LTO_CLANG issue commit 8b8e6b5d3b013b0
("kallsyms: strip ThinLTO hashes from static functions") describes
the issue with userspace tools (it doesn't explain which ones)
which don't expect the function name to change. This seems to happen
to static routines so I can only suspect this isn't an issue with
modversioning as the only symbols that would be used there wouldn't be
static.

Sami, what was the exact userspace issue with CONFIG_LTO_CLANG and these
long symbols?

  Luis