Re: [RFC PATCH 2/3] objtool/klp: Include symbol identities in reference checksums

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] objtool/klp: unstable checksums for local symbols in prepare_identities()
--

commit 547eafe74bd5b34cf5af6c927344337c25025230
Author: Longjun Luo <luolongjuna@gmail.com>

objtool/klp: Include symbol identities in reference checksums

This commit prepares symbol identities during klp checksum and hashes the
target symbol's source/scope/demangled_name path instead of only its
demangled name when computing instruction and data object reference
checksums.

> diff --git a/tools/objtool/klp-identity.c b/tools/objtool/klp-identity.c
> index 627eda694389e..ca8e65d5d3927 100644
> --- a/tools/objtool/klp-identity.c
> +++ b/tools/objtool/klp-identity.c
> @@ -156,12 +156,19 @@ int prepare_identities(struct elf *elf)
>  
>  	for_each_sym(elf, sym) {
>  		struct identity *identity = &sym->identity;
> +		char *path;
>  
>  		/* Default to STT_FILE and "/" for missing DWARF information. */
>  		if (!identity->external && !*identity->source && sym->file)
>  			identity->source = sym->file->identity.source;
>  		if (!identity->scope)
>  			identity->scope = "/";
> +		if (asprintf(&path, "%s%s%s", identity->source, identity->scope,
> +			     identity->demangled_name) < 0) {
> +			ERROR_GLIBC("asprintf");
> +			return -1;
> +		}
> +		identity->path = path;

[Severity: Medium]
Can baking sym->file->identity.source into identity->path cause unstable
reference checksums for unchanged functions and data objects when a livepatch
adds or removes the only correlated static symbol in a translation unit?

In cmd_klp_checksum(), sym->dont_correlate = dont_correlate(sym) is populated
before calling prepare_identities(file->elf). In prepare_identities(), the
first loop initializes each STT_FILE symbol's identity->source to sym->name
(typically a short basename such as "foo.c") and skips DWARF lookup whenever
is_dwarf_identity_candidate(sym) returns false:

tools/objtool/klp-identity.c:is_dwarf_identity_candidate() {
	return !sym->dont_correlate && sym->len && !is_undef_sym(sym) &&
	       (is_func_sym(sym) || is_object_sym(sym));
}

In the second loop of prepare_identities(), external symbols clear
identity->source to "", and complete_file_source() only upgrades
sym->file->identity.source from the short STT_FILE name to the full
normalized DWARF compilation unit path when the translation unit contains at
least one correlated (!sym->dont_correlate), non-external STB_LOCAL symbol
with DWARF information:

tools/objtool/klp-identity.c:prepare_identities() {
    ...
		if (identity->external)
			identity->source = "";
		else if (complete_file_source(sym, identity->source))
			return -1;
    ...
}

In the third loop above, all local symbols without a DWARF identity (including
dont_correlate symbols such as .data..once flags from WARN_ONCE/DO_ONCE,
pr_debug descriptors, __func__, lockdep keys, switch tables, anonymous
rodata, and .altinstr_replacement symbols) fall back to
sym->file->identity.source and format it into identity->path, which is then
hashed by checksum_update_insn() and checksum_update_object().

If a translation unit in orig.o has only external symbols (or static symbols
that were all inlined or promoted to STB_GLOBAL by ThinLTO),
sym->file->identity.source remains the short STT_FILE name. If patched.o
then introduces a static helper or static variable in that translation unit
(or removes the last one), sym->file->identity.source flips between the short
STT_FILE name and the full DWARF path.

Wouldn't that change identity->path for every fallback and dont_correlate
local symbol in the translation unit, causing unchanged functions referencing
them to get different checksums and causing unchanged correlated data objects
referencing them to fail klp diff in mark_changed_functions() with a false
"changed data" error?

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