[PATCH v2 0/7] objtool/klp: sympos/module/alternative/etc fixes

[PATCH v2 0/7] objtool/klp: sympos/module/alternative/etc fixes

[Josh Poimboeuf]

v2: 
- rebased on tip/master (first 6 patches were merged)
- dropped original patches 7-8 (will post followup)
- added .klp.symid fix

v1: https://lore.kernel.org/cover.1785727106.git.jpoimboe@kernel.org

This consolidates fixes for the klp-build issues reported by Joe over
the last several weeks, plus some more things I found while
testing/reviewing.

Fun stuff like symbol resolution, module dependencies, alternatives.

Joe Lawrence (1):
  objtool/klp: Allow new references to module exports

Josh Poimboeuf (6):
  objtool/klp: Fix vmlinux .klp.symid link error for .no_trim_symbol
    symbols
  objtool/klp: Fix size of empty special section entries
  objtool/klp: Ignore replacement offset of empty x86 alternatives
  objtool/klp: Explicitly disallow patching or referencing init
    code/data
  objtool/klp: Fix cross-module klp relocation section naming
  objtool/klp: Don't match local symbols against exports

 tools/objtool/arch/x86/special.c        | 27 ++++++++++
 tools/objtool/include/objtool/klp.h     | 10 ++--
 tools/objtool/include/objtool/special.h |  7 +++
 tools/objtool/klp-diff.c                | 65 ++++++++++++++++++++++---
 tools/objtool/klp-post-link.c           | 53 +++++++++++---------
 tools/objtool/klp-symid.c               |  1 +
 tools/objtool/klp-sympos.c              | 10 ++++
 7 files changed, 141 insertions(+), 32 deletions(-)

-- 
2.54.0

[PATCH v2 1/7] objtool/klp: Fix vmlinux .klp.symid link error for .no_trim_symbol symbols

[Josh Poimboeuf]

Testing klp-build with arm64 produced the following linker error during
the original kernel build:

  `__notrim.1' referenced in section `.klp.symid' of vmlinux.o: defined in discarded section `.no_trim_symbol' of vmlinux.o

symbol_get() puts a static __notrim[] in .no_trim_symbol, which GCC
names __notrim.1, __notrim.2, etc.  Two or more built-in translation
units calling symbol_get() thus produce duplicate names, resulting in
corresponding .klp.symid references which trigger the above error.

Add .no_trim_symbol to the discarded section list so its symbols don't
get symids.

Note this issue is not specific to arm64: it just needs two built-in
symbol_get() callers.  arm64 trips over it easily because it has KVM
always compiled in vmlinux, whereas on x86 it's typically a module.

Fixes: 029223d30162 ("objtool/klp: Add .klp.symid for sympos disambiguation")
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/klp-symid.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/objtool/klp-symid.c b/tools/objtool/klp-symid.c
index cf188cdfa6079..21d8708013aba 100644
--- a/tools/objtool/klp-symid.c
+++ b/tools/objtool/klp-symid.c
@@ -31,6 +31,7 @@
 static const char * const discarded_secs[] = {
 	".discard",
 	".modinfo",
+	".no_trim_symbol",
 	"__tracepoint_check",
 };
 
-- 
2.54.0

[PATCH v2 2/7] objtool/klp: Fix size of empty special section entries

[Josh Poimboeuf]

create_fake_symbols() sizes each ANNOTATE_DATA_SPECIAL entry from the
offset of the next annotation, falling back to the end of the section
for the last entry.  But the last entry is detected by a zero size,
which also happens for an *empty* entry: ALTERNATIVE(oldinstr, "", ft)
still annotates its zero-length replacement, at the same offset as the
next entry's annotation.

So every empty replacement gets a fake symbol spanning the entire rest
of .altinstr_replacement.  That's harmless today only because
find_symbol_containing() picks the smaller of two overlapping symbols.

Track whether a next annotation was found rather than inferring it from
the size.  A zero-length fake symbol is fine: find_symbol_containing()
skips those, so the properly sized symbol at the same offset still wins.

