Re: [PATCH v2] objtool/klp: Fix checksums for constant pool references

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The checksum calculation for anonymous constants does not align the relocation addend, potentially reading past the end of the constant and out-of-bounds of the section.
--

commit 40a0e3ccdee64d957ccae4cb27dbc7318ac09355
Author: Josh Poimboeuf <jpoimboe@kernel.org>

objtool/klp: Fix checksums for constant pool references

This commit addresses false positive changed functions in livepatches by
including the underlying data of anonymous constant pool references in the
objtool checksum calculation, rather than relying on arbitrary symbol names.

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

[ ... ]

> @@ -129,6 +142,14 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func,
>  		goto alts;
>  	}
>  
> +	if (is_anonymous_const_data(sym)) {
> +		void *cst;
> +
> +		cst = sym->sec->data->d_buf + sym->offset + offset;
> +		__checksum_update_insn(func, insn, cst, sym->sec->sh.sh_entsize);

[Severity: Medium]
Could this result in an out-of-bounds read if the relocation addend is
unaligned?

In checksum_update_insn(), the code calculates the cst pointer by adding the
potentially unaligned offset to the symbol base, but then unconditionally
hashes a full sym->sec->sh.sh_entsize bytes.

If the compiler emits a relocation to a constant pool section with an
unaligned addend (for example, when an instruction accesses a sub-element of
a 16-byte constant), reading a full sh_entsize bytes from a mid-constant
offset could cross the section buffer boundary if this constant is at the
end of the section.

Can this lead to non-deterministic checksums by incorporating unrelated memory
into the hash, or potentially cause a segfault during the kernel build?

> +		goto alts;
> +	}
> +
>  	if (is_sec_sym(sym)) {
>  		sym = find_symbol_containing(reloc->sym->sec, offset);
>  		if (!sym)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/f3a9e74ceebc6475ce94bcfe985401140857814a.1787939301.git.jpoimboe@kernel.org?part=1