Received-SPF: pass (google.com: domain of linux-kernel+bounces-206106-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Fri, 7 Jun 2024 15:06:14 +0200 (CEST)
From: Miroslav Benes <mbenes@suse.cz>
To: Song Liu <song@kernel.org>
cc: live-patching@vger.kernel.org, linux-kernel@vger.kernel.org, 
    jpoimboe@kernel.org, jikos@kernel.org, pmladek@suse.com, 
    joe.lawrence@redhat.com, nathan@kernel.org, morbo@google.com, 
    justinstitt@google.com, mcgrof@kernel.org, thunder.leizhen@huawei.com, 
    kees@kernel.org, kernel-team@meta.com
Subject: Re: [PATCH] kallsyms, livepatch: Fix livepatch with
 CONFIG_LTO_CLANG
In-Reply-To: <20240605032120.3179157-1-song@kernel.org>
Message-ID: <alpine.LSU.2.21.2406071458531.29080@pobox.suse.cz>
References: <20240605032120.3179157-1-song@kernel.org>
User-Agent: Alpine 2.21 (LSU 202 2017-01-01)
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII

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@kernel.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?

Moreover, isn't there a similar problem for ftrace, kprobes, ebpf,...?

Thank you,
Miroslav