Re: [RFC 2/4] objtool/klp: allow special section entry size overrides

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The custom entry size logic bypasses section size validation, allowing the creation of out-of-bounds ELF symbols.
--

commit 07468f70445bbc595d50f5edb50b5c394dc62f05
Author: Joe Lawrence <[email protected]>

objtool/klp: allow special section entry size overrides

This commit allows users to override compiled-in defaults for special
section entry sizes via environment variables. This helps process cached
unit test objects built from different kernel versions where the compiled-in
sizes may not match the actual entry sizes.

> diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
> index 0774ab312d033..84c00b673085b 100644
> --- a/tools/objtool/klp-diff.c
> +++ b/tools/objtool/klp-diff.c

[ ... ]

> @@ -1700,6 +1716,21 @@ static int create_fake_symbols(struct elf *elf)
>  		}
>  
>  		entry_size = sec->sh.sh_entsize;
> +
> +		/*
> +		 * Some special sections have multiple relocs per entry,
> +		 * so the reloc-based heuristic below doesn't work.  Use
> +		 * the arch-defined entry sizes for known special sections.
> +		 */
> +		if (!entry_size) {
> +			if (!strcmp(sec->name, ".altinstructions"))
> +				entry_size = entry_size_from_env("ALT_ENTRY_SIZE", ALT_ENTRY_SIZE);
> +			else if (!strcmp(sec->name, "__jump_table"))
> +				entry_size = entry_size_from_env("JUMP_ENTRY_SIZE", JUMP_ENTRY_SIZE);
> +			else if (!strcmp(sec->name, "__ex_table"))
> +				entry_size = entry_size_from_env("EX_ENTRY_SIZE", EX_ENTRY_SIZE);
> +		}
> +
>  		if (!entry_size) {
>  			entry_size = arch_reloc_size(sec->rsec->relocs);
>  			if (sec_size(sec) != entry_size * sec_num_entries(sec->rsec)) {

If entry_size is populated from the environment variables, it skips this
validation check entirely.

If an invalid entry size that isn't a clean divisor of the section size is
provided, could this lead to an out-of-bounds memory access regression?

>  				ERROR("%s: missing special section entsize or annotations", sec->name);
>  				return -1;
>  			}
>  		}
>  
>  		for (offset = 0; offset < sec_size(sec); offset += entry_size) {
>  			if (create_fake_symbol(elf, sec, offset, entry_size))
>  				return -1;
>  		}

For example, if sec_size(sec) is 20 and entry_size is provided as 12, the
loop will execute for offset 0 and offset 12. During the second iteration,
the fake symbol will extend from 12 to 24, which exceeds the section bounds
and may cause out-of-bounds accesses during subsequent objtool passes.

Should there be an explicit validation check to ensure the overridden
entry_size divides evenly into sec_size(sec) before proceeding to this loop?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2