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

Reply via email to