[PATCH 00/48] objtool/klp: Some klp-build fixes and improvements

[PATCH 00/48] objtool/klp: Some klp-build fixes and improvements

[Josh Poimboeuf]

While working on the (upcoming) arm64 support, I ended up shaking out a
lot of bugs by tested several patches on a variety of configs (distro,
LTO, FineIBT, kCFI, etc).

While arm64 support seems to be working well, I decided to leave those
patches out of this set to try to keep the number of patches
"reasonable".

And these stand alone as nice improvements for x86 anyway.

Full arm64 support (this set + 19 more) can be found here:

  git://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git klp-build-arm64


Joe Lawrence (2):
  objtool/klp: Fix is_uncorrelated_static_local() for Clang
  objtool/klp: Fix create_fake_symbols() skipping entsize-based sections

Josh Poimboeuf (46):
  objtool/klp: Fix .data..once static local non-correlation
  objtool/klp: Don't correlate __ADDRESSABLE() symbols
  objtool/klp: Ignore __UNIQUE_ID_*() PCI stub functions
  objtool: Move mark_rodata() to elf.c
  objtool/klp: Don't correlate rodata symbols
  objtool/klp: Don't correlate absolute symbols
  objtool/klp: Don't correlate __initstub__ symbols
  objtool/klp: Fix --debug-checksum for duplicate symbol names
  objtool/klp: Fix handling of zero-length .altinstr_replacement
    sections
  objtool/klp: Fix cloning of zero-length section symbols
  objtool/klp: Fix XXH3 state memory leak
  objtool/klp: Fix extraction of text annotations for alternatives
  objtool/klp: Fix kCFI trap handling
  objtool/klp: Fix relocation conversion failures for R_X86_64_NONE
  objtool: Fix reloc hash collision in find_reloc_by_dest_range()
  klp-build: Fix hang on out-of-date .config
  klp-build: Fix checksum comparison for changed offsets
  klp-build: Don't use errexit
  klp-build: Validate patch file existence
  klp-build: Suppress excessive fuzz output by default
  klp-build: Fix patch cleanup on interrupt
  klp-build: Reject patches to vDSO
  klp-build: Reject patches to realmode
  objtool/klp: Don't set sym->file for section symbols
  objtool: Include libsubcmd headers directly from source tree
  objtool/klp: Create empty checksum sections for function-less object
    files
  klp-build: Print "objtool klp diff" command in verbose mode
  objtool/klp: Handle Clang .data..Lanon anonymous data sections
  objtool: Add is_alias_sym() helper
  objtool: Add is_cold_func() helper
  objtool/klp: Extricate checksum calculation from validate_branch()
  objtool: Consolidate file decoding into decode_file()
  objtool/klp: Add "objtool klp checksum" subcommand
  klp-build: Use "objtool klp checksum" subcommand
  objtool/klp: Remove "objtool --checksum"
  klp-build: Validate short-circuit prerequisites
  objtool: Replace iterator callbacks with for_each_sym_by_*()
  objtool/klp: Calculate object checksums
  objtool/klp: Rewrite symbol correlation algorithm
  objtool/klp: Add correlation debugging output
  objtool: Add insn_sym() helper
  objtool/klp: Fix position-dependent checksums for non-relocated
    jumps/calls
  x86/Kconfig: Enable CONFIG_PREFIX_SYMBOLS for FineIBT
  objtool/klp: Make function prefix handling more generic
  objtool: Improve and simplify prefix symbol detection
  objtool/klp: Cache dont_correlate() result

 arch/x86/Kconfig                         |   2 +-
 scripts/livepatch/klp-build              | 183 +++--
 tools/objtool/Build                      |   2 +-
 tools/objtool/Makefile                   |   4 +-
 tools/objtool/arch/x86/decode.c          |  17 +-
 tools/objtool/builtin-check.c            |  17 +-
 tools/objtool/builtin-klp.c              |   1 +
 tools/objtool/check.c                    | 363 ++--------
 tools/objtool/disas.c                    |  22 +-
 tools/objtool/elf.c                      |  96 +--
 tools/objtool/include/objtool/arch.h     |   3 +
 tools/objtool/include/objtool/check.h    |  33 +-
 tools/objtool/include/objtool/checksum.h |  52 +-
 tools/objtool/include/objtool/elf.h      |  69 +-
 tools/objtool/include/objtool/klp.h      |   1 +
 tools/objtool/include/objtool/warn.h     |  57 +-
 tools/objtool/klp-checksum.c             | 346 +++++++++
 tools/objtool/klp-diff.c                 | 861 +++++++++++++++++------
 tools/objtool/objtool.c                  |   3 -
 tools/objtool/trace.c                    |   8 +-
 20 files changed, 1431 insertions(+), 709 deletions(-)
 create mode 100644 tools/objtool/klp-checksum.c

-- 
2.53.0

[PATCH 01/48] objtool/klp: Fix is_uncorrelated_static_local() for Clang

[Josh Poimboeuf]

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

For naming function-local static locals, GCC uses <var>.<id>, e.g.
__already_done.15, while Clang uses <func>.<var> with optional .<id>,
e.g. create_worker.__already_done.111

The existing is_uncorrelated_static_local() check only matches the GCC
convention where the variable name is a prefix.  Handle both cases by
checking for a prefix match (GCC) and by checking after the first dot
separator (Clang).

