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")
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 11e8f3ddbb0e6..07cc8e2703260 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