Fixes: dd590d4d57eb ("objtool/klp: Introduce klp diff subcommand for diffing object files")
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/klp-diff.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 492d7a012cffe..11e8f3ddbb0e6 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -1627,6 +1627,7 @@ static int create_fake_symbols(struct elf *elf)
 	for_each_reloc(sec->rsec, reloc) {
 		unsigned long offset, size;
 		struct reloc *next_reloc;
+		bool last = true;
 
 		if (annotype(elf, sec, reloc) != ANNOTYPE_DATA_SPECIAL)
 			continue;
@@ -1641,10 +1642,11 @@ static int create_fake_symbols(struct elf *elf)
 				continue;
 
 			size = reloc_addend(next_reloc) - offset;
+			last = false;
 			break;
 		}
 
-		if (!size)
+		if (last)
 			size = sec_size(reloc->sym->sec) - offset;
 
 		if (create_fake_symbol(elf, reloc->sym->sec, offset, size))
-- 
2.54.0

[PATCH v2 3/7] objtool/klp: Ignore replacement offset of empty x86 alternatives

[Josh Poimboeuf]

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 <jpoimboe@kernel.org>
---
 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

[PATCH v2 4/7] objtool/klp: Explicitly disallow patching or referencing init code/data

[Josh Poimboeuf]

Explicitly disallow the patching and referencing of init code/data.
Otherwise it could potentially introduce some odd edge cases depending
on whether the target object's init section has been freed yet (note
that the init code still exists in the target module when doing late
module patching).

Such edge cases include sympos calculation and the patching and/or
referencing of non-existent (init-freed) code/data.  Not to mention the
inherent differences in behavior that occur when the init code is only
patched *some* of the time depending on module loading order or kernel
config.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/klp-sympos.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tools/objtool/klp-sympos.c b/tools/objtool/klp-sympos.c
index bbfae516d3395..dfca9dd746812 100644
--- a/tools/objtool/klp-sympos.c
+++ b/tools/objtool/klp-sympos.c
@@ -367,6 +367,11 @@ static unsigned long find_vmlinux_sympos(struct symbol *sym)
 	return sympos;
 }
 
+static bool is_init_sym(struct symbol *sym)
+{
+	return strstarts(sym->sec->name, ".init");
+}
+
 /*
  * "sympos" is used by livepatch to disambiguate duplicate symbol names.
  */
@@ -376,6 +381,11 @@ unsigned long klp_find_sympos(struct elf *elf, struct symbol *sym)
 	bool has_dup = false;
 	struct symbol *s;
 
+	if (is_init_sym(sym)) {
+		ERROR("%s: can't patch or reference init code/data", sym->name);
+		return ULONG_MAX;
+	}
+
 	if (sym->bind != STB_LOCAL)
 		return 0;
 
-- 
2.54.0

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

[Josh Poimboeuf]

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 <joe.lawrence@redhat.com>
Link: https://lore.kernel.org/20260720145658.1103243-2-joe.lawrence@redhat.com
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 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

[PATCH v2 6/7] objtool/klp: Don't match local symbols against exports

[Josh Poimboeuf]

While cloning a reloc, klp diff calls find_export() to determine whether
the referenced symbol is exported.  That decides whether the reference
needs a klp reloc, which object the klp symbol belongs to, and whether
the symbol's data needs to be copied into the patch module.

But find_export() matches purely on symbol name, so a static function or
variable which happens to share its name with an export is mistaken for
a reference to that export:

  - klp_reloc_needed() creates a klp reloc pointing at the exporting
    module's symbol rather than the local one.  For a vmlinux export it
    skips the klp reloc altogether, leaving a normal reloc which the
    module loader resolves to the vmlinux symbol.

  - clone_reloc() treats the symbol as external and clones it without
    its data, leaving a dangling reference.

  - validate_special_section_klp_reloc() attributes a static branch or
    call key to the wrong module, and for a vmlinux export skips the
    unsupported-key check entirely.

Exports are always global, so ignore local symbols in find_export().