Fixes: dd590d4d57eb ("objtool/klp: Introduce klp diff subcommand for diffing object files")
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/klp-diff.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 0b0d1503851f..b1b068e9b4c7 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -242,16 +242,17 @@ static struct symbol *next_file_symbol(struct elf *elf, struct symbol *sym)
 static bool is_uncorrelated_static_local(struct symbol *sym)
 {
 	static const char * const vars[] = {
-		"__already_done.",
-		"__func__.",
-		"__key.",
-		"__warned.",
-		"_entry.",
-		"_entry_ptr.",
-		"_rs.",
-		"descriptor.",
-		"CSWTCH.",
+		"__already_done",
+		"__func__",
+		"__key",
+		"__warned",
+		"_entry",
+		"_entry_ptr",
+		"_rs",
+		"descriptor",
+		"CSWTCH",
 	};
+	const char *dot;
 
 	if (!is_object_sym(sym) || !is_local_sym(sym))
 		return false;
@@ -259,8 +260,20 @@ static bool is_uncorrelated_static_local(struct symbol *sym)
 	if (!strcmp(sym->sec->name, ".data.once"))
 		return true;
 
+	dot = strchr(sym->name, '.');
+	if (!dot)
+		return false;
+
 	for (int i = 0; i < ARRAY_SIZE(vars); i++) {
-		if (strstarts(sym->name, vars[i]))
+		size_t len = strlen(vars[i]);
+
+		/* GCC: <var>.<id> */
+		if (strstarts(sym->name, vars[i]) && (sym->name[len] == '.'))
+			return true;
+
+		/* Clang: <func>.<var>[.<id>] */
+		if (strstarts(dot + 1, vars[i]) &&
+		    (dot[1 + len] == '.' || dot[1 + len] == '\0'))
 			return true;
 	}
 
-- 
2.53.0

[PATCH 02/48] objtool/klp: Fix .data..once static local non-correlation

[Josh Poimboeuf]

While there was once a section named .data.once, it has since been
renamed to .data..once with commit dbefa1f31a91 ("Rename .data.once to
.data..once to fix resetting WARN*_ONCE").  Fix it.

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, 2 insertions(+), 1 deletion(-)

diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index b1b068e9b4c7..cb26c1c92a74 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -257,7 +257,8 @@ static bool is_uncorrelated_static_local(struct symbol *sym)
 	if (!is_object_sym(sym) || !is_local_sym(sym))
 		return false;
 
-	if (!strcmp(sym->sec->name, ".data.once"))
+	/* WARN_ONCE, etc */
+	if (!strcmp(sym->sec->name, ".data..once"))
 		return true;
 
 	dot = strchr(sym->name, '.');
-- 
2.53.0

[PATCH 03/48] objtool/klp: Don't correlate __ADDRESSABLE() symbols

[Josh Poimboeuf]

Symbols created by __ADDRESSABLE() are only used to convince the
toolchain not to optimize out the referenced symbol.

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

diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index cb26c1c92a74..36753eeba58c 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -352,6 +352,15 @@ static bool is_special_section_aux(struct section *sec)
 	return false;
 }
 
+/*
+ * Symbols created by ___ADDRESSABLE() are only used to convince the toolchain
+ * not to optimize out the referenced symbol.
+ */
+static bool is_addressable_sym(struct symbol *sym)
+{
+	return !strcmp(sym->sec->name, ".discard.addressable");
+}
+
 /*
  * These symbols should never be correlated, so their local patched versions
  * are used instead of linking to the originals.
@@ -365,6 +374,7 @@ static bool dont_correlate(struct symbol *sym)
 	       is_uncorrelated_static_local(sym) ||
 	       is_clang_tmp_label(sym) ||
 	       is_string_sec(sym->sec) ||
+	       is_addressable_sym(sym) ||
 	       is_special_section(sym->sec) ||
 	       is_special_section_aux(sym->sec) ||
 	       strstarts(sym->name, "__initcall__");
-- 
2.53.0

[PATCH 04/48] objtool/klp: Ignore __UNIQUE_ID_*() PCI stub functions

[Josh Poimboeuf]

With Clang LTO enabled, DECLARE_PCI_FIXUP_SECTION() uses __UNIQUE_ID()
to generate uniquely named wrapper functions, which are being reported
as new functions and unnecessarily included in the patch module:

  vmlinux.o: new function: __UNIQUE_ID_quirk_f0_vpd_link_661

These stub functions only exist to make the compiler happy.  Just ignore
them along with any other dont_correlate() symbols.  Note that
dont_correlate() already includes prefix functions.

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

diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 36753eeba58c..ea9ccf8c4ea9 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -786,7 +786,7 @@ static int mark_changed_functions(struct elfs *e)
 
 	/* Find changed functions */
 	for_each_sym(e->orig, sym_orig) {
-		if (!is_func_sym(sym_orig) || is_prefix_func(sym_orig))
+		if (!is_func_sym(sym_orig) || dont_correlate(sym_orig))
 			continue;
 
 		patched_sym = sym_orig->twin;
@@ -802,7 +802,7 @@ static int mark_changed_functions(struct elfs *e)
 
 	/* Find added functions and print them */
 	for_each_sym(e->patched, patched_sym) {
-		if (!is_func_sym(patched_sym) || is_prefix_func(patched_sym))
+		if (!is_func_sym(patched_sym) || dont_correlate(patched_sym))
 			continue;
 
 		if (!patched_sym->twin) {
-- 
2.53.0

[PATCH 05/48] objtool: Move mark_rodata() to elf.c

[Josh Poimboeuf]

Move the sec->rodata marking from check.c to elf.c so it's set during
ELF reading rather than during the check pipeline.  This makes the
rodata flag available to all objtool users, including klp-diff which
reads ELF files directly without running check().

Add an is_rodata_sec() helper to elf.h for consistency with
is_text_sec() and is_string_sec().

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/check.c               | 11 +++--------
 tools/objtool/elf.c                 | 13 +++++++++++++
 tools/objtool/include/objtool/elf.h |  5 +++++
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 9b11cf3193b9..5722d4568401 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2566,7 +2566,6 @@ static int classify_symbols(struct objtool_file *file)
 static void mark_rodata(struct objtool_file *file)
 {
 	struct section *sec;
-	bool found = false;
 
 	/*
 	 * Search for the following rodata sections, each of which can
@@ -2579,15 +2578,11 @@ static void mark_rodata(struct objtool_file *file)
 	 * .rodata.str1.* sections are ignored; they don't contain jump tables.
 	 */
 	for_each_sec(file->elf, sec) {
-		if ((!strncmp(sec->name, ".rodata", 7) &&
-		     !strstr(sec->name, ".str1.")) ||
-		    !strncmp(sec->name, ".data.rel.ro", 12)) {
-			sec->rodata = true;
-			found = true;
+		if (is_rodata_sec(sec)) {
+			file->rodata = true;
+			return;
 		}
 	}
-
-	file->rodata = found;
 }
 
 static void mark_holes(struct objtool_file *file)
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index f3df2bde119f..ac9da81a7a2f 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -1172,6 +1172,17 @@ static int read_relocs(struct elf *elf)
 	return 0;
 }
 
+static void mark_rodata(struct elf *elf)
+{
+	struct section *sec;
+
+	for_each_sec(elf, sec) {
+		if ((strstarts(sec->name, ".rodata") && !strstr(sec->name, ".str1.")) ||
+		    strstarts(sec->name, ".data.rel.ro"))
+			sec->rodata = true;
+	}
+}
+
 struct elf *elf_open_read(const char *name, int flags)
 {
 	struct elf *elf;
@@ -1222,6 +1233,8 @@ struct elf *elf_open_read(const char *name, int flags)
 	if (read_sections(elf))
 		goto err;
 
+	mark_rodata(elf);
+
 	if (read_symbols(elf))
 		goto err;
 
diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
index 25573e5af76e..c61bd57767f9 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -296,6 +296,11 @@ static inline bool is_text_sec(struct section *sec)
 	return sec->sh.sh_flags & SHF_EXECINSTR;
 }
 
+static inline bool is_rodata_sec(struct section *sec)
+{
+	return sec->rodata;
+}
+
 static inline bool sec_changed(struct section *sec)
 {
 	return sec->_changed;
-- 
2.53.0

[PATCH 06/48] objtool/klp: Don't correlate rodata symbols

[Josh Poimboeuf]

Some crypto assembly codes define the same local rodata labels (e.g.,
K256) which get duplicated when multiple .S files are linked into the
same composite object, triggering "Multiple correlation candidates"
errors.

Correlating rodata is tricky anyway, and not all rodata is associated
with a symbol.  So just don't correlate any rodata, so that any
referenced data will get duplicated in the livepatch module.

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

diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index ea9ccf8c4ea9..f6597015b33b 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -374,6 +374,7 @@ static bool dont_correlate(struct symbol *sym)
 	       is_uncorrelated_static_local(sym) ||
 	       is_clang_tmp_label(sym) ||
 	       is_string_sec(sym->sec) ||
+	       is_rodata_sec(sym->sec) ||
 	       is_addressable_sym(sym) ||
 	       is_special_section(sym->sec) ||
 	       is_special_section_aux(sym->sec) ||
-- 
2.53.0

[PATCH 07/48] objtool/klp: Don't correlate absolute symbols

[Josh Poimboeuf]

Some arch/x86/crypto/*.S files define local .set/.equ constants that get
duplicated in vmlinux.o.  This causes klp-diff to fail with "Multiple
correlation candidates" errors since it can't uniquely match these
between orig and patched builds.

Skip ABS symbols in dont_correlate().  They're purely compile-time
assembly constants that are never referenced by relocations, so they
don't need correlation.

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

diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index f6597015b33b..05071d691b5f 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -361,6 +361,15 @@ static bool is_addressable_sym(struct symbol *sym)
 	return !strcmp(sym->sec->name, ".discard.addressable");
 }
 
+/*
+ * ABS symbols are typically assembly .set/.equ constants which are never
+ * referenced by relocations.  (Exclude FILE symbols which are also SHN_ABS.)
+ */
+static bool is_abs_sym(struct symbol *sym)
+{
+	return sym->sym.st_shndx == SHN_ABS && !is_file_sym(sym);
+}
+
 /*
  * These symbols should never be correlated, so their local patched versions
  * are used instead of linking to the originals.
@@ -370,6 +379,7 @@ static bool dont_correlate(struct symbol *sym)
 	return is_file_sym(sym) ||
 	       is_null_sym(sym) ||
 	       is_sec_sym(sym) ||
+	       is_abs_sym(sym) ||
 	       is_prefix_func(sym) ||
 	       is_uncorrelated_static_local(sym) ||
 	       is_clang_tmp_label(sym) ||
-- 
2.53.0

[PATCH 08/48] objtool/klp: Don't correlate __initstub__ symbols

[Josh Poimboeuf]

With LTO, the initcall infrastructure generates __initstub__kmod_*
wrapper functions in .init.text.  These are the LTO equivalent of
__initcall__kmod_* data pointers, which are already excluded from
correlation.

These are __init functions whose memory is freed after boot, so there's
no reason to include or reference them in a livepatch module.

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

diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 05071d691b5f..022522cd9b6c 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -370,6 +370,12 @@ static bool is_abs_sym(struct symbol *sym)
 	return sym->sym.st_shndx == SHN_ABS && !is_file_sym(sym);
 }
 
+static bool is_initcall_sym(struct symbol *sym)
+{
+	return strstarts(sym->name, "__initcall__") ||
+	       strstarts(sym->name, "__initstub__");
+}
+
 /*
  * These symbols should never be correlated, so their local patched versions
  * are used instead of linking to the originals.
@@ -385,10 +391,10 @@ static bool dont_correlate(struct symbol *sym)
 	       is_clang_tmp_label(sym) ||
 	       is_string_sec(sym->sec) ||
 	       is_rodata_sec(sym->sec) ||
+	       is_initcall_sym(sym) ||
 	       is_addressable_sym(sym) ||
 	       is_special_section(sym->sec) ||
-	       is_special_section_aux(sym->sec) ||
-	       strstarts(sym->name, "__initcall__");
+	       is_special_section_aux(sym->sec);
 }
 
 struct process_demangled_name_data {
-- 
2.53.0

[PATCH 09/48] objtool/klp: Fix create_fake_symbols() skipping entsize-based sections

[Josh Poimboeuf]

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

create_fake_symbols() has two phases: creating symbols from
ANNOTATE_DATA_SPECIAL entries, and a fallback that uses sh_entsize for
special sections like .static_call_sites.

When .discard.annotate_data is absent, the function returns early,
skipping the entsize fallback and silently allowing unsupported
module-local static call keys through.

Fix it by jumping to the entsize phase instead of returning early.

Fixes: dd590d4d57eb ("objtool/klp: Introduce klp diff subcommand for diffing object files")
Assisted-by: Claude:claude-4-opus
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/klp-diff.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 022522cd9b6c..767716766d41 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -1375,7 +1375,7 @@ static int create_fake_symbols(struct elf *elf)
 
 	sec = find_section_by_name(elf, ".discard.annotate_data");
 	if (!sec || !sec->rsec)
-		return 0;
+		goto entsize;
 
 	for_each_reloc(sec->rsec, reloc) {
 		unsigned long offset, size;
@@ -1407,7 +1407,7 @@ static int create_fake_symbols(struct elf *elf)
 	/*
 	 * 2) Make symbols for sh_entsize, and simple arrays of pointers:
 	 */
-
+entsize:
 	for_each_sec(elf, sec) {
 		unsigned int entry_size;
 		unsigned long offset;
-- 
2.53.0

[PATCH 10/48] objtool/klp: Fix --debug-checksum for duplicate symbol names

[Josh Poimboeuf]

find_symbol_by_name() only returns the first match, so
--debug-checksum=<func> silently ignores any subsequent duplicately
named functions after the first.

Add a new iterate_sym_by_name() to fix that.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/check.c               | 19 ++++++++++++++-----
 tools/objtool/elf.c                 | 12 ++++++++++++
 tools/objtool/include/objtool/elf.h |  3 +++
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 5722d4568401..f14212a8c179 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3657,6 +3657,17 @@ static bool skip_alt_group(struct instruction *insn)
 	return alt_insn->type == INSN_CLAC || alt_insn->type == INSN_STAC;
 }
 
+static void enable_debug_checksum_cb(struct symbol *sym, void *d)
+{
+	bool *found = d;
+
+	if (!is_func_sym(sym))
+		return;
+
+	sym->debug_checksum = 1;
+	*found = true;
+}
+
 static int checksum_debug_init(struct objtool_file *file)
 {
 	char *dup, *s;
@@ -3672,18 +3683,16 @@ static int checksum_debug_init(struct objtool_file *file)
 
 	s = dup;
 	while (*s) {
-		struct symbol *func;
+		bool found = false;
 		char *comma;
 
 		comma = strchr(s, ',');
 		if (comma)
 			*comma = '\0';
 
-		func = find_symbol_by_name(file->elf, s);
-		if (!func || !is_func_sym(func))
+		iterate_sym_by_name(file->elf, s, enable_debug_checksum_cb, &found);
+		if (!found)
 			WARN("--debug-checksum: can't find '%s'", s);
-		else
-			func->debug_checksum = 1;
 
 		if (!comma)
 			break;
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index ac9da81a7a2f..a5486e172e5c 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -335,6 +335,18 @@ void iterate_global_symbol_by_demangled_name(const struct elf *elf,
 	}
 }
 
+void iterate_sym_by_name(const struct elf *elf, const char *name,
+			 void (*process)(struct symbol *sym, void *data),
+			 void *data)
+{
+	struct symbol *sym;
+
+	elf_hash_for_each_possible(symbol_name, sym, name_hash, str_hash_demangled(name)) {
+		if (!strcmp(sym->name, name))
+			process(sym, data);
+	}
+}
+
 struct reloc *find_reloc_by_dest_range(const struct elf *elf, struct section *sec,
 				     unsigned long offset, unsigned int len)
 {
diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
index c61bd57767f9..cd5844c7b4e2 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -189,6 +189,9 @@ struct symbol *find_global_symbol_by_name(const struct elf *elf, const char *nam
 void iterate_global_symbol_by_demangled_name(const struct elf *elf, const char *demangled_name,
 					     void (*process)(struct symbol *sym, void *data),
 					     void *data);
+void iterate_sym_by_name(const struct elf *elf, const char *name,
+			 void (*process)(struct symbol *sym, void *data),
+			 void *data);
 struct symbol *find_symbol_containing(const struct section *sec, unsigned long offset);
 int find_symbol_hole_containing(const struct section *sec, unsigned long offset);
 struct reloc *find_reloc_by_dest(const struct elf *elf, struct section *sec, unsigned long offset);
-- 
2.53.0

[PATCH 11/48] objtool/klp: Fix handling of zero-length .altinstr_replacement sections

[Josh Poimboeuf]

When a section is empty (e.g. only zero-length alternative
replacements), there are no symbols to convert a section symbol
reference to.  Skip the reloc instead of erroring out.

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 | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 767716766d41..7f6f86117394 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -1020,6 +1020,13 @@ static int convert_reloc_secsym_to_sym(struct elf *elf, struct reloc *reloc)
 	/* No dedicated section; find the symbol manually */
 	sym = find_symbol_containing(sec, arch_adjusted_addend(reloc));
 	if (!sym) {
+		/*
+		 * This is presumably an .altinstr_replacement section which is
+		 * empty due to it only having zero-length replacement(s).
+		 */
+		if (!sec_size(sec))
+			return 1;
+
 		/*
 		 * This can happen for special section references to weak code
 		 * whose symbol has been stripped by the linker.
@@ -1280,6 +1287,7 @@ static int clone_sym_relocs(struct elfs *e, struct symbol *patched_sym)
 
 	for_each_reloc(patched_rsec, patched_reloc) {
 		unsigned long offset;
+		int ret;
 
 		if (reloc_offset(patched_reloc) < start ||
 		    reloc_offset(patched_reloc) >= end)
@@ -1293,12 +1301,15 @@ static int clone_sym_relocs(struct elfs *e, struct symbol *patched_sym)
 		    !strcmp(patched_reloc->sym->sec->name, ".altinstr_aux"))
 			continue;
 
-		if (convert_reloc_sym(e->patched, patched_reloc)) {
+		ret = convert_reloc_sym(e->patched, patched_reloc);
+		if (ret < 0) {
 			ERROR_FUNC(patched_rsec->base, reloc_offset(patched_reloc),
 				   "failed to convert reloc sym '%s' to its proper format",
 				   patched_reloc->sym->name);
 			return -1;
 		}
+		if (ret > 0)
+			continue;
 
 		offset = out_sym->offset + (reloc_offset(patched_reloc) - patched_sym->offset);
 
-- 
2.53.0

[PATCH 12/48] objtool/klp: Fix cloning of zero-length section symbols

[Josh Poimboeuf]

Fix NULL dereference when cloning a symbol from an empty section.
sec->data is only populated for sections with non-zero size.

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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 7f6f86117394..3303664a39d7 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -696,7 +696,7 @@ static struct symbol *__clone_symbol(struct elf *elf, struct symbol *patched_sym
 			size_t size;
 
 			/* bss doesn't have data */
-			if (patched_sym->sec->data->d_buf)
+			if (patched_sym->sec->data && patched_sym->sec->data->d_buf)
 				data = patched_sym->sec->data->d_buf + patched_sym->offset;
 
 			if (is_sec_sym(patched_sym))
-- 
2.53.0

[PATCH 13/48] objtool/klp: Fix XXH3 state memory leak

[Josh Poimboeuf]

The XXH3 state allocated in checksum_init() is never freed.  Free it in
checksum_finish().

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/include/objtool/checksum.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/objtool/include/objtool/checksum.h b/tools/objtool/include/objtool/checksum.h
index 7fe21608722a..0bd16fe9168b 100644
--- a/tools/objtool/include/objtool/checksum.h
+++ b/tools/objtool/include/objtool/checksum.h
@@ -26,6 +26,7 @@ static inline void checksum_finish(struct symbol *func)
 {
 	if (func && func->csum.state) {
 		func->csum.checksum = XXH3_64bits_digest(func->csum.state);
+		XXH3_freeState(func->csum.state);
 		func->csum.state = NULL;
 	}
 }
-- 
2.53.0

[PATCH 14/48] objtool/klp: Fix extraction of text annotations for alternatives

[Josh Poimboeuf]

Objtool is failing to extract text annotations which reference
.altinstr_replacement instructions:

  1) Alternative replacement fake symbols are NOTYPE rather than FUNC,
     and they don't have sym->included set, thus they aren't recognized
     by should_keep_special_sym().

  2) .discard.annotate_insn gets processed before .altinstr_replacement,
     so the referenced (fake) symbols don't have clones yet.

Fix the first issue by checking for a valid clone instead of
sym->included and by accepting NOTYPE symbols when processing
.discard.annotate_insn.

Fix the second issue by deferring text annotation processing until after
the other special sections have been cloned.

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 | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 3303664a39d7..22942f394745 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -1452,6 +1452,7 @@ static int create_fake_symbols(struct elf *elf)
 /* Keep a special section entry if it references an included function */
 static bool should_keep_special_sym(struct elf *elf, struct symbol *sym)
 {
+	bool annotate_insn = !strcmp(sym->sec->name, ".discard.annotate_insn");
 	struct reloc *reloc;
 
 	if (is_sec_sym(sym) || !sym->sec->rsec)
@@ -1461,7 +1462,16 @@ static bool should_keep_special_sym(struct elf *elf, struct symbol *sym)
 		if (convert_reloc_sym(elf, reloc))
 			continue;
 
-		if (is_func_sym(reloc->sym) && reloc->sym->included)
+		if (!reloc->sym->clone || is_undef_sym(reloc->sym->clone))
+			continue;
+
+		/*
+		 * Keep special section references to cloned functions.
+		 * In some cases annotate_insn can also reference cloned alt
+		 * replacement fake symbols; keep those references as well.
+		 */
+		if (is_func_sym(reloc->sym) ||
+		    (annotate_insn && is_notype_sym(reloc->sym)))
 			return true;
 	}
 
@@ -1605,15 +1615,28 @@ static int clone_special_section(struct elfs *e, struct section *patched_sec)
 /* Extract only the needed bits from special sections */
 static int clone_special_sections(struct elfs *e)
 {
-	struct section *patched_sec;
+	struct section *sec, *annotate_insn = NULL;
 
-	for_each_sec(e->patched, patched_sec) {
-		if (is_special_section(patched_sec)) {
-			if (clone_special_section(e, patched_sec))
+	for_each_sec(e->patched, sec) {
+		if (is_special_section(sec)) {
+			if (!strcmp(sec->name, ".discard.annotate_insn")) {
+				annotate_insn = sec;
+				continue;
+			}
+			if (clone_special_section(e, sec))
 				return -1;
 		}
 	}
 
+	/*
+	 * Do .discard.annotate_insn last, it can reference other special
+	 * sections (alt replacements) so they need to be cloned first.
+	 */
+	if (annotate_insn) {
+		if (clone_special_section(e, annotate_insn))
+			return -1;
+	}
+
 	return 0;
 }
 
-- 
2.53.0

[PATCH 15/48] objtool/klp: Fix kCFI trap handling

[Josh Poimboeuf]

.kcfi_traps contains references to kCFI trap instruction locations.
When a KCFI type check fails at an indirect call, the trap handler looks
up the faulting address in this section.

Add it to the special sections list so the entries get extracted for the
changed functions they reference.

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

diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 22942f394745..a8b9a1441e7e 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -296,6 +296,7 @@ static bool is_special_section(struct section *sec)
 {
 	static const char * const specials[] = {
 		".altinstructions",
+		".kcfi_traps",
 		".smp_locks",
 		"__bug_table",
 		"__ex_table",
-- 
2.53.0

[PATCH 16/48] objtool/klp: Fix relocation conversion failures for R_X86_64_NONE

[Josh Poimboeuf]

Objtool has some hacks which NOP out certain calls/jumps and replace
their relocations with R_X86_64_NONE.  The klp-diff relocation
extraction code will error out when trying to copy these relocations due
to their negative addend, which would only makes sense for a PC-relative
branch instruction.  Just ignore them.

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 a8b9a1441e7e..57d2af98a33c 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -1048,6 +1048,9 @@ static int convert_reloc_secsym_to_sym(struct elf *elf, struct reloc *reloc)
  */
 static int convert_reloc_sym(struct elf *elf, struct reloc *reloc)
 {
+	if (reloc_type(reloc) == R_NONE)
+		return 1;
+
 	if (is_reloc_allowed(reloc))
 		return 0;
 
-- 
2.53.0

[PATCH 17/48] objtool: Fix reloc hash collision in find_reloc_by_dest_range()

[Josh Poimboeuf]

In find_reloc_by_dest_range(), hash collisions can cause a high-offset
relocation to appear when probing a low-offset hash bucket.

Only return early when the best match found so far genuinely belongs to
the current bucket (its offset is within the bucket's stride range).
Otherwise, continue scanning later buckets which may contain
lower-offset matches.

Fixes: 74b873e49d92 ("objtool: Optimize find_rela_by_dest_range()")
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/elf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index a5486e172e5c..c4cb371e72b2 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -370,11 +370,11 @@ struct reloc *find_reloc_by_dest_range(const struct elf *elf, struct section *se
 					r = reloc;
 			}
 		}
-		if (r)
+		if (r && (reloc_offset(r) & OFFSET_STRIDE_MASK) == o)
 			return r;
 	}
 
-	return NULL;
+	return r;
 }
 
 struct reloc *find_reloc_by_dest(const struct elf *elf, struct section *sec, unsigned long offset)
-- 
2.53.0

[PATCH 18/48] klp-build: Fix hang on out-of-date .config

[Josh Poimboeuf]

If .config is out of date with the kernel source, 'make syncconfig'
hangs while waiting for user input on new config options.  Detect the
mismatch and return an error.

Fixes: 6f93f7b06810 ("livepatch/klp-build: Fix inconsistent kernel version")
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 scripts/livepatch/klp-build | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index 0ad7e6631314..81b35fc10877 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -306,7 +306,12 @@ set_kernelversion() {
 
 	stash_file "$file"
 
-	kernelrelease="$(cd "$SRC" && make syncconfig &>/dev/null && make -s kernelrelease)"
+	if [[ -n "$(make -s listnewconfig 2>/dev/null)" ]]; then
+		die ".config mismatch, check your .config or run 'make olddefconfig'"
+	fi
+	make syncconfig &>/dev/null || die "make syncconfig failed"
+
+	kernelrelease="$(cd "$SRC" && make -s kernelrelease)"
 	[[ -z "$kernelrelease" ]] && die "failed to get kernel version"
 
 	sed -i "2i echo $kernelrelease; exit 0" scripts/setlocalversion
-- 
2.53.0

[PATCH 19/48] klp-build: Fix checksum comparison for changed offsets

[Josh Poimboeuf]

The klp-build -f/--show-first-changed feature uses diff to compare
checksum log lines between original and patched objects.  However, diff
compares entire lines, including the offset field.  When a function is
at a different section offset, the offset field differs even though the
instruction checksum is identical, causing the wrong instruction to be
printed.

Only compare the checksum field when looking for the first changed
instruction.  Also print both the original and patched offsets when they
differ.

Fixes: 78be9facfb5e ("livepatch/klp-build: Add --show-first-changed option to show function divergence")
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 scripts/livepatch/klp-build | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index 81b35fc10877..2b8b3c338a87 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -727,13 +727,29 @@ diff_checksums() {
 		)
 
 		for func in ${funcs[$file]}; do
-			diff <( grep0 -E "^DEBUG: .*checksum: $func " "$orig_log"    | sed "s|$ORIG_DIR/||")	\
-			     <( grep0 -E "^DEBUG: .*checksum: $func " "$patched_log" | sed "s|$PATCHED_DIR/||")	\
-				| gawk '/^< DEBUG: / {
-					gsub(/:/, "")
-					printf "%s: %s: %s\n", $3, $5, $6
-					exit
-			}' || true
+			local -a orig patched
+			paste <(grep0 -E "^DEBUG: .*checksum: $func " "$orig_log") \
+			      <(grep0 -E "^DEBUG: .*checksum: $func " "$patched_log") |
+			while IFS=$'\t' read -r orig patched; do
+				read -ra orig <<< "$orig"
+				read -ra patched <<< "$patched"
+
+				if [[ ${#patched[@]} -eq 0 ]]; then
+					printf "%s: %s: %s (removed)\n" "${orig[1]%:}" "${orig[3]}" "${orig[-2]}"
+					break
+				elif [[ ${#orig[@]} -eq 0 ]]; then
+					printf "%s: %s: %s (added)\n" "${patched[1]%:}" "${patched[3]}" "${patched[-2]}"
+					break
+				fi
+
+				[[ "${orig[-1]}" == "${patched[-1]}" ]] && continue
+
+				printf "%s: %s: %s" "${orig[1]%:}" "${orig[3]}" "${orig[-2]}"
+				[[ "${orig[-2]}" != "${patched[-2]}" ]] && \
+					printf " (patched: %s)" "${patched[-2]}"
+				printf "\n"
+				break
+			done || true
 		done
 	done
 }
-- 
2.53.0

[PATCH 20/48] klp-build: Don't use errexit

[Josh Poimboeuf]

The errtrace option (combined with the ERR trap) already serves the same
function (and more) as errexit, so errexit is redundant.  And it has
more pitfalls.  Remove it.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 scripts/livepatch/klp-build | 1 -
 1 file changed, 1 deletion(-)

diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index 2b8b3c338a87..e2f0eb8fdc7f 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -11,7 +11,6 @@ if (( BASH_VERSINFO[0]  < 4 || \
 	exit 1
 fi
 
-set -o errexit
 set -o errtrace
 set -o pipefail
 set -o nounset
-- 
2.53.0

[PATCH 21/48] klp-build: Validate patch file existence

[Josh Poimboeuf]

Make sure all patch files actually exist.  Otherwise there can be
confusing errors later.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 scripts/livepatch/klp-build | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index e2f0eb8fdc7f..115f68db49c9 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -157,6 +157,7 @@ process_args() {
 	local short
 	local long
 	local args
+	local patch
 
 	short="hfj:o:vdS:T"
 	long="help,show-first-changed,jobs:,output:,no-replace,verbose,debug,short-circuit:,keep-tmp"
@@ -235,6 +236,10 @@ process_args() {
 
 	KEEP_TMP="$keep_tmp"
 	PATCHES=("$@")
+
+	for patch in "${PATCHES[@]}"; do
+		[[ -f "$patch" ]] || die "$patch doesn't exist"
+	done
 }
 
 # temporarily disable xtrace for especially verbose code
-- 
2.53.0

[PATCH 22/48] klp-build: Suppress excessive fuzz output by default

[Josh Poimboeuf]

When a patch applies with fuzz, the detailed output from the patch tool
can be very noisy, especially for big patches.

Suppress the fuzz details by default, while keeping the "applied with
fuzz" warning.  The noise can be restored with '--verbose'.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 scripts/livepatch/klp-build | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index 115f68db49c9..a7f571df7813 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -19,12 +19,11 @@ set -o nounset
 # This helps keep execution in pipes so pipefail+errexit can catch errors.
 shopt -s lastpipe
 
-unset DEBUG_CLONE DIFF_CHECKSUM SKIP_CLEANUP XTRACE
+unset DEBUG_CLONE DIFF_CHECKSUM SKIP_CLEANUP VERBOSE XTRACE
 
 REPLACE=1
 SHORT_CIRCUIT=0
 JOBS="$(getconf _NPROCESSORS_ONLN)"
-VERBOSE="-s"
 shopt -o xtrace | grep -q 'on' && XTRACE=1
 
 # Avoid removing the previous $TMP_DIR until args have been fully processed.
@@ -194,7 +193,7 @@ process_args() {
 				shift
 				;;
 			-v | --verbose)
-				VERBOSE="V=1"
+				VERBOSE=1
 				shift
 				;;
 			-d | --debug)
@@ -381,7 +380,7 @@ apply_patch() {
 		echo "$output" >&2
 		die "$patch did not apply"
 	elif [[ "$output" =~ $drift_regex ]]; then
-		echo "$output" >&2
+		[[ -v VERBOSE ]] && echo "$output" >&2
 		warn "${patch} applied with fuzz"
 	fi
 
@@ -544,7 +543,11 @@ build_kernel() {
 	#
 	cmd+=("KBUILD_MODPOST_WARN=1")
 
-	cmd+=("$VERBOSE")
+	if [[ -v VERBOSE ]]; then
+		cmd+=("V=1")
+	else
+		cmd+=("-s")
+	fi
 	cmd+=("-j$JOBS")
 	cmd+=("KCFLAGS=-ffunction-sections -fdata-sections")
 	cmd+=("OBJTOOL_ARGS=${objtool_args[*]}")
@@ -805,7 +808,11 @@ build_patch_module() {
 	[[ $REPLACE -eq 0 ]] && cflags+=("-DKLP_NO_REPLACE")
 
 	cmd=("make")
-	cmd+=("$VERBOSE")
+	if [[ -v VERBOSE ]]; then
+		cmd+=("V=1")
+	else
+		cmd+=("-s")
+	fi
 	cmd+=("-j$JOBS")
 	cmd+=("--directory=.")
 	cmd+=("M=$KMOD_DIR")
-- 
2.53.0

[PATCH 23/48] klp-build: Fix patch cleanup on interrupt

[Josh Poimboeuf]

If a build error occurs and the user hits Ctrl-C while a large patch is
being reverted during cleanup, the cleanup EXIT trap gets re-triggered
and tries to re-revert the already partially-reverted patch.  That
causes 'patch -R' to repeatedly prompt

  "Unreversed patch detected!  Ignore -R? [n]"

for each already-reverted hunk, with no way to break out.

Fix it by adding '--force' to the patch revert command in
revert_patch(), which causes it to silently ignore already-reverted
hunks.  And ignore errors, as the cleanup is always best-effort.

For similar reasons, add to APPLIED_PATCHES before (rather than after)
applying the patch in apply_patch() so an interrupted apply will also
get cleaned up.

Fixes: d36a7343f4ba ("livepatch/klp-build: switch to GNU patch and recountdiff")
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 scripts/livepatch/klp-build | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index a7f571df7813..60231cf49e5c 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -384,15 +384,15 @@ apply_patch() {
 		warn "${patch} applied with fuzz"
 	fi
 
-	patch -d "$SRC" -p1 --no-backup-if-mismatch -r /dev/null "${extra_args[@]}" --silent < "$patch"
 	APPLIED_PATCHES+=("$patch")
+	patch -d "$SRC" -p1 --no-backup-if-mismatch -r /dev/null "${extra_args[@]}" --silent < "$patch"
 }
 
 revert_patch() {
 	local patch="$1"
 	local tmp=()
 
-	patch -d "$SRC" -p1 -R --silent --no-backup-if-mismatch -r /dev/null < "$patch"
+	patch -d "$SRC" -p1 -R --force --no-backup-if-mismatch -r /dev/null &> /dev/null < "$patch" || true
 
 	for p in "${APPLIED_PATCHES[@]}"; do
 		[[ "$p" == "$patch" ]] && continue
-- 
2.53.0

[PATCH 24/48] klp-build: Reject patches to vDSO

[Josh Poimboeuf]

vDSO code runs in userspace and can't be livepatched.  Such patches also
cause spurious "new function" errors due to generated files like
vdso*-image.c having unstable line numbers across builds.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 scripts/livepatch/klp-build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index 60231cf49e5c..deb1723b70de 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -357,7 +357,7 @@ check_unsupported_patches() {
 
 		for file in "${files[@]}"; do
 			case "$file" in
-				lib/*|*.S)
+				lib/*|*/vdso/*|*.S)
 					die "${patch}: unsupported patch to $file"
 					;;
 			esac
-- 
2.53.0

[PATCH 25/48] klp-build: Reject patches to realmode

[Josh Poimboeuf]

Realmode code is compiled as a separate 16-bit binary and embedded into
the kernel image via rmpiggy.S.  It can't be livepatched.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 scripts/livepatch/klp-build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index deb1723b70de..48abbe43f1c9 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -357,7 +357,7 @@ check_unsupported_patches() {
 
 		for file in "${files[@]}"; do
 			case "$file" in
-				lib/*|*/vdso/*|*.S)
+				lib/*|*/vdso/*|*/realmode/rm/*|*.S)
 					die "${patch}: unsupported patch to $file"
 					;;
 			esac
-- 
2.53.0

[PATCH 26/48] objtool/klp: Don't set sym->file for section symbols

[Josh Poimboeuf]

Section symbols aren't grouped after their corresponding FILE symbols.
Their sym->file should really be NULL rather than whatever random FILE
happened to be last.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/elf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index c4cb371e72b2..00c2389f345f 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -680,7 +680,7 @@ static int read_symbols(struct elf *elf)
 
 		if (is_file_sym(sym))
 			file = sym;
-		else if (sym->bind == STB_LOCAL)
+		else if (sym->bind == STB_LOCAL && !is_sec_sym(sym))
 			sym->file = file;
 	}
 
-- 
2.53.0

[PATCH 27/48] objtool: Include libsubcmd headers directly from source tree

[Josh Poimboeuf]

Instead of installing libsubcmd headers to a build output directory and
including from there, include directly from tools/lib/ where they
already exist.  This fixes clangd indexing which otherwise can't find
libsubcmd headers.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index b71d1886022e..a4484fd22a96 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile
@@ -58,7 +58,7 @@ INCLUDES := -I$(srctree)/tools/include \
 	    -I$(srctree)/tools/arch/$(SRCARCH)/include	\
 	    -I$(srctree)/tools/objtool/include \
 	    -I$(srctree)/tools/objtool/arch/$(SRCARCH)/include \
-	    -I$(LIBSUBCMD_OUTPUT)/include
+	    -I$(srctree)/tools/lib
 
 OBJTOOL_CFLAGS  := -std=gnu11 -fomit-frame-pointer -O2 -g $(WARNINGS)	\
 		   $(INCLUDES) $(LIBELF_FLAGS) $(LIBXXHASH_CFLAGS) $(HOSTCFLAGS)
@@ -135,7 +135,7 @@ $(LIBSUBCMD): fixdep $(LIBSUBCMD_OUTPUT) FORCE
 	$(Q)$(MAKE) -C $(LIBSUBCMD_DIR) O=$(LIBSUBCMD_OUTPUT) \
 		DESTDIR=$(LIBSUBCMD_OUTPUT) prefix= subdir= \
 		$(HOST_OVERRIDES) EXTRA_CFLAGS="$(OBJTOOL_CFLAGS)" \
-		$@ install_headers
+		$@
 
 $(LIBSUBCMD)-clean:
 	$(call QUIET_CLEAN, libsubcmd)
-- 
2.53.0

[PATCH 28/48] objtool/klp: Create empty checksum sections for function-less object files

[Josh Poimboeuf]

If an object file has no functions, objtool has nothing to checksum, so
it doesn't create the .discard.sym_checksum symbol.

Then when 'objtool klp diff' reads symbol checksums, it errors out due
to the missing .discard.sym_checksum section.

Instead, just create an empty checksum section to signal to
read_sym_checksums() that the file has been processed.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/check.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index f14212a8c179..54ceac857979 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1044,9 +1044,6 @@ static int create_sym_checksum_section(struct objtool_file *file)
 		if (sym->csum.checksum)
 			idx++;
 
-	if (!idx)
-		return 0;
-
 	sec = elf_create_section_pair(file->elf, ".discard.sym_checksum", entsize,
 				      idx, idx);
 	if (!sec)
-- 
2.53.0

[PATCH 29/48] klp-build: Print "objtool klp diff" command in verbose mode

[Josh Poimboeuf]

Print the full objtool command line when '--verbose' is given to help
with debugging.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 scripts/livepatch/klp-build | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index 48abbe43f1c9..84053e8aadd3 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -681,6 +681,7 @@ diff_objects() {
 
 		(
 			cd "$ORIG_DIR"
+			[[ -v VERBOSE ]] && echo "${cmd[@]}"
 			"${cmd[@]}"							\
 				1> >(tee -a "$log")					\
 				2> >(tee -a "$log" | "${filter[@]}" >&2) ||		\
-- 
2.53.0

[PATCH 30/48] objtool/klp: Handle Clang .data..Lanon anonymous data sections

[Josh Poimboeuf]

Clang generates anonymous data sections named .data..Lanon.<hash>.
These need section-symbol references in the same way as .data..Lubsan
(GCC) and .data..L__unnamed_ (Clang UBSAN) sections.  Without this,
convert_reloc_sym() fails when processing relocations that reference
these sections.

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

diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 57d2af98a33c..1951a8b2df44 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -873,9 +873,10 @@ static bool section_reference_needed(struct section *sec)
 	if (strstarts(sec->name, ".rodata"))
 		return true;
 
-	/* UBSAN anonymous data */
+	/* Anonymous data (UBSAN, Clang anonymous constants, etc.) */
 	if (strstarts(sec->name, ".data..Lubsan") ||	/* GCC */
-	    strstarts(sec->name, ".data..L__unnamed_"))	/* Clang */
+	    strstarts(sec->name, ".data..L__unnamed_") ||	/* Clang */
+	    strstarts(sec->name, ".data..Lanon."))	/* Clang */
 		return true;
 
 	return false;
-- 
2.53.0

[PATCH 31/48] objtool: Add is_alias_sym() helper

[Josh Poimboeuf]

Improve readability with a new is_alias_sym() helper.

No functional changes intended.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/check.c               | 6 +++---
 tools/objtool/include/objtool/elf.h | 5 +++++
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 54ceac857979..4c18d6e7f6c3 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -491,7 +491,7 @@ static int decode_instructions(struct objtool_file *file)
 				return -1;
 			}
 
-			if (func->embedded_insn || func->alias != func)
+			if (func->embedded_insn || is_alias_sym(func))
 				continue;
 
 			if (!find_insn(file, sec, func->offset)) {
@@ -2229,7 +2229,7 @@ static int add_jump_table_alts(struct objtool_file *file)
 		return 0;
 
 	for_each_sym(file->elf, func) {
-		if (!is_func_sym(func) || func->alias != func)
+		if (!is_func_sym(func) || is_alias_sym(func))
 			continue;
 
 		mark_func_jump_tables(file, func);
@@ -4527,7 +4527,7 @@ static int validate_symbol(struct objtool_file *file, struct section *sec,
 		return 1;
 	}
 
-	if (sym->pfunc != sym || sym->alias != sym)
+	if (sym->pfunc != sym || is_alias_sym(sym))
 		return 0;
 
 	insn = find_insn(file, sec, sym->offset);
diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
index cd5844c7b4e2..3abe4cbc584c 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -279,6 +279,11 @@ static inline bool is_local_sym(struct symbol *sym)
 	return sym->bind == STB_LOCAL;
 }
 
+static inline bool is_alias_sym(struct symbol *sym)
+{
+	return sym->alias != sym;
+}
+
 static inline bool is_prefix_func(struct symbol *sym)
 {
 	return sym->prefix;
-- 
2.53.0

[PATCH 32/48] objtool: Add is_cold_func() helper

[Josh Poimboeuf]

Add an is_cold_func() helper.  The sym->cold bit is redundant and can be
removed.

No functional changes intended.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/check.c               | 6 +++---
 tools/objtool/elf.c                 | 9 +++++----
 tools/objtool/include/objtool/elf.h | 6 +++++-
 tools/objtool/klp-diff.c            | 3 ++-
 4 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 4c18d6e7f6c3..4ed27c53c718 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2614,7 +2614,7 @@ static void mark_holes(struct objtool_file *file)
 		if (insn->jump_dest) {
 			struct symbol *dest_func = insn_func(insn->jump_dest);
 
-			if (dest_func && dest_func->cold)
+			if (dest_func && is_cold_func(dest_func))
 				dest_func->ignore = true;
 		}
 	}
@@ -4426,8 +4426,8 @@ static int create_prefix_symbol(struct objtool_file *file, struct symbol *func)
 	char name[SYM_NAME_LEN];
 	struct cfi_state *cfi;
 
-	if (!is_func_sym(func) || is_prefix_func(func) ||
-	    func->cold || func->static_call_tramp)
+	if (!is_func_sym(func) || is_prefix_func(func) || is_cold_func(func) ||
+	    func->static_call_tramp)
 		return 0;
 
 	if ((strlen(func->name) + sizeof("__pfx_") > SYM_NAME_LEN)) {
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 00c2389f345f..8a6e1338af97 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -586,8 +586,11 @@ static int elf_add_symbol(struct elf *elf, struct symbol *sym)
 	if (strstarts(sym->name, ".klp.sym"))
 		sym->klp = 1;
 
+	sym->pfunc = sym->cfunc = sym;
+
 	if (!sym->klp && !is_sec_sym(sym) && strstr(sym->name, ".cold")) {
-		sym->cold = 1;
+		/* Tell read_symbols() this is a cold subfunction */
+		sym->pfunc = NULL;
 
 		/*
 		 * Clang doesn't mark cold subfunctions as STT_FUNC, which
@@ -596,8 +599,6 @@ static int elf_add_symbol(struct elf *elf, struct symbol *sym)
 		sym->type = STT_FUNC;
 	}
 
-	sym->pfunc = sym->cfunc = sym;
-
 	return 0;
 }
 
@@ -695,7 +696,7 @@ static int read_symbols(struct elf *elf)
 			char *pname;
 			size_t pnamelen;
 
-			if (!sym->cold)
+			if (sym->pfunc)
 				continue;
 
 			coldstr = strstr(sym->name, ".cold");
diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
index 3abe4cbc584c..82b9fb05af26 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -83,7 +83,6 @@ struct symbol {
 	u8 frame_pointer     : 1;
 	u8 ignore	     : 1;
 	u8 nocfi             : 1;
-	u8 cold		     : 1;
 	u8 prefix	     : 1;
 	u8 debug_checksum    : 1;
 	u8 changed	     : 1;
@@ -289,6 +288,11 @@ static inline bool is_prefix_func(struct symbol *sym)
 	return sym->prefix;
 }
 
+static inline bool is_cold_func(struct symbol *sym)
+{
+	return sym->pfunc != sym;
+}
+
 static inline bool is_reloc_sec(struct section *sec)
 {
 	return sec->sh.sh_type == SHT_RELA || sec->sh.sh_type == SHT_REL;
diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 1951a8b2df44..266f0d2ba4fe 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -1718,7 +1718,8 @@ static int create_klp_sections(struct elfs *e)
 		unsigned long sympos;
 		void *func_data;
 
-		if (!is_func_sym(sym) || sym->cold || !sym->clone || !sym->clone->changed)
+		if (!is_func_sym(sym) || is_cold_func(sym) ||
+		    !sym->clone || !sym->clone->changed)
 			continue;
 
 		/* allocate klp_func_ext */
-- 
2.53.0

[PATCH 33/48] objtool/klp: Extricate checksum calculation from validate_branch()

[Josh Poimboeuf]

In preparation for porting the checksum code to other arches, make its
functionality independent from the CFG reverse engineering code.

Move it into a standalone calculate_checksums() function which iterates
all functions and instructions directly, rather than being called inline
from do_validate_branch().

Since checksum_update_insn() is no longer called during CFG traversal,
it needs to manually iterate the alternatives.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/check.c                    | 106 +++++++++++++++++------
 tools/objtool/include/objtool/checksum.h |   6 +-
 2 files changed, 80 insertions(+), 32 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 4ed27c53c718..c8208caa4b2c 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1350,10 +1350,7 @@ static struct reloc *insn_reloc(struct objtool_file *file, struct instruction *i
 {
 	struct reloc *reloc;
 
-	if (insn->no_reloc)
-		return NULL;
-
-	if (!file)
+	if (!file || insn->no_reloc || insn->fake)
 		return NULL;
 
 	reloc = find_reloc_by_dest_range(file->elf, insn->sec,
@@ -2622,9 +2619,17 @@ static void mark_holes(struct objtool_file *file)
 
 static bool validate_branch_enabled(void)
 {
-	return opts.stackval ||
-	       opts.orc ||
-	       opts.uaccess ||
+	return opts.stackval	||
+	       opts.orc		||
+	       opts.uaccess;
+}
+
+static bool alts_needed(void)
+{
+	return validate_branch_enabled()	||
+	       opts.noinstr			||
+	       opts.hack_jump_label		||
+	       opts.disas			||
 	       opts.checksum;
 }
 
@@ -2658,7 +2663,7 @@ static int decode_sections(struct objtool_file *file)
 	 * Must be before add_jump_destinations(), which depends on 'func'
 	 * being set for alternatives, to enable proper sibling call detection.
 	 */
-	if (validate_branch_enabled() || opts.noinstr || opts.hack_jump_label || opts.disas) {
+	if (alts_needed()) {
 		if (add_special_section_alts(file))
 			return -1;
 	}
@@ -3654,6 +3659,7 @@ static bool skip_alt_group(struct instruction *insn)
 	return alt_insn->type == INSN_CLAC || alt_insn->type == INSN_STAC;
 }
 
+#ifdef BUILD_KLP
 static void enable_debug_checksum_cb(struct symbol *sym, void *d)
 {
 	bool *found = d;
@@ -3705,8 +3711,10 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func,
 				 struct instruction *insn)
 {
 	struct reloc *reloc = insn_reloc(file, insn);
+	struct alternative *alt;
 	unsigned long offset;
 	struct symbol *sym;
+	static bool in_alt;
 
 	if (insn->fake)
 		return;
@@ -3719,7 +3727,7 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func,
 		if (call_dest)
 			checksum_update(func, insn, call_dest->demangled_name,
 					strlen(call_dest->demangled_name));
-		return;
+		goto alts;
 	}
 
 	sym = reloc->sym;
@@ -3730,21 +3738,78 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func,
 
 		str = sym->sec->data->d_buf + sym->offset + offset;
 		checksum_update(func, insn, str, strlen(str));
-		return;
+		goto alts;
 	}
 
 	if (is_sec_sym(sym)) {
 		sym = find_symbol_containing(reloc->sym->sec, offset);
 		if (!sym)
-			return;
+			goto alts;
 
 		offset -= sym->offset;
 	}
 
 	checksum_update(func, insn, sym->demangled_name, strlen(sym->demangled_name));
 	checksum_update(func, insn, &offset, sizeof(offset));
+
+alts:
+	for (alt = insn->alts; alt; alt = alt->next) {
+		struct alt_group *alt_group = alt->insn->alt_group;
+
+		/* Prevent __ex_table recursion, e.g. LOAD_SEGMENT() */
+		if (in_alt)
+			break;
+		in_alt = true;
+
+		checksum_update(func, insn, &alt->type, sizeof(alt->type));
+
+		if (alt_group && alt_group->orig_group) {
+			struct instruction *alt_insn;
+
+			checksum_update(func, insn, &alt_group->feature, sizeof(alt_group->feature));
+
+			for (alt_insn = alt->insn; alt_insn; alt_insn = next_insn_same_sec(file, alt_insn)) {
+				checksum_update_insn(file, func, alt_insn);
+				if (!alt_group->last_insn || alt_insn == alt_group->last_insn)
+					break;
+			}
+		} else {
+			checksum_update_insn(file, func, alt->insn);
+		}
+
+		in_alt = false;
+	}
 }
 
+static int calculate_checksums(struct objtool_file *file)
+{
+	struct instruction *insn;
+	struct symbol *func;
+
+	if (checksum_debug_init(file))
+		return -1;
+
+	for_each_sym(file->elf, func) {
+		/*
+		 * Skip cold subfunctions and aliases: they share the
+		 * parent's checksum via func_for_each_insn() which
+		 * follows func->cfunc into the cold subfunction.
+		 */
+		if (!is_func_sym(func) || is_cold_func(func) ||
+		    is_alias_sym(func) || !func->len)
+			continue;
+
+		checksum_init(func);
+
+		func_for_each_insn(file, func, insn)
+			checksum_update_insn(file, func, insn);
+
+		checksum_finish(func);
+	}
+	return 0;
+}
+#endif /* BUILD_KLP */
+
 static int validate_branch(struct objtool_file *file, struct symbol *func,
 			   struct instruction *insn, struct insn_state state);
 static int do_validate_branch(struct objtool_file *file, struct symbol *func,
@@ -4026,9 +4091,6 @@ static int do_validate_branch(struct objtool_file *file, struct symbol *func,
 		insn->trace = 0;
 		next_insn = next_insn_to_validate(file, insn);
 
-		if (opts.checksum && func && insn->sec)
-			checksum_update_insn(file, func, insn);
-
 		if (func && insn_func(insn) && func != insn_func(insn)->pfunc) {
 			/* Ignore KCFI type preambles, which always fall through */
 			if (is_prefix_func(func))
@@ -4094,9 +4156,6 @@ static int validate_unwind_hint(struct objtool_file *file,
 		struct symbol *func = insn_func(insn);
 		int ret;
 
-		if (opts.checksum)
-			checksum_init(func);
-
 		ret = validate_branch(file, func, insn, *state);
 		if (ret)
 			BT_INSN(insn, "<=== (hint)");
@@ -4539,9 +4598,6 @@ static int validate_symbol(struct objtool_file *file, struct section *sec,
 
 	func = insn_func(insn);
 
-	if (opts.checksum)
-		checksum_init(func);
-
 	if (opts.trace && !fnmatch(opts.trace, sym->name, 0)) {
 		trace_enable();
 		TRACE("%s: validation begin\n", sym->name);
@@ -4554,9 +4610,6 @@ static int validate_symbol(struct objtool_file *file, struct section *sec,
 	TRACE("%s: validation %s\n\n", sym->name, ret ? "failed" : "end");
 	trace_disable();
 
-	if (opts.checksum)
-		checksum_finish(func);
-
 	return ret;
 }
 
@@ -5011,10 +5064,6 @@ int check(struct objtool_file *file)
 	cfi_hash_add(&init_cfi);
 	cfi_hash_add(&func_cfi);
 
-	ret = checksum_debug_init(file);
-	if (ret)
-		goto out;
-
 	ret = decode_sections(file);
 	if (ret)
 		goto out;
@@ -5105,6 +5154,9 @@ int check(struct objtool_file *file)
 		warnings += check_abs_references(file);
 
 	if (opts.checksum) {
+		ret = calculate_checksums(file);
+		if (ret)
+			goto out;
 		ret = create_sym_checksum_section(file);
 		if (ret)
 			goto out;
diff --git a/tools/objtool/include/objtool/checksum.h b/tools/objtool/include/objtool/checksum.h
index 0bd16fe9168b..3f25df90305d 100644
--- a/tools/objtool/include/objtool/checksum.h
+++ b/tools/objtool/include/objtool/checksum.h
@@ -33,11 +33,7 @@ static inline void checksum_finish(struct symbol *func)
 
 #else /* !BUILD_KLP */
 
-static inline void checksum_init(struct symbol *func) {}
-static inline void checksum_update(struct symbol *func,
-				   struct instruction *insn,
-				   const void *data, size_t size) {}
-static inline void checksum_finish(struct symbol *func) {}
+static inline int calculate_checksums(struct objtool_file *file) { return -ENOSYS; }
 
 #endif /* !BUILD_KLP */
 
-- 
2.53.0

[PATCH 34/48] objtool: Consolidate file decoding into decode_file()

[Josh Poimboeuf]

decode_sections() relies on CFI and cfi_hash initialization done
separately in check(), making it unusable outside of check().

Consolidate the initialization into decode_sections() and rename it to
decode_file(), and make it global along with free_insns() and
insn_reloc() for use by other objtool components -- namely, the checksum
code which will be moving to another file.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/check.c                 | 36 +++++++++++++--------------
 tools/objtool/include/objtool/check.h |  5 ++++
 2 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index c8208caa4b2c..17cb9265973d 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1346,7 +1346,7 @@ __weak bool arch_is_embedded_insn(struct symbol *sym)
 	return false;
 }
 
-static struct reloc *insn_reloc(struct objtool_file *file, struct instruction *insn)
+struct reloc *insn_reloc(struct objtool_file *file, struct instruction *insn)
 {
 	struct reloc *reloc;
 
@@ -2633,8 +2633,21 @@ static bool alts_needed(void)
 	       opts.checksum;
 }
 
-static int decode_sections(struct objtool_file *file)
+int decode_file(struct objtool_file *file)
 {
+	arch_initial_func_cfi_state(&initial_func_cfi);
+	init_cfi_state(&init_cfi);
+	init_cfi_state(&func_cfi);
+	set_func_state(&func_cfi);
+	init_cfi_state(&force_undefined_cfi);
+	force_undefined_cfi.force_undefined = true;
+
+	if (!cfi_hash_alloc(1UL << (file->elf->symbol_bits - 3)))
+		return -1;
+
+	cfi_hash_add(&init_cfi);
+	cfi_hash_add(&func_cfi);
+
 	file->klp = is_livepatch_module(file);
 
 	mark_rodata(file);
@@ -5002,7 +5015,7 @@ struct insn_chunk {
  * which can trigger more allocations for .debug_* sections whose data hasn't
  * been read yet.
  */
-static void free_insns(struct objtool_file *file)
+void free_insns(struct objtool_file *file)
 {
 	struct instruction *insn;
 	struct insn_chunk *chunks = NULL, *chunk;
@@ -5049,22 +5062,7 @@ int check(struct objtool_file *file)
 		objtool_disas_ctx = disas_ctx;
 	}
 
-	arch_initial_func_cfi_state(&initial_func_cfi);
-	init_cfi_state(&init_cfi);
-	init_cfi_state(&func_cfi);
-	set_func_state(&func_cfi);
-	init_cfi_state(&force_undefined_cfi);
-	force_undefined_cfi.force_undefined = true;
-
-	if (!cfi_hash_alloc(1UL << (file->elf->symbol_bits - 3))) {
-		ret = -1;
-		goto out;
-	}
-
-	cfi_hash_add(&init_cfi);
-	cfi_hash_add(&func_cfi);
-
-	ret = decode_sections(file);
+	ret = decode_file(file);
 	if (ret)
 		goto out;
 
diff --git a/tools/objtool/include/objtool/check.h b/tools/objtool/include/objtool/check.h
index 5f2f77bd9b41..6489e52ea2f2 100644
--- a/tools/objtool/include/objtool/check.h
+++ b/tools/objtool/include/objtool/check.h
@@ -155,6 +155,11 @@ struct instruction *next_insn_same_sec(struct objtool_file *file, struct instruc
 	     insn && insn->offset < sym->offset + sym->len;		\
 	     insn = next_insn_same_sec(file, insn))
 
+struct reloc *insn_reloc(struct objtool_file *file, struct instruction *insn);
+
+int decode_file(struct objtool_file *file);
+void free_insns(struct objtool_file *file);
+
 const char *objtool_disas_insn(struct instruction *insn);
 
 extern size_t sym_name_max_len;
-- 
2.53.0

[PATCH 35/48] objtool/klp: Add "objtool klp checksum" subcommand

[Josh Poimboeuf]

Move the checksum functionality out of the main objtool command into a
new "objtool klp checksum" subcommand.

This has the benefit of making the code (and the patch generation
process itself) more modular.

For bisectability, both "objtool --checksum" and "objtool klp checksum"
work for now.  The former will be removed after klp-build has been
converted to use the new subcommand.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/Build                      |   2 +-
 tools/objtool/builtin-klp.c              |   1 +
 tools/objtool/check.c                    | 209 +-----------------
 tools/objtool/include/objtool/check.h    |   6 +
 tools/objtool/include/objtool/checksum.h |   4 +
 tools/objtool/include/objtool/klp.h      |   1 +
 tools/objtool/klp-checksum.c             | 257 +++++++++++++++++++++++
 tools/objtool/klp-diff.c                 |   2 +-
 8 files changed, 273 insertions(+), 209 deletions(-)
 create mode 100644 tools/objtool/klp-checksum.c

diff --git a/tools/objtool/Build b/tools/objtool/Build
index 600da051af12..93a37b0dfd31 100644
--- a/tools/objtool/Build
+++ b/tools/objtool/Build
@@ -12,7 +12,7 @@ objtool-$(BUILD_DISAS) += disas.o
 objtool-$(BUILD_DISAS) += trace.o
 
 objtool-$(BUILD_ORC) += orc_gen.o orc_dump.o
-objtool-$(BUILD_KLP) += builtin-klp.o klp-diff.o klp-post-link.o
+objtool-$(BUILD_KLP) += builtin-klp.o klp-checksum.o klp-diff.o klp-post-link.o
 
 objtool-y += libstring.o
 objtool-y += libctype.o
diff --git a/tools/objtool/builtin-klp.c b/tools/objtool/builtin-klp.c
index 56d5a5b92f72..58c3b9bda3eb 100644
--- a/tools/objtool/builtin-klp.c
+++ b/tools/objtool/builtin-klp.c
@@ -13,6 +13,7 @@ struct subcmd {
 };
 
 static struct subcmd subcmds[] = {
+	{ "checksum",		"Generate per-function checksums",			cmd_klp_checksum, },
 	{ "diff",		"Generate binary diff of two object files",		cmd_klp_diff, },
 	{ "post-link",		"Finalize klp symbols/relocs after module linking",	cmd_klp_post_link, },
 };
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 17cb9265973d..3e5d335d0e29 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -64,8 +64,8 @@ struct instruction *next_insn_same_sec(struct objtool_file *file,
 	return insn;
 }
 
-static struct instruction *next_insn_same_func(struct objtool_file *file,
-					       struct instruction *insn)
+struct instruction *next_insn_same_func(struct objtool_file *file,
+				       struct instruction *insn)
 {
 	struct instruction *next = next_insn_same_sec(file, insn);
 	struct symbol *func = insn_func(insn);
@@ -113,10 +113,6 @@ static struct instruction *prev_insn_same_sym(struct objtool_file *file,
 		for_each_sec(file->elf, __sec)				\
 			sec_for_each_insn(file, __sec, insn)
 
-#define func_for_each_insn(file, func, insn)				\
-	for (insn = find_insn(file, func->sec, func->offset);		\
-	     insn;							\
-	     insn = next_insn_same_func(file, insn))
 
 #define sym_for_each_insn(file, sym, insn)				\
 	for (insn = find_insn(file, sym->sec, sym->offset);		\
@@ -1023,56 +1019,6 @@ static int create_direct_call_sections(struct objtool_file *file)
 	return 0;
 }
 
-#ifdef BUILD_KLP
-static int create_sym_checksum_section(struct objtool_file *file)
-{
-	struct section *sec;
-	struct symbol *sym;
-	unsigned int idx = 0;
-	struct sym_checksum *checksum;
-	size_t entsize = sizeof(struct sym_checksum);
-
-	sec = find_section_by_name(file->elf, ".discard.sym_checksum");
-	if (sec) {
-		if (!opts.dryrun)
-			WARN("file already has .discard.sym_checksum section, skipping");
-
-		return 0;
-	}
-
-	for_each_sym(file->elf, sym)
-		if (sym->csum.checksum)
-			idx++;
-
-	sec = elf_create_section_pair(file->elf, ".discard.sym_checksum", entsize,
-				      idx, idx);
-	if (!sec)
-		return -1;
-
-	idx = 0;
-	for_each_sym(file->elf, sym) {
-		if (!sym->csum.checksum)
-			continue;
-
-		if (!elf_init_reloc(file->elf, sec->rsec, idx, idx * entsize,
-				    sym, 0, R_TEXT64))
-			return -1;
-
-		checksum = (struct sym_checksum *)sec->data->d_buf + idx;
-		checksum->addr = 0; /* reloc */
-		checksum->checksum = sym->csum.checksum;
-
-		mark_sec_changed(file->elf, sec, true);
-
-		idx++;
-	}
-
-	return 0;
-}
-#else
-static int create_sym_checksum_section(struct objtool_file *file) { return -EINVAL; }
-#endif
-
 /*
  * Warnings shouldn't be reported for ignored functions.
  */
@@ -3672,157 +3618,6 @@ static bool skip_alt_group(struct instruction *insn)
 	return alt_insn->type == INSN_CLAC || alt_insn->type == INSN_STAC;
 }
 
-#ifdef BUILD_KLP
-static void enable_debug_checksum_cb(struct symbol *sym, void *d)
-{
-	bool *found = d;
-
-	if (!is_func_sym(sym))
-		return;
-
-	sym->debug_checksum = 1;
-	*found = true;
-}
-
-static int checksum_debug_init(struct objtool_file *file)
-{
-	char *dup, *s;
-
-	if (!opts.debug_checksum)
-		return 0;
-
-	dup = strdup(opts.debug_checksum);
-	if (!dup) {
-		ERROR_GLIBC("strdup");
-		return -1;
-	}
-
-	s = dup;
-	while (*s) {
-		bool found = false;
-		char *comma;
-
-		comma = strchr(s, ',');
-		if (comma)
-			*comma = '\0';
-
-		iterate_sym_by_name(file->elf, s, enable_debug_checksum_cb, &found);
-		if (!found)
-			WARN("--debug-checksum: can't find '%s'", s);
-
-		if (!comma)
-			break;
-
-		s = comma + 1;
-	}
-
-	free(dup);
-	return 0;
-}
-
-static void checksum_update_insn(struct objtool_file *file, struct symbol *func,
-				 struct instruction *insn)
-{
-	struct reloc *reloc = insn_reloc(file, insn);
-	struct alternative *alt;
-	unsigned long offset;
-	struct symbol *sym;
-	static bool in_alt;
-
-	if (insn->fake)
-		return;
-
-	checksum_update(func, insn, insn->sec->data->d_buf + insn->offset, insn->len);
-
-	if (!reloc) {
-		struct symbol *call_dest = insn_call_dest(insn);
-
-		if (call_dest)
-			checksum_update(func, insn, call_dest->demangled_name,
-					strlen(call_dest->demangled_name));
-		goto alts;
-	}
-
-	sym = reloc->sym;
-	offset = arch_insn_adjusted_addend(insn, reloc);
-
-	if (is_string_sec(sym->sec)) {
-		char *str;
-
-		str = sym->sec->data->d_buf + sym->offset + offset;
-		checksum_update(func, insn, str, strlen(str));
-		goto alts;
-	}
-
-	if (is_sec_sym(sym)) {
-		sym = find_symbol_containing(reloc->sym->sec, offset);
-		if (!sym)
-			goto alts;
-
-		offset -= sym->offset;
-	}
-
-	checksum_update(func, insn, sym->demangled_name, strlen(sym->demangled_name));
-	checksum_update(func, insn, &offset, sizeof(offset));
-
-alts:
-	for (alt = insn->alts; alt; alt = alt->next) {
-		struct alt_group *alt_group = alt->insn->alt_group;
-
-		/* Prevent __ex_table recursion, e.g. LOAD_SEGMENT() */
-		if (in_alt)
-			break;
-		in_alt = true;
-
-		checksum_update(func, insn, &alt->type, sizeof(alt->type));
-
-		if (alt_group && alt_group->orig_group) {
-			struct instruction *alt_insn;
-
-			checksum_update(func, insn, &alt_group->feature, sizeof(alt_group->feature));
-
-			for (alt_insn = alt->insn; alt_insn; alt_insn = next_insn_same_sec(file, alt_insn)) {
-				checksum_update_insn(file, func, alt_insn);
-				if (!alt_group->last_insn || alt_insn == alt_group->last_insn)
-					break;
-			}
-		} else {
-			checksum_update_insn(file, func, alt->insn);
-		}
-
-		in_alt = false;
-	}
-}
-
-static int calculate_checksums(struct objtool_file *file)
-{
-	struct instruction *insn;
-	struct symbol *func;
-
-	if (checksum_debug_init(file))
-		return -1;
-
-	for_each_sym(file->elf, func) {
-		/*
-		 * Skip cold subfunctions and aliases: they share the
-		 * parent's checksum via func_for_each_insn() which
-		 * follows func->cfunc into the cold subfunction.
-		 */
-		if (!is_func_sym(func) || is_cold_func(func) ||
-		    is_alias_sym(func) || !func->len)
-			continue;
-
-		checksum_init(func);
-
-		func_for_each_insn(file, func, insn)
-			checksum_update_insn(file, func, insn);
-
-		checksum_finish(func);
-	}
-	return 0;
-}
-#endif /* BUILD_KLP */
-
 static int validate_branch(struct objtool_file *file, struct symbol *func,
 			   struct instruction *insn, struct insn_state state);
 static int do_validate_branch(struct objtool_file *file, struct symbol *func,
diff --git a/tools/objtool/include/objtool/check.h b/tools/objtool/include/objtool/check.h
index 6489e52ea2f2..eea64728d39b 100644
--- a/tools/objtool/include/objtool/check.h
+++ b/tools/objtool/include/objtool/check.h
@@ -144,6 +144,12 @@ struct instruction *find_insn(struct objtool_file *file,
 			      struct section *sec, unsigned long offset);
 
 struct instruction *next_insn_same_sec(struct objtool_file *file, struct instruction *insn);
+struct instruction *next_insn_same_func(struct objtool_file *file, struct instruction *insn);
+
+#define func_for_each_insn(file, func, insn)				\
+	for (insn = find_insn(file, func->sec, func->offset);		\
+	     insn;							\
+	     insn = next_insn_same_func(file, insn))
 
 #define sec_for_each_insn(file, _sec, insn)				\
 	for (insn = find_insn(file, _sec, 0);				\
diff --git a/tools/objtool/include/objtool/checksum.h b/tools/objtool/include/objtool/checksum.h
index 3f25df90305d..be4eb7dfe6f2 100644
--- a/tools/objtool/include/objtool/checksum.h
+++ b/tools/objtool/include/objtool/checksum.h
@@ -31,9 +31,13 @@ static inline void checksum_finish(struct symbol *func)
 	}
 }
 
+int calculate_checksums(struct objtool_file *file);
+int create_sym_checksum_section(struct objtool_file *file);
+
 #else /* !BUILD_KLP */
 
 static inline int calculate_checksums(struct objtool_file *file) { return -ENOSYS; }
+static inline int create_sym_checksum_section(struct objtool_file *file) { return -EINVAL; }
 
 #endif /* !BUILD_KLP */
 
diff --git a/tools/objtool/include/objtool/klp.h b/tools/objtool/include/objtool/klp.h
index e32e5e8bc631..6f60cf05db86 100644
--- a/tools/objtool/include/objtool/klp.h
+++ b/tools/objtool/include/objtool/klp.h
@@ -29,6 +29,7 @@ struct klp_reloc {
 	u32 type;
 };
 
+int cmd_klp_checksum(int argc, const char **argv);
 int cmd_klp_diff(int argc, const char **argv);
 int cmd_klp_post_link(int argc, const char **argv);
 
diff --git a/tools/objtool/klp-checksum.c b/tools/objtool/klp-checksum.c
new file mode 100644
index 000000000000..4edd29028bff
--- /dev/null
+++ b/tools/objtool/klp-checksum.c
@@ -0,0 +1,257 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#include <string.h>
+#include <subcmd/parse-options.h>
+
+#include <objtool/arch.h>
+#include <objtool/builtin.h>
+#include <objtool/check.h>
+#include <objtool/elf.h>
+#include <objtool/klp.h>
+#include <objtool/objtool.h>
+#include <objtool/warn.h>
+#include <objtool/checksum.h>
+
+static void enable_debug_checksum_cb(struct symbol *sym, void *d)
+{
+	bool *found = d;
+
+	if (!is_func_sym(sym))
+		return;
+
+	sym->debug_checksum = 1;
+	*found = true;
+}
+
+static int checksum_debug_init(struct objtool_file *file)
+{
+	char *dup, *s;
+
+	if (!opts.debug_checksum)
+		return 0;
+
+	dup = strdup(opts.debug_checksum);
+	if (!dup) {
+		ERROR_GLIBC("strdup");
+		return -1;
+	}
+
+	s = dup;
+	while (*s) {
+		bool found = false;
+		char *comma;
+
+		comma = strchr(s, ',');
+		if (comma)
+			*comma = '\0';
+
+		iterate_sym_by_name(file->elf, s, enable_debug_checksum_cb, &found);
+		if (!found)
+			WARN("--debug-checksum: can't find '%s'", s);
+
+		if (!comma)
+			break;
+
+		s = comma + 1;
+	}
+
+	free(dup);
+	return 0;
+}
+
+static void checksum_update_insn(struct objtool_file *file, struct symbol *func,
+				 struct instruction *insn)
+{
+	struct reloc *reloc = insn_reloc(file, insn);
+	struct alternative *alt;
+	unsigned long offset;
+	struct symbol *sym;
+	static bool in_alt;
+
+	if (insn->fake)
+		return;
+
+	checksum_update(func, insn, insn->sec->data->d_buf + insn->offset, insn->len);
+
+	if (!reloc) {
+		struct symbol *call_dest = insn_call_dest(insn);
+
+		if (call_dest)
+			checksum_update(func, insn, call_dest->demangled_name,
+					strlen(call_dest->demangled_name));
+		goto alts;
+	}
+
+	sym = reloc->sym;
+	offset = arch_insn_adjusted_addend(insn, reloc);
+
+	if (is_string_sec(sym->sec)) {
+		char *str;
+
+		str = sym->sec->data->d_buf + sym->offset + offset;
+		checksum_update(func, insn, str, strlen(str));
+		goto alts;
+	}
+
+	if (is_sec_sym(sym)) {
+		sym = find_symbol_containing(reloc->sym->sec, offset);
+		if (!sym)
+			goto alts;
+
+		offset -= sym->offset;
+	}
+
+	checksum_update(func, insn, sym->demangled_name, strlen(sym->demangled_name));
+	checksum_update(func, insn, &offset, sizeof(offset));
+
+alts:
+	for (alt = insn->alts; alt; alt = alt->next) {
+		struct alt_group *alt_group = alt->insn->alt_group;
+
+		/* Prevent __ex_table recursion, e.g. LOAD_SEGMENT() */
+		if (in_alt)
+			break;
+		in_alt = true;
+
+		checksum_update(func, insn, &alt->type, sizeof(alt->type));
+
+		if (alt_group && alt_group->orig_group) {
+			struct instruction *alt_insn;
+
+			checksum_update(func, insn, &alt_group->feature, sizeof(alt_group->feature));
+
+			for (alt_insn = alt->insn; alt_insn; alt_insn = next_insn_same_sec(file, alt_insn)) {
+				checksum_update_insn(file, func, alt_insn);
+				if (!alt_group->last_insn || alt_insn == alt_group->last_insn)
+					break;
+			}
+		} else {
+			checksum_update_insn(file, func, alt->insn);
+		}
+
+		in_alt = false;
+	}
+}
+
+int calculate_checksums(struct objtool_file *file)
+{
+	struct instruction *insn;
+	struct symbol *func;
+
+	if (checksum_debug_init(file))
+		return -1;
+
+	for_each_sym(file->elf, func) {
+		/*
+		 * Skip cold subfunctions and aliases: they share the
+		 * parent's checksum via func_for_each_insn() which
+		 * follows func->cfunc into the cold subfunction.
+		 */
+		if (!is_func_sym(func) || is_cold_func(func) ||
+		    is_alias_sym(func) || !func->len)
+			continue;
+
+		checksum_init(func);
+
+		func_for_each_insn(file, func, insn)
+			checksum_update_insn(file, func, insn);
+
+		checksum_finish(func);
+	}
+	return 0;
+}
+
+int create_sym_checksum_section(struct objtool_file *file)
+{
+	struct section *sec;
+	struct symbol *sym;
+	unsigned int idx = 0;
+	struct sym_checksum *checksum;
+	size_t entsize = sizeof(struct sym_checksum);
+
+	sec = find_section_by_name(file->elf, ".discard.sym_checksum");
+	if (sec) {
+		if (!opts.dryrun)
+			WARN("file already has .discard.sym_checksum section, skipping");
+
+		return 0;
+	}
+
+	for_each_sym(file->elf, sym)
+		if (sym->csum.checksum)
+			idx++;
+
+	sec = elf_create_section_pair(file->elf, ".discard.sym_checksum", entsize,
+				      idx, idx);
+	if (!sec)
+		return -1;
+
+	idx = 0;
+	for_each_sym(file->elf, sym) {
+		if (!sym->csum.checksum)
+			continue;
+
+		if (!elf_init_reloc(file->elf, sec->rsec, idx, idx * entsize,
+				    sym, 0, R_TEXT64))
+			return -1;
+
+		checksum = (struct sym_checksum *)sec->data->d_buf + idx;
+		checksum->addr = 0; /* reloc */
+		checksum->checksum = sym->csum.checksum;
+
+		mark_sec_changed(file->elf, sec, true);
+
+		idx++;
+	}
+
+	return 0;
+}
+
+static const char * const klp_checksum_usage[] = {
+	"objtool klp checksum [<options>] file.o",
+	NULL,
+};
+
+int cmd_klp_checksum(int argc, const char **argv)
+{
+	struct objtool_file *file;
+	int ret;
+
+	const struct option options[] = {
+		OPT_STRING(0,	"debug-checksum", &opts.debug_checksum,	"funcs", "enable checksum debug output"),
+		OPT_BOOLEAN(0,	"dry-run", &opts.dryrun, "don't write modifications"),
+		OPT_END(),
+	};
+
+	argc = parse_options(argc, argv, options, klp_checksum_usage, 0);
+	if (argc != 1)
+		usage_with_options(klp_checksum_usage, options);
+
+	opts.checksum = true;
+
+	objname = argv[0];
+
+	file = objtool_open_read(objname);
+	if (!file)
+		return 1;
+
+	ret = decode_file(file);
+	if (ret)
+		goto out;
+
+	ret = calculate_checksums(file);
+	if (ret)
+		goto out;
+
+	ret = create_sym_checksum_section(file);
+
+out:
+	free_insns(file);
+
+	if (ret)
+		return ret;
+
+	if (!opts.dryrun && file->elf->changed && elf_write(file->elf))
+		return 1;
+
+	return elf_close(file->elf);
+}
diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 266f0d2ba4fe..c903aa65d4b6 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -171,7 +171,7 @@ static int read_sym_checksums(struct elf *elf)
 
 	sec = find_section_by_name(elf, ".discard.sym_checksum");
 	if (!sec) {
-		ERROR("'%s' missing .discard.sym_checksum section, file not processed by 'objtool --checksum'?",
+		ERROR("'%s' missing .discard.sym_checksum section, file not processed by 'objtool klp checksum'?",
 		      elf->name);
 		return -1;
 	}
-- 
2.53.0

[PATCH 36/48] klp-build: Use "objtool klp checksum" subcommand

[Josh Poimboeuf]

Use the new "objtool klp checksum" subcommand instead of injecting
--checksum into every objtool invocation via OBJTOOL_ARGS during the
kernel build.

This decouples checksum generation from the build, running it in
separate post-build passes, making the code (and the patch generation
pipeline itself) more modular.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 scripts/livepatch/klp-build | 93 +++++++++++++++++++++++++------------
 1 file changed, 64 insertions(+), 29 deletions(-)

diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index 84053e8aadd3..d29ef3022556 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -39,10 +39,12 @@ OBJ="$(pwd)"
 CONFIG="$OBJ/.config"
 TMP_DIR="$OBJ/klp-tmp"
 
-ORIG_DIR="$TMP_DIR/orig"
-PATCHED_DIR="$TMP_DIR/patched"
-DIFF_DIR="$TMP_DIR/diff"
-KMOD_DIR="$TMP_DIR/kmod"
+ORIG_DIR="$TMP_DIR/1-orig"
+PATCHED_DIR="$TMP_DIR/2-patched"
+ORIG_CSUM_DIR="$TMP_DIR/3-checksum-orig"
+PATCHED_CSUM_DIR="$TMP_DIR/3-checksum-patched"
+DIFF_DIR="$TMP_DIR/4-diff"
+KMOD_DIR="$TMP_DIR/5-kmod"
 
 STASH_DIR="$TMP_DIR/stash"
 TIMESTAMP="$TMP_DIR/timestamp"
@@ -138,10 +140,11 @@ Options:
 Advanced Options:
    -d, --debug			Show symbol/reloc cloning decisions
    -S, --short-circuit=STEP	Start at build step (requires prior --keep-tmp)
-				   1|orig	Build original kernel (default)
-				   2|patched	Build patched kernel
-				   3|diff	Diff objects
-				   4|kmod	Build patch module
+				   1|orig		Build original kernel (default)
+				   2|patched		Build patched kernel
+				   3|checksum		Generate checksums
+				   4|diff		Diff objects
+				   5|kmod		Build patch module
    -T, --keep-tmp		Preserve tmp dir on exit
 
 EOF
@@ -205,10 +208,11 @@ process_args() {
 				[[ ! -d "$TMP_DIR" ]] && die "--short-circuit requires preserved klp-tmp dir"
 				keep_tmp=1
 				case "$2" in
-					1 | orig)	SHORT_CIRCUIT=1; ;;
-					2 | patched)	SHORT_CIRCUIT=2; ;;
-					3 | diff)	SHORT_CIRCUIT=3; ;;
-					4 | mod)	SHORT_CIRCUIT=4; ;;
+					1 | orig)		SHORT_CIRCUIT=1; ;;
+					2 | patched)		SHORT_CIRCUIT=2; ;;
+					3 | checksum)		SHORT_CIRCUIT=3; ;;
+					4 | diff)		SHORT_CIRCUIT=4; ;;
+					5 | kmod)		SHORT_CIRCUIT=5; ;;
 					*)		die "invalid short-circuit step '$2'" ;;
 				esac
 				shift 2
@@ -519,11 +523,8 @@ clean_kernel() {
 build_kernel() {
 	local build="$1"
 	local log="$TMP_DIR/build.log"
-	local objtool_args=()
 	local cmd=()
 
-	objtool_args=("--checksum")
-
 	cmd=("make")
 
 	# When a patch to a kernel module references a newly created unexported
@@ -550,7 +551,6 @@ build_kernel() {
 	fi
 	cmd+=("-j$JOBS")
 	cmd+=("KCFLAGS=-ffunction-sections -fdata-sections")
-	cmd+=("OBJTOOL_ARGS=${objtool_args[*]}")
 	cmd+=("vmlinux")
 	cmd+=("modules")
 
@@ -582,7 +582,7 @@ copy_orig_objects() {
 
 	find_objects | mapfile -t files
 
-	xtrace_save "copying orig objects"
+	xtrace_save "copying original objects"
 	for _file in "${files[@]}"; do
 		local rel_file="${_file/.ko/.o}"
 		local file="$OBJ/$rel_file"
@@ -638,6 +638,35 @@ copy_patched_objects() {
 	mv -f "$TMP_DIR/build.log" "$PATCHED_DIR"
 }
 
+# Copy .o files to a separate directory and run "objtool klp checksum" on each
+# copy.  The checksums are written to a .discard.sym_checksum section.
+#
+# If match_dir is given, only process files which also exist there.
+generate_checksums() {
+	local src_dir="$1"
+	local dest_dir="$2"
+	local match_dir="${3:-}"
+	local files=()
+	local file
+
+	rm -rf "$dest_dir"
+	mkdir -p "$dest_dir"
+
+	find "$src_dir" -type f -name "*.o" | mapfile -t files
+	for file in "${files[@]}"; do
+		local rel="${file#"$src_dir"/}"
+		local dest="$dest_dir/$rel"
+
+		[[ -n "$match_dir" && ! -f "$match_dir/$rel" ]] && continue
+
+		mkdir -p "$(dirname "$dest")"
+		cp -f "$file" "$dest"
+		"$SRC/tools/objtool/objtool" klp checksum "$dest"
+	done
+
+	touch "$dest_dir/.complete"
+}
+
 # Diff changed objects, writing output object to $DIFF_DIR
 diff_objects() {
 	local log="$KLP_DIFF_LOG"
@@ -647,16 +676,16 @@ diff_objects() {
 	rm -rf "$DIFF_DIR"
 	mkdir -p "$DIFF_DIR"
 
-	find "$PATCHED_DIR" -type f -name "*.o" | mapfile -t files
+	find "$PATCHED_CSUM_DIR" -type f -name "*.o" | mapfile -t files
 	[[ ${#files[@]} -eq 0 ]] && die "no changes detected"
 
 	[[ -v DEBUG_CLONE ]] && opts=("--debug")
 
 	# Diff all changed objects
 	for file in "${files[@]}"; do
-		local rel_file="${file#"$PATCHED_DIR"/}"
+		local rel_file="${file#"$PATCHED_CSUM_DIR"/}"
 		local orig_file="$rel_file"
-		local patched_file="$PATCHED_DIR/$rel_file"
+		local patched_file="$PATCHED_CSUM_DIR/$rel_file"
 		local out_file="$DIFF_DIR/$rel_file"
 		local filter=()
 		local cmd=()
@@ -680,7 +709,7 @@ diff_objects() {
 		fi
 
 		(
-			cd "$ORIG_DIR"
+			cd "$ORIG_CSUM_DIR"
 			[[ -v VERBOSE ]] && echo "${cmd[@]}"
 			"${cmd[@]}"							\
 				1> >(tee -a "$log")					\
@@ -690,9 +719,9 @@ diff_objects() {
 	done
 }
 
-# For each changed object, run objtool with --debug-checksum to get the
-# per-instruction checksums, and then diff those to find the first changed
-# instruction for each function.
+# For each changed object, run "objtool klp checksum" with --debug-checksum to
+# get the per-instruction checksums, and then diff those to find the first
+# changed instruction for each function.
 diff_checksums() {
 	local orig_log="$ORIG_DIR/checksum.log"
 	local patched_log="$PATCHED_DIR/checksum.log"
@@ -717,8 +746,7 @@ diff_checksums() {
 	done
 
 	cmd=("$SRC/tools/objtool/objtool")
-	cmd+=("--checksum")
-	cmd+=("--link")
+	cmd+=("klp" "checksum")
 	cmd+=("--dry-run")
 
 	for file in "${!funcs[@]}"; do
@@ -727,11 +755,11 @@ diff_checksums() {
 		(
 			cd "$ORIG_DIR"
 			"${cmd[@]}" "$opt" "$file" &> "$orig_log" || \
-				( cat "$orig_log" >&2; die "objtool --debug-checksum failed" )
+				( cat "$orig_log" >&2; die "objtool klp checksum failed" )
 
 			cd "$PATCHED_DIR"
 			"${cmd[@]}" "$opt" "$file" &> "$patched_log" ||	\
-				( cat "$patched_log" >&2; die "objtool --debug-checksum failed" )
+				( cat "$patched_log" >&2; die "objtool klp checksum failed" )
 		)
 
 		for func in ${funcs[$file]}; do
@@ -872,6 +900,13 @@ if (( SHORT_CIRCUIT <= 2 )); then
 fi
 
 if (( SHORT_CIRCUIT <= 3 )); then
+	status "Generating original checksums"
+	generate_checksums "$ORIG_DIR" "$ORIG_CSUM_DIR" "$PATCHED_DIR"
+	status "Generating patched checksums"
+	generate_checksums "$PATCHED_DIR" "$PATCHED_CSUM_DIR"
+fi
+
+if (( SHORT_CIRCUIT <= 4 )); then
 	status "Diffing objects"
 	diff_objects
 	if [[ -v DIFF_CHECKSUM ]]; then
@@ -880,7 +915,7 @@ if (( SHORT_CIRCUIT <= 3 )); then
 	fi
 fi
 
-if (( SHORT_CIRCUIT <= 4 )); then
+if (( SHORT_CIRCUIT <= 5 )); then
 	status "Building patch module: $OUTFILE"
 	build_patch_module
 fi
-- 
2.53.0

[PATCH 37/48] objtool/klp: Remove "objtool --checksum"

[Josh Poimboeuf]

The checksum functionality has been moved to "objtool klp checksum"
which is now used by klp-build.  Remove the now-dead --checksum and
--debug-checksum options from the default objtool command.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 scripts/livepatch/klp-build   |  3 +++
 tools/objtool/builtin-check.c | 17 +----------------
 tools/objtool/check.c         | 10 ----------
 3 files changed, 4 insertions(+), 26 deletions(-)

diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index d29ef3022556..eda690b297cc 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -277,6 +277,9 @@ validate_config() {
 		[[ "$CONFIG_AS_VERSION" -lt 200000 ]] &&	\
 		die "Clang assembler version < 20 not supported"
 
+	"$SRC/tools/objtool/objtool" klp 2>&1 | command grep -q "not implemented" && \
+		die "objtool not built with KLP support; install xxhash-devel/libxxhash-dev (version >= 0.8) and recompile"
+
 	return 0
 }
 
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index b780df513715..ec7f10a5ef19 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -73,7 +73,6 @@ static int parse_hacks(const struct option *opt, const char *str, int unset)
 
 static const struct option check_options[] = {
 	OPT_GROUP("Actions:"),
-	OPT_BOOLEAN(0,		 "checksum", &opts.checksum, "generate per-function checksums"),
 	OPT_BOOLEAN(0,		 "cfi", &opts.cfi, "annotate kernel control flow integrity (kCFI) function preambles"),
 	OPT_STRING_OPTARG('d',	 "disas", &opts.disas, "function-pattern", "disassemble functions", "*"),
 	OPT_CALLBACK_OPTARG('h', "hacks", NULL, NULL, "jump_label,noinstr,skylake", "patch toolchain bugs/limitations", parse_hacks),
@@ -95,7 +94,6 @@ static const struct option check_options[] = {
 	OPT_GROUP("Options:"),
 	OPT_BOOLEAN(0,		 "backtrace", &opts.backtrace, "unwind on error"),
 	OPT_BOOLEAN(0,		 "backup", &opts.backup, "create backup (.orig) file on warning/error"),
-	OPT_STRING(0,		 "debug-checksum", &opts.debug_checksum,  "funcs", "enable checksum debug output"),
 	OPT_BOOLEAN(0,		 "dry-run", &opts.dryrun, "don't write modifications"),
 	OPT_BOOLEAN(0,		 "link", &opts.link, "object is a linked object"),
 	OPT_BOOLEAN(0,		 "module", &opts.module, "object is part of a kernel module"),
@@ -165,20 +163,7 @@ static bool opts_valid(void)
 		return false;
 	}
 
-#ifndef BUILD_KLP
-	if (opts.checksum) {
-		ERROR("--checksum not supported; install xxhash-devel/libxxhash-dev (version >= 0.8) and recompile");
-		return false;
-	}
-#endif
-
-	if (opts.debug_checksum && !opts.checksum) {
-		ERROR("--debug-checksum requires --checksum");
-		return false;
-	}
-
-	if (opts.checksum		||
-	    opts.disas			||
+	if (opts.disas			||
 	    opts.hack_jump_label	||
 	    opts.hack_noinstr		||
 	    opts.ibt			||
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 3e5d335d0e29..ae047be919c5 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -18,7 +18,6 @@
 #include <objtool/special.h>
 #include <objtool/trace.h>
 #include <objtool/warn.h>
-#include <objtool/checksum.h>
 #include <objtool/util.h>
 
 #include <linux/objtool_types.h>
@@ -4946,15 +4945,6 @@ int check(struct objtool_file *file)
 	if (opts.noabs)
 		warnings += check_abs_references(file);
 
-	if (opts.checksum) {
-		ret = calculate_checksums(file);
-		if (ret)
-			goto out;
-		ret = create_sym_checksum_section(file);
-		if (ret)
-			goto out;
-	}
-
 	if (opts.orc && nr_insns) {
 		ret = orc_create(file);
 		if (ret)
-- 
2.53.0

[PATCH 38/48] klp-build: Validate short-circuit prerequisites

[Josh Poimboeuf]

The --short-circuit option implicitly requires that certain directories
are already in klp-tmp.  Enforce that to prevent confusing errors.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 scripts/livepatch/klp-build | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index eda690b297cc..b44924d097a5 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -440,6 +440,20 @@ do_init() {
 	[[ ! "$SRC" -ef "$SCRIPT_DIR/../.." ]] && die "please run from the kernel root directory"
 	[[ ! "$OBJ" -ef "$SCRIPT_DIR/../.." ]] && die "please run from the kernel root directory"
 
+	if (( SHORT_CIRCUIT >= 2 )); then
+		[[ -f "$ORIG_DIR/.complete" ]] || die "-S $SHORT_CIRCUIT requires completed $ORIG_DIR"
+		if (( SHORT_CIRCUIT >= 3 )); then
+			[[ -f "$PATCHED_DIR/.complete" ]] || die "-S $SHORT_CIRCUIT requires completed $PATCHED_DIR"
+			if (( SHORT_CIRCUIT >= 4 )); then
+				[[ -f "$ORIG_CSUM_DIR/.complete" ]] || die "-S $SHORT_CIRCUIT requires completed $ORIG_CSUM_DIR"
+				[[ -f "$PATCHED_CSUM_DIR/.complete" ]] || die "-S $SHORT_CIRCUIT requires completed $PATCHED_CSUM_DIR"
+				if (( SHORT_CIRCUIT >= 5 )); then
+					[[ -f "$DIFF_DIR/.complete" ]] || die "-S $SHORT_CIRCUIT requires completed $DIFF_DIR"
+				fi
+			fi
+		fi
+	fi
+
 	(( SHORT_CIRCUIT <= 1 )) && rm -rf "$TMP_DIR"
 	mkdir -p "$TMP_DIR"
 
@@ -601,6 +615,7 @@ copy_orig_objects() {
 
 	mv -f "$TMP_DIR/build.log" "$ORIG_DIR"
 	touch "$TIMESTAMP"
+	touch "$ORIG_DIR/.complete"
 }
 
 # Copy all changed objects to $PATCHED_DIR
@@ -639,6 +654,7 @@ copy_patched_objects() {
 	(( found == 0 )) && die "no changes detected"
 
 	mv -f "$TMP_DIR/build.log" "$PATCHED_DIR"
+	touch "$PATCHED_DIR/.complete"
 }
 
 # Copy .o files to a separate directory and run "objtool klp checksum" on each
@@ -720,6 +736,8 @@ diff_objects() {
 				die "objtool klp diff failed"
 		)
 	done
+
+	touch "$DIFF_DIR/.complete"
 }
 
 # For each changed object, run "objtool klp checksum" with --debug-checksum to
-- 
2.53.0

[PATCH 39/48] objtool: Replace iterator callbacks with for_each_sym_by_*()

[Josh Poimboeuf]

Convert the callback-based iterate_sym_by_name() and
iterate_sym_by_demangled_name() callers to use new
for_each_sym_by[_demangled]_name() macros.  This eliminates the callback
structs and functions and makes the code more compact and readable.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/elf.c                 | 80 ++++++-----------------------
 tools/objtool/include/objtool/elf.h | 40 ++++++++++++---
 tools/objtool/klp-checksum.c        | 20 +++-----
 tools/objtool/klp-diff.c            | 42 +++++----------
 4 files changed, 73 insertions(+), 109 deletions(-)

diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 8a6e1338af97..8d823e96b7c9 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -27,27 +27,16 @@
 
 static ssize_t demangled_name_len(const char *name);
 
-static inline u32 str_hash(const char *str)
-{
-	return jhash(str, strlen(str), 0);
-}
-
-static inline u32 str_hash_demangled(const char *str)
+u32 str_hash_demangled(const char *str)
 {
 	return jhash(str, demangled_name_len(str), 0);
 }
 
-#define __elf_table(name)	(elf->name##_hash)
-#define __elf_bits(name)	(elf->name##_bits)
-
-#define __elf_table_entry(name, key) \
-	__elf_table(name)[hash_min(key, __elf_bits(name))]
-
 #define elf_hash_add(name, node, key)					\
 ({									\
 	struct elf_hash_node *__node = node;				\
-	__node->next = __elf_table_entry(name, key);			\
-	__elf_table_entry(name, key) = __node;				\
+	__node->next = __elf_table_entry(elf, name, key);		\
+	__elf_table_entry(elf, name, key) = __node;			\
 })
 
 static inline void __elf_hash_del(struct elf_hash_node *node,
@@ -69,30 +58,20 @@ static inline void __elf_hash_del(struct elf_hash_node *node,
 }
 
 #define elf_hash_del(name, node, key) \
-	__elf_hash_del(node, &__elf_table_entry(name, key))
-
-#define elf_list_entry(ptr, type, member)				\
-({									\
-	typeof(ptr) __ptr = (ptr);					\
-	__ptr ? container_of(__ptr, type, member) : NULL;		\
-})
-
-#define elf_hash_for_each_possible(name, obj, member, key)		\
-	for (obj = elf_list_entry(__elf_table_entry(name, key), typeof(*obj), member); \
-	     obj;							\
-	     obj = elf_list_entry(obj->member.next, typeof(*(obj)), member))
+	__elf_hash_del(node, &__elf_table_entry(elf, name, key))
 
 #define elf_alloc_hash(name, size)					\
 ({									\
-	__elf_bits(name) = max(10, ilog2(size));			\
-	__elf_table(name) = mmap(NULL, sizeof(struct elf_hash_node *) << __elf_bits(name), \
+	__elf_bits(elf, name) = max(10, ilog2(size));			\
+	__elf_table(elf, name) = mmap(NULL,				\
+				 sizeof(struct elf_hash_node *) << __elf_bits(elf, name), \
 				 PROT_READ|PROT_WRITE,			\
 				 MAP_PRIVATE|MAP_ANON, -1, 0);		\
-	if (__elf_table(name) == (void *)-1L) {				\
+	if (__elf_table(elf, name) == (void *)-1L) {			\
 		ERROR_GLIBC("mmap fail " #name);			\
-		__elf_table(name) = NULL;				\
+		__elf_table(elf, name) = NULL;				\
 	}								\
-	__elf_table(name);						\
+	__elf_table(elf, name);						\
 })
 
 static inline unsigned long __sym_start(struct symbol *s)
@@ -141,7 +120,7 @@ struct section *find_section_by_name(const struct elf *elf, const char *name)
 {
 	struct section *sec;
 
-	elf_hash_for_each_possible(section_name, sec, name_hash, str_hash(name)) {
+	elf_hash_for_each_possible(elf, section_name, sec, name_hash, str_hash(name)) {
 		if (!strcmp(sec->name, name))
 			return sec;
 	}
@@ -154,7 +133,7 @@ static struct section *find_section_by_index(struct elf *elf,
 {
 	struct section *sec;
 
-	elf_hash_for_each_possible(section, sec, hash, idx) {
+	elf_hash_for_each_possible(elf, section, sec, hash, idx) {
 		if (sec->idx == idx)
 			return sec;
 	}
@@ -166,7 +145,7 @@ static struct symbol *find_symbol_by_index(struct elf *elf, unsigned int idx)
 {
 	struct symbol *sym;
 
-	elf_hash_for_each_possible(symbol, sym, hash, idx) {
+	elf_hash_for_each_possible(elf, symbol, sym, hash, idx) {
 		if (sym->idx == idx)
 			return sym;
 	}
@@ -285,7 +264,7 @@ struct symbol *find_symbol_by_name(const struct elf *elf, const char *name)
 {
 	struct symbol *sym;
 
-	elf_hash_for_each_possible(symbol_name, sym, name_hash, str_hash(name)) {
+	elf_hash_for_each_possible(elf, symbol_name, sym, name_hash, str_hash(name)) {
 		if (!strcmp(sym->name, name))
 			return sym;
 	}
@@ -300,7 +279,7 @@ static struct symbol *find_local_symbol_by_file_and_name(const struct elf *elf,
 {
 	struct symbol *sym;
 
-	elf_hash_for_each_possible(symbol_name, sym, name_hash, str_hash_demangled(name)) {
+	elf_hash_for_each_possible(elf, symbol_name, sym, name_hash, str_hash_demangled(name)) {
 		if (sym->bind == STB_LOCAL && sym->file == file &&
 		    !strcmp(sym->name, name)) {
 			return sym;
@@ -314,7 +293,7 @@ struct symbol *find_global_symbol_by_name(const struct elf *elf, const char *nam
 {
 	struct symbol *sym;
 
-	elf_hash_for_each_possible(symbol_name, sym, name_hash, str_hash_demangled(name)) {
+	elf_hash_for_each_possible(elf, symbol_name, sym, name_hash, str_hash_demangled(name)) {
 		if (!strcmp(sym->name, name) && !is_local_sym(sym))
 			return sym;
 	}
@@ -322,31 +301,6 @@ struct symbol *find_global_symbol_by_name(const struct elf *elf, const char *nam
 	return NULL;
 }
 
-void iterate_global_symbol_by_demangled_name(const struct elf *elf,
-					     const char *demangled_name,
-					     void (*process)(struct symbol *sym, void *data),
-					     void *data)
-{
-	struct symbol *sym;
-
-	elf_hash_for_each_possible(symbol_name, sym, name_hash, str_hash(demangled_name)) {
-		if (!strcmp(sym->demangled_name, demangled_name) && !is_local_sym(sym))
-			process(sym, data);
-	}
-}
-
-void iterate_sym_by_name(const struct elf *elf, const char *name,
-			 void (*process)(struct symbol *sym, void *data),
-			 void *data)
-{
-	struct symbol *sym;
-
-	elf_hash_for_each_possible(symbol_name, sym, name_hash, str_hash_demangled(name)) {
-		if (!strcmp(sym->name, name))
-			process(sym, data);
-	}
-}
-
 struct reloc *find_reloc_by_dest_range(const struct elf *elf, struct section *sec,
 				     unsigned long offset, unsigned int len)
 {
@@ -359,7 +313,7 @@ struct reloc *find_reloc_by_dest_range(const struct elf *elf, struct section *se
 		return NULL;
 
 	for_offset_range(o, offset, offset + len) {
-		elf_hash_for_each_possible(reloc, reloc, hash,
+		elf_hash_for_each_possible(elf, reloc, reloc, hash,
 					   sec_offset_hash(rsec, o)) {
 			if (reloc->sec != rsec)
 				continue;
diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
index 82b9fb05af26..ba13dd67cf26 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -21,6 +21,13 @@
 #define SEC_NAME_LEN		1024
 #define SYM_NAME_LEN		512
 
+static inline u32 str_hash(const char *str)
+{
+	return jhash(str, strlen(str), 0);
+}
+
+u32 str_hash_demangled(const char *str);
+
 #define bswap_if_needed(elf, val) __bswap_if_needed(&elf->ehdr, val)
 
 #ifdef LIBELF_USE_DEPRECATED
@@ -129,6 +136,23 @@ struct elf {
 	struct symbol *symbol_data;
 };
 
+#define __elf_table(elf, name)	((elf)->name##_hash)
+#define __elf_bits(elf, name)	((elf)->name##_bits)
+
+#define __elf_table_entry(elf, name, key) \
+	__elf_table(elf, name)[hash_min(key, __elf_bits(elf, name))]
+
+#define elf_list_entry(ptr, type, member)				\
+({									\
+	typeof(ptr) __ptr = (ptr);					\
+	__ptr ? container_of(__ptr, type, member) : NULL;		\
+})
+
+#define elf_hash_for_each_possible(elf, name, obj, member, key)		\
+	for (obj = elf_list_entry(__elf_table_entry(elf, name, key), typeof(*obj), member); \
+	     obj;							\
+	     obj = elf_list_entry(obj->member.next, typeof(*(obj)), member))
+
 struct elf *elf_open_read(const char *name, int flags);
 struct elf *elf_create_file(GElf_Ehdr *ehdr, const char *name);
 
@@ -185,12 +209,6 @@ struct symbol *find_func_by_offset(struct section *sec, unsigned long offset);
 struct symbol *find_symbol_by_offset(struct section *sec, unsigned long offset);
 struct symbol *find_symbol_by_name(const struct elf *elf, const char *name);
 struct symbol *find_global_symbol_by_name(const struct elf *elf, const char *name);
-void iterate_global_symbol_by_demangled_name(const struct elf *elf, const char *demangled_name,
-					     void (*process)(struct symbol *sym, void *data),
-					     void *data);
-void iterate_sym_by_name(const struct elf *elf, const char *name,
-			 void (*process)(struct symbol *sym, void *data),
-			 void *data);
 struct symbol *find_symbol_containing(const struct section *sec, unsigned long offset);
 int find_symbol_hole_containing(const struct section *sec, unsigned long offset);
 struct reloc *find_reloc_by_dest(const struct elf *elf, struct section *sec, unsigned long offset);
@@ -485,6 +503,16 @@ static inline void set_sym_next_reloc(struct reloc *reloc, struct reloc *next)
 #define for_each_sym_continue(elf, sym)					\
 	list_for_each_entry_continue(sym, &elf->symbols, global_list)
 
+#define for_each_sym_by_name(elf, _name, sym)				\
+	elf_hash_for_each_possible(elf, symbol_name, sym, name_hash,	\
+				   str_hash_demangled(_name))		\
+		if (!strcmp(sym->name, _name))
+
+#define for_each_sym_by_demangled_name(elf, name, sym)			\
+	elf_hash_for_each_possible(elf, symbol_name, sym, name_hash,	\
+				   str_hash(name))			\
+		if (!strcmp(sym->demangled_name, name))
+
 #define rsec_next_reloc(rsec, reloc)					\
 	reloc_idx(reloc) < sec_num_entries(rsec) - 1 ? reloc + 1 : NULL
 
diff --git a/tools/objtool/klp-checksum.c b/tools/objtool/klp-checksum.c
index 4edd29028bff..e4a910f3211c 100644
--- a/tools/objtool/klp-checksum.c
+++ b/tools/objtool/klp-checksum.c
@@ -11,17 +11,6 @@
 #include <objtool/warn.h>
 #include <objtool/checksum.h>
 
-static void enable_debug_checksum_cb(struct symbol *sym, void *d)
-{
-	bool *found = d;
-
-	if (!is_func_sym(sym))
-		return;
-
-	sym->debug_checksum = 1;
-	*found = true;
-}
-
 static int checksum_debug_init(struct objtool_file *file)
 {
 	char *dup, *s;
@@ -38,13 +27,20 @@ static int checksum_debug_init(struct objtool_file *file)
 	s = dup;
 	while (*s) {
 		bool found = false;
+		struct symbol *sym;
 		char *comma;
 
 		comma = strchr(s, ',');
 		if (comma)
 			*comma = '\0';
 
-		iterate_sym_by_name(file->elf, s, enable_debug_checksum_cb, &found);
+		for_each_sym_by_name(file->elf, s, sym) {
+			if (!is_func_sym(sym))
+				continue;
+			sym->debug_checksum = 1;
+			found = true;
+		}
+
 		if (!found)
 			WARN("--debug-checksum: can't find '%s'", s);
 
diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index c903aa65d4b6..33e401b85001 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -46,11 +46,6 @@ static const struct option klp_diff_options[] = {
 
 static DEFINE_HASHTABLE(exports, 15);
 
-static inline u32 str_hash(const char *str)
-{
-	return jhash(str, strlen(str), 0);
-}
-
 static char *escape_str(const char *orig)
 {
 	size_t len = 0;
@@ -398,22 +393,6 @@ static bool dont_correlate(struct symbol *sym)
 	       is_special_section_aux(sym->sec);
 }
 
-struct process_demangled_name_data {
-	struct symbol *ret;
-	int count;
-};
-
-static void process_demangled_name(struct symbol *sym, void *d)
-{
-	struct process_demangled_name_data *data = d;
-
-	if (sym->twin)
-		return;
-
-	data->count++;
-	data->ret = sym;
-}
-
 /*
  * When there is no full name match, try match demangled_name. This would
  * match original foo.llvm.123 to patched foo.llvm.456.
@@ -425,16 +404,23 @@ static void process_demangled_name(struct symbol *sym, void *d)
 static int find_global_symbol_by_demangled_name(struct elf *elf, struct symbol *sym,
 						struct symbol **out_sym)
 {
-	struct process_demangled_name_data data = {};
+	struct symbol *sym2, *result = NULL;
+	int count = 0;
 
-	iterate_global_symbol_by_demangled_name(elf, sym->demangled_name,
-						process_demangled_name,
-						&data);
-	if (data.count > 1) {
-		ERROR("Multiple (%d) correlation candidates for %s", data.count, sym->name);
+	for_each_sym_by_demangled_name(elf, sym->demangled_name, sym2) {
+		if (is_local_sym(sym2) || sym2->twin)
+			continue;
+
+		count++;
+		result = sym2;
+	}
+
+	if (count > 1) {
+		ERROR("Multiple (%d) correlation candidates for %s", count, sym->name);
 		return -1;
 	}
-	*out_sym = data.ret;
+
+	*out_sym = result;
 	return 0;
 }
 
-- 
2.53.0

[PATCH 40/48] objtool/klp: Calculate object checksums

[Josh Poimboeuf]

Start checksumming data objects in preparation for revamping the
correlation algorithm.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/include/objtool/checksum.h | 43 ++++++++----
 tools/objtool/include/objtool/warn.h     | 29 ++++----
 tools/objtool/klp-checksum.c             | 88 +++++++++++++++++++-----
 tools/objtool/klp-diff.c                 |  2 +-
 4 files changed, 115 insertions(+), 47 deletions(-)

diff --git a/tools/objtool/include/objtool/checksum.h b/tools/objtool/include/objtool/checksum.h
index be4eb7dfe6f2..ccaf57c7df38 100644
--- a/tools/objtool/include/objtool/checksum.h
+++ b/tools/objtool/include/objtool/checksum.h
@@ -6,28 +6,43 @@
 
 #ifdef BUILD_KLP
 
-static inline void checksum_init(struct symbol *func)
+static inline void checksum_init(struct symbol *sym)
 {
-	if (func && !func->csum.state) {
-		func->csum.state = XXH3_createState();
-		XXH3_64bits_reset(func->csum.state);
+	if (sym && !sym->csum.state) {
+		sym->csum.state = XXH3_createState();
+		XXH3_64bits_reset(sym->csum.state);
 	}
 }
 
-static inline void checksum_update(struct symbol *func,
-				   struct instruction *insn,
-				   const void *data, size_t size)
+static inline void __checksum_update(struct symbol *sym, const void *data,
+				     size_t size)
 {
-	XXH3_64bits_update(func->csum.state, data, size);
-	dbg_checksum(func, insn, XXH3_64bits_digest(func->csum.state));
+	XXH3_64bits_update(sym->csum.state, data, size);
 }
 
-static inline void checksum_finish(struct symbol *func)
+static inline void __checksum_update_insn(struct symbol *sym,
+					  struct instruction *insn,
+					  const void *data, size_t size)
 {
-	if (func && func->csum.state) {
-		func->csum.checksum = XXH3_64bits_digest(func->csum.state);
-		XXH3_freeState(func->csum.state);
-		func->csum.state = NULL;
+	__checksum_update(sym, data, size);
+	dbg_checksum_insn(sym, insn, XXH3_64bits_digest(sym->csum.state));
+}
+
+static inline void __checksum_update_object(struct symbol *sym,
+					    unsigned long offset,
+					    const char *what, const void *data,
+					    size_t size)
+{
+	__checksum_update(sym, data, size);
+	dbg_checksum_object(sym, offset, what, XXH3_64bits_digest(sym->csum.state));
+}
+
+static inline void checksum_finish(struct symbol *sym)
+{
+	if (sym && sym->csum.state) {
+		sym->csum.checksum = XXH3_64bits_digest(sym->csum.state);
+		XXH3_freeState(sym->csum.state);
+		sym->csum.state = NULL;
 	}
 }
 
diff --git a/tools/objtool/include/objtool/warn.h b/tools/objtool/include/objtool/warn.h
index fa8b7d292e83..595ee8009667 100644
--- a/tools/objtool/include/objtool/warn.h
+++ b/tools/objtool/include/objtool/warn.h
@@ -130,10 +130,22 @@ static inline void unindent(int *unused) { indent--; }
 		objname ? ": " : "",					\
 		##__VA_ARGS__)
 
-#define dbg(args...)							\
+#define dbg_checksum_insn(func, insn, checksum)				\
 ({									\
-	if (unlikely(debug))						\
-		__dbg(args);						\
+	if (unlikely(func->debug_checksum)) {				\
+		char *insn_off = offstr(insn->sec, insn->offset);	\
+		__dbg("checksum: %s(): %s %016llx",			\
+		      func->name, insn_off, (unsigned long long)checksum);\
+		free(insn_off);						\
+	}								\
+})
+
+#define dbg_checksum_object(sym, offset, what, checksum)		\
+({									\
+	if (unlikely(sym->debug_checksum))				\
+		__dbg("checksum: %s+0x%lx: %s %016llx",			\
+		      sym->name, offset, what,				\
+		      (unsigned long long)checksum);			\
 })
 
 #define __dbg_indent(format, ...)					\
@@ -147,15 +159,4 @@ static inline void unindent(int *unused) { indent--; }
 	__dbg_indent(args);						\
 	indent++
 
-#define dbg_checksum(func, insn, checksum)				\
-({									\
-	if (unlikely(insn->sym && insn->sym->pfunc &&			\
-		     insn->sym->pfunc->debug_checksum)) {		\
-		char *insn_off = offstr(insn->sec, insn->offset);	\
-		__dbg("checksum: %s %s %016llx",			\
-		      func->name, insn_off, (unsigned long long)checksum);\
-		free(insn_off);						\
-	}								\
-})
-
 #endif /* _WARN_H */
diff --git a/tools/objtool/klp-checksum.c b/tools/objtool/klp-checksum.c
index e4a910f3211c..adfd02447a45 100644
--- a/tools/objtool/klp-checksum.c
+++ b/tools/objtool/klp-checksum.c
@@ -35,7 +35,7 @@ static int checksum_debug_init(struct objtool_file *file)
 			*comma = '\0';
 
 		for_each_sym_by_name(file->elf, s, sym) {
-			if (!is_func_sym(sym))
+			if (!is_func_sym(sym) && !is_object_sym(sym))
 				continue;
 			sym->debug_checksum = 1;
 			found = true;
@@ -66,14 +66,14 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func,
 	if (insn->fake)
 		return;
 
-	checksum_update(func, insn, insn->sec->data->d_buf + insn->offset, insn->len);
+	__checksum_update_insn(func, insn, insn->sec->data->d_buf + insn->offset, insn->len);
 
 	if (!reloc) {
 		struct symbol *call_dest = insn_call_dest(insn);
 
 		if (call_dest)
-			checksum_update(func, insn, call_dest->demangled_name,
-					strlen(call_dest->demangled_name));
+			__checksum_update_insn(func, insn, call_dest->demangled_name,
+					       strlen(call_dest->demangled_name));
 		goto alts;
 	}
 
@@ -84,7 +84,7 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func,
 		char *str;
 
 		str = sym->sec->data->d_buf + sym->offset + offset;
-		checksum_update(func, insn, str, strlen(str));
+		__checksum_update_insn(func, insn, str, strlen(str));
 		goto alts;
 	}
 
@@ -96,8 +96,9 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func,
 		offset -= sym->offset;
 	}
 
-	checksum_update(func, insn, sym->demangled_name, strlen(sym->demangled_name));
-	checksum_update(func, insn, &offset, sizeof(offset));
+	__checksum_update_insn(func, insn, sym->demangled_name,
+			       strlen(sym->demangled_name));
+	__checksum_update_insn(func, insn, &offset, sizeof(offset));
 
 alts:
 	for (alt = insn->alts; alt; alt = alt->next) {
@@ -108,12 +109,13 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func,
 			break;
 		in_alt = true;
 
-		checksum_update(func, insn, &alt->type, sizeof(alt->type));
+		__checksum_update_insn(func, insn, &alt->type,
+				       sizeof(alt->type));
 
 		if (alt_group && alt_group->orig_group) {
 			struct instruction *alt_insn;
 
-			checksum_update(func, insn, &alt_group->feature, sizeof(alt_group->feature));
+			__checksum_update_insn(func, insn, &alt_group->feature,sizeof(alt_group->feature));
 
 			for (alt_insn = alt->insn; alt_insn; alt_insn = next_insn_same_sec(file, alt_insn)) {
 				checksum_update_insn(file, func, alt_insn);
@@ -128,31 +130,81 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func,
 	}
 }
 
+static void checksum_update_object(struct objtool_file *file, struct symbol *sym)
+{
+	struct reloc *reloc;
+
+	__checksum_update_object(sym, 0, "len", &sym->len, sizeof(sym->len));
+
+	if (sym->sec->data->d_buf)
+		__checksum_update_object(sym, 0, "data",
+					 sym->sec->data->d_buf + sym->offset,
+					 sym->len);
+
+	sym_for_each_reloc(file->elf, sym, reloc) {
+		struct symbol *target = reloc->sym;
+		s64 offset;
+
+		offset = reloc_addend(reloc);
+
+		if (is_string_sec(target->sec)) {
+			char *str;
+
+			str = target->sec->data->d_buf + target->offset + offset;
+			__checksum_update_object(sym, reloc_offset(reloc),
+						 "reloc string", str, strlen(str));
+			continue;
+		}
+
+		if (is_sec_sym(target)) {
+			target = find_symbol_containing(reloc->sym->sec, offset);
+			if (!target)
+				continue;
+
+			offset -= target->offset;
+		}
+
+		__checksum_update_object(sym, reloc_offset(reloc), "reloc name",
+					 target->demangled_name,
+					 strlen(target->demangled_name));
+		__checksum_update_object(sym, reloc_offset(reloc), "reloc addend",
+					 &offset, sizeof(offset));
+	}
+}
+
 int calculate_checksums(struct objtool_file *file)
 {
 	struct instruction *insn;
-	struct symbol *func;
+	struct symbol *sym;
 
 	if (checksum_debug_init(file))
 		return -1;
 
-	for_each_sym(file->elf, func) {
+	for_each_sym(file->elf, sym) {
+
 		/*
 		 * Skip cold subfunctions and aliases: they share the
 		 * parent's checksum via func_for_each_insn() which
 		 * follows func->cfunc into the cold subfunction.
 		 */
-		if (!is_func_sym(func) || is_cold_func(func) ||
-		    is_alias_sym(func) || !func->len)
+		if (is_cold_func(sym) || is_alias_sym(sym) || !sym->len ||
+		    !sym->sec || !sym->sec->data)
 			continue;
 
-		checksum_init(func);
+		if (is_func_sym(sym)) {
+			checksum_init(sym);
+			func_for_each_insn(file, sym, insn)
+				checksum_update_insn(file, sym, insn);
+			checksum_finish(sym);
 
-		func_for_each_insn(file, func, insn)
-			checksum_update_insn(file, func, insn);
+		} else if (is_object_sym(sym)) {
+			checksum_init(sym);
+			checksum_update_object(file, sym);
+			checksum_finish(sym);
+		}
 
-		checksum_finish(func);
 	}
+
 	return 0;
 }
 
@@ -213,7 +265,7 @@ int cmd_klp_checksum(int argc, const char **argv)
 	int ret;
 
 	const struct option options[] = {
-		OPT_STRING(0,	"debug-checksum", &opts.debug_checksum,	"funcs", "enable checksum debug output"),
+		OPT_STRING(0,	"debug-checksum", &opts.debug_checksum,	"syms", "enable checksum debug output"),
 		OPT_BOOLEAN(0,	"dry-run", &opts.dryrun, "don't write modifications"),
 		OPT_END(),
 	};
diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 33e401b85001..8d64d4c691cb 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -201,7 +201,7 @@ static int read_sym_checksums(struct elf *elf)
 			return -1;
 		}
 
-		if (is_func_sym(sym))
+		if (is_func_sym(sym) || is_object_sym(sym))
 			sym->csum.checksum = sym_checksum->checksum;
 	}
 
-- 
2.53.0

[PATCH 41/48] objtool/klp: Rewrite symbol correlation algorithm

[Josh Poimboeuf]

Rewrite the symbol correlation code, using a tiered list of
deterministic strategies in a loop.  For duplicately named symbols, each
tier applies a filter with the goal of finding a 1:1 deterministic
correlation between the original and patched version of the symbol.

Overall this works much better than the existing algorithm, particularly
with LTO kernels.

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

diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 8d64d4c691cb..6d7fbb16e59c 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -393,78 +393,319 @@ static bool dont_correlate(struct symbol *sym)
 	       is_special_section_aux(sym->sec);
 }
 
-/*
- * When there is no full name match, try match demangled_name. This would
- * match original foo.llvm.123 to patched foo.llvm.456.
- *
- * Note that, in very rare cases, it is possible to have multiple
- * foo.llvm.<hash> in the same kernel. When this happens, report error and
- * fail the diff.
- */
-static int find_global_symbol_by_demangled_name(struct elf *elf, struct symbol *sym,
-						struct symbol **out_sym)
+static const char *llvm_suffix(const char *name)
 {
-	struct symbol *sym2, *result = NULL;
-	int count = 0;
+	return strstr(name, ".llvm.");
+}
 
-	for_each_sym_by_demangled_name(elf, sym->demangled_name, sym2) {
-		if (is_local_sym(sym2) || sym2->twin)
+static bool is_llvm_sym(struct symbol *sym)
+{
+	return llvm_suffix(sym->name);
+}
+
+/*
+ * Determine if two symbols have compatible source file origins:
+ *
+ *   - If both symbols are local, only return true if they belong to the same
+ *     ELF file symbol.
+ *
+ *   - If both symbols are global, always return true, as globals don't have
+ *     file associations.
+ *
+ *   - If they have different scopes, also return true, as the patch might have
+ *     changed the symbol's scope.
+ *
+ * Works for both same-ELF (direct pointer compare) and cross-ELF
+ * (compare via file->twin) cases.
+ */
+static bool maybe_same_file(struct symbol *sym1, struct symbol *sym2)
+{
+	if (!sym1->file || !sym2->file)
+		return true;
+	if (sym1->file == sym2->file)
+		return true;
+	return sym1->file->twin == sym2->file;
+}
+
+/*
+ * Similar to maybe_same_file(), but strict: no scope changes allowed.
+ *
+ * Works for both same-ELF (direct pointer compare) and cross-ELF
+ * (compare via file->twin) cases.
+ */
+static bool same_file(struct symbol *sym1, struct symbol *sym2)
+{
+	if (llvm_suffix(sym1->name) && llvm_suffix(sym2->name))
+		return true;
+	if (!sym1->file && !sym2->file)
+		return true;
+	if (!sym1->file || !sym2->file)
+		return false;
+	if (sym1->file == sym2->file)
+		return true;
+	return sym1->file->twin == sym2->file;
+}
+
+/*
+ * Is it a local symbol, or at least was it local in the translation unit
+ * before LLVM promoted it?
+ */
+static bool is_tu_local_sym(struct symbol *sym)
+{
+	return is_local_sym(sym) || is_llvm_sym(sym);
+}
+
+/*
+ * Try to find sym1's twin in patched using deterministic matching.
+ *
+ * Multiple symbols can share a demangled name (e.g., static functions in
+ * different TUs).  This function counts same-named candidates through a
+ * funnel of progressively tighter filters.  Each level is a strict subset
+ * of the previous one.
+ *
+ * The widest level that yields a 1:1 match wins.  Narrower levels are only
+ * needed when the wider level is ambiguous (count > 1).
+ */
+static struct symbol *find_twin(struct elfs *e, struct symbol *sym1)
+{
+	struct symbol *name_last = NULL, *scope_last = NULL,
+		      *file_last = NULL, *csum_last = NULL;
+	unsigned int name_orig = 0, name_patched = 0;
+	unsigned int scope_orig = 0, scope_patched = 0;
+	unsigned int file_orig = 0, file_patched = 0;
+	unsigned int csum_orig = 0, csum_patched = 0;
+	struct symbol *sym2, *match = NULL;
+
+	/* Count orig candidates */
+	for_each_sym_by_demangled_name(e->orig, sym1->demangled_name, sym2) {
+		if (sym2->twin || sym1->type != sym2->type || dont_correlate(sym2) ||
+		    (!maybe_same_file(sym1, sym2)))
 			continue;
 
-		count++;
-		result = sym2;
+		/* Level 1: name match (widest filter)  */
+		name_orig++;
+
+		/* Level 2: scope (scope changes allowed) */
+		if (is_tu_local_sym(sym1) != is_tu_local_sym(sym2))
+			continue;
+		scope_orig++;
+
+		/* Level 3: file (scope changes disallowed) */
+		if (!same_file(sym1, sym2))
+			continue;
+		file_orig++;
+
+		/* Level 4: checksum (unchanged symbols) */
+		if (sym1->len != sym2->len || !sym1->csum.checksum ||
+		    sym1->csum.checksum != sym2->csum.checksum)
+			continue;
+		csum_orig++;
 	}
 
-	if (count > 1) {
-		ERROR("Multiple (%d) correlation candidates for %s", count, sym->name);
-		return -1;
+	/* Count patched candidates */
+	for_each_sym_by_demangled_name(e->patched, sym1->demangled_name, sym2) {
+		if (sym2->twin || sym1->type != sym2->type || dont_correlate(sym2))
+			continue;
+
+		/* Level 1 */
+		if (!maybe_same_file(sym1, sym2))
+			continue;
+		name_patched++;
+		name_last = sym2;
+
+		/* Level 2 */
+		if (is_tu_local_sym(sym1) != is_tu_local_sym(sym2))
+			continue;
+		scope_patched++;
+		scope_last = sym2;
+
+		/* Level 3 */
+		if (!same_file(sym1, sym2))
+			continue;
+		file_patched++;
+		file_last = sym2;
+
+		/* Level 4 */
+		if (sym1->len != sym2->len || !sym1->csum.checksum ||
+		    sym1->csum.checksum != sym2->csum.checksum)
+			continue;
+		csum_patched++;
+		csum_last = sym2;
+	}
+
+	/* Return the widest level that yields a unique (1:1) match */
+	if (name_orig == 1 && name_patched == 1)
+		match = name_last;
+	else if (scope_orig == 1 && scope_patched == 1)
+		match = scope_last;
+	else if (file_orig == 1 && file_patched == 1)
+		match = file_last;
+	else if (csum_orig == 1 && csum_patched == 1)
+		match = csum_last;
+
+	return match;
+}
+
+struct llvm_suffix_pair {
+	struct hlist_node hash;
+	const char *orig;
+	const char *patched;
+};
+
+static DECLARE_HASHTABLE(suffix_map, 7);
+
+/*
+ * Build a mapping of known orig-to-patched LLVM suffixes based on
+ * already-correlated symbol pairs.  All promoted symbols from the same TU
+ * share the same .llvm.<hash> suffix, so one correlated pair seeds the map
+ * for the entire TU.
+ */
+static int update_suffix_map(struct elf *elf)
+{
+	struct llvm_suffix_pair *entry;
+	struct symbol *sym;
+
+	for_each_sym(elf, sym) {
+		const char *s1, *s2;
+		bool found;
+
+		if (!sym->twin)
+			continue;
+
+		s1 = llvm_suffix(sym->name);
+		s2 = llvm_suffix(sym->twin->name);
+
+		if (!s1 || !s2)
+			continue;
+
+		found = false;
+		hash_for_each_possible(suffix_map, entry, hash, str_hash(s1)) {
+			if (!strcmp(entry->orig, s1)) {
+				found = true;
+				break;
+			}
+		}
+		if (found)
+			continue;
+
+		entry = calloc(1, sizeof(*entry));
+		if (!entry) {
+			ERROR_GLIBC("calloc");
+			return -1;
+		}
+
+		entry->orig = s1;
+		entry->patched = s2;
+		hash_add(suffix_map, &entry->hash, str_hash(s1));
 	}
 
-	*out_sym = result;
 	return 0;
 }
 
 /*
- * For each symbol in the original kernel, find its corresponding "twin" in the
- * patched kernel.
+ * Match by translating the symbol's .llvm.<hash> suffix through the suffix
+ * map to find the corresponding hash suffix for the patched object.
+ */
+static struct symbol *find_twin_suffixed(struct elf *elf, struct symbol *sym1)
+{
+	const char *suffix, *patched_suffix = NULL;
+	struct symbol *sym2, *match = NULL;
+	char name[SYM_NAME_LEN];
+	struct llvm_suffix_pair *entry;
+	int count = 0;
+
+	suffix = llvm_suffix(sym1->name);
+	if (!suffix)
+		return NULL;
+
+	hash_for_each_possible(suffix_map, entry, hash, str_hash(suffix)) {
+		if (!strcmp(entry->orig, suffix)) {
+			patched_suffix = entry->patched;
+			break;
+		}
+	}
+	if (!patched_suffix)
+		return NULL;
+
+	if (snprintf_check(name, SYM_NAME_LEN, "%s%s",
+			   sym1->demangled_name, patched_suffix))
+		return NULL;
+
+	for_each_sym_by_name(elf, name, sym2) {
+		if (sym2->twin || sym1->type != sym2->type || dont_correlate(sym2))
+			continue;
+		count++;
+		match = sym2;
+	}
+
+	if (count == 1)
+		return match;
+
+	return NULL;
+}
+
+/*
+ * Last-resort positional matching.
+ *
+ * Finds a symbol with the same position in the symbol table among
+ * same-demangled-name candidates, similar to livepatch sympos.  Note that
+ * LLVM-promoted symbols are globals, which come after locals in the symbol
+ * table, so we have to be careful not to compare different scopes.
+ *
+ * This is a bit less deterministic than the other matching strategies, so it
+ * should be done last.
+ */
+static struct symbol *find_twin_positional(struct elfs *e, struct symbol *sym1)
+{
+	unsigned int idx_orig = 0, idx_patched = 0;
+	unsigned int sym1_pos = 0;
+	struct symbol *sym2, *match = NULL;
+
+	for_each_sym_by_demangled_name(e->orig, sym1->demangled_name, sym2) {
+		if (sym2->twin || sym1->type != sym2->type || dont_correlate(sym2) ||
+		    !maybe_same_file(sym1, sym2))
+			continue;
+		if (is_tu_local_sym(sym1) != is_tu_local_sym(sym2) ||
+		    is_llvm_sym(sym1) != is_llvm_sym(sym2))
+			continue;
+		if (sym1 == sym2)
+			sym1_pos = idx_orig;
+		idx_orig++;
+	}
+
+	for_each_sym_by_demangled_name(e->patched, sym1->demangled_name, sym2) {
+		if (sym2->twin || sym1->type != sym2->type || dont_correlate(sym2) ||
+		    !maybe_same_file(sym1, sym2))
+			continue;
+		if (is_tu_local_sym(sym1) != is_tu_local_sym(sym2) ||
+		    is_llvm_sym(sym1) != is_llvm_sym(sym2))
+			continue;
+		if (idx_patched == sym1_pos)
+			match = sym2;
+		idx_patched++;
+	}
+
+	if (idx_orig != idx_patched)
+		return NULL;
+
+	return match;
+}
+
+/*
+ * Correlate symbols between the orig and patched objects.  This is a
+ * prerequisite for detecting changed functions, as well as for properly
+ * translating relocations so they point to the correct symbol.
  */
 static int correlate_symbols(struct elfs *e)
 {
 	struct symbol *file1_sym, *file2_sym;
 	struct symbol *sym1, *sym2;
+	bool progress;
 
+	/* Correlate FILE symbols */
 	file1_sym = first_file_symbol(e->orig);
 	file2_sym = first_file_symbol(e->patched);
 
-	/*
-	 * Correlate any locals before the first FILE symbol.  This has been
-	 * seen when LTO inexplicably strips the initramfs_data.o FILE symbol
-	 * due to the file only containing data and no code.
-	 */
-	for_each_sym(e->orig, sym1) {
-		if (sym1 == file1_sym || !is_local_sym(sym1))
-			break;
-
-		if (dont_correlate(sym1))
-			continue;
-
-		for_each_sym(e->patched, sym2) {
-			if (sym2 == file2_sym || !is_local_sym(sym2))
-				break;
-
-			if (sym2->twin || dont_correlate(sym2))
-				continue;
-
-			if (strcmp(sym1->demangled_name, sym2->demangled_name))
-				continue;
-
-			sym1->twin = sym2;
-			sym2->twin = sym1;
-			break;
-		}
-	}
-
-	/* Correlate locals after the first FILE symbol */
 	for (; ; file1_sym = next_file_symbol(e->orig, file1_sym),
 		 file2_sym = next_file_symbol(e->patched, file2_sym)) {
 
@@ -488,92 +729,52 @@ static int correlate_symbols(struct elfs *e)
 
 		file1_sym->twin = file2_sym;
 		file2_sym->twin = file1_sym;
-
-		sym1 = file1_sym;
-
-		for_each_sym_continue(e->orig, sym1) {
-			if (is_file_sym(sym1) || !is_local_sym(sym1))
-				break;
-
-			if (dont_correlate(sym1))
-				continue;
-
-			sym2 = file2_sym;
-			for_each_sym_continue(e->patched, sym2) {
-				if (is_file_sym(sym2) || !is_local_sym(sym2))
-					break;
-
-				if (sym2->twin || dont_correlate(sym2))
-					continue;
-
-				if (strcmp(sym1->demangled_name, sym2->demangled_name))
-					continue;
-
-				sym1->twin = sym2;
-				sym2->twin = sym1;
-				break;
-			}
-		}
 	}
 
-	/* Correlate globals */
-	for_each_sym(e->orig, sym1) {
-		if (sym1->bind == STB_LOCAL)
-			continue;
-
-		sym2 = find_global_symbol_by_name(e->patched, sym1->name);
-		if (sym2 && !sym2->twin) {
-			sym1->twin = sym2;
-			sym2->twin = sym1;
-		}
-	}
 
 	/*
-	 * Correlate globals with demangled_name.
-	 * A separate loop is needed because we want to finish all the
-	 * full name correlations first.
+	 * Correlate in two phases: loop deterministic levels until no more
+	 * progress, then use positional fallback for the rest.  This prevents
+	 * the nondeterministic positional matching from stealing symbols that
+	 * have deterministic matches.
 	 */
+	hash_init(suffix_map);
+	do {
+		progress = false;
+		for_each_sym(e->orig, sym1) {
+			if (sym1->twin || dont_correlate(sym1))
+				continue;
+			sym2 = find_twin(e, sym1);
+			if (!sym2)
+				continue;
+			sym1->twin = sym2;
+			sym2->twin = sym1;
+			progress = true;
+		}
+
+		if (update_suffix_map(e->orig))
+			return -1;
+
+		for_each_sym(e->orig, sym1) {
+			if (sym1->twin || dont_correlate(sym1))
+				continue;
+			sym2 = find_twin_suffixed(e->patched, sym1);
+			if (!sym2)
+				continue;
+			sym1->twin = sym2;
+			sym2->twin = sym1;
+			progress = true;
+		}
+	} while (progress);
+
 	for_each_sym(e->orig, sym1) {
-		if (sym1->bind == STB_LOCAL || sym1->twin)
+		if (sym1->twin || dont_correlate(sym1))
 			continue;
-
-		if (find_global_symbol_by_demangled_name(e->patched, sym1, &sym2))
-			return -1;
-
-		if (sym2 && !sym2->twin) {
-			sym1->twin = sym2;
-			sym2->twin = sym1;
-		}
-	}
-
-	/* Correlate original locals with patched globals */
-	for_each_sym(e->orig, sym1) {
-		if (sym1->twin || dont_correlate(sym1) || !is_local_sym(sym1))
+		sym2 = find_twin_positional(e, sym1);
+		if (!sym2)
 			continue;
-
-		sym2 = find_global_symbol_by_name(e->patched, sym1->name);
-		if (!sym2 && find_global_symbol_by_demangled_name(e->patched, sym1, &sym2))
-			return -1;
-
-		if (sym2 && !sym2->twin) {
-			sym1->twin = sym2;
-			sym2->twin = sym1;
-		}
-	}
-
-	/* Correlate original globals with patched locals */
-	for_each_sym(e->patched, sym2) {
-		if (sym2->twin || dont_correlate(sym2) || !is_local_sym(sym2))
-			continue;
-
-		sym1 = find_global_symbol_by_name(e->orig, sym2->name);
-		if (!sym1 && find_global_symbol_by_demangled_name(e->orig, sym2, &sym1))
-			return -1;
-
-		if (sym1 && !sym1->twin) {
-			sym2->twin = sym1;
-			sym1->twin = sym2;
-		}
+		sym1->twin = sym2;
+		sym2->twin = sym1;
 	}
 
 	for_each_sym(e->orig, sym1) {
@@ -785,19 +986,24 @@ static void mark_included_function(struct symbol *func)
  */
 static int mark_changed_functions(struct elfs *e)
 {
-	struct symbol *sym_orig, *patched_sym;
+	struct symbol *orig_sym, *patched_sym;
 	bool changed = false;
 
 	/* Find changed functions */
-	for_each_sym(e->orig, sym_orig) {
-		if (!is_func_sym(sym_orig) || dont_correlate(sym_orig))
+	for_each_sym(e->orig, orig_sym) {
+		if (dont_correlate(orig_sym))
 			continue;
 
-		patched_sym = sym_orig->twin;
+		patched_sym = orig_sym->twin;
 		if (!patched_sym)
 			continue;
 
-		if (sym_orig->csum.checksum != patched_sym->csum.checksum) {
+		if (orig_sym->csum.checksum != patched_sym->csum.checksum) {
+			if (!is_func_sym(orig_sym)) {
+				ERROR("changed data: %s", orig_sym->name);
+				return -1;
+			}
+
 			patched_sym->changed = 1;
 			mark_included_function(patched_sym);
 			changed = true;
@@ -822,7 +1028,7 @@ static int mark_changed_functions(struct elfs *e)
 			printf("%s: changed function: %s\n", objname, patched_sym->name);
 	}
 
-	return !changed ? -1 : 0;
+	return !changed ? 1 : 0;
 }
 
 static int clone_included_functions(struct elfs *e)
@@ -1865,6 +2071,7 @@ static int copy_import_ns(struct elfs *e)
 int cmd_klp_diff(int argc, const char **argv)
 {
 	struct elfs e = {0};
+	int ret;
 
 	argc = parse_options(argc, argv, klp_diff_options, klp_diff_usage, 0);
 	if (argc != 3)
@@ -1891,7 +2098,10 @@ int cmd_klp_diff(int argc, const char **argv)
 	if (correlate_symbols(&e))
 		return -1;
 
-	if (mark_changed_functions(&e))
+	ret = mark_changed_functions(&e);
+	if (ret < 0)
+		return -1;
+	if (ret > 0)
 		return 0;
 
 	e.out = elf_create_file(&e.orig->ehdr, argv[2]);
-- 
2.53.0

[PATCH 42/48] objtool/klp: Add correlation debugging output

[Josh Poimboeuf]

Add debugging messages to show how duplicate symbols get correlated, and
split the --debug feature into --debug-correlate and --debug-clone.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/include/objtool/warn.h | 16 +++++++----
 tools/objtool/klp-diff.c             | 42 ++++++++++++++++++++++------
 tools/objtool/objtool.c              |  3 --
 3 files changed, 45 insertions(+), 16 deletions(-)

diff --git a/tools/objtool/include/objtool/warn.h b/tools/objtool/include/objtool/warn.h
index 595ee8009667..a9936d60980c 100644
--- a/tools/objtool/include/objtool/warn.h
+++ b/tools/objtool/include/objtool/warn.h
@@ -109,7 +109,7 @@ static inline char *offstr(struct section *sec, unsigned long offset)
 #define ERROR_FUNC(sec, offset, format, ...) __WARN_FUNC(ERROR_STR, sec, offset, format, ##__VA_ARGS__)
 #define ERROR_INSN(insn, format, ...) ERROR_FUNC(insn->sec, insn->offset, format, ##__VA_ARGS__)
 
-extern bool debug;
+extern bool debug, debug_correlate, debug_clone;
 extern int indent;
 
 static inline void unindent(int *unused) { indent--; }
@@ -148,15 +148,21 @@ static inline void unindent(int *unused) { indent--; }
 		      (unsigned long long)checksum);			\
 })
 
-#define __dbg_indent(format, ...)					\
+#define dbg_correlate(args...)						\
 ({									\
-	if (unlikely(debug))						\
+	if (unlikely(debug_correlate))					\
+		__dbg(args);						\
+})
+
+#define __dbg_clone(format, ...)					\
+({									\
+	if (unlikely(debug_clone))					\
 		__dbg("%*s" format, indent * 8, "", ##__VA_ARGS__);	\
 })
 
-#define dbg_indent(args...)						\
+#define dbg_clone(args...)						\
 	int __cleanup(unindent) __dummy_##__COUNTER__;			\
-	__dbg_indent(args);						\
+	__dbg_clone(args);						\
 	indent++
 
 #endif /* _WARN_H */
diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 6d7fbb16e59c..acb76aefd04f 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -33,6 +33,9 @@ struct export {
 	char *mod, *sym;
 };
 
+bool debug, debug_correlate, debug_clone;
+int indent;
+
 static const char * const klp_diff_usage[] = {
 	"objtool klp diff [<options>] <in1.o> <in2.o> <out.o>",
 	NULL,
@@ -40,7 +43,9 @@ static const char * const klp_diff_usage[] = {
 
 static const struct option klp_diff_options[] = {
 	OPT_GROUP("Options:"),
-	OPT_BOOLEAN('d', "debug", &debug, "enable debug output"),
+	OPT_BOOLEAN('d', "debug", &debug, "enable all debug output"),
+	OPT_BOOLEAN(0, "debug-correlate", &debug_correlate, "enable correlation debug output"),
+	OPT_BOOLEAN(0, "debug-clone", &debug_clone, "enable cloning debug output"),
 	OPT_END(),
 };
 
@@ -543,6 +548,14 @@ static struct symbol *find_twin(struct elfs *e, struct symbol *sym1)
 	else if (csum_orig == 1 && csum_patched == 1)
 		match = csum_last;
 
+	if (!match)
+		return NULL;
+
+	if (name_orig != 1 || name_patched != 1)
+		dbg_correlate("find_twin(): %s%s -> %s%s",
+			      sym1->name, is_func_sym(sym1) ? "()" : "",
+			      match->name, is_func_sym(match) ? "()" : "");
+
 	return match;
 }
 
@@ -638,10 +651,14 @@ static struct symbol *find_twin_suffixed(struct elf *elf, struct symbol *sym1)
 		match = sym2;
 	}
 
-	if (count == 1)
-		return match;
+	if (count != 1)
+		return NULL;
 
-	return NULL;
+	dbg_correlate("find_suffixed_twin(): %s%s -> %s%s",
+		      sym1->name, is_func_sym(sym1) ? "()" : "",
+		      match->name, is_func_sym(match) ? "()" : "");
+
+	return match;
 }
 
 /*
@@ -688,6 +705,10 @@ static struct symbol *find_twin_positional(struct elfs *e, struct symbol *sym1)
 	if (idx_orig != idx_patched)
 		return NULL;
 
+	dbg_correlate("find_twin_positional(): %s%s -> %s%s",
+	    sym1->name, is_func_sym(sym1) ? "()" : "",
+	    match->name, is_func_sym(match) ? "()" : "");
+
 	return match;
 }
 
@@ -944,7 +965,7 @@ static struct symbol *clone_symbol(struct elfs *e, struct symbol *patched_sym,
 	if (patched_sym->clone)
 		return patched_sym->clone;
 
-	dbg_indent("%s%s", patched_sym->name, data_too ? " [+DATA]" : "");
+	dbg_clone("%s%s", patched_sym->name, data_too ? " [+DATA]" : "");
 
 	/* Make sure the prefix gets cloned first */
 	if (is_func_sym(patched_sym) && data_too) {
@@ -1324,7 +1345,7 @@ static int clone_reloc_klp(struct elfs *e, struct reloc *patched_reloc,
 
 	klp_sym = find_symbol_by_name(e->out, sym_name);
 	if (!klp_sym) {
-		__dbg_indent("%s", sym_name);
+		__dbg_clone("%s", sym_name);
 
 		/* STB_WEAK: avoid modpost undefined symbol warnings */
 		klp_sym = elf_create_symbol(e->out, sym_name, NULL,
@@ -1375,7 +1396,7 @@ static int clone_reloc_klp(struct elfs *e, struct reloc *patched_reloc,
 }
 
 #define dbg_clone_reloc(sec, offset, patched_sym, addend, export, klp)			\
-	dbg_indent("%s+0x%lx: %s%s0x%lx [%s%s%s%s%s%s]",				\
+	dbg_clone("%s+0x%lx: %s%s0x%lx [%s%s%s%s%s%s]",				\
 		   sec->name, offset, patched_sym->name,				\
 		   addend >= 0 ? "+" : "-", labs(addend),				\
 		   sym_type(patched_sym),						\
@@ -1437,7 +1458,7 @@ static int clone_reloc(struct elfs *e, struct reloc *patched_reloc,
 	if (is_string_sec(patched_sym->sec)) {
 		const char *str = patched_sym->sec->data->d_buf + addend;
 
-		__dbg_indent("\"%s\"", escape_str(str));
+		__dbg_clone("\"%s\"", escape_str(str));
 
 		addend = elf_add_string(e->out, out_sym->sec, str);
 		if (addend == -1)
@@ -2077,6 +2098,11 @@ int cmd_klp_diff(int argc, const char **argv)
 	if (argc != 3)
 		usage_with_options(klp_diff_usage, klp_diff_options);
 
+	if (debug) {
+		debug_correlate = true;
+		debug_clone = true;
+	}
+
 	objname = argv[0];
 
 	e.orig = elf_open_read(argv[0], O_RDONLY);
diff --git a/tools/objtool/objtool.c b/tools/objtool/objtool.c
index 1c3622117c33..a4e139dee7e9 100644
--- a/tools/objtool/objtool.c
+++ b/tools/objtool/objtool.c
@@ -16,9 +16,6 @@
 #include <objtool/objtool.h>
 #include <objtool/warn.h>
 
-bool debug;
-int indent;
-
 static struct objtool_file file;
 
 struct objtool_file *objtool_open_read(const char *filename)
-- 
2.53.0

[PATCH 43/48] objtool: Add insn_sym() helper

[Josh Poimboeuf]

Alternative replacement instructions awkwardly have insn->sym set to the
function they get patched to rather than the symbol (or rather lack
thereof) they belong to in the file.

This makes it difficult to know where a given instruction actually
lives.

Add a new insn_sym() helper which preserves the existing semantic of
insn->sym.  Rename insn->sym to insn->_sym, which contains the actual
ELF binary symbol (or NULL, for alternative replacements) an instruction
lives in.

The private insn->_sym value will be needed for a subsequent patch.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/check.c                 | 31 ++++++++++++---------------
 tools/objtool/disas.c                 | 22 +++++++++----------
 tools/objtool/include/objtool/check.h | 19 ++++++++++++++--
 tools/objtool/include/objtool/warn.h  |  6 +++---
 tools/objtool/trace.c                 |  8 +++----
 5 files changed, 48 insertions(+), 38 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index ae047be919c5..410061aeed26 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -495,7 +495,7 @@ static int decode_instructions(struct objtool_file *file)
 			}
 
 			sym_for_each_insn(file, func, insn) {
-				insn->sym = func;
+				insn->_sym = func;
 				if (is_func_sym(func) &&
 				    insn->type == INSN_ENDBR &&
 				    list_empty(&insn->call_node)) {
@@ -859,15 +859,14 @@ static int create_ibt_endbr_seal_sections(struct objtool_file *file)
 	list_for_each_entry(insn, &file->endbr_list, call_node) {
 
 		int *site = (int *)sec->data->d_buf + idx;
-		struct symbol *sym = insn->sym;
+		struct symbol *func = insn_func(insn);
 		*site = 0;
 
-		if (opts.module && sym && is_func_sym(sym) &&
-		    insn->offset == sym->offset &&
-		    (!strcmp(sym->name, "init_module") ||
-		     !strcmp(sym->name, "cleanup_module"))) {
+		if (opts.module && func && insn->offset == func->offset &&
+		    (!strcmp(func->name, "init_module") ||
+		     !strcmp(func->name, "cleanup_module"))) {
 			ERROR("%s(): Magic init_module() function name is deprecated, use module_init(fn) instead",
-			      sym->name);
+			      func->name);
 			return -1;
 		}
 
@@ -1581,7 +1580,7 @@ static int add_jump_destinations(struct objtool_file *file)
 		}
 
 		if (!dest_sym || is_sec_sym(dest_sym)) {
-			dest_sym = dest_insn->sym;
+			dest_sym = insn_sym(dest_insn);
 			if (!dest_sym)
 				goto set_jump_dest;
 		}
@@ -1597,7 +1596,7 @@ static int add_jump_destinations(struct objtool_file *file)
 			continue;
 		}
 
-		if (!insn->sym || insn->sym->pfunc == dest_sym->pfunc)
+		if (!insn_sym(insn) || insn_sym(insn)->pfunc == dest_sym->pfunc)
 			goto set_jump_dest;
 
 		/*
@@ -1770,7 +1769,6 @@ static int handle_group_alt(struct objtool_file *file,
 		nop->offset = special_alt->new_off + special_alt->new_len;
 		nop->len = special_alt->orig_len - special_alt->new_len;
 		nop->type = INSN_NOP;
-		nop->sym = orig_insn->sym;
 		nop->alt_group = new_alt_group;
 		nop->fake = 1;
 	}
@@ -1789,7 +1787,6 @@ static int handle_group_alt(struct objtool_file *file,
 
 		last_new_insn = insn;
 
-		insn->sym = orig_insn->sym;
 		insn->alt_group = new_alt_group;
 
 		/*
@@ -2432,12 +2429,12 @@ static int __annotate_late(struct objtool_file *file, int type, struct instructi
 		break;
 
 	case ANNOTYPE_NOCFI:
-		sym = insn->sym;
+		sym = insn_sym(insn);
 		if (!sym) {
 			ERROR_INSN(insn, "dodgy NOCFI annotation");
 			return -1;
 		}
-		insn->sym->nocfi = 1;
+		insn_sym(insn)->nocfi = 1;
 		break;
 
 	default:
@@ -2538,7 +2535,7 @@ static void mark_holes(struct objtool_file *file)
 	 * favour of a regular symbol, but leaves the code in place.
 	 */
 	for_each_insn(file, insn) {
-		if (insn->sym || !find_symbol_hole_containing(insn->sec, insn->offset)) {
+		if (insn_sym(insn) || !find_symbol_hole_containing(insn->sec, insn->offset)) {
 			in_hole = false;
 			continue;
 		}
@@ -2982,7 +2979,7 @@ static int update_cfi_state(struct instruction *insn,
 			}
 
 			if (op->dest.reg == CFI_BP && op->src.reg == CFI_SP &&
-			    insn->sym->frame_pointer) {
+			    insn_sym(insn)->frame_pointer) {
 				/* addi.d fp,sp,imm on LoongArch */
 				if (cfa->base == CFI_SP && cfa->offset == op->src.offset) {
 					cfa->base = CFI_BP;
@@ -2994,7 +2991,7 @@ static int update_cfi_state(struct instruction *insn,
 			if (op->dest.reg == CFI_SP && op->src.reg == CFI_BP) {
 				/* addi.d sp,fp,imm on LoongArch */
 				if (cfa->base == CFI_BP && cfa->offset == 0) {
-					if (insn->sym->frame_pointer) {
+					if (insn_sym(insn)->frame_pointer) {
 						cfa->base = CFI_SP;
 						cfa->offset = -op->src.offset;
 					}
@@ -4171,7 +4168,7 @@ static int validate_retpoline(struct objtool_file *file)
 	 * broken.
 	 */
 	list_for_each_entry(insn, &file->retpoline_call_list, call_node) {
-		struct symbol *sym = insn->sym;
+		struct symbol *sym = insn_sym(insn);
 
 		if (sym && (is_notype_sym(sym) ||
 			    is_func_sym(sym)) && !sym->nocfi) {
diff --git a/tools/objtool/disas.c b/tools/objtool/disas.c
index 59090234af19..e6a54a83605c 100644
--- a/tools/objtool/disas.c
+++ b/tools/objtool/disas.c
@@ -210,7 +210,7 @@ static bool disas_print_addr_alt(bfd_vma addr, struct disassemble_info *dinfo)
 	offset = addr - alt_group->first_insn->offset;
 
 	addr = orig_first_insn->offset + offset;
-	sym = orig_first_insn->sym;
+	sym = insn_sym(orig_first_insn);
 
 	disas_print_addr_sym(orig_first_insn->sec, sym, addr, dinfo);
 
@@ -222,15 +222,13 @@ static void disas_print_addr_noreloc(bfd_vma addr,
 {
 	struct disas_context *dctx = dinfo->application_data;
 	struct instruction *insn = dctx->insn;
-	struct symbol *sym = NULL;
+	struct symbol *sym = insn_sym(insn);
 
 	if (disas_print_addr_alt(addr, dinfo))
 		return;
 
-	if (insn->sym && addr >= insn->sym->offset &&
-	    addr < insn->sym->offset + insn->sym->len) {
-		sym = insn->sym;
-	}
+	if (sym && (addr < sym->offset || addr >= sym->offset + sym->len))
+		sym = NULL;
 
 	disas_print_addr_sym(insn->sec, sym, addr, dinfo);
 }
@@ -291,9 +289,9 @@ static void disas_print_address(bfd_vma addr, struct disassemble_info *dinfo)
 	 * up. So check it first.
 	 */
 	jump_dest = insn->jump_dest;
-	if (jump_dest && jump_dest->sym && jump_dest->offset == addr) {
+	if (jump_dest && insn_sym(jump_dest) && jump_dest->offset == addr) {
 		if (!disas_print_addr_alt(addr, dinfo))
-			disas_print_addr_sym(jump_dest->sec, jump_dest->sym,
+			disas_print_addr_sym(jump_dest->sec, insn_sym(jump_dest),
 					     addr, dinfo);
 		return;
 	}
@@ -768,8 +766,8 @@ static int disas_alt_jump(struct disas_alt *dalt)
 		if (orig_insn->len == 5)
 			suffix[0] = 'q';
 		str = strfmt("jmp%-3s %lx <%s+0x%lx>", suffix,
-			     dest_insn->offset, dest_insn->sym->name,
-			     dest_insn->offset - dest_insn->sym->offset);
+			     dest_insn->offset, insn_sym(dest_insn)->name,
+			     dest_insn->offset - insn_sym(dest_insn)->offset);
 		nops = 0;
 	} else {
 		str = strfmt("nop%d", orig_insn->len);
@@ -794,8 +792,8 @@ static int disas_alt_extable(struct disas_alt *dalt)
 
 	alt_insn = dalt->alt->insn;
 	str = strfmt("resume at 0x%lx <%s+0x%lx>",
-		     alt_insn->offset, alt_insn->sym->name,
-		     alt_insn->offset - alt_insn->sym->offset);
+		     alt_insn->offset, insn_sym(alt_insn)->name,
+		     alt_insn->offset - insn_sym(alt_insn)->offset);
 	if (!str)
 		return -1;
 
diff --git a/tools/objtool/include/objtool/check.h b/tools/objtool/include/objtool/check.h
index eea64728d39b..0c53476528a8 100644
--- a/tools/objtool/include/objtool/check.h
+++ b/tools/objtool/include/objtool/check.h
@@ -94,14 +94,29 @@ struct instruction {
 		};
 	};
 	struct alternative *alts;
-	struct symbol *sym;
+	struct symbol *_sym;
 	struct stack_op *stack_ops;
 	struct cfi_state *cfi;
 };
 
+/*
+ * Return the symbol associated with an instruction.  For alternative
+ * replacements, return the symbol of the original code being replaced rather
+ * than NULL.  insn->_sym reflects the actual location in the ELF file.
+ */
+static inline struct symbol *insn_sym(struct instruction *insn)
+{
+	struct symbol *sym = insn->_sym;
+
+	if (!sym && insn->alt_group && insn->alt_group->orig_group)
+		sym = insn->alt_group->orig_group->first_insn->_sym;
+
+	return sym;
+}
+
 static inline struct symbol *insn_func(struct instruction *insn)
 {
-	struct symbol *sym = insn->sym;
+	struct symbol *sym = insn_sym(insn);
 
 	if (sym && sym->type != STT_FUNC)
 		sym = NULL;
diff --git a/tools/objtool/include/objtool/warn.h b/tools/objtool/include/objtool/warn.h
index a9936d60980c..870e147f3a56 100644
--- a/tools/objtool/include/objtool/warn.h
+++ b/tools/objtool/include/objtool/warn.h
@@ -77,13 +77,13 @@ static inline char *offstr(struct section *sec, unsigned long offset)
 #define WARN_INSN(insn, format, ...)					\
 ({									\
 	struct instruction *_insn = (insn);				\
-	if (!_insn->sym || !_insn->sym->warned)	{			\
+	if (!insn_sym(_insn) || !insn_sym(_insn)->warned)	{	\
 		WARN_FUNC(_insn->sec, _insn->offset, format,		\
 			  ##__VA_ARGS__);				\
 		BT_INSN(_insn, "");					\
 	}								\
-	if (_insn->sym)							\
-		_insn->sym->warned = 1;					\
+	if (insn_sym(_insn))						\
+		insn_sym(_insn)->warned = 1;				\
 })
 
 #define BT_INSN(insn, format, ...)				\
diff --git a/tools/objtool/trace.c b/tools/objtool/trace.c
index 5dec44dab781..61c6aa302bc3 100644
--- a/tools/objtool/trace.c
+++ b/tools/objtool/trace.c
@@ -169,8 +169,8 @@ void trace_alt_begin(struct instruction *orig_insn, struct alternative *alt,
 		 */
 		TRACE_ALT_INFO_NOADDR(orig_insn, "/ ", "%s for instruction at 0x%lx <%s+0x%lx>",
 				      alt_name,
-				      orig_insn->offset, orig_insn->sym->name,
-				      orig_insn->offset - orig_insn->sym->offset);
+				      orig_insn->offset, insn_sym(orig_insn)->name,
+				      orig_insn->offset - insn_sym(orig_insn)->offset);
 	} else {
 		TRACE_ALT_INFO_NOADDR(orig_insn, "/ ", "%s", alt_name);
 	}
@@ -185,8 +185,8 @@ void trace_alt_begin(struct instruction *orig_insn, struct alternative *alt,
 		if (orig_insn->type == INSN_NOP) {
 			suffix[0] = (orig_insn->len == 5) ? 'q' : '\0';
 			TRACE_ADDR(orig_insn, "jmp%-3s %lx <%s+0x%lx>", suffix,
-				   alt_insn->offset, alt_insn->sym->name,
-				   alt_insn->offset - alt_insn->sym->offset);
+				   alt_insn->offset, insn_sym(alt_insn)->name,
+				   alt_insn->offset - insn_sym(alt_insn)->offset);
 		} else {
 			TRACE_ADDR(orig_insn, "nop%d", orig_insn->len);
 			trace_depth--;
-- 
2.53.0

[PATCH 44/48] objtool/klp: Fix position-dependent checksums for non-relocated jumps/calls

[Josh Poimboeuf]

When computing klp checksums, instructions with non-relocated jump/call
destination offsets are problematic because the offset values can change
when surrounding code has moved, causing the function to be incorrectly
marked as changed.

Specifically, that includes jumps from alternatives to the end of the
alternative, which from objtool's perspective are jumps to the end of
the alternative instruction block in the original function.

Note that 'jump_dest' jumps don't include sibling calls (those use
call_dest), nor do they include jumps to/from .cold sub functions (those
are cross-section and need a reloc).

Fix it by hashing the opcode bytes (excluding the immediate operand)
along with a position-independent representation of the destination.
For calls, use the function name, and for jumps, use the destination's
offset within its function.

[Note the "9 bit hole" comment was wrong: it has been 8 bits since
commit 70589843b36f ("objtool: Add option to trace function validation")
added the 'trace' field.  Adding the 4-bit 'immediate_len' field now
leaves a 4-bit hole.]

Fixes: 0d83da43b1e1 ("objtool/klp: Add --checksum option to generate per-function checksums")
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/arch/x86/decode.c       | 17 ++++++++-
 tools/objtool/include/objtool/arch.h  |  3 ++
 tools/objtool/include/objtool/check.h |  3 +-
 tools/objtool/klp-checksum.c          | 53 ++++++++++++++++++++++++---
 4 files changed, 67 insertions(+), 9 deletions(-)

diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 350b8ee6e776..1b387d5a195b 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -805,14 +805,27 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec
 		break;
 	}
 
-	if (ins.immediate.nbytes)
+	if (ins.immediate.nbytes) {
 		insn->immediate = ins.immediate.value;
-	else if (ins.displacement.nbytes)
+		insn->immediate_len = ins.immediate.nbytes;
+	} else if (ins.displacement.nbytes) {
 		insn->immediate = ins.displacement.value;
+		insn->immediate_len = ins.displacement.nbytes;
+	}
 
 	return 0;
 }
 
+size_t arch_jump_opcode_bytes(struct objtool_file *file, struct instruction *insn,
+			      unsigned char *buf)
+{
+	size_t len;
+
+	len = insn->len - insn->immediate_len;
+	memcpy(buf, insn->sec->data->d_buf + insn->offset, len);
+	return len;
+}
+
 void arch_initial_func_cfi_state(struct cfi_init_state *state)
 {
 	int i;
diff --git a/tools/objtool/include/objtool/arch.h b/tools/objtool/include/objtool/arch.h
index 8866158975fc..96d828a8401f 100644
--- a/tools/objtool/include/objtool/arch.h
+++ b/tools/objtool/include/objtool/arch.h
@@ -79,6 +79,9 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec
 			    unsigned long offset, unsigned int maxlen,
 			    struct instruction *insn);
 
+size_t arch_jump_opcode_bytes(struct objtool_file *file, struct instruction *insn,
+			      unsigned char *buf);
+
 bool arch_callee_saved_reg(unsigned char reg);
 
 unsigned long arch_jump_destination(struct instruction *insn);
diff --git a/tools/objtool/include/objtool/check.h b/tools/objtool/include/objtool/check.h
index 0c53476528a8..bc99355e66e9 100644
--- a/tools/objtool/include/objtool/check.h
+++ b/tools/objtool/include/objtool/check.h
@@ -68,6 +68,7 @@ struct instruction {
 	s8 instr;
 
 	u32 idx			: INSN_CHUNK_BITS,
+	    immediate_len	: 4,
 	    dead_end		: 1,
 	    ignore_alts		: 1,
 	    hint		: 1,
@@ -81,7 +82,7 @@ struct instruction {
 	    hole		: 1,
 	    fake		: 1,
 	    trace		: 1;
-		/* 9 bit hole */
+		/* 4 bit hole */
 
 	struct alt_group *alt_group;
 	struct instruction *jump_dest;
diff --git a/tools/objtool/klp-checksum.c b/tools/objtool/klp-checksum.c
index adfd02447a45..4100b4dfba86 100644
--- a/tools/objtool/klp-checksum.c
+++ b/tools/objtool/klp-checksum.c
@@ -66,17 +66,58 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func,
 	if (insn->fake)
 		return;
 
-	__checksum_update_insn(func, insn, insn->sec->data->d_buf + insn->offset, insn->len);
-
 	if (!reloc) {
 		struct symbol *call_dest = insn_call_dest(insn);
+		struct instruction *jump_dest = insn->jump_dest;
 
-		if (call_dest)
-			__checksum_update_insn(func, insn, call_dest->demangled_name,
-					       strlen(call_dest->demangled_name));
-		goto alts;
+		/*
+		 * For a jump/call non-relocated dest offset embedded in the
+		 * instruction, the offset may vary due to changes in
+		 * surrounding code.  Just hash the opcode and a
+		 * position-independent representation of the destination.
+		 */
+
+		if (call_dest || jump_dest) {
+			unsigned char buf[16];
+			size_t len;
+
+			len = arch_jump_opcode_bytes(file, insn, buf);
+			__checksum_update_insn(func, insn, buf, len);
+
+			if (call_dest) {
+				__checksum_update_insn(func, insn, call_dest->demangled_name,
+						       strlen(call_dest->demangled_name));
+
+			} else if (jump_dest) {
+				struct symbol *dest_sym;
+				unsigned long offset;
+
+				/*
+				 * use insn->_sym instead of insn_sym() here.
+				 * For alternative replacements, the latter
+				 * would give the function of the code being
+				 * replaced.
+				 */
+				dest_sym = jump_dest->_sym;
+				if (!dest_sym)
+					goto alts;
+
+				__checksum_update_insn(func, insn, dest_sym->demangled_name,
+						       strlen(dest_sym->demangled_name));
+
+				offset = jump_dest->offset - dest_sym->offset;
+				__checksum_update_insn(func, insn, &offset, sizeof(offset));
+			}
+
+			goto alts;
+		}
 	}
 
+	__checksum_update_insn(func, insn, insn->sec->data->d_buf + insn->offset, insn->len);
+
+	if (!reloc)
+		goto alts;
+
 	sym = reloc->sym;
 	offset = arch_insn_adjusted_addend(insn, reloc);
 
-- 
2.53.0

[PATCH 45/48] x86/Kconfig: Enable CONFIG_PREFIX_SYMBOLS for FineIBT

[Josh Poimboeuf]

PREFIX_SYMBOLS has a !CFI dependency because the compiler already
generates __cfi_ prefix symbols for kCFI builds, so objtool-generated
__pfx_ symbols were considered redundant.

However, the __cfi_ symbols only cover the 5-byte kCFI type hash.  With
FUNCTION_CALL_PADDING, there are also 11 bytes of NOP padding between
the hash and the function entry which have no symbol to claim them.

The NOPs can be rewritten with call depth tracking thunks at runtime.
Without a symbol, unwinders and other tools that symbolize code
locations misattribute those bytes.

Remove the !CFI guard so objtool creates __pfx_ symbols for all
CALL_PADDING configs, covering the full padding area regardless of
whether there's also a __cfi_ symbol.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 arch/x86/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index f3f7cb01d69d..493b0038ac8d 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2439,7 +2439,7 @@ config CALL_THUNKS
 
 config PREFIX_SYMBOLS
 	def_bool y
-	depends on CALL_PADDING && !CFI
+	depends on CALL_PADDING
 
 menuconfig CPU_MITIGATIONS
 	bool "Mitigations for CPU vulnerabilities"
-- 
2.53.0

[PATCH 46/48] objtool/klp: Make function prefix handling more generic

[Josh Poimboeuf]

The way x86 klp-diff handles function prefix symbols is a bit awkward,
Also, x86 is the only arch which needs the __pfx_/cfi_ prefix symbols,
so this approach isn't extensible to other arches.  And while other
arches *do* use __patchable_function_entries (PFEs), they use them in
completely different ways.

In preparation for supporting other arches, use a more generic approach
that will work for all arches with prefixed areas and/or PFE sections.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/include/objtool/elf.h |  15 +--
 tools/objtool/klp-diff.c            | 187 ++++++++++++++++++++++++----
 2 files changed, 161 insertions(+), 41 deletions(-)

diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
index ba13dd67cf26..21441bd72971 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -119,6 +119,7 @@ struct elf {
 	struct list_head sections;
 	struct list_head symbols;
 	unsigned long num_relocs;
+	int pfe_offset;
 
 	int symbol_bits;
 	int symbol_name_bits;
@@ -532,20 +533,6 @@ static inline void set_sym_next_reloc(struct reloc *reloc, struct reloc *next)
 	     reloc && reloc_offset(reloc) <  sym->offset + sym->len;	\
 	     reloc = rsec_next_reloc(sym->sec->rsec, reloc))
 
-static inline struct symbol *get_func_prefix(struct symbol *func)
-{
-	struct symbol *prev;
-
-	if (!is_func_sym(func))
-		return NULL;
-
-	prev = sec_prev_sym(func);
-	if (prev && is_prefix_func(prev))
-		return prev;
-
-	return NULL;
-}
-
 #define OFFSET_STRIDE_BITS	4
 #define OFFSET_STRIDE		(1UL << OFFSET_STRIDE_BITS)
 #define OFFSET_STRIDE_MASK	(~(OFFSET_STRIDE - 1))
diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index acb76aefd04f..420d05633aba 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -213,6 +213,88 @@ static int read_sym_checksums(struct elf *elf)
 	return 0;
 }
 
+/*
+ * Detect the offset from the function entry point to its
+ * __patchable_function_entries (PFE) relocation target.
+ *
+ * offset < 0 (before function entry):
+ *
+ *    CONFIG_FINEIBT (x86)
+ *    CONFIG_MITIGATION_CALL_DEPTH_TRACKING (x86)
+ */
+static int read_pfe_offset(struct elf *elf)
+{
+	bool has_pfe = false;
+	struct section *sec;
+
+	for_each_sec(elf, sec) {
+		struct reloc *reloc;
+
+		if (strcmp(sec->name, "__patchable_function_entries"))
+			continue;
+		if (!sec->rsec)
+			continue;
+
+		has_pfe = true;
+
+		for_each_reloc(sec->rsec, reloc) {
+			unsigned long target = reloc->sym->offset + reloc_addend(reloc);
+			struct symbol *func;
+
+			func = find_func_containing(reloc->sym->sec, target);
+			if (func) {
+				if (is_prefix_func(func))
+					elf->pfe_offset = target - (func->offset + func->len);
+				else
+					elf->pfe_offset = target - func->offset;
+				return 0;
+			}
+		}
+	}
+
+	if (has_pfe) {
+		ERROR("can't find __patchable_function_entries offset");
+		return -1;
+	}
+
+	return 0;
+}
+
+/*
+ * Detect the size of the area before a function's entry point.  This prefix
+ * area is used for CFI type hashes, call thunks, or ftrace call ops.
+ *
+ *  __pfx_ prefix function (x86):
+ *
+ *    CONFIG_MITIGATION_CALL_DEPTH_TRACKING
+ *
+ *  __cfi_ prefix function (x86):
+ *
+ *    CONFIG_CFI
+ */
+static unsigned long func_pfx_size(struct elf *elf, struct symbol *func)
+{
+	struct symbol *pfx;
+
+	/* x86 __pfx_ and/or __cfi_ */
+	if (func->offset) {
+		pfx = find_func_containing(func->sec, func->offset - 1);
+		if (pfx && pfx->prefix) {
+			struct symbol *pfx2;
+
+			/* FineIBT has both */
+			if (pfx->offset) {
+				pfx2 = find_func_containing(func->sec, pfx->offset - 1);
+				if (pfx2 && pfx2->prefix)
+					pfx = pfx2;
+			}
+
+			return func->offset - pfx->offset;
+		}
+	}
+	return 0;
+}
+
 static struct symbol *first_file_symbol(struct elf *elf)
 {
 	struct symbol *sym;
@@ -302,6 +384,7 @@ static bool is_special_section(struct section *sec)
 		"__ex_table",
 		"__jump_table",
 		"__mcount_loc",
+		"__patchable_function_entries",
 
 		/*
 		 * Extract .static_call_sites here to inherit non-module
@@ -872,7 +955,7 @@ static struct symbol *__clone_symbol(struct elf *elf, struct symbol *patched_sym
 				     bool data_too)
 {
 	struct section *out_sec = NULL;
-	unsigned long offset = 0;
+	unsigned long offset = 0, pfx_size = 0;
 	struct symbol *out_sym;
 
 	if (data_too && !is_undef_sym(patched_sym)) {
@@ -901,20 +984,26 @@ static struct symbol *__clone_symbol(struct elf *elf, struct symbol *patched_sym
 			offset = ALIGN(sec_size(out_sec), out_sec->sh.sh_addralign);
 
 		if (patched_sym->len || is_sec_sym(patched_sym)) {
-			void *data = NULL;
 			size_t size;
+			void *data = NULL;
+
+			/* Clone function prefix area */
+			if (is_func_sym(patched_sym))
+				pfx_size = func_pfx_size(elf, patched_sym);
 
 			/* bss doesn't have data */
 			if (patched_sym->sec->data && patched_sym->sec->data->d_buf)
-				data = patched_sym->sec->data->d_buf + patched_sym->offset;
+				data = patched_sym->sec->data->d_buf + patched_sym->offset - pfx_size;
 
 			if (is_sec_sym(patched_sym))
 				size = sec_size(patched_sym->sec);
 			else
-				size = patched_sym->len;
+				size = patched_sym->len + pfx_size;
 
 			if (!elf_add_data(elf, out_sec, data, size))
 				return NULL;
+
+			offset += pfx_size;
 		}
 	}
 
@@ -924,6 +1013,23 @@ static struct symbol *__clone_symbol(struct elf *elf, struct symbol *patched_sym
 	if (!out_sym)
 		return NULL;
 
+	/*
+	 * The copied prefixed area may have had a __cfi_ symbol which needs to
+	 * be copied.  During the module link, objtool collates these in a
+	 * .cfi_sites section for FineIBT.
+	 */
+	if (pfx_size && is_func_sym(patched_sym)) {
+		struct symbol *cfi_sym;
+
+		cfi_sym = find_func_containing(patched_sym->sec, patched_sym->offset - pfx_size);
+		if (cfi_sym && strstarts(cfi_sym->name, "__cfi_")) {
+			if (!elf_create_symbol(elf, cfi_sym->name, out_sec,
+					       cfi_sym->bind, cfi_sym->type,
+					       offset - pfx_size, cfi_sym->len))
+				return NULL;
+		}
+	}
+
 sym_created:
 	patched_sym->clone = out_sym;
 	out_sym->clone = patched_sym;
@@ -960,20 +1066,11 @@ static const char *sym_bind(struct symbol *sym)
 static struct symbol *clone_symbol(struct elfs *e, struct symbol *patched_sym,
 				   bool data_too)
 {
-	struct symbol *pfx;
-
 	if (patched_sym->clone)
 		return patched_sym->clone;
 
 	dbg_clone("%s%s", patched_sym->name, data_too ? " [+DATA]" : "");
 
-	/* Make sure the prefix gets cloned first */
-	if (is_func_sym(patched_sym) && data_too) {
-		pfx = get_func_prefix(patched_sym);
-		if (pfx)
-			clone_symbol(e, pfx, true);
-	}
-
 	if (!__clone_symbol(e->out, patched_sym, data_too))
 		return NULL;
 
@@ -985,15 +1082,8 @@ static struct symbol *clone_symbol(struct elfs *e, struct symbol *patched_sym,
 
 static void mark_included_function(struct symbol *func)
 {
-	struct symbol *pfx;
-
 	func->included = 1;
 
-	/* Include prefix function */
-	pfx = get_func_prefix(func);
-	if (pfx)
-		pfx->included = 1;
-
 	/* Make sure .cold parent+child always stay together */
 	if (func->cfunc && func->cfunc != func)
 		func->cfunc->included = 1;
@@ -1222,17 +1312,37 @@ static int convert_reloc_sym_to_secsym(struct elf *elf, struct reloc *reloc)
 	return 0;
 }
 
+/*
+ * __patchable_function_entries relocs point to the patchable entry NOPs,
+ * which are at 'pfe_offset' bytes from the function symbol.
+ *
+ * Some entries (e.g., removed weak functions, syscall -ENOSYS stubs) don't
+ * have a corresponding function symbol.  Skip those with a return value of 1.
+ */
+static int convert_pfe_reloc(struct elf *elf, struct reloc *reloc)
+{
+	struct symbol *func;
+
+	func = find_func_by_offset(reloc->sym->sec,
+				   reloc->sym->offset +
+				   reloc_addend(reloc) - elf->pfe_offset);
+	if (!func)
+		return 1;
+
+	reloc->sym = func;
+	set_reloc_sym(elf, reloc, func->idx);
+	set_reloc_addend(elf, reloc, elf->pfe_offset);
+	return 0;
+}
+
 static int convert_reloc_secsym_to_sym(struct elf *elf, struct reloc *reloc)
 {
 	struct symbol *sym = reloc->sym;
 	struct section *sec = sym->sec;
 
-	/* If the symbol has a dedicated section, it's easy to find */
-	sym = find_symbol_by_offset(sec, 0);
-	if (sym && sym->len == sec_size(sec))
-		goto found_sym;
+	if (!strcmp(reloc->sec->name, ".rela__patchable_function_entries"))
+		return convert_pfe_reloc(elf, reloc);
 
-	/* No dedicated section; find the symbol manually */
 	sym = find_symbol_containing(sec, arch_adjusted_addend(reloc));
 	if (!sym) {
 		/*
@@ -1249,7 +1359,6 @@ static int convert_reloc_secsym_to_sym(struct elf *elf, struct reloc *reloc)
 		return -1;
 	}
 
-found_sym:
 	reloc->sym = sym;
 	set_reloc_sym(elf, reloc, sym->idx);
 	set_reloc_addend(elf, reloc, reloc_addend(reloc) - sym->offset);
@@ -1802,6 +1911,9 @@ static int validate_special_section_klp_reloc(struct elfs *e, struct symbol *sym
 
 static int clone_special_section(struct elfs *e, struct section *patched_sec)
 {
+	bool is_pfe = !strcmp(patched_sec->name, "__patchable_function_entries");
+	struct section *out_sec = NULL;
+	struct reloc *patched_reloc;
 	struct symbol *patched_sym;
 
 	/*
@@ -1809,6 +1921,7 @@ static int clone_special_section(struct elfs *e, struct section *patched_sec)
 	 * reference included functions.
 	 */
 	sec_for_each_sym(patched_sec, patched_sym) {
+		struct symbol *out_sym;
 		int ret;
 
 		if (!is_object_sym(patched_sym))
@@ -1823,8 +1936,23 @@ static int clone_special_section(struct elfs *e, struct section *patched_sec)
 		if (ret > 0)
 			continue;
 
-		if (!clone_symbol(e, patched_sym, true))
+		out_sym = clone_symbol(e, patched_sym, true);
+		if (!out_sym)
 			return -1;
+
+		if (!is_pfe || (out_sec && out_sec->sh.sh_link))
+			continue;
+
+		/*
+		 * For reasons, the patched object has multiple PFE sections,
+		 * but we only need to create one combined section for the
+		 * output.  Link the single PFE ouput section to a random text
+		 * section to satisfy the linker for SHF_LINK_ORDER.
+		 */
+		out_sec = out_sym->sec;
+		patched_reloc = find_reloc_by_dest(e->patched, patched_sec,
+						   patched_sym->offset);
+		out_sec->sh.sh_link = patched_reloc->sym->clone->sec->idx;
 	}
 
 	return 0;
@@ -2121,6 +2249,9 @@ int cmd_klp_diff(int argc, const char **argv)
 	if (read_sym_checksums(e.patched))
 		return -1;
 
+	if (read_pfe_offset(e.patched))
+		return -1;
+
 	if (correlate_symbols(&e))
 		return -1;
 
@@ -2134,6 +2265,8 @@ int cmd_klp_diff(int argc, const char **argv)
 	if (!e.out)
 		return -1;
 
+	e.out->pfe_offset = e.patched->pfe_offset;
+
 	/*
 	 * Special section fake symbols are needed so that individual special
 	 * section entries can be extracted by clone_special_sections().
-- 
2.53.0

[PATCH 47/48] objtool: Improve and simplify prefix symbol detection

[Josh Poimboeuf]

Only create prefix symbols for functions that have
__patchable_function_entries entries, since those are the only functions
where prefix NOPs are intentional.

This both simplifies the detection and makes it more accurate.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/check.c | 90 ++++++++++---------------------------------
 1 file changed, 21 insertions(+), 69 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 410061aeed26..d438c5a4444b 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4271,17 +4271,6 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio
  * For FineIBT or kCFI, a certain number of bytes preceding the function may be
  * NOPs.  Those NOPs may be rewritten at runtime and executed, so give them a
  * proper function name: __pfx_<func>.
- *
- * The NOPs may not exist for the following cases:
- *
- *   - compiler cloned functions (*.cold, *.part0, etc)
- *   - asm functions created with inline asm or without SYM_FUNC_START()
- *
- * Also, the function may already have a prefix from a previous objtool run
- * (livepatch extracted functions, or manually running objtool multiple times).
- *
- * So return 0 if the NOPs are missing or the function already has a prefix
- * symbol.
  */
 static int create_prefix_symbol(struct objtool_file *file, struct symbol *func)
 {
@@ -4289,10 +4278,6 @@ static int create_prefix_symbol(struct objtool_file *file, struct symbol *func)
 	char name[SYM_NAME_LEN];
 	struct cfi_state *cfi;
 
-	if (!is_func_sym(func) || is_prefix_func(func) || is_cold_func(func) ||
-	    func->static_call_tramp)
-		return 0;
-
 	if ((strlen(func->name) + sizeof("__pfx_") > SYM_NAME_LEN)) {
 		WARN("%s: symbol name too long, can't create __pfx_ symbol",
 		      func->name);
@@ -4302,59 +4287,21 @@ static int create_prefix_symbol(struct objtool_file *file, struct symbol *func)
 	if (snprintf_check(name, SYM_NAME_LEN, "__pfx_%s", func->name))
 		return -1;
 
-	if (file->klp) {
-		struct symbol *pfx;
-
-		pfx = find_symbol_by_offset(func->sec, func->offset - opts.prefix);
-		if (pfx && is_prefix_func(pfx) && !strcmp(pfx->name, name))
-			return 0;
-	}
-
-	insn = find_insn(file, func->sec, func->offset);
-	if (!insn) {
-		WARN("%s: can't find starting instruction", func->name);
+	if (!elf_create_symbol(file->elf, name, func->sec,
+			       GELF_ST_BIND(func->sym.st_info),
+			       GELF_ST_TYPE(func->sym.st_info),
+			       func->offset - opts.prefix, opts.prefix))
 		return -1;
-	}
-
-	for (prev = prev_insn_same_sec(file, insn);
-	     prev;
-	     prev = prev_insn_same_sec(file, prev)) {
-		u64 offset;
-
-		if (prev->type != INSN_NOP)
-			return 0;
-
-		offset = func->offset - prev->offset;
-
-		if (offset > opts.prefix)
-			return 0;
-
-		if (offset < opts.prefix)
-			continue;
-
-		if (!elf_create_symbol(file->elf, name, func->sec,
-				       GELF_ST_BIND(func->sym.st_info),
-				       GELF_ST_TYPE(func->sym.st_info),
-				       prev->offset, opts.prefix))
-			return -1;
-
-		break;
-	}
-
-	if (!prev)
-		return 0;
-
-	if (!insn->cfi) {
-		/*
-		 * This can happen if stack validation isn't enabled or the
-		 * function is annotated with STACK_FRAME_NON_STANDARD.
-		 */
-		return 0;
-	}
 
 	/* Propagate insn->cfi to the prefix code */
+	insn = find_insn(file, func->sec, func->offset);
+	if (!insn || !insn->cfi)
+		return 0;
+
 	cfi = cfi_hash_find_or_add(insn->cfi);
-	for (; prev != insn; prev = next_insn_same_sec(file, prev))
+	for (prev = find_insn(file, func->sec, func->offset - opts.prefix);
+	     prev && prev != insn;
+	     prev = next_insn_same_sec(file, prev))
 		prev->cfi = cfi;
 
 	return 0;
@@ -4362,15 +4309,20 @@ static int create_prefix_symbol(struct objtool_file *file, struct symbol *func)
 
 static int create_prefix_symbols(struct objtool_file *file)
 {
-	struct section *sec;
+	struct section *pfe_sec;
 	struct symbol *func;
+	struct reloc *reloc;
 
-	for_each_sec(file->elf, sec) {
-		if (!is_text_sec(sec))
+	for_each_sec(file->elf, pfe_sec) {
+		if (strcmp(pfe_sec->name, "__patchable_function_entries"))
+			continue;
+		if (!pfe_sec->rsec)
 			continue;
 
-		sec_for_each_sym(sec, func) {
-			if (create_prefix_symbol(file, func))
+		for_each_reloc(pfe_sec->rsec, reloc) {
+			func = find_func_by_offset(reloc->sym->sec,
+						   reloc->sym->offset + reloc_addend(reloc) + opts.prefix);
+			if (func && create_prefix_symbol(file, func))
 				return -1;
 		}
 	}
-- 
2.53.0

[PATCH 48/48] objtool/klp: Cache dont_correlate() result

[Josh Poimboeuf]

Cache the dont_correlate() result once per symbol at the start of
correlate_symbols().  This reduces klp diff time on an arm64 LTO
vmlinux.o from 2m51s to 35s.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/include/objtool/elf.h |  1 +
 tools/objtool/klp-diff.c            | 29 +++++++++++++++++------------
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
index 21441bd72971..fb4fec7d8a6e 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -95,6 +95,7 @@ struct symbol {
 	u8 changed	     : 1;
 	u8 included	     : 1;
 	u8 klp		     : 1;
+	u8 dont_correlate     : 1;
 	struct list_head pv_target;
 	struct reloc *relocs;
 	struct section *group_sec;
diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 420d05633aba..a848015b477c 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -566,7 +566,7 @@ static struct symbol *find_twin(struct elfs *e, struct symbol *sym1)
 
 	/* Count orig candidates */
 	for_each_sym_by_demangled_name(e->orig, sym1->demangled_name, sym2) {
-		if (sym2->twin || sym1->type != sym2->type || dont_correlate(sym2) ||
+		if (sym2->twin || sym1->type != sym2->type || sym2->dont_correlate ||
 		    (!maybe_same_file(sym1, sym2)))
 			continue;
 
@@ -592,7 +592,7 @@ static struct symbol *find_twin(struct elfs *e, struct symbol *sym1)
 
 	/* Count patched candidates */
 	for_each_sym_by_demangled_name(e->patched, sym1->demangled_name, sym2) {
-		if (sym2->twin || sym1->type != sym2->type || dont_correlate(sym2))
+		if (sym2->twin || sym1->type != sym2->type || sym2->dont_correlate)
 			continue;
 
 		/* Level 1 */
@@ -728,7 +728,7 @@ static struct symbol *find_twin_suffixed(struct elf *elf, struct symbol *sym1)
 		return NULL;
 
 	for_each_sym_by_name(elf, name, sym2) {
-		if (sym2->twin || sym1->type != sym2->type || dont_correlate(sym2))
+		if (sym2->twin || sym1->type != sym2->type || sym2->dont_correlate)
 			continue;
 		count++;
 		match = sym2;
@@ -762,7 +762,7 @@ static struct symbol *find_twin_positional(struct elfs *e, struct symbol *sym1)
 	struct symbol *sym2, *match = NULL;
 
 	for_each_sym_by_demangled_name(e->orig, sym1->demangled_name, sym2) {
-		if (sym2->twin || sym1->type != sym2->type || dont_correlate(sym2) ||
+		if (sym2->twin || sym1->type != sym2->type || sym2->dont_correlate ||
 		    !maybe_same_file(sym1, sym2))
 			continue;
 		if (is_tu_local_sym(sym1) != is_tu_local_sym(sym2) ||
@@ -774,7 +774,7 @@ static struct symbol *find_twin_positional(struct elfs *e, struct symbol *sym1)
 	}
 
 	for_each_sym_by_demangled_name(e->patched, sym1->demangled_name, sym2) {
-		if (sym2->twin || sym1->type != sym2->type || dont_correlate(sym2) ||
+		if (sym2->twin || sym1->type != sym2->type || sym2->dont_correlate ||
 		    !maybe_same_file(sym1, sym2))
 			continue;
 		if (is_tu_local_sym(sym1) != is_tu_local_sym(sym2) ||
@@ -806,6 +806,11 @@ static int correlate_symbols(struct elfs *e)
 	struct symbol *sym1, *sym2;
 	bool progress;
 
+	for_each_sym(e->orig, sym1)
+		sym1->dont_correlate = dont_correlate(sym1);
+	for_each_sym(e->patched, sym2)
+		sym2->dont_correlate = dont_correlate(sym2);
+
 	/* Correlate FILE symbols */
 	file1_sym = first_file_symbol(e->orig);
 	file2_sym = first_file_symbol(e->patched);
@@ -846,7 +851,7 @@ static int correlate_symbols(struct elfs *e)
 	do {
 		progress = false;
 		for_each_sym(e->orig, sym1) {
-			if (sym1->twin || dont_correlate(sym1))
+			if (sym1->twin || sym1->dont_correlate)
 				continue;
 			sym2 = find_twin(e, sym1);
 			if (!sym2)
@@ -860,7 +865,7 @@ static int correlate_symbols(struct elfs *e)
 			return -1;
 
 		for_each_sym(e->orig, sym1) {
-			if (sym1->twin || dont_correlate(sym1))
+			if (sym1->twin || sym1->dont_correlate)
 				continue;
 			sym2 = find_twin_suffixed(e->patched, sym1);
 			if (!sym2)
@@ -872,7 +877,7 @@ static int correlate_symbols(struct elfs *e)
 	} while (progress);
 
 	for_each_sym(e->orig, sym1) {
-		if (sym1->twin || dont_correlate(sym1))
+		if (sym1->twin || sym1->dont_correlate)
 			continue;
 		sym2 = find_twin_positional(e, sym1);
 		if (!sym2)
@@ -882,7 +887,7 @@ static int correlate_symbols(struct elfs *e)
 	}
 
 	for_each_sym(e->orig, sym1) {
-		if (sym1->twin || dont_correlate(sym1))
+		if (sym1->twin || sym1->dont_correlate)
 			continue;
 		WARN("no correlation: %s", sym1->name);
 	}
@@ -1102,7 +1107,7 @@ static int mark_changed_functions(struct elfs *e)
 
 	/* Find changed functions */
 	for_each_sym(e->orig, orig_sym) {
-		if (dont_correlate(orig_sym))
+		if (orig_sym->dont_correlate)
 			continue;
 
 		patched_sym = orig_sym->twin;
@@ -1123,7 +1128,7 @@ static int mark_changed_functions(struct elfs *e)
 
 	/* Find added functions and print them */
 	for_each_sym(e->patched, patched_sym) {
-		if (!is_func_sym(patched_sym) || dont_correlate(patched_sym))
+		if (!is_func_sym(patched_sym) || patched_sym->dont_correlate)
 			continue;
 
 		if (!patched_sym->twin) {
@@ -1263,7 +1268,7 @@ static bool klp_reloc_needed(struct reloc *patched_reloc)
 	struct export *export;
 
 	/* no external symbol to reference */
-	if (dont_correlate(patched_sym))
+	if (patched_sym->dont_correlate)
 		return false;
 
 	/* For included functions, a regular reloc will do. */
-- 
2.53.0

Re: [PATCH 05/48] objtool: Move mark_rodata() to elf.c

[Peter Zijlstra]

On Wed, Apr 22, 2026 at 09:03:33PM -0700, Josh Poimboeuf wrote:
> Move the sec->rodata marking from check.c to elf.c so it's set during
> ELF reading rather than during the check pipeline.  This makes the
> rodata flag available to all objtool users, including klp-diff which
> reads ELF files directly without running check().
> 
> Add an is_rodata_sec() helper to elf.h for consistency with
> is_text_sec() and is_string_sec().
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

> ---
>  tools/objtool/check.c               | 11 +++--------
>  tools/objtool/elf.c                 | 13 +++++++++++++
>  tools/objtool/include/objtool/elf.h |  5 +++++
>  3 files changed, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 9b11cf3193b9..5722d4568401 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -2566,7 +2566,6 @@ static int classify_symbols(struct objtool_file *file)
>  static void mark_rodata(struct objtool_file *file)
>  {
>  	struct section *sec;
> -	bool found = false;
>  
>  	/*
>  	 * Search for the following rodata sections, each of which can
> @@ -2579,15 +2578,11 @@ static void mark_rodata(struct objtool_file *file)
>  	 * .rodata.str1.* sections are ignored; they don't contain jump tables.
>  	 */
>  	for_each_sec(file->elf, sec) {
> -		if ((!strncmp(sec->name, ".rodata", 7) &&
> -		     !strstr(sec->name, ".str1.")) ||
> -		    !strncmp(sec->name, ".data.rel.ro", 12)) {
> -			sec->rodata = true;
> -			found = true;
> +		if (is_rodata_sec(sec)) {
> +			file->rodata = true;
> +			return;
>  		}
>  	}
> -
> -	file->rodata = found;
>  }
>  
>  static void mark_holes(struct objtool_file *file)
> diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> index f3df2bde119f..ac9da81a7a2f 100644
> --- a/tools/objtool/elf.c
> +++ b/tools/objtool/elf.c
> @@ -1172,6 +1172,17 @@ static int read_relocs(struct elf *elf)
>  	return 0;
>  }
>  
> +static void mark_rodata(struct elf *elf)
> +{
> +	struct section *sec;
> +
> +	for_each_sec(elf, sec) {
> +		if ((strstarts(sec->name, ".rodata") && !strstr(sec->name, ".str1.")) ||
> +		    strstarts(sec->name, ".data.rel.ro"))
> +			sec->rodata = true;
> +	}
> +}
> +
>  struct elf *elf_open_read(const char *name, int flags)
>  {
>  	struct elf *elf;
> @@ -1222,6 +1233,8 @@ struct elf *elf_open_read(const char *name, int flags)
>  	if (read_sections(elf))
>  		goto err;
>  
> +	mark_rodata(elf);
> +
>  	if (read_symbols(elf))
>  		goto err;
>  
> diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
> index 25573e5af76e..c61bd57767f9 100644
> --- a/tools/objtool/include/objtool/elf.h
> +++ b/tools/objtool/include/objtool/elf.h
> @@ -296,6 +296,11 @@ static inline bool is_text_sec(struct section *sec)
>  	return sec->sh.sh_flags & SHF_EXECINSTR;
>  }
>  
> +static inline bool is_rodata_sec(struct section *sec)
> +{
> +	return sec->rodata;
> +}
> +
>  static inline bool sec_changed(struct section *sec)
>  {
>  	return sec->_changed;
> -- 
> 2.53.0
> 

Re: [PATCH 17/48] objtool: Fix reloc hash collision in find_reloc_by_dest_range()

[Peter Zijlstra]

On Wed, Apr 22, 2026 at 09:03:45PM -0700, Josh Poimboeuf wrote:
> In find_reloc_by_dest_range(), hash collisions can cause a high-offset
> relocation to appear when probing a low-offset hash bucket.
> 
> Only return early when the best match found so far genuinely belongs to
> the current bucket (its offset is within the bucket's stride range).
> Otherwise, continue scanning later buckets which may contain
> lower-offset matches.

Maybe mention (and or add a comment to the function) that in case of
multiple matches in the given range, it will return the lowest address
one.

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

> Fixes: 74b873e49d92 ("objtool: Optimize find_rela_by_dest_range()")
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> ---
>  tools/objtool/elf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> index a5486e172e5c..c4cb371e72b2 100644
> --- a/tools/objtool/elf.c
> +++ b/tools/objtool/elf.c
> @@ -370,11 +370,11 @@ struct reloc *find_reloc_by_dest_range(const struct elf *elf, struct section *se
>  					r = reloc;
>  			}
>  		}
> -		if (r)
> +		if (r && (reloc_offset(r) & OFFSET_STRIDE_MASK) == o)
>  			return r;
>  	}
>  
> -	return NULL;
> +	return r;
>  }
>  
>  struct reloc *find_reloc_by_dest(const struct elf *elf, struct section *sec, unsigned long offset)
> -- 
> 2.53.0
> 

Re: [PATCH 26/48] objtool/klp: Don't set sym->file for section symbols

[Peter Zijlstra]

On Wed, Apr 22, 2026 at 09:03:54PM -0700, Josh Poimboeuf wrote:
> Section symbols aren't grouped after their corresponding FILE symbols.
> Their sym->file should really be NULL rather than whatever random FILE
> happened to be last.
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

> ---
>  tools/objtool/elf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> index c4cb371e72b2..00c2389f345f 100644
> --- a/tools/objtool/elf.c
> +++ b/tools/objtool/elf.c
> @@ -680,7 +680,7 @@ static int read_symbols(struct elf *elf)
>  
>  		if (is_file_sym(sym))
>  			file = sym;
> -		else if (sym->bind == STB_LOCAL)
> +		else if (sym->bind == STB_LOCAL && !is_sec_sym(sym))
>  			sym->file = file;
>  	}
>  
> -- 
> 2.53.0
> 

Re: [PATCH 31/48] objtool: Add is_alias_sym() helper

[Peter Zijlstra]

On Wed, Apr 22, 2026 at 09:03:59PM -0700, Josh Poimboeuf wrote:
> Improve readability with a new is_alias_sym() helper.
> 
> No functional changes intended.
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

> ---
>  tools/objtool/check.c               | 6 +++---
>  tools/objtool/include/objtool/elf.h | 5 +++++
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 54ceac857979..4c18d6e7f6c3 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -491,7 +491,7 @@ static int decode_instructions(struct objtool_file *file)
>  				return -1;
>  			}
>  
> -			if (func->embedded_insn || func->alias != func)
> +			if (func->embedded_insn || is_alias_sym(func))
>  				continue;
>  
>  			if (!find_insn(file, sec, func->offset)) {
> @@ -2229,7 +2229,7 @@ static int add_jump_table_alts(struct objtool_file *file)
>  		return 0;
>  
>  	for_each_sym(file->elf, func) {
> -		if (!is_func_sym(func) || func->alias != func)
> +		if (!is_func_sym(func) || is_alias_sym(func))
>  			continue;
>  
>  		mark_func_jump_tables(file, func);
> @@ -4527,7 +4527,7 @@ static int validate_symbol(struct objtool_file *file, struct section *sec,
>  		return 1;
>  	}
>  
> -	if (sym->pfunc != sym || sym->alias != sym)
> +	if (sym->pfunc != sym || is_alias_sym(sym))
>  		return 0;
>  
>  	insn = find_insn(file, sec, sym->offset);
> diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
> index cd5844c7b4e2..3abe4cbc584c 100644
> --- a/tools/objtool/include/objtool/elf.h
> +++ b/tools/objtool/include/objtool/elf.h
> @@ -279,6 +279,11 @@ static inline bool is_local_sym(struct symbol *sym)
>  	return sym->bind == STB_LOCAL;
>  }
>  
> +static inline bool is_alias_sym(struct symbol *sym)
> +{
> +	return sym->alias != sym;
> +}
> +
>  static inline bool is_prefix_func(struct symbol *sym)
>  {
>  	return sym->prefix;
> -- 
> 2.53.0
> 

Re: [PATCH 32/48] objtool: Add is_cold_func() helper

[Peter Zijlstra]

On Wed, Apr 22, 2026 at 09:04:00PM -0700, Josh Poimboeuf wrote:

> diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> index 00c2389f345f..8a6e1338af97 100644
> --- a/tools/objtool/elf.c
> +++ b/tools/objtool/elf.c
> @@ -586,8 +586,11 @@ static int elf_add_symbol(struct elf *elf, struct symbol *sym)
>  	if (strstarts(sym->name, ".klp.sym"))
>  		sym->klp = 1;
>  
> +	sym->pfunc = sym->cfunc = sym;
> +
>  	if (!sym->klp && !is_sec_sym(sym) && strstr(sym->name, ".cold")) {
> -		sym->cold = 1;
> +		/* Tell read_symbols() this is a cold subfunction */
> +		sym->pfunc = NULL;
>  
>  		/*
>  		 * Clang doesn't mark cold subfunctions as STT_FUNC, which
> @@ -596,8 +599,6 @@ static int elf_add_symbol(struct elf *elf, struct symbol *sym)
>  		sym->type = STT_FUNC;
>  	}
>  
> -	sym->pfunc = sym->cfunc = sym;
> -
>  	return 0;
>  }

So now the cold subfunction has a NULL parent-function and a
child-function that points to the parent?

I'm confused.

Re: [PATCH 34/48] objtool: Consolidate file decoding into decode_file()

[Peter Zijlstra]

On Wed, Apr 22, 2026 at 09:04:02PM -0700, Josh Poimboeuf wrote:
> decode_sections() relies on CFI and cfi_hash initialization done
> separately in check(), making it unusable outside of check().
> 
> Consolidate the initialization into decode_sections() and rename it to
> decode_file(), and make it global along with free_insns() and
> insn_reloc() for use by other objtool components -- namely, the checksum
> code which will be moving to another file.
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Re: [PATCH 43/48] objtool: Add insn_sym() helper

[Peter Zijlstra]

On Wed, Apr 22, 2026 at 09:04:11PM -0700, Josh Poimboeuf wrote:
> Alternative replacement instructions awkwardly have insn->sym set to the
> function they get patched to rather than the symbol (or rather lack
> thereof) they belong to in the file.
> 
> This makes it difficult to know where a given instruction actually
> lives.
> 
> Add a new insn_sym() helper which preserves the existing semantic of
> insn->sym.  Rename insn->sym to insn->_sym, which contains the actual
> ELF binary symbol (or NULL, for alternative replacements) an instruction
> lives in.
> 
> The private insn->_sym value will be needed for a subsequent patch.
> 

> +/*
> + * Return the symbol associated with an instruction.  For alternative
> + * replacements, return the symbol of the original code being replaced rather
> + * than NULL.  insn->_sym reflects the actual location in the ELF file.
> + */
> +static inline struct symbol *insn_sym(struct instruction *insn)
> +{
> +	struct symbol *sym = insn->_sym;
> +
> +	if (!sym && insn->alt_group && insn->alt_group->orig_group)
> +		sym = insn->alt_group->orig_group->first_insn->_sym;
> +
> +	return sym;
> +}

That is a bit of a deref fest -- this does not affect performance
negatively?

Re: [PATCH 45/48] x86/Kconfig: Enable CONFIG_PREFIX_SYMBOLS for FineIBT

[Peter Zijlstra]

On Wed, Apr 22, 2026 at 09:04:13PM -0700, Josh Poimboeuf wrote:
> PREFIX_SYMBOLS has a !CFI dependency because the compiler already
> generates __cfi_ prefix symbols for kCFI builds, so objtool-generated
> __pfx_ symbols were considered redundant.
> 
> However, the __cfi_ symbols only cover the 5-byte kCFI type hash.  With
> FUNCTION_CALL_PADDING, there are also 11 bytes of NOP padding between
> the hash and the function entry which have no symbol to claim them.

If you force the function alignment to 64 bytes, the prefix will also be
64bytes, rather than the normal 16.

> The NOPs can be rewritten with call depth tracking thunks at runtime.
> Without a symbol, unwinders and other tools that symbolize code
> locations misattribute those bytes.
> 
> Remove the !CFI guard so objtool creates __pfx_ symbols for all
> CALL_PADDING configs, covering the full padding area regardless of
> whether there's also a __cfi_ symbol.

Egads, that a ton of symbols :/ Does it not make sense to 'fix' up the
__cfi_ symbols to cover the whole prefix?

Re: [PATCH 47/48] objtool: Improve and simplify prefix symbol detection

[Peter Zijlstra]

On Wed, Apr 22, 2026 at 09:04:15PM -0700, Josh Poimboeuf wrote:
> Only create prefix symbols for functions that have
> __patchable_function_entries entries, since those are the only functions
> where prefix NOPs are intentional.

__CFI_TYPE() as used in SYM_TYPED_ENTRY() will also generate the NOPs
but will not have __patchable_function_entries, because ASM not
compiler.

Re: [PATCH 32/48] objtool: Add is_cold_func() helper

[Josh Poimboeuf]

On Thu, Apr 23, 2026 at 10:38:49AM +0200, Peter Zijlstra wrote:
> On Wed, Apr 22, 2026 at 09:04:00PM -0700, Josh Poimboeuf wrote:
> 
> > diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> > index 00c2389f345f..8a6e1338af97 100644
> > --- a/tools/objtool/elf.c
> > +++ b/tools/objtool/elf.c
> > @@ -586,8 +586,11 @@ static int elf_add_symbol(struct elf *elf, struct symbol *sym)
> >  	if (strstarts(sym->name, ".klp.sym"))
> >  		sym->klp = 1;
> >  
> > +	sym->pfunc = sym->cfunc = sym;
> > +
> >  	if (!sym->klp && !is_sec_sym(sym) && strstr(sym->name, ".cold")) {
> > -		sym->cold = 1;
> > +		/* Tell read_symbols() this is a cold subfunction */
> > +		sym->pfunc = NULL;
> >  
> >  		/*
> >  		 * Clang doesn't mark cold subfunctions as STT_FUNC, which
> > @@ -596,8 +599,6 @@ static int elf_add_symbol(struct elf *elf, struct symbol *sym)
> >  		sym->type = STT_FUNC;
> >  	}
> >  
> > -	sym->pfunc = sym->cfunc = sym;
> > -
> >  	return 0;
> >  }
> 
> So now the cold subfunction has a NULL parent-function and a
> child-function that points to the parent?
> 
> I'm confused.

It's a bit clunky.  As the comment implies, 'sym->pfunc = NULL' is a
signal to it caller read_symbols() that this is a .cold function.  Then,
after all the symbols have been added, read_symbols() goes and finds the
parent.

I think I did it this way because klp-diff.c calls elf_add_symbol() (via
elf_create_symbol()) and later needs to call is_cold_func() on it.  In
that case, even though the parent isn't set, it still works because
is_cold_func() returns true for sym->pfunc != sym;

-- 
Josh

Re: [PATCH 32/48] objtool: Add is_cold_func() helper

[Peter Zijlstra]

On Thu, Apr 23, 2026 at 08:12:07AM -0700, Josh Poimboeuf wrote:
> On Thu, Apr 23, 2026 at 10:38:49AM +0200, Peter Zijlstra wrote:
> > On Wed, Apr 22, 2026 at 09:04:00PM -0700, Josh Poimboeuf wrote:
> > 
> > > diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> > > index 00c2389f345f..8a6e1338af97 100644
> > > --- a/tools/objtool/elf.c
> > > +++ b/tools/objtool/elf.c
> > > @@ -586,8 +586,11 @@ static int elf_add_symbol(struct elf *elf, struct symbol *sym)
> > >  	if (strstarts(sym->name, ".klp.sym"))
> > >  		sym->klp = 1;
> > >  
> > > +	sym->pfunc = sym->cfunc = sym;
> > > +
> > >  	if (!sym->klp && !is_sec_sym(sym) && strstr(sym->name, ".cold")) {
> > > -		sym->cold = 1;
> > > +		/* Tell read_symbols() this is a cold subfunction */
> > > +		sym->pfunc = NULL;
> > >  
> > >  		/*
> > >  		 * Clang doesn't mark cold subfunctions as STT_FUNC, which
> > > @@ -596,8 +599,6 @@ static int elf_add_symbol(struct elf *elf, struct symbol *sym)
> > >  		sym->type = STT_FUNC;
> > >  	}
> > >  
> > > -	sym->pfunc = sym->cfunc = sym;
> > > -
> > >  	return 0;
> > >  }
> > 
> > So now the cold subfunction has a NULL parent-function and a
> > child-function that points to the parent?
> > 
> > I'm confused.
> 
> It's a bit clunky.  As the comment implies, 'sym->pfunc = NULL' is a
> signal to it caller read_symbols() that this is a .cold function.  Then,
> after all the symbols have been added, read_symbols() goes and finds the
> parent.
> 
> I think I did it this way because klp-diff.c calls elf_add_symbol() (via
> elf_create_symbol()) and later needs to call is_cold_func() on it.  In
> that case, even though the parent isn't set, it still works because
> is_cold_func() returns true for sym->pfunc != sym;

I'm thinking this needs more comments if it stays like this. Is most
confusing.

Re: [PATCH 43/48] objtool: Add insn_sym() helper

[Josh Poimboeuf]

On Thu, Apr 23, 2026 at 10:45:37AM +0200, Peter Zijlstra wrote:
> On Wed, Apr 22, 2026 at 09:04:11PM -0700, Josh Poimboeuf wrote:
> > Alternative replacement instructions awkwardly have insn->sym set to the
> > function they get patched to rather than the symbol (or rather lack
> > thereof) they belong to in the file.
> > 
> > This makes it difficult to know where a given instruction actually
> > lives.
> > 
> > Add a new insn_sym() helper which preserves the existing semantic of
> > insn->sym.  Rename insn->sym to insn->_sym, which contains the actual
> > ELF binary symbol (or NULL, for alternative replacements) an instruction
> > lives in.
> > 
> > The private insn->_sym value will be needed for a subsequent patch.
> > 
> 
> > +/*
> > + * Return the symbol associated with an instruction.  For alternative
> > + * replacements, return the symbol of the original code being replaced rather
> > + * than NULL.  insn->_sym reflects the actual location in the ELF file.
> > + */
> > +static inline struct symbol *insn_sym(struct instruction *insn)
> > +{
> > +	struct symbol *sym = insn->_sym;
> > +
> > +	if (!sym && insn->alt_group && insn->alt_group->orig_group)
> > +		sym = insn->alt_group->orig_group->first_insn->_sym;
> > +
> > +	return sym;
> > +}
> 
> That is a bit of a deref fest -- this does not affect performance
> negatively?

Ha, the "deref fest" only happens for alternative replacement
instructions, so presumably not?

-- 
Josh

Re: [PATCH 45/48] x86/Kconfig: Enable CONFIG_PREFIX_SYMBOLS for FineIBT

[Josh Poimboeuf]

On Thu, Apr 23, 2026 at 10:47:58AM +0200, Peter Zijlstra wrote:
> On Wed, Apr 22, 2026 at 09:04:13PM -0700, Josh Poimboeuf wrote:
> > PREFIX_SYMBOLS has a !CFI dependency because the compiler already
> > generates __cfi_ prefix symbols for kCFI builds, so objtool-generated
> > __pfx_ symbols were considered redundant.
> > 
> > However, the __cfi_ symbols only cover the 5-byte kCFI type hash.  With
> > FUNCTION_CALL_PADDING, there are also 11 bytes of NOP padding between
> > the hash and the function entry which have no symbol to claim them.
> 
> If you force the function alignment to 64 bytes, the prefix will also be
> 64bytes, rather than the normal 16.

Sorry, how do you get 64 here?

> > The NOPs can be rewritten with call depth tracking thunks at runtime.
> > Without a symbol, unwinders and other tools that symbolize code
> > locations misattribute those bytes.
> > 
> > Remove the !CFI guard so objtool creates __pfx_ symbols for all
> > CALL_PADDING configs, covering the full padding area regardless of
> > whether there's also a __cfi_ symbol.
> 
> Egads, that a ton of symbols :/ Does it not make sense to 'fix' up the
> __cfi_ symbols to cover the whole prefix?

Yeah, I suppose that would be better, via objtool I presume.

-- 
Josh

Re: [PATCH 45/48] x86/Kconfig: Enable CONFIG_PREFIX_SYMBOLS for FineIBT

[Peter Zijlstra]

On Thu, Apr 23, 2026 at 08:16:08AM -0700, Josh Poimboeuf wrote:
> On Thu, Apr 23, 2026 at 10:47:58AM +0200, Peter Zijlstra wrote:
> > On Wed, Apr 22, 2026 at 09:04:13PM -0700, Josh Poimboeuf wrote:
> > > PREFIX_SYMBOLS has a !CFI dependency because the compiler already
> > > generates __cfi_ prefix symbols for kCFI builds, so objtool-generated
> > > __pfx_ symbols were considered redundant.
> > > 
> > > However, the __cfi_ symbols only cover the 5-byte kCFI type hash.  With
> > > FUNCTION_CALL_PADDING, there are also 11 bytes of NOP padding between
> > > the hash and the function entry which have no symbol to claim them.
> > 
> > If you force the function alignment to 64 bytes, the prefix will also be
> > 64bytes, rather than the normal 16.
> 
> Sorry, how do you get 64 here?

DEBUG_FORCE_FUNCTION_ALIGNMENT_64B=y

> > > The NOPs can be rewritten with call depth tracking thunks at runtime.
> > > Without a symbol, unwinders and other tools that symbolize code
> > > locations misattribute those bytes.
> > > 
> > > Remove the !CFI guard so objtool creates __pfx_ symbols for all
> > > CALL_PADDING configs, covering the full padding area regardless of
> > > whether there's also a __cfi_ symbol.
> > 
> > Egads, that a ton of symbols :/ Does it not make sense to 'fix' up the
> > __cfi_ symbols to cover the whole prefix?
> 
> Yeah, I suppose that would be better, via objtool I presume.

Yup.

Re: [PATCH 47/48] objtool: Improve and simplify prefix symbol detection

[Josh Poimboeuf]

On Thu, Apr 23, 2026 at 10:55:20AM +0200, Peter Zijlstra wrote:
> On Wed, Apr 22, 2026 at 09:04:15PM -0700, Josh Poimboeuf wrote:
> > Only create prefix symbols for functions that have
> > __patchable_function_entries entries, since those are the only functions
> > where prefix NOPs are intentional.
> 
> __CFI_TYPE() as used in SYM_TYPED_ENTRY() will also generate the NOPs
> but will not have __patchable_function_entries, because ASM not
> compiler.

Hm, but those already have __cfi_ symbols, no?

-- 
Josh

Re: [PATCH 47/48] objtool: Improve and simplify prefix symbol detection

[Peter Zijlstra]

On Thu, Apr 23, 2026 at 08:19:57AM -0700, Josh Poimboeuf wrote:
> On Thu, Apr 23, 2026 at 10:55:20AM +0200, Peter Zijlstra wrote:
> > On Wed, Apr 22, 2026 at 09:04:15PM -0700, Josh Poimboeuf wrote:
> > > Only create prefix symbols for functions that have
> > > __patchable_function_entries entries, since those are the only functions
> > > where prefix NOPs are intentional.
> > 
> > __CFI_TYPE() as used in SYM_TYPED_ENTRY() will also generate the NOPs
> > but will not have __patchable_function_entries, because ASM not
> > compiler.
> 
> Hm, but those already have __cfi_ symbols, no?

Yes, but you said those were 'short' -- but fair, I did not check if the
asm stub generated symbols of the correct length.

Re: [PATCH 45/48] x86/Kconfig: Enable CONFIG_PREFIX_SYMBOLS for FineIBT

[Josh Poimboeuf]

On Thu, Apr 23, 2026 at 05:19:25PM +0200, Peter Zijlstra wrote:
> On Thu, Apr 23, 2026 at 08:16:08AM -0700, Josh Poimboeuf wrote:
> > On Thu, Apr 23, 2026 at 10:47:58AM +0200, Peter Zijlstra wrote:
> > > On Wed, Apr 22, 2026 at 09:04:13PM -0700, Josh Poimboeuf wrote:
> > > > PREFIX_SYMBOLS has a !CFI dependency because the compiler already
> > > > generates __cfi_ prefix symbols for kCFI builds, so objtool-generated
> > > > __pfx_ symbols were considered redundant.
> > > > 
> > > > However, the __cfi_ symbols only cover the 5-byte kCFI type hash.  With
> > > > FUNCTION_CALL_PADDING, there are also 11 bytes of NOP padding between
> > > > the hash and the function entry which have no symbol to claim them.
> > > 
> > > If you force the function alignment to 64 bytes, the prefix will also be
> > > 64bytes, rather than the normal 16.
> > 
> > Sorry, how do you get 64 here?
> 
> DEBUG_FORCE_FUNCTION_ALIGNMENT_64B=y

Ok, so in that case it would be 5-byte cfi symbol and 59-byte NOP gap.
Or a 64-byte pfx for the !CFI case.

> > > > The NOPs can be rewritten with call depth tracking thunks at runtime.
> > > > Without a symbol, unwinders and other tools that symbolize code
> > > > locations misattribute those bytes.
> > > > 
> > > > Remove the !CFI guard so objtool creates __pfx_ symbols for all
> > > > CALL_PADDING configs, covering the full padding area regardless of
> > > > whether there's also a __cfi_ symbol.
> > > 
> > > Egads, that a ton of symbols :/ Does it not make sense to 'fix' up the
> > > __cfi_ symbols to cover the whole prefix?
> > 
> > Yeah, I suppose that would be better, via objtool I presume.
> 
> Yup.

-- 
Josh

Re: [PATCH 45/48] x86/Kconfig: Enable CONFIG_PREFIX_SYMBOLS for FineIBT

[Peter Zijlstra]

On Thu, Apr 23, 2026 at 09:23:12AM -0700, Josh Poimboeuf wrote:

> > > Sorry, how do you get 64 here?
> > 
> > DEBUG_FORCE_FUNCTION_ALIGNMENT_64B=y
> 
> Ok, so in that case it would be 5-byte cfi symbol and 59-byte NOP gap.
> Or a 64-byte pfx for the !CFI case.

Just so.

Re: [PATCH 17/48] objtool: Fix reloc hash collision in find_reloc_by_dest_range()

[Josh Poimboeuf]

On Thu, Apr 23, 2026 at 10:32:31AM +0200, Peter Zijlstra wrote:
> On Wed, Apr 22, 2026 at 09:03:45PM -0700, Josh Poimboeuf wrote:
> > In find_reloc_by_dest_range(), hash collisions can cause a high-offset
> > relocation to appear when probing a low-offset hash bucket.
> > 
> > Only return early when the best match found so far genuinely belongs to
> > the current bucket (its offset is within the bucket's stride range).
> > Otherwise, continue scanning later buckets which may contain
> > lower-offset matches.
> 
> Maybe mention (and or add a comment to the function) that in case of
> multiple matches in the given range, it will return the lowest address
> one.
> 
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index c4cb371e72b2..af2841b8e095 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -347,8 +347,9 @@ void iterate_sym_by_name(const struct elf *elf, const char *name,
 	}
 }
 
+/* If there are multiple matches, return the first one in the range */
 struct reloc *find_reloc_by_dest_range(const struct elf *elf, struct section *sec,
-				     unsigned long offset, unsigned int len)
+				       unsigned long offset, unsigned int len)
 {
 	struct reloc *reloc, *r = NULL;
 	struct section *rsec;

Re: [PATCH 01/48] objtool/klp: Fix is_uncorrelated_static_local() for Clang

[Song Liu]

On Wed, Apr 22, 2026 at 9:04 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> From: Joe Lawrence <joe.lawrence@redhat.com>
>
> For naming function-local static locals, GCC uses <var>.<id>, e.g.
> __already_done.15, while Clang uses <func>.<var> with optional .<id>,
> e.g. create_worker.__already_done.111
>
> The existing is_uncorrelated_static_local() check only matches the GCC
> convention where the variable name is a prefix.  Handle both cases by
> checking for a prefix match (GCC) and by checking after the first dot
> separator (Clang).
>
> Fixes: dd590d4d57eb ("objtool/klp: Introduce klp diff subcommand for diffing object files")
> 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 02/48] objtool/klp: Fix .data..once static local non-correlation

[Song Liu]

On Wed, Apr 22, 2026 at 9:04 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> While there was once a section named .data.once, it has since been
> renamed to .data..once with commit dbefa1f31a91 ("Rename .data.once to
> .data..once to fix resetting WARN*_ONCE").  Fix it.
>
> 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>

Nitpick: shall we match both ".data.once" and ".data..once", so that whoever
backports klp-build to older kernels will not have a surprise.

> ---
>  tools/objtool/klp-diff.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
> index b1b068e9b4c7..cb26c1c92a74 100644
> --- a/tools/objtool/klp-diff.c
> +++ b/tools/objtool/klp-diff.c
> @@ -257,7 +257,8 @@ static bool is_uncorrelated_static_local(struct symbol *sym)
>         if (!is_object_sym(sym) || !is_local_sym(sym))
>                 return false;
>
> -       if (!strcmp(sym->sec->name, ".data.once"))
> +       /* WARN_ONCE, etc */
> +       if (!strcmp(sym->sec->name, ".data..once"))
>                 return true;
>
>         dot = strchr(sym->name, '.');
> --
> 2.53.0
>
>

Re: [PATCH 04/48] objtool/klp: Ignore __UNIQUE_ID_*() PCI stub functions

[Song Liu]

On Wed, Apr 22, 2026 at 9:04 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> With Clang LTO enabled, DECLARE_PCI_FIXUP_SECTION() uses __UNIQUE_ID()
> to generate uniquely named wrapper functions, which are being reported
> as new functions and unnecessarily included in the patch module:
>
>   vmlinux.o: new function: __UNIQUE_ID_quirk_f0_vpd_link_661
>
> These stub functions only exist to make the compiler happy.  Just ignore
> them along with any other dont_correlate() symbols.  Note that
> dont_correlate() already includes prefix functions.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>

The actual change appears to be much bigger than the subject line.
Maybe rephrase it a bit?

> ---
>  tools/objtool/klp-diff.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
> index 36753eeba58c..ea9ccf8c4ea9 100644
> --- a/tools/objtool/klp-diff.c
> +++ b/tools/objtool/klp-diff.c
> @@ -786,7 +786,7 @@ static int mark_changed_functions(struct elfs *e)
>
>         /* Find changed functions */
>         for_each_sym(e->orig, sym_orig) {
> -               if (!is_func_sym(sym_orig) || is_prefix_func(sym_orig))
> +               if (!is_func_sym(sym_orig) || dont_correlate(sym_orig))
>                         continue;
>
>                 patched_sym = sym_orig->twin;
> @@ -802,7 +802,7 @@ static int mark_changed_functions(struct elfs *e)
>
>         /* Find added functions and print them */
>         for_each_sym(e->patched, patched_sym) {
> -               if (!is_func_sym(patched_sym) || is_prefix_func(patched_sym))
> +               if (!is_func_sym(patched_sym) || dont_correlate(patched_sym))
>                         continue;
>
>                 if (!patched_sym->twin) {
> --
> 2.53.0
>

Re: [PATCH 32/48] objtool: Add is_cold_func() helper

[Josh Poimboeuf]

On Thu, Apr 23, 2026 at 05:14:05PM +0200, Peter Zijlstra wrote:
> On Thu, Apr 23, 2026 at 08:12:07AM -0700, Josh Poimboeuf wrote:
> > On Thu, Apr 23, 2026 at 10:38:49AM +0200, Peter Zijlstra wrote:
> > > On Wed, Apr 22, 2026 at 09:04:00PM -0700, Josh Poimboeuf wrote:
> > > 
> > > > diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> > > > index 00c2389f345f..8a6e1338af97 100644
> > > > --- a/tools/objtool/elf.c
> > > > +++ b/tools/objtool/elf.c
> > > > @@ -586,8 +586,11 @@ static int elf_add_symbol(struct elf *elf, struct symbol *sym)
> > > >  	if (strstarts(sym->name, ".klp.sym"))
> > > >  		sym->klp = 1;
> > > >  
> > > > +	sym->pfunc = sym->cfunc = sym;
> > > > +
> > > >  	if (!sym->klp && !is_sec_sym(sym) && strstr(sym->name, ".cold")) {
> > > > -		sym->cold = 1;
> > > > +		/* Tell read_symbols() this is a cold subfunction */
> > > > +		sym->pfunc = NULL;
> > > >  
> > > >  		/*
> > > >  		 * Clang doesn't mark cold subfunctions as STT_FUNC, which
> > > > @@ -596,8 +599,6 @@ static int elf_add_symbol(struct elf *elf, struct symbol *sym)
> > > >  		sym->type = STT_FUNC;
> > > >  	}
> > > >  
> > > > -	sym->pfunc = sym->cfunc = sym;
> > > > -
> > > >  	return 0;
> > > >  }
> > > 
> > > So now the cold subfunction has a NULL parent-function and a
> > > child-function that points to the parent?
> > > 
> > > I'm confused.
> > 
> > It's a bit clunky.  As the comment implies, 'sym->pfunc = NULL' is a
> > signal to it caller read_symbols() that this is a .cold function.  Then,
> > after all the symbols have been added, read_symbols() goes and finds the
> > parent.
> > 
> > I think I did it this way because klp-diff.c calls elf_add_symbol() (via
> > elf_create_symbol()) and later needs to call is_cold_func() on it.  In
> > that case, even though the parent isn't set, it still works because
> > is_cold_func() returns true for sym->pfunc != sym;
> 
> I'm thinking this needs more comments if it stays like this. Is most
> confusing.

So we can just keep the 'cold' bit and keep the confusingness at its
current level :-)

From: Josh Poimboeuf <jpoimboe@kernel.org>
Subject: [PATCH] objtool: Add is_cold_func() helper

Add an is_cold_func() helper.  No functional changes intended.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/check.c               | 6 +++---
 tools/objtool/include/objtool/elf.h | 5 +++++
 tools/objtool/klp-diff.c            | 3 ++-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 4c18d6e7f6c3..4ed27c53c718 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2614,7 +2614,7 @@ static void mark_holes(struct objtool_file *file)
 		if (insn->jump_dest) {
 			struct symbol *dest_func = insn_func(insn->jump_dest);
 
-			if (dest_func && dest_func->cold)
+			if (dest_func && is_cold_func(dest_func))
 				dest_func->ignore = true;
 		}
 	}
@@ -4426,8 +4426,8 @@ static int create_prefix_symbol(struct objtool_file *file, struct symbol *func)
 	char name[SYM_NAME_LEN];
 	struct cfi_state *cfi;
 
-	if (!is_func_sym(func) || is_prefix_func(func) ||
-	    func->cold || func->static_call_tramp)
+	if (!is_func_sym(func) || is_prefix_func(func) || is_cold_func(func) ||
+	    func->static_call_tramp)
 		return 0;
 
 	if ((strlen(func->name) + sizeof("__pfx_") > SYM_NAME_LEN)) {
diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
index 3abe4cbc584c..ad0cc57a9d5f 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -289,6 +289,11 @@ static inline bool is_prefix_func(struct symbol *sym)
 	return sym->prefix;
 }
 
+static inline bool is_cold_func(struct symbol *sym)
+{
+	return sym->cold;
+}
+
 static inline bool is_reloc_sec(struct section *sec)
 {
 	return sec->sh.sh_type == SHT_RELA || sec->sh.sh_type == SHT_REL;
diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 1951a8b2df44..266f0d2ba4fe 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -1718,7 +1718,8 @@ static int create_klp_sections(struct elfs *e)
 		unsigned long sympos;
 		void *func_data;
 
-		if (!is_func_sym(sym) || sym->cold || !sym->clone || !sym->clone->changed)
+		if (!is_func_sym(sym) || is_cold_func(sym) ||
+		    !sym->clone || !sym->clone->changed)
 			continue;
 
 		/* allocate klp_func_ext */
-- 
2.53.0

Re: [PATCH 04/48] objtool/klp: Ignore __UNIQUE_ID_*() PCI stub functions

[Josh Poimboeuf]

On Thu, Apr 23, 2026 at 12:05:03PM -0700, Song Liu wrote:
> On Wed, Apr 22, 2026 at 9:04 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> > With Clang LTO enabled, DECLARE_PCI_FIXUP_SECTION() uses __UNIQUE_ID()
> > to generate uniquely named wrapper functions, which are being reported
> > as new functions and unnecessarily included in the patch module:
> >
> >   vmlinux.o: new function: __UNIQUE_ID_quirk_f0_vpd_link_661
> >
> > These stub functions only exist to make the compiler happy.  Just ignore
> > them along with any other dont_correlate() symbols.  Note that
> > dont_correlate() already includes prefix functions.
> >
> > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> 
> The actual change appears to be much bigger than the subject line.
> Maybe rephrase it a bit?

Hm, in fact this is a relic from a previous iteration of the patches: it
longer fixes what it claims to fix, as __UNIQUE_ID_ (other than
__ADDRESSABLE()) are now correlated.  The claimed issue actually gets
fixed later by the rewriting of the correlation algorithm.

That said, I still think the below is needed, I just need to rewrite the
commit log.

> 
> > ---
> >  tools/objtool/klp-diff.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
> > index 36753eeba58c..ea9ccf8c4ea9 100644
> > --- a/tools/objtool/klp-diff.c
> > +++ b/tools/objtool/klp-diff.c
> > @@ -786,7 +786,7 @@ static int mark_changed_functions(struct elfs *e)
> >
> >         /* Find changed functions */
> >         for_each_sym(e->orig, sym_orig) {
> > -               if (!is_func_sym(sym_orig) || is_prefix_func(sym_orig))
> > +               if (!is_func_sym(sym_orig) || dont_correlate(sym_orig))
> >                         continue;
> >
> >                 patched_sym = sym_orig->twin;
> > @@ -802,7 +802,7 @@ static int mark_changed_functions(struct elfs *e)
> >
> >         /* Find added functions and print them */
> >         for_each_sym(e->patched, patched_sym) {
> > -               if (!is_func_sym(patched_sym) || is_prefix_func(patched_sym))
> > +               if (!is_func_sym(patched_sym) || dont_correlate(patched_sym))
> >                         continue;
> >
> >                 if (!patched_sym->twin) {
> > --
> > 2.53.0
> >

-- 
Josh

Re: [PATCH 04/48] objtool/klp: Ignore __UNIQUE_ID_*() PCI stub functions

[Song Liu]

On Thu, Apr 23, 2026 at 12:31 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Thu, Apr 23, 2026 at 12:05:03PM -0700, Song Liu wrote:
> > On Wed, Apr 22, 2026 at 9:04 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > >
> > > With Clang LTO enabled, DECLARE_PCI_FIXUP_SECTION() uses __UNIQUE_ID()
> > > to generate uniquely named wrapper functions, which are being reported
> > > as new functions and unnecessarily included in the patch module:
> > >
> > >   vmlinux.o: new function: __UNIQUE_ID_quirk_f0_vpd_link_661
> > >
> > > These stub functions only exist to make the compiler happy.  Just ignore
> > > them along with any other dont_correlate() symbols.  Note that
> > > dont_correlate() already includes prefix functions.
> > >
> > > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> >
> > The actual change appears to be much bigger than the subject line.
> > Maybe rephrase it a bit?
>
> Hm, in fact this is a relic from a previous iteration of the patches: it
> longer fixes what it claims to fix, as __UNIQUE_ID_ (other than
> __ADDRESSABLE()) are now correlated.  The claimed issue actually gets
> fixed later by the rewriting of the correlation algorithm.
>
> That said, I still think the below is needed, I just need to rewrite the
> commit log.

Agreed.

Thanks,
Song

Re: [PATCH 45/48] x86/Kconfig: Enable CONFIG_PREFIX_SYMBOLS for FineIBT

[Josh Poimboeuf]

On Thu, Apr 23, 2026 at 09:23:12AM -0700, Josh Poimboeuf wrote:
> On Thu, Apr 23, 2026 at 05:19:25PM +0200, Peter Zijlstra wrote:
> > On Thu, Apr 23, 2026 at 08:16:08AM -0700, Josh Poimboeuf wrote:
> > > On Thu, Apr 23, 2026 at 10:47:58AM +0200, Peter Zijlstra wrote:
> > > > On Wed, Apr 22, 2026 at 09:04:13PM -0700, Josh Poimboeuf wrote:
> > > > > PREFIX_SYMBOLS has a !CFI dependency because the compiler already
> > > > > generates __cfi_ prefix symbols for kCFI builds, so objtool-generated
> > > > > __pfx_ symbols were considered redundant.
> > > > > 
> > > > > However, the __cfi_ symbols only cover the 5-byte kCFI type hash.  With
> > > > > FUNCTION_CALL_PADDING, there are also 11 bytes of NOP padding between
> > > > > the hash and the function entry which have no symbol to claim them.
> > > > 
> > > > If you force the function alignment to 64 bytes, the prefix will also be
> > > > 64bytes, rather than the normal 16.
> > > 
> > > Sorry, how do you get 64 here?
> > 
> > DEBUG_FORCE_FUNCTION_ALIGNMENT_64B=y
> 
> Ok, so in that case it would be 5-byte cfi symbol and 59-byte NOP gap.
> Or a 64-byte pfx for the !CFI case.
> 
> > > > > The NOPs can be rewritten with call depth tracking thunks at runtime.
> > > > > Without a symbol, unwinders and other tools that symbolize code
> > > > > locations misattribute those bytes.
> > > > > 
> > > > > Remove the !CFI guard so objtool creates __pfx_ symbols for all
> > > > > CALL_PADDING configs, covering the full padding area regardless of
> > > > > whether there's also a __cfi_ symbol.
> > > > 
> > > > Egads, that a ton of symbols :/ Does it not make sense to 'fix' up the
> > > > __cfi_ symbols to cover the whole prefix?
> > > 
> > > Yeah, I suppose that would be better, via objtool I presume.
> > 
> > Yup.

From: Josh Poimboeuf <jpoimboe@kernel.org>
Subject: [PATCH] objtool: Grow __cfi_* symbols for FineIBT

For FineIBT, the __cfi_ symbols only cover the 5-byte kCFI type hash.
After that there also N bytes of NOP padding between the hash and the
function entry which aren't associated with any symbol.

The NOPs can be replaced with actual code at runtime.  Without a symbol,
unwinders, objtool, and other tools have no way of knowing where those
bytes belong.

Grow the existing __cfi_* symbols to fill that gap.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 scripts/Makefile.lib                |  2 +-
 tools/objtool/check.c               | 13 ++++++++++++-
 tools/objtool/elf.c                 | 20 ++++++++++++++++++++
 tools/objtool/include/objtool/elf.h |  1 +
 4 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 0718e39cedda..baaf9f6c6bb5 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -187,7 +187,7 @@ objtool-args-$(CONFIG_HAVE_JUMP_LABEL_HACK)		+= --hacks=jump_label
 objtool-args-$(CONFIG_HAVE_NOINSTR_HACK)		+= --hacks=noinstr
 objtool-args-$(CONFIG_MITIGATION_CALL_DEPTH_TRACKING)	+= --hacks=skylake
 objtool-args-$(CONFIG_X86_KERNEL_IBT)			+= --ibt
-objtool-args-$(CONFIG_FINEIBT)				+= --cfi
+objtool-args-$(CONFIG_FINEIBT)				+= --cfi --prefix=$(CONFIG_FUNCTION_PADDING_BYTES)
 objtool-args-$(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL)	+= --mcount
 ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
 objtool-args-$(CONFIG_HAVE_OBJTOOL_NOP_MCOUNT)		+= --mnop
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 410061aeed26..fb24fd284e09 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -923,6 +923,17 @@ static int create_cfi_sections(struct objtool_file *file)
 			return -1;
 
 		idx++;
+
+		/*
+		 * Grow the __cfi_ symbol to fill the NOP gap between the
+		 * 'mov <hash>, %rax' and the start of the function.
+		 */
+		if (sym->len == 5) {
+			sym->len += opts.prefix;
+			sym->sym.st_size = sym->len;
+			if (elf_write_symbol(file->elf, sym))
+				return -1;
+		}
 	}
 
 	return 0;
@@ -4927,7 +4938,7 @@ int check(struct objtool_file *file)
 			goto out;
 	}
 
-	if (opts.prefix) {
+	if (opts.prefix && !opts.cfi) {
 		ret = create_prefix_symbols(file);
 		if (ret)
 			goto out;
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 2ca1151de815..ede87dd9644c 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -983,6 +983,26 @@ struct symbol *elf_create_symbol(struct elf *elf, const char *name,
 	return sym;
 }
 
+int elf_write_symbol(struct elf *elf, struct symbol *sym)
+{
+	struct section *symtab, *symtab_shndx;
+
+	symtab = find_section_by_name(elf, ".symtab");
+	if (!symtab) {
+		ERROR("no .symtab");
+		return -1;
+	}
+
+	symtab_shndx = find_section_by_name(elf, ".symtab_shndx");
+
+	if (elf_update_symbol(elf, symtab, symtab_shndx, sym))
+		return -1;
+
+	mark_sec_changed(elf, symtab, true);
+
+	return 0;
+}
+
 struct symbol *elf_create_section_symbol(struct elf *elf, struct section *sec)
 {
 	struct symbol *sym = calloc(1, sizeof(*sym));
diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
index 0fd1a9b563e9..4c8a67a68063 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -199,6 +199,7 @@ struct reloc *elf_init_reloc_data_sym(struct elf *elf, struct section *sec,
 				      struct symbol *sym,
 				      s64 addend);
 
+int elf_write_symbol(struct elf *elf, struct symbol *sym);
 int elf_write_insn(struct elf *elf, struct section *sec, unsigned long offset,
 		   unsigned int len, const char *insn);
 
-- 
2.53.0

Re: [PATCH 02/48] objtool/klp: Fix .data..once static local non-correlation

[Josh Poimboeuf]

On Thu, Apr 23, 2026 at 11:54:39AM -0700, Song Liu wrote:
> On Wed, Apr 22, 2026 at 9:04 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> > While there was once a section named .data.once, it has since been
> > renamed to .data..once with commit dbefa1f31a91 ("Rename .data.once to
> > .data..once to fix resetting WARN*_ONCE").  Fix it.
> >
> > 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>
> 
> Nitpick: shall we match both ".data.once" and ".data..once", so that whoever
> backports klp-build to older kernels will not have a surprise.

Hm, I'm a bit hesitant to do that.  One of the nice things about having
this code upstream is that we don't have to start collecting all the
cruft for old kernels.

-- 
Josh

Re: [PATCH 04/48] objtool/klp: Ignore __UNIQUE_ID_*() PCI stub functions

[Josh Poimboeuf]

On Thu, Apr 23, 2026 at 02:33:00PM -0700, Song Liu wrote:
> On Thu, Apr 23, 2026 at 12:31 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> > On Thu, Apr 23, 2026 at 12:05:03PM -0700, Song Liu wrote:
> > > On Wed, Apr 22, 2026 at 9:04 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > > >
> > > > With Clang LTO enabled, DECLARE_PCI_FIXUP_SECTION() uses __UNIQUE_ID()
> > > > to generate uniquely named wrapper functions, which are being reported
> > > > as new functions and unnecessarily included in the patch module:
> > > >
> > > >   vmlinux.o: new function: __UNIQUE_ID_quirk_f0_vpd_link_661
> > > >
> > > > These stub functions only exist to make the compiler happy.  Just ignore
> > > > them along with any other dont_correlate() symbols.  Note that
> > > > dont_correlate() already includes prefix functions.
> > > >
> > > > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> > >
> > > The actual change appears to be much bigger than the subject line.
> > > Maybe rephrase it a bit?
> >
> > Hm, in fact this is a relic from a previous iteration of the patches: it
> > longer fixes what it claims to fix, as __UNIQUE_ID_ (other than
> > __ADDRESSABLE()) are now correlated.  The claimed issue actually gets
> > fixed later by the rewriting of the correlation algorithm.
> >
> > That said, I still think the below is needed, I just need to rewrite the
> > commit log.
> 
> Agreed.

From: Josh Poimboeuf <jpoimboe@kernel.org>
Subject: [PATCH] objtool/klp: Don't report uncorrelated functions as new

Clang LTO uses __UNIQUE_ID() to generate some uniquely named wrapper
functions, like initstubs.  If they're uncorrelated, prevent them from
being reported as new functions and included unnecessarily.

Note that dont_correlate() already includes prefix functions, so prefix
functions are still being ignored here.

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

diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 36753eeba58c..ea9ccf8c4ea9 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -786,7 +786,7 @@ static int mark_changed_functions(struct elfs *e)
 
 	/* Find changed functions */
 	for_each_sym(e->orig, sym_orig) {
-		if (!is_func_sym(sym_orig) || is_prefix_func(sym_orig))
+		if (!is_func_sym(sym_orig) || dont_correlate(sym_orig))
 			continue;
 
 		patched_sym = sym_orig->twin;
@@ -802,7 +802,7 @@ static int mark_changed_functions(struct elfs *e)
 
 	/* Find added functions and print them */
 	for_each_sym(e->patched, patched_sym) {
-		if (!is_func_sym(patched_sym) || is_prefix_func(patched_sym))
+		if (!is_func_sym(patched_sym) || dont_correlate(patched_sym))
 			continue;
 
 		if (!patched_sym->twin) {
-- 
2.53.0

Re: [PATCH 02/48] objtool/klp: Fix .data..once static local non-correlation

[Song Liu]

On Thu, Apr 23, 2026 at 4:34 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Thu, Apr 23, 2026 at 11:54:39AM -0700, Song Liu wrote:
> > On Wed, Apr 22, 2026 at 9:04 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > >
> > > While there was once a section named .data.once, it has since been
> > > renamed to .data..once with commit dbefa1f31a91 ("Rename .data.once to
> > > .data..once to fix resetting WARN*_ONCE").  Fix it.
> > >
> > > 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>
> >
> > Nitpick: shall we match both ".data.once" and ".data..once", so that whoever
> > backports klp-build to older kernels will not have a surprise.
>
> Hm, I'm a bit hesitant to do that.  One of the nice things about having
> this code upstream is that we don't have to start collecting all the
> cruft for old kernels.

Agreed. Instead of matching both, we can probably cover this with a
test case.

Thanks,
Song

Re: [PATCH 04/48] objtool/klp: Ignore __UNIQUE_ID_*() PCI stub functions

[Song Liu]

On Thu, Apr 23, 2026 at 4:50 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Thu, Apr 23, 2026 at 02:33:00PM -0700, Song Liu wrote:
> > On Thu, Apr 23, 2026 at 12:31 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > >
> > > On Thu, Apr 23, 2026 at 12:05:03PM -0700, Song Liu wrote:
> > > > On Wed, Apr 22, 2026 at 9:04 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > > > >
> > > > > With Clang LTO enabled, DECLARE_PCI_FIXUP_SECTION() uses __UNIQUE_ID()
> > > > > to generate uniquely named wrapper functions, which are being reported
> > > > > as new functions and unnecessarily included in the patch module:
> > > > >
> > > > >   vmlinux.o: new function: __UNIQUE_ID_quirk_f0_vpd_link_661
> > > > >
> > > > > These stub functions only exist to make the compiler happy.  Just ignore
> > > > > them along with any other dont_correlate() symbols.  Note that
> > > > > dont_correlate() already includes prefix functions.
> > > > >
> > > > > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> > > >
> > > > The actual change appears to be much bigger than the subject line.
> > > > Maybe rephrase it a bit?
> > >
> > > Hm, in fact this is a relic from a previous iteration of the patches: it
> > > longer fixes what it claims to fix, as __UNIQUE_ID_ (other than
> > > __ADDRESSABLE()) are now correlated.  The claimed issue actually gets
> > > fixed later by the rewriting of the correlation algorithm.
> > >
> > > That said, I still think the below is needed, I just need to rewrite the
> > > commit log.
> >
> > Agreed.
>
> From: Josh Poimboeuf <jpoimboe@kernel.org>
> Subject: [PATCH] objtool/klp: Don't report uncorrelated functions as new
>
> Clang LTO uses __UNIQUE_ID() to generate some uniquely named wrapper
> functions, like initstubs.  If they're uncorrelated, prevent them from
> being reported as new functions and included unnecessarily.
>
> Note that dont_correlate() already includes prefix functions, so prefix
> functions are still being ignored here.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>

LGTM.

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

Re: [PATCH 45/48] x86/Kconfig: Enable CONFIG_PREFIX_SYMBOLS for FineIBT

[Josh Poimboeuf]

On Thu, Apr 23, 2026 at 04:30:47PM -0700, Josh Poimboeuf wrote:
> On Thu, Apr 23, 2026 at 09:23:12AM -0700, Josh Poimboeuf wrote:
> > On Thu, Apr 23, 2026 at 05:19:25PM +0200, Peter Zijlstra wrote:
> > > On Thu, Apr 23, 2026 at 08:16:08AM -0700, Josh Poimboeuf wrote:
> > > > On Thu, Apr 23, 2026 at 10:47:58AM +0200, Peter Zijlstra wrote:
> > > > > On Wed, Apr 22, 2026 at 09:04:13PM -0700, Josh Poimboeuf wrote:
> > > > > > PREFIX_SYMBOLS has a !CFI dependency because the compiler already
> > > > > > generates __cfi_ prefix symbols for kCFI builds, so objtool-generated
> > > > > > __pfx_ symbols were considered redundant.
> > > > > > 
> > > > > > However, the __cfi_ symbols only cover the 5-byte kCFI type hash.  With
> > > > > > FUNCTION_CALL_PADDING, there are also 11 bytes of NOP padding between
> > > > > > the hash and the function entry which have no symbol to claim them.
> > > > > 
> > > > > If you force the function alignment to 64 bytes, the prefix will also be
> > > > > 64bytes, rather than the normal 16.
> > > > 
> > > > Sorry, how do you get 64 here?
> > > 
> > > DEBUG_FORCE_FUNCTION_ALIGNMENT_64B=y
> > 
> > Ok, so in that case it would be 5-byte cfi symbol and 59-byte NOP gap.
> > Or a 64-byte pfx for the !CFI case.
> > 
> > > > > > The NOPs can be rewritten with call depth tracking thunks at runtime.
> > > > > > Without a symbol, unwinders and other tools that symbolize code
> > > > > > locations misattribute those bytes.
> > > > > > 
> > > > > > Remove the !CFI guard so objtool creates __pfx_ symbols for all
> > > > > > CALL_PADDING configs, covering the full padding area regardless of
> > > > > > whether there's also a __cfi_ symbol.
> > > > > 
> > > > > Egads, that a ton of symbols :/ Does it not make sense to 'fix' up the
> > > > > __cfi_ symbols to cover the whole prefix?
> > > > 
> > > > Yeah, I suppose that would be better, via objtool I presume.
> > > 
> > > Yup.

I discovered it's not just FineIBT, it's basically any CALL_PADDING+CFI,
like so:

From: Josh Poimboeuf <jpoimboe@kernel.org>
Subject: [PATCH] objtool: Grow __cfi_* symbols for all kCFI+CALL_PADDING

For all CONFIG_CFI+CONFIG_CALL_PADDING configs, the __cfi_ symbols only
cover the 5-byte kCFI type hash.  After that there also N bytes of NOP
padding between the hash and the function entry which aren't associated
with any symbol.

The NOPs can be replaced with actual code at runtime.  Without a symbol,
unwinders and tooling have no way of knowing where those bytes belong.

Grow the existing __cfi_* symbols to fill that gap.

Also, CONFIG_PREFIX_SYMBOLS has no reason to exist: CONFIG_CALL_PADDING
is what causes the compiler to emit NOP padding before function entry
(via -fpatchable-function-entry), so it's the right condition for
creating prefix symbols.

Remove CONFIG_PREFIX_SYMBOLS, as it's no longer needed.  Simplify the
LONGEST_SYM_KUNIT_TEST dependency accordingly.

Update the --cfi and --prefix usage strings to reflect their current
scope.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 arch/x86/Kconfig                    |  4 ----
 lib/Kconfig.debug                   |  2 +-
 scripts/Makefile.lib                |  5 ++++-
 tools/objtool/builtin-check.c       |  9 +++++++--
 tools/objtool/check.c               | 13 ++++++++++++-
 tools/objtool/elf.c                 | 20 ++++++++++++++++++++
 tools/objtool/include/objtool/elf.h |  1 +
 7 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index f3f7cb01d69d..3eb3c48d764a 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2437,10 +2437,6 @@ config CALL_THUNKS
 	def_bool n
 	select CALL_PADDING
 
-config PREFIX_SYMBOLS
-	def_bool y
-	depends on CALL_PADDING && !CFI
-
 menuconfig CPU_MITIGATIONS
 	bool "Mitigations for CPU vulnerabilities"
 	default y
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 77c3774c1c49..8b41720069b3 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -3059,7 +3059,7 @@ config FORTIFY_KUNIT_TEST
 config LONGEST_SYM_KUNIT_TEST
 	tristate "Test the longest symbol possible" if !KUNIT_ALL_TESTS
 	depends on KUNIT && KPROBES
-	depends on !PREFIX_SYMBOLS && !CFI && !GCOV_KERNEL
+	depends on !CALL_PADDING && !GCOV_KERNEL
 	default KUNIT_ALL_TESTS
 	help
 	  Tests the longest symbol possible
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 0718e39cedda..562d89f051f0 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -187,7 +187,10 @@ objtool-args-$(CONFIG_HAVE_JUMP_LABEL_HACK)		+= --hacks=jump_label
 objtool-args-$(CONFIG_HAVE_NOINSTR_HACK)		+= --hacks=noinstr
 objtool-args-$(CONFIG_MITIGATION_CALL_DEPTH_TRACKING)	+= --hacks=skylake
 objtool-args-$(CONFIG_X86_KERNEL_IBT)			+= --ibt
-objtool-args-$(CONFIG_FINEIBT)				+= --cfi
+objtool-args-$(CONFIG_CALL_PADDING)			+= --prefix=$(CONFIG_FUNCTION_PADDING_BYTES)
+ifdef CONFIG_CFI
+objtool-args-$(CONFIG_CALL_PADDING)			+= --cfi
+endif
 objtool-args-$(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL)	+= --mcount
 ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
 objtool-args-$(CONFIG_HAVE_OBJTOOL_NOP_MCOUNT)		+= --mnop
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index ec7f10a5ef19..254ceb6b0e2c 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -73,7 +73,6 @@ static int parse_hacks(const struct option *opt, const char *str, int unset)
 
 static const struct option check_options[] = {
 	OPT_GROUP("Actions:"),
-	OPT_BOOLEAN(0,		 "cfi", &opts.cfi, "annotate kernel control flow integrity (kCFI) function preambles"),
 	OPT_STRING_OPTARG('d',	 "disas", &opts.disas, "function-pattern", "disassemble functions", "*"),
 	OPT_CALLBACK_OPTARG('h', "hacks", NULL, NULL, "jump_label,noinstr,skylake", "patch toolchain bugs/limitations", parse_hacks),
 	OPT_BOOLEAN('i',	 "ibt", &opts.ibt, "validate and annotate IBT"),
@@ -84,7 +83,7 @@ static const struct option check_options[] = {
 	OPT_BOOLEAN('r',	 "retpoline", &opts.retpoline, "validate and annotate retpoline usage"),
 	OPT_BOOLEAN(0,		 "rethunk", &opts.rethunk, "validate and annotate rethunk usage"),
 	OPT_BOOLEAN(0,		 "unret", &opts.unret, "validate entry unret placement"),
-	OPT_INTEGER(0,		 "prefix", &opts.prefix, "generate prefix symbols"),
+	OPT_INTEGER(0,		 "prefix", &opts.prefix, "generate or grow prefix symbols for N-byte function padding"),
 	OPT_BOOLEAN('l',	 "sls", &opts.sls, "validate straight-line-speculation mitigations"),
 	OPT_BOOLEAN('s',	 "stackval", &opts.stackval, "validate frame pointer rules"),
 	OPT_BOOLEAN('t',	 "static-call", &opts.static_call, "annotate static calls"),
@@ -92,6 +91,7 @@ static const struct option check_options[] = {
 	OPT_CALLBACK_OPTARG(0,	 "dump", NULL, NULL, "orc", "dump metadata", parse_dump),
 
 	OPT_GROUP("Options:"),
+	OPT_BOOLEAN(0,		 "cfi", &opts.cfi, "annotate and grow kCFI preamble symbols (use with --prefix)"),
 	OPT_BOOLEAN(0,		 "backtrace", &opts.backtrace, "unwind on error"),
 	OPT_BOOLEAN(0,		 "backup", &opts.backup, "create backup (.orig) file on warning/error"),
 	OPT_BOOLEAN(0,		 "dry-run", &opts.dryrun, "don't write modifications"),
@@ -163,6 +163,11 @@ static bool opts_valid(void)
 		return false;
 	}
 
+	if (opts.cfi && !opts.prefix) {
+		ERROR("--cfi requires --prefix");
+		return false;
+	}
+
 	if (opts.disas			||
 	    opts.hack_jump_label	||
 	    opts.hack_noinstr		||
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 410061aeed26..fb24fd284e09 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -923,6 +923,17 @@ static int create_cfi_sections(struct objtool_file *file)
 			return -1;
 
 		idx++;
+
+		/*
+		 * Grow the __cfi_ symbol to fill the NOP gap between the
+		 * 'mov <hash>, %rax' and the start of the function.
+		 */
+		if (sym->len == 5) {
+			sym->len += opts.prefix;
+			sym->sym.st_size = sym->len;
+			if (elf_write_symbol(file->elf, sym))
+				return -1;
+		}
 	}
 
 	return 0;
@@ -4927,7 +4938,7 @@ int check(struct objtool_file *file)
 			goto out;
 	}
 
-	if (opts.prefix) {
+	if (opts.prefix && !opts.cfi) {
 		ret = create_prefix_symbols(file);
 		if (ret)
 			goto out;
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 2ca1151de815..ede87dd9644c 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -983,6 +983,26 @@ struct symbol *elf_create_symbol(struct elf *elf, const char *name,
 	return sym;
 }
 
+int elf_write_symbol(struct elf *elf, struct symbol *sym)
+{
+	struct section *symtab, *symtab_shndx;
+
+	symtab = find_section_by_name(elf, ".symtab");
+	if (!symtab) {
+		ERROR("no .symtab");
+		return -1;
+	}
+
+	symtab_shndx = find_section_by_name(elf, ".symtab_shndx");
+
+	if (elf_update_symbol(elf, symtab, symtab_shndx, sym))
+		return -1;
+
+	mark_sec_changed(elf, symtab, true);
+
+	return 0;
+}
+
 struct symbol *elf_create_section_symbol(struct elf *elf, struct section *sec)
 {
 	struct symbol *sym = calloc(1, sizeof(*sym));
diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
index 0fd1a9b563e9..4c8a67a68063 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -199,6 +199,7 @@ struct reloc *elf_init_reloc_data_sym(struct elf *elf, struct section *sec,
 				      struct symbol *sym,
 				      s64 addend);
 
+int elf_write_symbol(struct elf *elf, struct symbol *sym);
 int elf_write_insn(struct elf *elf, struct section *sec, unsigned long offset,
 		   unsigned int len, const char *insn);
 
-- 
2.53.0