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 <[email protected]>
---
include/linux/kallsyms.h | 13 +++++++++++++
kernel/kallsyms.c | 21 ++++++++++++++++-----
kernel/livepatch/core.c | 32 +++++++++++++++++++++++++++++++-
3 files changed, 60 insertions(+), 6 deletions(-)
diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index c3f075e8f60c..d7d07a134ae4 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -97,6 +97,10 @@ extern int sprint_backtrace_build_id(char *buffer, unsigned long address);
int lookup_symbol_name(unsigned long addr, char *symname);
+int kallsyms_lookup_symbol_full_name(unsigned long addr, char *symname);
+
+void kallsyms_cleanup_symbol_name(char *s);
+
#else /* !CONFIG_KALLSYMS */
static inline unsigned long kallsyms_lookup_name(const char *name)
@@ -165,6 +169,15 @@ static inline int kallsyms_on_each_match_symbol(int (*fn)(void *, unsigned long)
{
return -EOPNOTSUPP;
}
+
+static inline int kallsyms_lookup_symbol_full_name(unsigned long addr, char *symname)
+{
+ return -ERANGE;
+}
+
+static inline void kallsyms_cleanup_symbol_name(char *s)
+{
+}
#endif /*CONFIG_KALLSYMS*/
static inline void print_ip_sym(const char *loglvl, unsigned long ip)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 22ea19a36e6e..f0d04fa1bbb4 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -163,7 +163,7 @@ unsigned long kallsyms_sym_address(int idx)
return kallsyms_relative_base - 1 - kallsyms_offsets[idx];
}
-static void cleanup_symbol_name(char *s)
+void kallsyms_cleanup_symbol_name(char *s)
{
char *res;
@@ -191,7 +191,7 @@ static int compare_symbol_name(const char *name, char *namebuf)
* To ensure correct bisection in kallsyms_lookup_names(), do
* cleanup_symbol_name(namebuf) before comparing name and namebuf.
*/
- cleanup_symbol_name(namebuf);
+ kallsyms_cleanup_symbol_name(namebuf);
return strcmp(name, namebuf);
}
@@ -426,7 +426,7 @@ static const char *kallsyms_lookup_buildid(unsigned long addr,
offset, modname, namebuf);
found:
- cleanup_symbol_name(namebuf);
+ kallsyms_cleanup_symbol_name(namebuf);
return ret;
}
@@ -446,7 +446,7 @@ const char *kallsyms_lookup(unsigned long addr,
NULL, namebuf);
}
-int lookup_symbol_name(unsigned long addr, char *symname)
+static int __lookup_symbol_name(unsigned long addr, char *symname, bool cleanup)
{
int res;
@@ -468,10 +468,21 @@ int lookup_symbol_name(unsigned long addr, char *symname)
return res;
found:
- cleanup_symbol_name(symname);
+ if (cleanup)
+ kallsyms_cleanup_symbol_name(symname);
return 0;
}
+int lookup_symbol_name(unsigned long addr, char *symname)
+{
+ return __lookup_symbol_name(addr, symname, true);
+}
+
+int kallsyms_lookup_symbol_full_name(unsigned long addr, char *symname)
+{
+ return __lookup_symbol_name(addr, symname, false);
+}
+
/* Look up a kernel symbol and return it in a text buffer. */
static int __sprint_symbol(char *buffer, unsigned long address,
int symbol_offset, int add_offset, int add_buildid)
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 52426665eecc..284220e04801 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -128,6 +128,19 @@ struct klp_find_arg {
static int klp_match_callback(void *data, unsigned long addr)
{
struct klp_find_arg *args = data;
+#ifdef CONFIG_LTO_CLANG
+ char full_name[KSYM_NAME_LEN];
+
+ /*
+ * With CONFIG_LTO_CLANG, we need to compare the full name of the
+ * symbol (with .llvm.<hash> postfix).
+ */
+ if (kallsyms_lookup_symbol_full_name(addr, full_name))
+ return 0;
+
+ if (strcmp(args->name, full_name))
+ return 0;
+#endif
args->addr = addr;
args->count++;
@@ -145,10 +158,12 @@ static int klp_match_callback(void *data, unsigned long addr)
static int klp_find_callback(void *data, const char *name, unsigned long addr)
{
+#ifndef CONFIG_LTO_CLANG
struct klp_find_arg *args = data;
if (strcmp(args->name, name))
return 0;
+#endif
return klp_match_callback(data, addr);
}
@@ -162,11 +177,26 @@ static int klp_find_object_symbol(const char *objname, const char *name,
.count = 0,
.pos = sympos,
};
+ const char *lookup_name = name;
+#ifdef CONFIG_LTO_CLANG
+ char short_name[KSYM_NAME_LEN];
+
+ /*
+ * With CONFIG_LTO_CLANG, the symbol name make have .llvm.<hash>
+ * postfix, e.g. show_cpuinfo.llvm.12345.
+ * Call kallsyms_on_each_match_symbol with short_name, e.g.
+ * show_cpuinfo, args->name is still the full name, which is used
+ * checked in klp_match_callback.
+ */
+ strscpy(short_name, name, sizeof(short_name));
+ kallsyms_cleanup_symbol_name(short_name);
+ lookup_name = short_name;
+#endif
if (objname)
module_kallsyms_on_each_symbol(objname, klp_find_callback, &args);
else
- kallsyms_on_each_match_symbol(klp_match_callback, name, &args);
+ kallsyms_on_each_match_symbol(klp_match_callback, lookup_name, &args);
/*
* Ensure an address was found. If sympos is 0, ensure symbol is unique;
--
2.43.0
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 <[email protected]>
> ---
> 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
Hi Miroslav,
Thanks for reviewing the patch!
On Fri, Jun 7, 2024 at 6:06 AM Miroslav Benes <[email protected]> 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 <[email protected]>
> > ---
> > 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.
> 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.
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.
Given livepatch has a well defined API, while the APIs at tracing side
may still change, we can change kallsyms to make sure livepatch side
works. Work on the tracing side can wait.
Thanks,
Song