link_elf_obj: bounds-check a relocation section's sh_info

[ojanerif_amd.com]

Summary

sh_info is used to index the section header array without checking it
against e_shnum, so a malformed object can make the linker read out of
bounds. Reject such a file with ENOEXEC in both the preload and kldload
paths.

MFC after: 1 week

[kib]

This is fine but also it means that the object is corrupted. We do not perform exhaustive checks for the ELF consistency in kernel, generally.

[markj]

In D60937#1397548, @kib wrote:

This is fine but also it means that the object is corrupted. We do not perform exhaustive checks for the ELF consistency in kernel, generally.

Unfortunately, this sort of thing gets reported frequently by naive LLM scans which do not consider the threat model carefully. In the long run, it is better IMO to "harden" this ELF parser in order to silence such reports. It may also be important to do so in case someone in the future copies this code somewhere more sensitive or exposes it to a less-privileged context.

sys/kern/link_elf_obj.c

430

Wouldn't this indicate that the file is corrupt and should therefore be rejected?

[ojanerif_amd.com]

Yeah. Rejecting the whole object on a bad sh_info would also stop objects that are otherwise fine from loading, and the linker doesn't do full ELF validation elsewhere anyway. Bounds-checking and skipping the bad section seemed like the safer option, and it quiets the scan reports. Thanks for looking.

[markj]

In D60937#1398485, @ojanerif_amd.com wrote:

Yeah. Rejecting the whole object on a bad sh_info would also stop objects that are otherwise fine from loading, and the linker doesn't do full ELF validation elsewhere anyway. Bounds-checking and skipping the bad section seemed like the safer option, and it quiets the scan reports. Thanks for looking.

I prefer to fail loudly instead of trying to quietly work around a malformed linker file. Maybe @kib disagrees.

[kib]

I am mostly fine with the change as you see with my approval. I am curious how such object gets produced for you.

[ojanerif_amd.com]

Thanks both. kib: it isn't produced by any toolchain I know of; it came from a static code scan tool that I am researching and writing, and I reproduced the out-of-bounds read with a hand-crafted object. markj: fair enough, I updated the diff to reject the file with link_elf_error() and ENOEXEC instead of skipping the section.