[PATCH v3 3/9] objtool/klp: Ignore replacement offset of empty x86 alternatives

Josh Poimboeuf <[email protected]>
Newsgroups org.kernel.vger.live-patching,org.kernel.vger.linux-kernel
Message-ID <7a885b70974795c3417f3358869e62aafd4ef783.1786138493.git.jpoimboe@kernel.org>
An x86 alternative with an empty replacement, e.g. the second entry of

  ALTERNATIVE_2("orig", "repl", ft1, "", ft2)

has a replacementlen of zero.  Its replacement offset still gets a
relocation, but the label it points at is the end of the previous
replacement, which is also the beginning of the *next* alternative's
replacement.  The value is meaningless; get_alt_entry() already ignores
it for that reason.

klp diff doesn't ignore it.  When such an alternative belongs to a
changed function, cloning its relocations drags in the unrelated
neighboring replacement, along with everything that replacement
references.  On an x86 clang/lto build an empty alternative in
meminfo_proc_show() pulled in the replacement of an alternative in
proc_kcore_init(), silently emitting a klp relocation against init text
which has long since been freed by the time the patch is applied.

Add arch_alt_ignore_new_reloc() and skip such relocations when cloning.
This has to be arch specific: on arm64 a zero-length replacement instead
identifies an alternative callback, whose replacement offset points at
the callback function and must be preserved.

Fixes: dd590d4d57eb ("objtool/klp: Introduce klp diff subcommand for diffing object files")
Acked-by: Song Liu <[email protected]>
Signed-off-by: Josh Poimboeuf <[email protected]>
---
 tools/objtool/arch/x86/special.c        | 27 +++++++++++++++++++++++++
 tools/objtool/include/objtool/special.h |  7 +++++++
 tools/objtool/klp-diff.c                |  6 +++++-
 3 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/tools/objtool/arch/x86/special.c b/tools/objtool/arch/x86/special.c
index e817a3fff4491..1e84c81bfcd81 100644
--- a/tools/objtool/arch/x86/special.c
+++ b/tools/objtool/arch/x86/special.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 #include <string.h>
 
+#include <arch/special.h>
 #include <objtool/special.h>
 #include <objtool/builtin.h>
 #include <objtool/warn.h>
@@ -9,6 +10,32 @@
 /* cpu feature name array generated from cpufeatures.h */
 #include "cpu-feature-names.c"
 
+/*
+ * An alternative with an empty replacement, e.g. the second entry of
+ *
+ *   ALTERNATIVE_2("orig", "repl", ft1, "", ft2)
+ *
+ * still gets a relocation for its replacement offset.  But the label it points
+ * at is the end of the previous entry's replacement, which is also the
+ * beginning of the *next* entry's replacement.  The value is meaningless: it's
+ * only ever used with a length of zero.
+ */
+bool arch_alt_ignore_new_reloc(struct section *sec, unsigned long offset)
+{
+	unsigned long entry_off;
+
+	if (strcmp(sec->name, ".altinstructions"))
+		return false;
+
+	entry_off = offset - (offset % ALT_ENTRY_SIZE);
+
+	if (offset - entry_off != ALT_NEW_OFFSET)
+		return false;
+
+	return !*(unsigned char *)(sec->data->d_buf + entry_off +
+				   ALT_NEW_LEN_OFFSET);
+}
+
 void arch_handle_alternative(struct special_alt *alt)
 {
 	static struct special_alt *group, *prev;
diff --git a/tools/objtool/include/objtool/special.h b/tools/objtool/include/objtool/special.h
index 121c3761899c1..620dbf6cb0e58 100644
--- a/tools/objtool/include/objtool/special.h
+++ b/tools/objtool/include/objtool/special.h
@@ -32,6 +32,13 @@ int special_get_alts(struct elf *elf, struct list_head *alts);
 
 void arch_handle_alternative(struct special_alt *alt);
 
+/*
+ * Should the reloc at @offset -- the "new" (replacement) field of a special
+ * section group entry -- be ignored?  The meaning of a zero-length replacement
+ * is arch specific, so the arch decides.
+ */
+bool arch_alt_ignore_new_reloc(struct section *sec, unsigned long offset);
+
 bool arch_support_alt_relocation(struct special_alt *special_alt,
 				 struct instruction *insn,
 				 struct reloc *reloc);
diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 38fae861d12c7..3923fabc13331 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -12,7 +12,7 @@
 #include <objtool/arch.h>
 #include <objtool/klp.h>
 #include <objtool/util.h>
-#include <arch/special.h>
+#include <objtool/special.h>
 
 #include <linux/align.h>
 #include <linux/objtool_types.h>
@@ -1537,6 +1537,10 @@ static int clone_sym_relocs(struct elfs *e, struct symbol *patched_sym)
 		    !strcmp(patched_reloc->sym->sec->name, ".altinstr_aux"))
 			continue;
 
+		if (arch_alt_ignore_new_reloc(patched_sym->sec,
+					      reloc_offset(patched_reloc)))
+			continue;
+
 		ret = convert_reloc_sym(e->patched, patched_reloc);
 		if (ret < 0) {
 			ERROR_FUNC(patched_rsec->base, reloc_offset(patched_reloc),
-- 
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.