* [PATCH] objtool/klp: Fix checksums for constant pool references
@ 2026-08-28 3:54 Josh Poimboeuf
2026-08-28 4:05 ` sashiko-bot
0 siblings, 1 reply; 3+ messages in thread
From: Josh Poimboeuf @ 2026-08-28 3:54 UTC (permalink / raw)
To: x86
Cc: linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Song Liu, Miroslav Benes, Petr Mladek
Adding a line of code to __link_shadow_page() with a literal string
causes a false positive changed function with GCC:
arch/x86/kvm/kvm.ko.o: changed function: kvm_tdp_mmu_map_private_pfn
While the patch only touched __link_shadow_page(), the string addition
triggered a rename of .LC64 -> .LC65 in kvm_tdp_mmu_map_private_pfn()
even though the underlying referenced constant data didn't change.
So for .LC* symbols, the suffix is arbitrary but the data isn't. Add
the underlying data to the checksum calculation rather than the symbol
name.
Fixes: 0d83da43b1e1 ("objtool/klp: Add --checksum option to generate per-function checksums")
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
tools/objtool/klp-checksum.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/tools/objtool/klp-checksum.c b/tools/objtool/klp-checksum.c
index b8e47f28997e9..359fd27616013 100644
--- a/tools/objtool/klp-checksum.c
+++ b/tools/objtool/klp-checksum.c
@@ -129,6 +129,14 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func,
goto alts;
}
+ if (strstarts(sym->name, ".LC")) {
+ void *cst;
+
+ cst = sym->sec->data->d_buf + sym->offset + offset;
+ __checksum_update_insn(func, insn, cst, sym->sec->sh.sh_entsize);
+ goto alts;
+ }
+
if (is_sec_sym(sym)) {
sym = find_symbol_containing(reloc->sym->sec, offset);
if (!sym)
--
2.55.0
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] objtool/klp: Fix checksums for constant pool references 2026-08-28 3:54 [PATCH] objtool/klp: Fix checksums for constant pool references Josh Poimboeuf @ 2026-08-28 4:05 ` sashiko-bot 2026-08-28 17:45 ` Josh Poimboeuf 0 siblings, 1 reply; 3+ messages in thread From: sashiko-bot @ 2026-08-28 4:05 UTC (permalink / raw) To: Josh Poimboeuf, x86 Cc: Joe Lawrence, Song Liu, Petr Mladek, Miroslav Benes, linux-kernel, live-patching, Peter Zijlstra Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [Critical] Hashing 0 bytes for `.LC` constants in `.rodata` sections due to `sh_entsize == 0`, leading to livepatch false negatives. - [High] Out-of-bounds read and false positives when referencing `.LC` constants with a non-zero relocation offset. - [Medium] The `.LC*` symbol check is bypassed for section-relative relocations. -- commit fbe33684e2b71a1fc6c9da77412adb627592aa7f Author: Josh Poimboeuf <jpoimboe@kernel.org> objtool/klp: Fix checksums for constant pool references This commit fixes false positive changed functions in livepatching caused by changes to .LC* constant names. It does so by adding the underlying data to the checksum calculation instead of the symbol name. > diff --git a/tools/objtool/klp-checksum.c b/tools/objtool/klp-checksum.c > index b8e47f28997e9..359fd27616013 100644 > --- a/tools/objtool/klp-checksum.c > +++ b/tools/objtool/klp-checksum.c > @@ -129,6 +129,14 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func, > goto alts; > } > > + if (strstarts(sym->name, ".LC")) { ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [Severity: Medium] Will this bypass the check for section-relative relocations? If the compiler emits a section-relative relocation (where `reloc->sym` is a section symbol like `.rodata`) with an addend pointing to the `.LC` constant, this check will fail because the section name doesn't start with ".LC". The code then falls through, resolves the actual `.LC` symbol via find_symbol_containing(), and hashes its name instead of its data. Should this check be placed after is_sec_sym(sym) is processed? > + void *cst; > + > + cst = sym->sec->data->d_buf + sym->offset + offset; > + __checksum_update_insn(func, insn, cst, sym->sec->sh.sh_entsize); ^^^^^^^^^^^^^^^^^^^^^^^ [Severity: Critical] Does this silently hash 0 bytes for non-mergeable sections like standard `.rodata`? Because standard sections like `.rodata` have an `sh_entsize` of 0, objtool now hashes 0 bytes for these constants. This completely excludes the constant from the checksum. If a vulnerable constant is fixed, the livepatch checksum will not change, and the fix will be silently omitted. [Severity: High] Can this cause an out-of-bounds read and false positives when referencing `.LC` constants with a non-zero relocation offset? If an `.LC` constant in a section with `sh_entsize > 0` (e.g., `.rodata.cst8`) is referenced via a relocation with a non-zero addend (`offset > 0`), the start pointer `cst` is shifted forward by `offset`, but the length parameter remains the full `sh_entsize`. By reading `offset` bytes past the end of the constant, objtool hashes adjacent, unrelated constants. This falsely binds the function's checksum to unrelated data, and if the constant is at the end of the section buffer, it can cause an out-of-bounds read past the allocated `d_buf` boundary. > + goto alts; > + } > + > if (is_sec_sym(sym)) { > sym = find_symbol_containing(reloc->sym->sec, offset); > if (!sym) -- Sashiko AI review · https://sashiko.dev/#/patchset/22a92df358195864e83fb4cd16181c5faa63c489.1787889249.git.jpoimboe@kernel.org?part=1 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] objtool/klp: Fix checksums for constant pool references 2026-08-28 4:05 ` sashiko-bot @ 2026-08-28 17:45 ` Josh Poimboeuf 0 siblings, 0 replies; 3+ messages in thread From: Josh Poimboeuf @ 2026-08-28 17:45 UTC (permalink / raw) To: sashiko-bot Cc: x86, Joe Lawrence, Song Liu, Petr Mladek, Miroslav Benes, linux-kernel, live-patching, Peter Zijlstra On Fri, Aug 28, 2026 at 04:05:34AM +0000, sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: > - [Critical] Hashing 0 bytes for `.LC` constants in `.rodata` sections due to `sh_entsize == 0`, leading to livepatch false negatives. > - [High] Out-of-bounds read and false positives when referencing `.LC` constants with a non-zero relocation offset. > - [Medium] The `.LC*` symbol check is bypassed for section-relative relocations. > -- > > commit fbe33684e2b71a1fc6c9da77412adb627592aa7f > Author: Josh Poimboeuf <jpoimboe@kernel.org> > > objtool/klp: Fix checksums for constant pool references > > This commit fixes false positive changed functions in livepatching caused by > changes to .LC* constant names. It does so by adding the underlying data to the > checksum calculation instead of the symbol name. > > > diff --git a/tools/objtool/klp-checksum.c b/tools/objtool/klp-checksum.c > > index b8e47f28997e9..359fd27616013 100644 > > --- a/tools/objtool/klp-checksum.c > > +++ b/tools/objtool/klp-checksum.c > > @@ -129,6 +129,14 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func, > > goto alts; > > } > > > > + if (strstarts(sym->name, ".LC")) { > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > [Severity: Medium] > Will this bypass the check for section-relative relocations? > > If the compiler emits a section-relative relocation (where `reloc->sym` is a > section symbol like `.rodata`) with an addend pointing to the `.LC` constant, > this check will fail because the section name doesn't start with ".LC". > > The code then falls through, resolves the actual `.LC` symbol via > find_symbol_containing(), and hashes its name instead of its data. Should this > check be placed after is_sec_sym(sym) is processed? Doesn't happen... however there is a similar issue where in many cases Clang uses a section symbol reference instead of creating a symbol. I'll send a v2. > > > + void *cst; > > + > > + cst = sym->sec->data->d_buf + sym->offset + offset; > > + __checksum_update_insn(func, insn, cst, sym->sec->sh.sh_entsize); > ^^^^^^^^^^^^^^^^^^^^^^^ > > [Severity: Critical] > Does this silently hash 0 bytes for non-mergeable sections like standard > `.rodata`? > > Because standard sections like `.rodata` have an `sh_entsize` of 0, objtool > now hashes 0 bytes for these constants. This completely excludes the constant > from the checksum. If a vulnerable constant is fixed, the livepatch checksum > will not change, and the fix will be silently omitted. Doesn't happen, these are always in .rodata.cst<num> which always have entsize. > [Severity: High] > Can this cause an out-of-bounds read and false positives when referencing > `.LC` constants with a non-zero relocation offset? > > If an `.LC` constant in a section with `sh_entsize > 0` (e.g., `.rodata.cst8`) > is referenced via a relocation with a non-zero addend (`offset > 0`), the start > pointer `cst` is shifted forward by `offset`, but the length parameter remains > the full `sh_entsize`. > > By reading `offset` bytes past the end of the constant, objtool hashes > adjacent, unrelated constants. This falsely binds the function's checksum to > unrelated data, and if the constant is at the end of the section buffer, it can > cause an out-of-bounds read past the allocated `d_buf` boundary. Doesn't happen, the addend is already adjusted to match the symbol itself, and the middle is never referenced. -- Josh ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-08-28 17:45 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-08-28 3:54 [PATCH] objtool/klp: Fix checksums for constant pool references Josh Poimboeuf 2026-08-28 4:05 ` sashiko-bot 2026-08-28 17:45 ` Josh Poimboeuf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®