Re: [PATCH] kallsyms, livepatch: Fix livepatch with CONFIG_LTO_CLANG

From: Song Liu
Date: Tue Jul 09 2024 - 13:11:52 EST



> On Jul 9, 2024, at 8:07 AM, Sami Tolvanen <samitolvanen@xxxxxxxxxx> wrote:

[...]

>
>>> 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?
>
> The issue with LTO wasn't symbol length. IIRC the compiler renaming
> symbols with ThinLTO caused issues for folks using dynamic kprobes,
> and I seem to recall it also breaking systrace in Android, at which
> point we decided to strip the postfix in kallsyms to avoid breaking
> anything else.

Trying to understand all the requirements and constraints. IIUC, we
can mostly agree:

(1) A way to match a symbol exactly is crucial for users like live
patching.
(2) Original symbol name is useful for backtrace, etc. (IOW hash
alone is not enough)

With these two requirements/constraints, we need

original symbol name + something

for duplicate symbols. "Something" here could be a path name
(xxx_driver_xxx_yyy_c), or a hash, or sympos.

At the moment, (1) is not met with CONFIG_LTO_CLANG. The original
patch tries to fix this, but the solution seems not optimal. I will
send another version that allows kallsyms match exactly or without
suffix.

This work shouldn't cause any problem for Rust, as Rust also need
original symbol name + "something". If we finally decide "something"
should be some format of hash, we can change all users (live patch,
etc.) to use hash, which might be better than sympos. Note: I am
not trying to say "something" should be hash.

OTOH, there is also an open question: Shall we allow tracing with
only original symbol name (without specifying _something_). I think
this a separate question and we don't have to answer it here.

Does this make sense?

Thanks,
Song