[PATCH] objtool/klp: Fix missed changes to same-named symbol references

[PATCH] objtool/klp: Fix missed changes to same-named symbol references

[Longjun Luo]

Hashing symbol references by demangled_name alone can miss target
changes while the instruction or data bytes and normalized relocation
offsets remain unchanged. This occurs when:

  - A global function moves between files and calls a same-named static
    function with a different implementation.
  - A function's call target changes from a global function to a
    same-named static function, without moving the caller.
  - A data object, such as an ops structure, changes its function pointer
    from a global function to a same-named static function.

These false negatives can silently omit changed functions from a
livepatch or fail to reject changed data.

Hash referenced symbols as (filename, demangled_name) instead. Use an
empty filename when no FILE is associated, and include both terminating
NULs to delimit the fields.

Verified all three cases with klp-build on x86-64: the function cases
produce livepatch modules with the expected target relocations, and the
data case is rejected. Hand-built unchanged-input controls produce
identical checksums. Module loading was not tested.

Fixes: 0d83da43b1e1 ("objtool/klp: Add --checksum option to generate per-function checksums")
Assisted-by: LLM
Signed-off-by: Longjun Luo <luolongjuna@gmail.com>
---
 tools/objtool/include/objtool/checksum.h | 27 ++++++++++++++++++++++++
 tools/objtool/klp-checksum.c             | 13 ++++--------
 2 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/tools/objtool/include/objtool/checksum.h b/tools/objtool/include/objtool/checksum.h
index d46293f54716..1834d34dfd47 100644
--- a/tools/objtool/include/objtool/checksum.h
+++ b/tools/objtool/include/objtool/checksum.h
@@ -20,6 +20,16 @@ static inline void __checksum_update(struct symbol *sym, const void *data,
 	XXH3_64bits_update(sym->csum.state, data, size);
 }
 
+/* Include FILE identity to distinguish same-named local targets. */
+static inline void __checksum_update_symbol_identity(struct symbol *sym,
+						     struct symbol *target)
+{
+	const char *file_name = target->file ? target->file->name : "";
+
+	__checksum_update(sym, file_name, strlen(file_name) + 1);
+	__checksum_update(sym, target->demangled_name, strlen(target->demangled_name) + 1);
+}
+
 static inline void __checksum_update_insn(struct symbol *sym,
 					  struct instruction *insn,
 					  const void *data, size_t size)
@@ -28,6 +38,14 @@ static inline void __checksum_update_insn(struct symbol *sym,
 	dbg_checksum_insn(sym, insn, XXH3_64bits_digest(sym->csum.state));
 }
 
