Date: Tue,  4 Jun 2024 20:21:20 -0700
From: Song Liu <song@...nel.org>
To: live-patching@...r.kernel.org,
	linux-kernel@...r.kernel.org
Cc: jpoimboe@...nel.org,
	jikos@...nel.org,
	mbenes@...e.cz,
	pmladek@...e.com,
	joe.lawrence@...hat.com,
	nathan@...nel.org,
	morbo@...gle.com,
	justinstitt@...gle.com,
	mcgrof@...nel.org,
	thunder.leizhen@...wei.com,
	kees@...nel.org,
	kernel-team@...a.com,
	Song Liu <song@...nel.org>
Subject: [PATCH] kallsyms, livepatch: Fix livepatch with CONFIG_LTO_CLANG

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(-)

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

Powered by blists - more mailing lists