On Fri, Aug 28, 2026 at 04:05:34AM +0000, [email protected] 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 <[email protected]> > > 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