+static inline void __checksum_update_symbol(struct symbol *sym,
+					    struct instruction *insn,
+					    struct symbol *target)
+{
+	__checksum_update_symbol_identity(sym, target);
+	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,
@@ -38,6 +56,15 @@ static inline void __checksum_update_object(struct symbol *sym,
 	dbg_checksum_object(sym, offset, what, XXH3_64bits_digest(sym->csum.state));
 }
 
+static inline void __checksum_update_object_symbol(struct symbol *sym,
+						   unsigned long offset,
+						   struct symbol *target)
+{
+	__checksum_update(sym, &offset, sizeof(offset));
+	__checksum_update_symbol_identity(sym, target);
+	dbg_checksum_object(sym, offset, "reloc name", XXH3_64bits_digest(sym->csum.state));
+}
+
 static inline void checksum_finish(struct symbol *sym)
 {
 	if (sym && sym->csum.state) {
diff --git a/tools/objtool/klp-checksum.c b/tools/objtool/klp-checksum.c
index b8e47f28997e..3da3fe11bf76 100644
--- a/tools/objtool/klp-checksum.c
+++ b/tools/objtool/klp-checksum.c
@@ -85,8 +85,7 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func,
 			__checksum_update_insn(func, insn, buf, len);
 
 			if (call_dest) {
-				__checksum_update_insn(func, insn, call_dest->demangled_name,
-						       strlen(call_dest->demangled_name));
+				__checksum_update_symbol(func, insn, call_dest);
 
 			} else if (jump_dest) {
 				struct symbol *dest_sym;
@@ -102,8 +101,7 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func,
 				if (!dest_sym)
 					goto alts;
 
-				__checksum_update_insn(func, insn, dest_sym->demangled_name,
-						       strlen(dest_sym->demangled_name));
+				__checksum_update_symbol(func, insn, dest_sym);
 
 				offset = jump_dest->offset - dest_sym->offset;
 				__checksum_update_insn(func, insn, &offset, sizeof(offset));
@@ -137,8 +135,7 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func,
 		offset -= sym->offset;
 	}
 
-	__checksum_update_insn(func, insn, sym->demangled_name,
-			       strlen(sym->demangled_name));
+	__checksum_update_symbol(func, insn, sym);
 	__checksum_update_insn(func, insn, &offset, sizeof(offset));
 
 alts:
@@ -206,9 +203,7 @@ static void checksum_update_object(struct objtool_file *file, struct symbol *sym
 			offset -= target->offset;
 		}
 
-		__checksum_update_object(sym, sym_offset, "reloc name",
-					 target->demangled_name,
-					 strlen(target->demangled_name));
+		__checksum_update_object_symbol(sym, sym_offset, target);
 		__checksum_update_object(sym, sym_offset, "reloc addend",
 					 &offset, sizeof(offset));
 	}

base-commit: 50d05c7c76c96b90462f24debacca971d2e86713
-- 
2.43.7

Re: [PATCH] objtool/klp: Fix missed changes to same-named symbol references

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] `__checksum_update_object_symbol()` unintentionally hashes the `offset` parameter (the relocation's position in the object) using `sizeof(offset)`. Since `offset` is an `unsigned long`, this introduces host-dependent hashing (4 bytes on 32-bit hosts, 8 bytes on 64-bit hosts), which breaks reproducible builds when cross-compiling. Furthermore, this behavior is inconsistent with `__checksum_update_object()` (used for string section relocations), which only uses the offset for debugging and does not hash it.
--

commit 84526b95607bbfd01a9cb5c2ebf122ffa01327c1
Author: Longjun Luo <luolongjuna@gmail.com>

objtool/klp: Fix missed changes to same-named symbol references

This commit modifies objtool to hash referenced symbols using both the
filename and the demangled name. This aims to prevent false negatives
when a function or data object changes its target to a same-named
static function.

> diff --git a/tools/objtool/include/objtool/checksum.h b/tools/objtool/include/objtool/checksum.h
> index d46293f547166..1834d34dfd478 100644
> --- a/tools/objtool/include/objtool/checksum.h
> +++ b/tools/objtool/include/objtool/checksum.h

[ ... ]

> @@ -38,6 +56,15 @@ static inline void __checksum_update_object(struct symbol *sym,
>  	dbg_checksum_object(sym, offset, what, XXH3_64bits_digest(sym->csum.state));
>  }
>  
> +static inline void __checksum_update_object_symbol(struct symbol *sym,
> +						   unsigned long offset,
> +						   struct symbol *target)
> +{
> +	__checksum_update(sym, &offset, sizeof(offset));

[Severity: Medium]
Can hashing the offset parameter here introduce host-dependent behavior?

Since offset is an unsigned long, sizeof(offset) is 4 bytes on 32-bit hosts
and 8 bytes on 64-bit hosts. This can cause the generated livepatch checksums
to vary depending on the build host, which breaks reproducible builds when
cross-compiling.

Before this change, __checksum_update_object() received sym_offset but only
used it for debugging output without hashing it. Should this offset be
excluded from the hash calculation here as well to ensure consistent
checksums across different build hosts?

> +	__checksum_update_symbol_identity(sym, target);
> +	dbg_checksum_object(sym, offset, "reloc name", XXH3_64bits_digest(sym->csum.state));
> +}
> +

> diff --git a/tools/objtool/klp-checksum.c b/tools/objtool/klp-checksum.c
> index b8e47f28997e9..3da3fe11bf762 100644
> --- a/tools/objtool/klp-checksum.c
> +++ b/tools/objtool/klp-checksum.c

[ ... ]

> @@ -206,9 +203,7 @@ static void checksum_update_object(struct objtool_file *file, struct symbol *sym
>  			offset -= target->offset;
>  		}
>  
> -		__checksum_update_object(sym, sym_offset, "reloc name",
> -					 target->demangled_name,
> -					 strlen(target->demangled_name));
> +		__checksum_update_object_symbol(sym, sym_offset, target);
>  		__checksum_update_object(sym, sym_offset, "reloc addend",
>  					 &offset, sizeof(offset));
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910085218.66565-1-luolongjuna@gmail.com?part=1

Re: [PATCH] objtool/klp: Fix missed changes to same-named symbol references

[Longjun Luo]

> Before this change, __checksum_update_object() received sym_offset but only
> used it for debugging output without hashing it.

At this patch's base commit (50d05c7c76c9), __checksum_update_object()
begins with:

        __checksum_update(sym, &offset, sizeof(offset));
        __checksum_update(sym, data, size);

__checksum_update_object_symbol() preserves the same offset handling,
including the unsigned long type and sizeof(offset). The difference is
how the referenced symbol's identity is encoded.

The same &offset/sizeof(offset) pattern is also used in
checksum_update_insn(). Any host-width dependence here therefore
predates this patch.

Thanks,
Longjun

Re: [PATCH] objtool/klp: Fix missed changes to same-named symbol references

[Josh Poimboeuf]

On Thu, Sep 10, 2026 at 04:52:18PM +0800, Longjun Luo wrote:
> Hashing symbol references by demangled_name alone can miss target
> changes while the instruction or data bytes and normalized relocation
> offsets remain unchanged. This occurs when:
> 
>   - A global function moves between files and calls a same-named static
>     function with a different implementation.
>   - A function's call target changes from a global function to a
>     same-named static function, without moving the caller.
>   - A data object, such as an ops structure, changes its function pointer
>     from a global function to a same-named static function.

These seem like unusual edge cases, is this problem theoretical or was
it a real-world bug?

> +/* Include FILE identity to distinguish same-named local targets. */
> +static inline void __checksum_update_symbol_identity(struct symbol *sym,
> +						     struct symbol *target)
> +{
> +	const char *file_name = target->file ? target->file->name : "";
> +
> +	__checksum_update(sym, file_name, strlen(file_name) + 1);
> +	__checksum_update(sym, target->demangled_name, strlen(target->demangled_name) + 1);
> +}

If the .patch upgrades a function from static to global then the
function no longer has a FILE associated with it.  Then with the above,
all callers to that function would be marked as changed.  And note that
LTO does a lot of that, so static-to-global and global-to-static changes
are common between orig and patched objects.

Note that function change detection isn't intended to be 100% perfect
for all edge cases.  The patch author needs to verify the changed
function list matches what they expect.

-- 
Josh

Re: [PATCH] objtool/klp: Fix missed changes to same-named symbol references

[Longjun Luo]

Hi Josh,

> These seem like unusual edge cases, is this problem theoretical or was
> it a real-world bug?

These are constructed reproducers, not production failures. We have
been working on userspace livepatching, including symbol correlation
under LTO. I built and tested these cases while comparing klp-build
and kpatch-build.

> If the .patch upgrades a function from static to global then the
> function no longer has a FILE associated with it.  Then with the above,
> all callers to that function would be marked as changed.  And note that
> LTO does a lot of that, so static-to-global and global-to-static changes
> are common between orig and patched objects.

I confirmed locally that kernel LTO can change a function's binding
without changing its name.

For context, in our userspace ThinLTO tests, promoted locals usually
acquired a .llvm.<hash> suffix. The kernel's ThinLTO build uses
-always-rename-promoted-locals=false when supported by the toolchain,
so promotion need not add that suffix.

I agree that this FILE-based checksum change is too restrictive.
Please drop this patch.

You are right that patch authors need to verify the changed-function
list. However, in these cases the current checksums do not distinguish
the old and new targets, and no diagnostic points to the missed change.
That could make a resulting problem difficult and time-consuming to
diagnose. Would it be useful to document these cases as a known
limitation?

Thanks,
Longjun

Josh Poimboeuf <jpoimboe@kernel.org> 于2026年9月14日周一 08:27写道：
>
> On Thu, Sep 10, 2026 at 04:52:18PM +0800, Longjun Luo wrote:
> > Hashing symbol references by demangled_name alone can miss target
> > changes while the instruction or data bytes and normalized relocation
> > offsets remain unchanged. This occurs when:
> >
> >   - A global function moves between files and calls a same-named static
> >     function with a different implementation.
> >   - A function's call target changes from a global function to a
> >     same-named static function, without moving the caller.
> >   - A data object, such as an ops structure, changes its function pointer
> >     from a global function to a same-named static function.
>
> These seem like unusual edge cases, is this problem theoretical or was
> it a real-world bug?
>
> > +/* Include FILE identity to distinguish same-named local targets. */
> > +static inline void __checksum_update_symbol_identity(struct symbol *sym,
> > +                                                  struct symbol *target)
> > +{
> > +     const char *file_name = target->file ? target->file->name : "";
> > +
> > +     __checksum_update(sym, file_name, strlen(file_name) + 1);
> > +     __checksum_update(sym, target->demangled_name, strlen(target->demangled_name) + 1);
> > +}
>
> If the .patch upgrades a function from static to global then the
> function no longer has a FILE associated with it.  Then with the above,
> all callers to that function would be marked as changed.  And note that
> LTO does a lot of that, so static-to-global and global-to-static changes
> are common between orig and patched objects.
>
> Note that function change detection isn't intended to be 100% perfect
> for all edge cases.  The patch author needs to verify the changed
> function list matches what they expect.
>
> --
> Josh

Re: [PATCH] objtool/klp: Fix missed changes to same-named symbol references

[Josh Poimboeuf]

On Mon, Sep 14, 2026 at 08:12:33PM +0800, Longjun Luo wrote:
> Hi Josh,
> 
> > These seem like unusual edge cases, is this problem theoretical or was
> > it a real-world bug?
> 
> These are constructed reproducers, not production failures. We have
> been working on userspace livepatching, including symbol correlation
> under LTO. I built and tested these cases while comparing klp-build
> and kpatch-build.
> 
> > If the .patch upgrades a function from static to global then the
> > function no longer has a FILE associated with it.  Then with the above,
> > all callers to that function would be marked as changed.  And note that
> > LTO does a lot of that, so static-to-global and global-to-static changes
> > are common between orig and patched objects.
> 
> I confirmed locally that kernel LTO can change a function's binding
> without changing its name.
> 
> For context, in our userspace ThinLTO tests, promoted locals usually
> acquired a .llvm.<hash> suffix. The kernel's ThinLTO build uses
> -always-rename-promoted-locals=false when supported by the toolchain,
> so promotion need not add that suffix.
> 
> I agree that this FILE-based checksum change is too restrictive.
> Please drop this patch.
> 
> You are right that patch authors need to verify the changed-function
> list. However, in these cases the current checksums do not distinguish
> the old and new targets, and no diagnostic points to the missed change.
> That could make a resulting problem difficult and time-consuming to
> diagnose. Would it be useful to document these cases as a known
> limitation?

Yes, and in fact we need a "patch author guide" for humans/LLMs to read
when using klp-build, as there are pitfalls to watch out for.

Something similar to what kpatch has...

  https://github.com/dynup/kpatch/blob/master/doc/patch-author-guide.md

in case anybody wants to try porting that over.

-- 
Josh