Fixes: dd590d4d57eb ("objtool/klp: Introduce klp diff subcommand for diffing object files")
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/klp-diff.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 91a9562c45a6e..aebe68a401571 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -1101,6 +1101,9 @@ static struct export *find_export(struct symbol *sym)
 {
 	struct export *export;
 
+	if (is_local_sym(sym))
+		return NULL;
+
 	hash_for_each_possible(exports, export, hash, str_hash(sym->name)) {
 		if (!strcmp(export->sym, sym->name))
 			return export;
-- 
2.54.0

[PATCH v2 7/7] objtool/klp: Allow new references to module exports

[Josh Poimboeuf]

From: Joe Lawrence <joe.lawrence@redhat.com>

klp_reloc_needed() returns true for module exports to support
late-module patching.  However, clone_reloc_klp() unconditionally
rejects symbols without a twin (i.e., new references added by the
patch), even when the symbol is a known export from Module.symvers.

Relax the check: allow new references to exported symbols by only
erroring on !twin when there is no export.  The export metadata from
Module.symvers provides sufficient context to emit the klp-relocation
without a twin.

For a module export that isn't sufficient on its own though, as the
resulting klp relocation will only be resolved at patch-enable time if
the exporting module is loaded.

If the original (unpatched) module already depends on the exporting
module, the dependency is safe: the module loader ensures the dependency
is satisfied before the patched module can be loaded, so the
klp relocation target will exist.

However, if the patch introduces a reference to a module that the
original doesn't depend on, there is no such guarantee.  The exporting
module could be absent or could be unloaded at any time, leading to a
relocation failure or use-after-free.

So also add a build-time check: when a new symbol reference (no twin)
targets a module export, verify that the original module already has at
least one UNDEF symbol resolving to that same exporting module.  If not,
error out with a diagnostic message.

Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/klp-diff.c | 35 +++++++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index aebe68a401571..0314426abcd8d 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -1294,6 +1294,28 @@ static int convert_reloc_sym(struct elf *elf, struct reloc *reloc)
 	return convert_reloc_secsym_to_sym(elf, reloc);
 }
 
+/*
+ * Check if the original module already has a dependency on dep_mod, i.e. it
+ * already references at least one export from that module.
+ */
+static bool has_module_dep(struct elfs *e, const char *dep_mod)
+{
+	struct symbol *sym;
+
+	for_each_sym(e->orig, sym) {
+		struct export *exp;
+
+		if (!is_undef_sym(sym) || is_weak_sym(sym))
+			continue;
+
+		exp = find_export(sym);
+		if (exp && !strcmp(exp->mod, dep_mod))
+			return true;
+	}
+
+	return false;
+}
+
 /*
  * Convert a regular relocation to a klp relocation (sort of).
  */
@@ -1313,8 +1335,17 @@ static int clone_reloc_klp(struct elfs *e, struct reloc *patched_reloc,
 	unsigned long sympos;
 
 	if (!patched_sym->twin) {
-		ERROR("unexpected klp reloc for new symbol %s", patched_sym->name);
-		return -1;
+		if (!export) {
+			ERROR("unexpected klp reloc for new symbol %s", patched_sym->name);
+			return -1;
+		}
+
+		if (strcmp(export->mod, "vmlinux") &&
+		    !has_module_dep(e, export->mod)) {
+			ERROR("%s: new reference to %s (exported by %s) would create an undeclared module dependency",
+			      patched_sym->name, export->sym, export->mod);
+			return -1;
+		}
 	}
 
 	/*
-- 
2.54.0

Re: [PATCH v2 4/7] objtool/klp: Explicitly disallow patching or referencing init code/data

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `.init` safety check is bypassed for exported module symbols, allowing livepatches to silently reference freed init memory.
--

--- Patch [4]: [PATCH v2 4/7] objtool/klp: Explicitly disallow patching or referencing init code/data ---
commit fe2fe6cc51b729f1eb6addc3f9e161e818ce482f
Author: Josh Poimboeuf <jpoimboe@kernel.org>

objtool/klp: Explicitly disallow patching or referencing init code/data

This commit explicitly prevents the patching and referencing of init code
and data to avoid edge cases. It introduces an init check in the sympos
resolution to handle targets that might have their init section freed.

> diff --git a/tools/objtool/klp-sympos.c b/tools/objtool/klp-sympos.c
> index bbfae516d3395..dfca9dd746812 100644
> --- a/tools/objtool/klp-sympos.c
> +++ b/tools/objtool/klp-sympos.c
[ ... ]
> @@ -376,6 +381,11 @@ unsigned long klp_find_sympos(struct elf *elf, struct symbol *sym)
>  	bool has_dup = false;
>  	struct symbol *s;
>  
> +	if (is_init_sym(sym)) {
> +		ERROR("%s: can't patch or reference init code/data", sym->name);
> +		return ULONG_MAX;
> +	}
> +

[Severity: High]
Does this check adequately protect against exported module symbols?

Looking at clone_reloc_klp() in tools/objtool/klp-diff.c, if the symbol
is exported, the code skips calling klp_find_sympos() entirely:

tools/objtool/klp-diff.c:clone_reloc_klp() {
    if (export) {
        sym_modname = export->mod;
        sym_orig_name = export->sym;
        sympos = 0;
    } else {
        sym_modname = find_modname(e);
        if (!sym_modname)
            return -1;

        sym_orig_name = patched_sym->twin->name;
        sympos = klp_find_sympos(e->orig, patched_sym->twin);
    ...
}

If a livepatch object references an exported init symbol from a module,
would it bypass this new safety check and potentially result in a reference
to freed init memory once the target module finishes initialization?

>  	if (sym->bind != STB_LOCAL)
>  		return 0;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785939903.git.jpoimboe@kernel.org?part=4

Re: [PATCH v2 1/7] objtool/klp: Fix vmlinux .klp.symid link error for .no_trim_symbol symbols

[Song Liu]

On Wed, Aug 5, 2026 at 7:30 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> Testing klp-build with arm64 produced the following linker error during
> the original kernel build:
>
>   `__notrim.1' referenced in section `.klp.symid' of vmlinux.o: defined in discarded section `.no_trim_symbol' of vmlinux.o
>
> symbol_get() puts a static __notrim[] in .no_trim_symbol, which GCC
> names __notrim.1, __notrim.2, etc.  Two or more built-in translation
> units calling symbol_get() thus produce duplicate names, resulting in
> corresponding .klp.symid references which trigger the above error.
>
> Add .no_trim_symbol to the discarded section list so its symbols don't
> get symids.
>
> Note this issue is not specific to arm64: it just needs two built-in
> symbol_get() callers.  arm64 trips over it easily because it has KVM
> always compiled in vmlinux, whereas on x86 it's typically a module.

Interesting. I didn't notice this difference between x86 and arm64.

>
> Fixes: 029223d30162 ("objtool/klp: Add .klp.symid for sympos disambiguation")
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>

Acked-by: Song Liu <song@kernel.org>

Re: [PATCH v2 2/7] objtool/klp: Fix size of empty special section entries

[Song Liu]

On Wed, Aug 5, 2026 at 7:30 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
[...]
>
> diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
> index 492d7a012cffe..11e8f3ddbb0e6 100644
> --- a/tools/objtool/klp-diff.c
> +++ b/tools/objtool/klp-diff.c
> @@ -1627,6 +1627,7 @@ static int create_fake_symbols(struct elf *elf)
>         for_each_reloc(sec->rsec, reloc) {
>                 unsigned long offset, size;
>                 struct reloc *next_reloc;
> +               bool last = true;
>
>                 if (annotype(elf, sec, reloc) != ANNOTYPE_DATA_SPECIAL)
>                         continue;
> @@ -1641,10 +1642,11 @@ static int create_fake_symbols(struct elf *elf)
>                                 continue;
>
>                         size = reloc_addend(next_reloc) - offset;
> +                       last = false;
>                         break;
>                 }
>
> -               if (!size)
> +               if (last)
>                         size = sec_size(reloc->sym->sec) - offset;

Some comments about "last" logic here can be very helpful.

Also, with the last flag, "size = 0;" before the for_each_reloc_continue()
loop can be removed.

Other than these nitpicks:

Acked-by: Song Liu <song@kernel.org>

>
>                 if (create_fake_symbol(elf, reloc->sym->sec, offset, size))
> --
> 2.54.0
>

Re: [PATCH v2 3/7] objtool/klp: Ignore replacement offset of empty x86 alternatives

[Song Liu]

On Wed, Aug 5, 2026 at 7:30 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> 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 <jpoimboe@kernel.org>

The patch looks good to me.

Acked-by: Song Liu <song@kernel.org>

Maybe we should add a __weak version of arch_alt_ignore_new_reloc(),
but that can wait until we add arm64 support.

However, this reminds me the cross compile use case. With current
arch_* functions, we cannot run klp-build on x86_64 build server for
an arm64 kernel (right?). What's our plan with the cross-compile use
cases?

Thanks,
Song

Re: [PATCH v2 6/7] objtool/klp: Don't match local symbols against exports

[Song Liu]

On Wed, Aug 5, 2026 at 7:30 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
[...]
>
> Exports are always global, so ignore local symbols in find_export().
>
> Fixes: dd590d4d57eb ("objtool/klp: Introduce klp diff subcommand for diffing object files")
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>

Acked-by: Song Liu <song@kernel.org>

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

[Song Liu]

On Wed, Aug 5, 2026 at 7:30 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> 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 <joe.lawrence@redhat.com>
> Link: https://lore.kernel.org/20260720145658.1103243-2-joe.lawrence@redhat.com
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>

Acked-by: Song Liu <song@kernel.org>

Re: [PATCH v2 7/7] objtool/klp: Allow new references to module exports

[Song Liu]

On Wed, Aug 5, 2026 at 7:30 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> From: Joe Lawrence <joe.lawrence@redhat.com>
>
> klp_reloc_needed() returns true for module exports to support
> late-module patching.  However, clone_reloc_klp() unconditionally
> rejects symbols without a twin (i.e., new references added by the
> patch), even when the symbol is a known export from Module.symvers.
>
> Relax the check: allow new references to exported symbols by only
> erroring on !twin when there is no export.  The export metadata from
> Module.symvers provides sufficient context to emit the klp-relocation
> without a twin.
>
> For a module export that isn't sufficient on its own though, as the
> resulting klp relocation will only be resolved at patch-enable time if
> the exporting module is loaded.
>
> If the original (unpatched) module already depends on the exporting
> module, the dependency is safe: the module loader ensures the dependency
> is satisfied before the patched module can be loaded, so the
> klp relocation target will exist.
>
> However, if the patch introduces a reference to a module that the
> original doesn't depend on, there is no such guarantee.  The exporting
> module could be absent or could be unloaded at any time, leading to a
> relocation failure or use-after-free.
>
> So also add a build-time check: when a new symbol reference (no twin)
> targets a module export, verify that the original module already has at
> least one UNDEF symbol resolving to that same exporting module.  If not,
> error out with a diagnostic message.
>
> Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>

Acked-by: Song Liu <song@kernel.org>

Re: [PATCH v2 2/7] objtool/klp: Fix size of empty special section entries

[Josh Poimboeuf]

On Wed, Aug 05, 2026 at 01:36:26PM -0700, Song Liu wrote:
> On Wed, Aug 5, 2026 at 7:30 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> [...]
> >
> > diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
> > index 492d7a012cffe..11e8f3ddbb0e6 100644
> > --- a/tools/objtool/klp-diff.c
> > +++ b/tools/objtool/klp-diff.c
> > @@ -1627,6 +1627,7 @@ static int create_fake_symbols(struct elf *elf)
> >         for_each_reloc(sec->rsec, reloc) {
> >                 unsigned long offset, size;
> >                 struct reloc *next_reloc;
> > +               bool last = true;
> >
> >                 if (annotype(elf, sec, reloc) != ANNOTYPE_DATA_SPECIAL)
> >                         continue;
> > @@ -1641,10 +1642,11 @@ static int create_fake_symbols(struct elf *elf)
> >                                 continue;
> >
> >                         size = reloc_addend(next_reloc) - offset;
> > +                       last = false;
> >                         break;
> >                 }
> >
> > -               if (!size)
> > +               if (last)
> >                         size = sec_size(reloc->sym->sec) - offset;
> 
> Some comments about "last" logic here can be very helpful.
> 
> Also, with the last flag, "size = 0;" before the for_each_reloc_continue()
> loop can be removed.
> 
> Other than these nitpicks:
> 
> Acked-by: Song Liu <song@kernel.org>

How about this on top?

diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 11e8f3ddbb0e6..38fae861d12c7 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -1634,7 +1634,10 @@ static int create_fake_symbols(struct elf *elf)
 
 		offset = reloc_addend(reloc);
 
-		size = 0;
+		/*
+		 * Find the start of the next entry so the fake symbol size can
+		 * be calculated.
+		 */
 		next_reloc = reloc;
 		for_each_reloc_continue(sec->rsec, next_reloc) {
 			if (annotype(elf, sec, next_reloc) != ANNOTYPE_DATA_SPECIAL ||
@@ -1646,6 +1649,10 @@ static int create_fake_symbols(struct elf *elf)
 			break;
 		}
 
+		/*
+		 * If no next entry found, this is the last entry, so its size
+		 * is from the current offset to the end of the section.
+		 */
 		if (last)
 			size = sec_size(reloc->sym->sec) - offset;
 

Re: [PATCH v2 3/7] objtool/klp: Ignore replacement offset of empty x86 alternatives

[Josh Poimboeuf]

On Wed, Aug 05, 2026 at 01:51:44PM -0700, Song Liu wrote:
> On Wed, Aug 5, 2026 at 7:30 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> > 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 <jpoimboe@kernel.org>
> 
> The patch looks good to me.
> 
> Acked-by: Song Liu <song@kernel.org>
> 
> Maybe we should add a __weak version of arch_alt_ignore_new_reloc(),
> but that can wait until we add arm64 support.
> 
> However, this reminds me the cross compile use case. With current
> arch_* functions, we cannot run klp-build on x86_64 build server for
> an arm64 kernel (right?). What's our plan with the cross-compile use
> cases?

I suspect it will be easy, objtool already works fine being
cross-compiled today.  Feel free to try it ;-)

-- 
Josh

Re: [PATCH v2 4/7] objtool/klp: Explicitly disallow patching or referencing init code/data

[Josh Poimboeuf]

On Wed, Aug 05, 2026 at 03:12:02PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The `.init` safety check is bypassed for exported module symbols, allowing livepatches to silently reference freed init memory.
> --
> 
> --- Patch [4]: [PATCH v2 4/7] objtool/klp: Explicitly disallow patching or referencing init code/data ---
> commit fe2fe6cc51b729f1eb6addc3f9e161e818ce482f
> Author: Josh Poimboeuf <jpoimboe@kernel.org>
> 
> objtool/klp: Explicitly disallow patching or referencing init code/data
> 
> This commit explicitly prevents the patching and referencing of init code
> and data to avoid edge cases. It introduces an init check in the sympos
> resolution to handle targets that might have their init section freed.
> 
> > diff --git a/tools/objtool/klp-sympos.c b/tools/objtool/klp-sympos.c
> > index bbfae516d3395..dfca9dd746812 100644
> > --- a/tools/objtool/klp-sympos.c
> > +++ b/tools/objtool/klp-sympos.c
> [ ... ]
> > @@ -376,6 +381,11 @@ unsigned long klp_find_sympos(struct elf *elf, struct symbol *sym)
> >  	bool has_dup = false;
> >  	struct symbol *s;
> >  
> > +	if (is_init_sym(sym)) {
> > +		ERROR("%s: can't patch or reference init code/data", sym->name);
> > +		return ULONG_MAX;
> > +	}
> > +
> 
> [Severity: High]
> Does this check adequately protect against exported module symbols?
> 
> Looking at clone_reloc_klp() in tools/objtool/klp-diff.c, if the symbol
> is exported, the code skips calling klp_find_sympos() entirely:
> 
> tools/objtool/klp-diff.c:clone_reloc_klp() {
>     if (export) {
>         sym_modname = export->mod;
>         sym_orig_name = export->sym;
>         sympos = 0;
>     } else {
>         sym_modname = find_modname(e);
>         if (!sym_modname)
>             return -1;
> 
>         sym_orig_name = patched_sym->twin->name;
>         sympos = klp_find_sympos(e->orig, patched_sym->twin);
>     ...
> }
> 
> If a livepatch object references an exported init symbol from a module,
> would it bypass this new safety check and potentially result in a reference
> to freed init memory once the target module finishes initialization?

An exported init symbol?  That's not a thing :-)

-- 
Josh

Re: [PATCH v2 0/7] objtool/klp: sympos/module/alternative/etc fixes

[Joe Lawrence]

On 8/5/26 10:29 AM, Josh Poimboeuf wrote:
> v2: 
> - rebased on tip/master (first 6 patches were merged)
> - dropped original patches 7-8 (will post followup)
> - added .klp.symid fix
> 
> v1: https://lore.kernel.org/cover.1785727106.git.jpoimboe@kernel.org
> 
> This consolidates fixes for the klp-build issues reported by Joe over
> the last several weeks, plus some more things I found while
> testing/reviewing.
> 
> Fun stuff like symbol resolution, module dependencies, alternatives.
> 
> Joe Lawrence (1):
>   objtool/klp: Allow new references to module exports
> 
> Josh Poimboeuf (6):
>   objtool/klp: Fix vmlinux .klp.symid link error for .no_trim_symbol
>     symbols
>   objtool/klp: Fix size of empty special section entries
>   objtool/klp: Ignore replacement offset of empty x86 alternatives
>   objtool/klp: Explicitly disallow patching or referencing init
>     code/data
>   objtool/klp: Fix cross-module klp relocation section naming
>   objtool/klp: Don't match local symbols against exports
> 
>  tools/objtool/arch/x86/special.c        | 27 ++++++++++
>  tools/objtool/include/objtool/klp.h     | 10 ++--
>  tools/objtool/include/objtool/special.h |  7 +++
>  tools/objtool/klp-diff.c                | 65 ++++++++++++++++++++++---
>  tools/objtool/klp-post-link.c           | 53 +++++++++++---------
>  tools/objtool/klp-symid.c               |  1 +
>  tools/objtool/klp-sympos.c              | 10 ++++
>  7 files changed, 141 insertions(+), 32 deletions(-)
> 

These changes (and follow-up comments) look pretty good (lightly
tested).  Could "objtool/klp: Fix vmlinux .klp.symid link error for
.no_trim_symbol symbols" peel off and merge sooner since it fixes a
commit just recently merged?

Though I think there is a still gap for EXPORT_SYMBOL_FOR_MODULES
symbols: those exported that way from vmlinux would presumably need a
klp-relocation, but klp_reloc_needed() always returns false for all
vmlinux exports.  Maybe we need to parse the namespace column from
Module.symvers to special case those?  (At least, that is how I read the
code, I don't have full repro test cases handy.)

This wouldn't block any of these patches, but if you are spinning a v3,
we could consider tacking that on while we're here.

-- 
Joe

Re: [PATCH v2 2/7] objtool/klp: Fix size of empty special section entries

[Song Liu]

On Wed, Aug 5, 2026 at 10:11 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Wed, Aug 05, 2026 at 01:36:26PM -0700, Song Liu wrote:
> > On Wed, Aug 5, 2026 at 7:30 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > >
> > [...]
> > >
> > > diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
> > > index 492d7a012cffe..11e8f3ddbb0e6 100644
> > > --- a/tools/objtool/klp-diff.c
> > > +++ b/tools/objtool/klp-diff.c
> > > @@ -1627,6 +1627,7 @@ static int create_fake_symbols(struct elf *elf)
> > >         for_each_reloc(sec->rsec, reloc) {
> > >                 unsigned long offset, size;
> > >                 struct reloc *next_reloc;
> > > +               bool last = true;
> > >
> > >                 if (annotype(elf, sec, reloc) != ANNOTYPE_DATA_SPECIAL)
> > >                         continue;
> > > @@ -1641,10 +1642,11 @@ static int create_fake_symbols(struct elf *elf)
> > >                                 continue;
> > >
> > >                         size = reloc_addend(next_reloc) - offset;
> > > +                       last = false;
> > >                         break;
> > >                 }
> > >
> > > -               if (!size)
> > > +               if (last)
> > >                         size = sec_size(reloc->sym->sec) - offset;
> >
> > Some comments about "last" logic here can be very helpful.
> >
> > Also, with the last flag, "size = 0;" before the for_each_reloc_continue()
> > loop can be removed.
> >
> > Other than these nitpicks:
> >
> > Acked-by: Song Liu <song@kernel.org>
>
> How about this on top?

Looks great! Thanks!

Song

> diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
> index 11e8f3ddbb0e6..38fae861d12c7 100644
> --- a/tools/objtool/klp-diff.c
> +++ b/tools/objtool/klp-diff.c
> @@ -1634,7 +1634,10 @@ static int create_fake_symbols(struct elf *elf)
>
>                 offset = reloc_addend(reloc);
>
> -               size = 0;
> +               /*
> +                * Find the start of the next entry so the fake symbol size can
> +                * be calculated.
> +                */
>                 next_reloc = reloc;
>                 for_each_reloc_continue(sec->rsec, next_reloc) {
>                         if (annotype(elf, sec, next_reloc) != ANNOTYPE_DATA_SPECIAL ||
> @@ -1646,6 +1649,10 @@ static int create_fake_symbols(struct elf *elf)
>                         break;
>                 }
>
> +               /*
> +                * If no next entry found, this is the last entry, so its size
> +                * is from the current offset to the end of the section.
> +                */
>                 if (last)
>                         size = sec_size(reloc->sym->sec) - offset;
>

Re: [PATCH v2 0/7] objtool/klp: sympos/module/alternative/etc fixes

[Josh Poimboeuf]

On Thu, Aug 06, 2026 at 11:57:55AM -0400, Joe Lawrence wrote:
> On 8/5/26 10:29 AM, Josh Poimboeuf wrote:
> > v2: 
> > - rebased on tip/master (first 6 patches were merged)
> > - dropped original patches 7-8 (will post followup)
> > - added .klp.symid fix
> > 
> > v1: https://lore.kernel.org/cover.1785727106.git.jpoimboe@kernel.org
> > 
> > This consolidates fixes for the klp-build issues reported by Joe over
> > the last several weeks, plus some more things I found while
> > testing/reviewing.
> > 
> > Fun stuff like symbol resolution, module dependencies, alternatives.
> > 
> > Joe Lawrence (1):
> >   objtool/klp: Allow new references to module exports
> > 
> > Josh Poimboeuf (6):
> >   objtool/klp: Fix vmlinux .klp.symid link error for .no_trim_symbol
> >     symbols
> >   objtool/klp: Fix size of empty special section entries
> >   objtool/klp: Ignore replacement offset of empty x86 alternatives
> >   objtool/klp: Explicitly disallow patching or referencing init
> >     code/data
> >   objtool/klp: Fix cross-module klp relocation section naming
> >   objtool/klp: Don't match local symbols against exports
> > 
> >  tools/objtool/arch/x86/special.c        | 27 ++++++++++
> >  tools/objtool/include/objtool/klp.h     | 10 ++--
> >  tools/objtool/include/objtool/special.h |  7 +++
> >  tools/objtool/klp-diff.c                | 65 ++++++++++++++++++++++---
> >  tools/objtool/klp-post-link.c           | 53 +++++++++++---------
> >  tools/objtool/klp-symid.c               |  1 +
> >  tools/objtool/klp-sympos.c              | 10 ++++
> >  7 files changed, 141 insertions(+), 32 deletions(-)
> > 
> 
> These changes (and follow-up comments) look pretty good (lightly
> tested).  Could "objtool/klp: Fix vmlinux .klp.symid link error for
> .no_trim_symbol symbols" peel off and merge sooner since it fixes a
> commit just recently merged?
> 
> Though I think there is a still gap for EXPORT_SYMBOL_FOR_MODULES
> symbols: those exported that way from vmlinux would presumably need a
> klp-relocation, but klp_reloc_needed() always returns false for all
> vmlinux exports.  Maybe we need to parse the namespace column from
> Module.symvers to special case those?  (At least, that is how I read the
> code, I don't have full repro test cases handy.)
> 
> This wouldn't block any of these patches, but if you are spinning a v3,
> we could consider tacking that on while we're here.

Indeed, I will do a followup patch for that one.

-- 
Josh