[PATCH v2 5/7] objtool/klp: Fix cross-module klp relocation section naming

Josh Poimboeuf <[email protected]>
Newsgroups org.kernel.vger.live-patching,org.kernel.vger.linux-kernel
Message-ID <e5a665072e9b5b63cfe95d5a4ca1d11dd0a7a96a.1785939903.git.jpoimboe@kernel.org>
A klp relocation section is .klp.rela.<objname>.<secname>, where objname
is the object being patched.

klp-build wrongly derives objname from where the referenced symbol
lives, not where it's referenced.  For a cross-module reference like
patched can_isotp code calling can.ko's can_rx_unregister(), that gives
.klp.rela.can..text rather than .klp.rela.can_isotp..text.  Unless the
patch happens to patch can.ko as well, the relocation never gets applied
and the call goes off into the weeds.

Name the intermediate section __klp_relocs.<objname> so post-link can
read the patched object's name from there.

Fixes: dd590d4d57eb ("objtool/klp: Introduce klp diff subcommand for diffing object files")
Reported-by: Joe Lawrence <[email protected]>
Link: https://lore.kernel.org/[email protected]
Signed-off-by: Josh Poimboeuf <[email protected]>
---
 tools/objtool/include/objtool/klp.h | 10 ++++--
 tools/objtool/klp-diff.c            | 17 +++++++--
 tools/objtool/klp-post-link.c       | 53 +++++++++++++++++------------
 3 files changed, 52 insertions(+), 28 deletions(-)

diff --git a/tools/objtool/include/objtool/klp.h b/tools/objtool/include/objtool/klp.h
index 0118c2c170c3f..646d8e1f12eff 100644
--- a/tools/objtool/include/objtool/klp.h
+++ b/tools/objtool/include/objtool/klp.h
@@ -14,11 +14,15 @@
 #define KLP_FUNCS_SEC	".init.klp_funcs"
 
 /*
- * __klp_relocs is an intermediate section which are created by klp diff and
- * converted into KLP symbols/relas by "objtool klp post-link".  This is needed
- * to work around the linker, which doesn't preserve SHN_LIVEPATCH or
+ * __klp_relocs.<objname> are intermediate sections which are created by klp
+ * diff and converted into KLP symbols/relas by "objtool klp post-link".  This
+ * is needed to work around the linker, which doesn't preserve SHN_LIVEPATCH or
  * SHF_RELA_LIVEPATCH, nor does it support having two RELA sections for a
  * single PROGBITS section.
+ *
+ * "objname" is the name of the object being patched ("vmlinux" or a module
+ * name).  post-link uses it to name the resulting
+ * .klp.rela.objname.section_name sections.
  */
 #define KLP_RELOCS_SEC	"__klp_relocs"
 #define KLP_STRINGS_SEC	".rodata.klp.str1.1"
diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 07cc8e2703260..91a9562c45a6e 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -1380,8 +1380,8 @@ static int clone_reloc_klp(struct elfs *e, struct reloc *patched_reloc,
 	}
 
 	/*
-	 * Create the __klp_relocs entry.  This will be converted to an actual
-	 * KLP rela by "objtool klp post-link".
+	 * Create the __klp_relocs.<objname> entry.  This will be converted to
+	 * an actual KLP rela by "objtool klp post-link".
 	 *
 	 * This intermediate step is necessary to prevent corruption by the
 	 * linker, which doesn't know how to properly handle two rela sections
@@ -1389,7 +1389,18 @@ static int clone_reloc_klp(struct elfs *e, struct reloc *patched_reloc,
 	 */
 
 	if (!klp_relocs) {
-		klp_relocs = elf_create_section(e->out, KLP_RELOCS_SEC, 0,
+		const char *objname = find_modname(e);
+		char sec_name[SEC_NAME_LEN];
+
+		if (!objname)
+			return -1;
+
+		/* section format: __klp_relocs.objname */
+		if (snprintf_check(sec_name, SEC_NAME_LEN,
+				   KLP_RELOCS_SEC ".%s", objname))
+			return -1;
+
+		klp_relocs = elf_create_section(e->out, sec_name, 0,
 						0, SHT_PROGBITS, 8, SHF_ALLOC);
 		if (!klp_relocs)
 			return -1;
diff --git a/tools/objtool/klp-post-link.c b/tools/objtool/klp-post-link.c
index c013e39957b11..350d20495897b 100644
--- a/tools/objtool/klp-post-link.c
+++ b/tools/objtool/klp-post-link.c
@@ -19,19 +19,11 @@
 #include <objtool/util.h>
 #include <linux/livepatch_external.h>
 
-static int fix_klp_relocs(struct elf *elf)
+static int fix_klp_reloc_sec(struct elf *elf, struct section *symtab,
+			     struct section *klp_relocs)
 {
-	struct section *symtab, *klp_relocs;
-
-	klp_relocs = find_section_by_name(elf, KLP_RELOCS_SEC);
-	if (!klp_relocs)
-		return 0;
-
-	symtab = find_section_by_name(elf, ".symtab");
-	if (!symtab) {
-		ERROR("missing .symtab");
-		return -1;
-	}
+	/* section format: __klp_relocs.sec_objname */
+	const char *sec_objname = klp_relocs->name + strlen(KLP_RELOCS_SEC ".");
 
 	for (int i = 0; i < sec_size(klp_relocs) / sizeof(struct klp_reloc); i++) {
 		struct klp_reloc *klp_reloc;
@@ -39,7 +31,6 @@ static int fix_klp_relocs(struct elf *elf)
 		struct section *sec, *tmp, *klp_rsec;
 		unsigned long offset;
 		struct reloc *reloc;
-		char sym_modname[64];
 		char rsec_name[SEC_NAME_LEN];
 		u64 addend;
 		struct symbol *sym, *klp_sym;
@@ -55,7 +46,7 @@ static int fix_klp_relocs(struct elf *elf)
 		reloc = find_reloc_by_dest(elf, klp_relocs,
 					   klp_reloc_off + offsetof(struct klp_reloc, offset));
 		if (!reloc) {
-			ERROR("malformed " KLP_RELOCS_SEC " section");
+			ERROR("malformed %s section", klp_relocs->name);
 			return -1;
 		}
 
@@ -66,17 +57,13 @@ static int fix_klp_relocs(struct elf *elf)
 		reloc = find_reloc_by_dest(elf, klp_relocs,
 					   klp_reloc_off + offsetof(struct klp_reloc, sym));
 		if (!reloc) {
-			ERROR("malformed " KLP_RELOCS_SEC " section");
+			ERROR("malformed %s section", klp_relocs->name);
 			return -1;
 		}
 
 		klp_sym = reloc->sym;
 		addend = reloc_addend(reloc);
 
-		/* symbol format: .klp.sym.modname.sym_name,sympos */
-		if (sscanf(klp_sym->name + strlen(KLP_SYM_PREFIX), "%55[^.]", sym_modname) != 1)
-			ERROR("can't find modname in klp symbol '%s'", klp_sym->name);
-
 		/*
 		 * Create the KLP rela:
 		 */
@@ -84,7 +71,7 @@ static int fix_klp_relocs(struct elf *elf)
 		/* section format: .klp.rela.sec_objname.section_name */
 		if (snprintf_check(rsec_name, SEC_NAME_LEN,
 				   KLP_RELOC_SEC_PREFIX "%s.%s",
-				   sym_modname, sec->name))
+				   sec_objname, sec->name))
 			return -1;
 
 		klp_rsec = find_section_by_name(elf, rsec_name);
@@ -134,10 +121,32 @@ static int fix_klp_relocs(struct elf *elf)
 	return 0;
 }
 
+static int fix_klp_relocs(struct elf *elf)
+{
+	struct section *symtab, *sec;
+
+	symtab = find_section_by_name(elf, ".symtab");
+	if (!symtab) {
+		ERROR("missing .symtab");
+		return -1;
+	}
+
+	for_each_sec(elf, sec) {
+		if (strncmp(sec->name, KLP_RELOCS_SEC ".",
+			    strlen(KLP_RELOCS_SEC ".")))
+			continue;
+
+		if (fix_klp_reloc_sec(elf, symtab, sec))
+			return -1;
+	}
+
+	return 0;
+}
+
 /*
  * This runs on the livepatch module after all other linking has been done.  It
- * converts the intermediate __klp_relocs section into proper KLP relocs to be
- * processed by livepatch.  This needs to run last to avoid linker wreckage.
+ * converts the intermediate __klp_relocs.* sections into proper KLP relocs to
+ * be processed by livepatch.  This needs to run last to avoid linker wreckage.
  * Linkers don't tend to handle the "two rela sections for a single base
  * section" case very well, nor do they appreciate SHN_LIVEPATCH.
  */
-- 
2.54.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.