Re: [PATCH v2 00/62] objtool,livepatch: klp-build livepatch module generation
On Fri, May 09, 2025 at 01:16:24PM -0700, Josh Poimboeuf wrote:
> I've tested with a variety of patches on defconfig and Fedora-config
> kernels with both GCC and Clang.
>
> These patches can also be found at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git
> klp-build-v2
>
> Please test!
I found a nasty bug while trying to patch copy_process(). The wrong
version of __refcount_add.constprop.0() was being called due to a bad
klp rela sympos value, causing refcount overflow/UAF warnings and hangs.
The problem was a mismatch between the sympos order of the vmlinux.o
archive (which klp-diff uses) and the final vmlinux.
The linker script manually emits .text.unlikely before .text instead of
emitting in them in section table order. So if a function name exists
in both sections, the calculated sympos (based on vmlinux.o) is wrong.
In my test kernel with GCC 14, only 25 out of 136,931 functions (0.018%)
have this problem. It was my lucky day.
The below hack fixes it by starting the sympos counting with
.text.unlikely*. It's a bit fragile, but it works fine for now.
I'm thinking the final fix would involve adding a checksum field to
kallsyms. Then sympos would no longer be needed. But that will need to
come later.
diff --git a/tools/include/linux/string.h b/tools/include/linux/string.h
index 8499f509f03e..51ad3cf4fa82 100644
--- a/tools/include/linux/string.h
+++ b/tools/include/linux/string.h
@@ -44,6 +44,20 @@ static inline bool strstarts(const char *str, const char
*prefix)
return strncmp(str, prefix, strlen(prefix)) == 0;
}
+/*
+ * Checks if a string ends with another.
+ */
+static inline bool str_ends_with(const char *str, const char *substr)
+{
+ size_t len = strlen(str);
+ size_t sublen = strlen(substr);
+
+ if (sublen > len)
+ return false;
+
+ return !strcmp(str + len - sublen, substr);
+}
+
extern char * __must_check skip_spaces(const char *);
extern char *strim(char *);
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index b5494b5ca78f..47ee010a7852 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -187,20 +187,6 @@ static bool is_sibling_call(struct instruction *insn)
return (is_static_jump(insn) && insn_call_dest(insn));
}
-/*
- * Checks if a string ends with another.
- */
-static bool str_ends_with(const char *s, const char *sub)
-{
- const int slen = strlen(s);
- const int sublen = strlen(sub);
-
- if (sublen > slen)
- return 0;
-
- return !memcmp(s + slen - sublen, sub, sublen);
-}
-
/*
* Checks if a function is a Rust "noreturn" one.
*/
diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 5276964ef123..0290cbc90c16 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -439,6 +439,7 @@ static int correlate_symbols(struct elfs *e)
/* "sympos" is used by livepatch to disambiguate duplicate symbol names */
static unsigned long find_sympos(struct elf *elf, struct symbol *sym)
{
+ bool vmlinux = str_ends_with(objname, "vmlinux.o");
unsigned long sympos = 0, nr_matches = 0;
bool has_dup = false;
struct symbol *s;
@@ -446,13 +447,43 @@ static unsigned long find_sympos(struct elf *elf, struct
symbol *sym)
if (sym->bind != STB_LOCAL)
return 0;
- for_each_sym(elf, s) {
- if (!strcmp(s->name, sym->name)) {
- nr_matches++;
- if (s == sym)
- sympos = nr_matches;
- else
- has_dup = true;
+ if (vmlinux && sym->type == STT_FUNC) {
+ /*
+* HACK: Unfortunately, symbol ordering can differ between
+* vmlinux.o and vmlinux due to the linker script emitting
+* .text.unlikely* before .text*. Count .text.unlikely* first.
+*
+* TODO: Disambiguate symbols more reliably (checksums?)
+*/
+ for_each_sym(elf, s) {
+ if (strstarts(s->sec->name, ".text.unlikely") &&
+ !strcmp(s->name, sym->name)) {
+ nr_matches++;
+ if (s == sym)
+ sympos = nr_matches;
+ else
+ has_dup = true;
+ }
+ }
+ for_each_sym(elf, s) {
+ if (!strstarts(s->sec->name, ".text.unlikely") &&
+ !strcmp(s->name, sym->name)) {
+ nr_matches++;
+ if (s == sym)
+ sympos = nr_matches;
+ else
+ has_dup = true;
+ }
+ }
+ }
[PATCH v2 00/62] objtool,livepatch: klp-build livepatch module generation
This series introduces new objtool features and a klp-build script to generate livepatch modules using a source .patch as input. This builds on concepts from the longstanding out-of-tree kpatch [1] project which began in 2012 and has been used for many years to generate livepatch modules for production kernels. However, this is a complete rewrite which incorporates hard-earned lessons from 12+ years of maintaining kpatch. Key improvements compared to kpatch-build: - Integrated with objtool: Leverages objtool's existing control-flow graph analysis to help detect changed functions. - Works on vmlinux.o: Supports late-linked objects, making it compatible with LTO, IBT, and similar. - Simplified code base: ~3k fewer lines of code. - Upstream: No more out-of-tree #ifdef hacks, far less cruft. - Cleaner internals: Vastly simplified logic for symbol/section/reloc inclusion and special section extraction. - Robust __LINE__ macro handling: Avoids false positive binary diffs caused by the __LINE__ macro by introducing a fix-patch-lines script which injects #line directives into the source .patch to preserve the original line numbers at compile time. The primary user interface is the klp-build script which does the following: - Builds an original kernel with -function-sections and -fdata-sections, plus objtool function checksumming. - Applies the .patch file and rebuilds the kernel using the same options. - Runs 'objtool klp diff' to detect changed functions and generate intermediate binary diff objects. - Builds a kernel module which links the diff objects with some livepatch module init code (scripts/livepatch/init.c). - Finalizes the livepatch module (aka work around linker wreckage) using 'objtool klp post-link'. I've tested with a variety of patches on defconfig and Fedora-config kernels with both GCC and Clang. These patches can also be found at: git://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git klp-build-v2 Please test! [1] https://github.com/dynup/kpatch Changes since the RFC: https://lore.kernel.org/[email protected] - Too many to list. Doubled the patch count exactly ;-) Josh Poimboeuf (62): s390/vmlinux.lds.S: Prevent thunk functions from getting placed with normal text vmlinux.lds: Unify TEXT_MAIN, DATA_MAIN, and related macros x86/module: Improve relocation error messages x86/kprobes: Remove STACK_FRAME_NON_STANDARD annotation compiler: Tweak __UNIQUE_ID() naming compiler.h: Make addressable symbols less of an eyesore elfnote: Change ELFNOTE() to use __UNIQUE_ID() kbuild: Remove 'kmod_' prefix from __KBUILD_MODNAME modpost: Ignore unresolved section bounds symbols x86/alternative: Refactor INT3 call emulation selftest objtool: Make find_symbol_containing() less arbitrary objtool: Speed up SHT_GROUP reindexing objtool: Fix broken error handling in read_symbols() objtool: Propagate elf_truncate_section() error in elf_write() objtool: Add empty symbols to the symbol tree again objtool: Fix interval tree insertion for zero-length symbols objtool: Fix weak symbol detection objtool: Fix x86 addend calculation objtool: Fix __pa_symbol() relocation handling objtool: Fix "unexpected end of section" warning for alternatives objtool: Check for missing annotation entries in read_annotate() objtool: Const string cleanup objtool: Clean up compiler flag usage objtool: Remove .parainstructions reference objtool: Convert elf iterator macros to use 'struct elf' objtool: Add section/symbol type helpers objtool: Mark .cold subfunctions objtool: Fix weak symbol hole detection for .cold functions objtool: Mark prefix functions objtool: Simplify reloc offset calculation in unwind_read_hints() objtool: Avoid emptying lists for duplicate sections objtool: Suppress section skipping warnings with --dryrun objtool: Rename --Werror to --werror objtool: Reindent check_options[] objtool: Refactor add_jump_destinations() objtool: Simplify special symbol handling in elf_update_symbol() objtool: Generalize elf_create_symbol() objtool: Generalize elf_create_section() objtool: Add elf_create_data() objtool: Introduce elf_create_reloc() and elf_init_reloc() objtool: Add elf_create_file() kbuild,x86: Fix module permissions for __jump_table and __bug_table x86/alternative: Define ELF section entry size for alternatives x86/jump_label: Define ELF section entry size for jump table x86/extable: Define ELF section entry size for exception tables x86/bug: Define ELF section entry size for the bug table x86/orc: Define ELF section entry size for unwind hints objtool: Make STACK_FRAME_NON_STANDARD consistent kbuild,objtool: Defer objtool validation step for CONFIG_LIVEPATCH objtool/klp: Add --checksum option to generate per-function checksums objtool/klp: Add --debug-checksum= to show per-instruction checksums
