Re: [PATCH] module: reject out-of-range relocation target indices

[Helge Deller <deller@gmx.de>]

On 9/9/26 01:08, Karl Mehltretter wrote:
> apply_relocations() skips relocation sections whose sh_info target index
> is outside the section table. ARM, ARM64, LoongArch, PA-RISC and RISC-V
> use sh_info earlier in module_frob_arch_sections(), before this check.
>
> ARM, ARM64, LoongArch and RISC-V use the unchecked index to read
> sh_flags outside the section header table. PA-RISC uses it to index an
> e_shnum-sized heap array for a read and an update. QEMU reproduced
> page-fault Oopses on ARM, ARM64, LoongArch and RISC-V, and a Data TLB
> miss on the PA-RISC array read.
> 
> Validate sh_info for SHT_REL and SHT_RELA sections in
> elf_validity_cache_sechdrs(). Reject the module with ENOEXEC before
> architecture code can use the index.
> 
> Fixes: c298be74492b ("parisc: fix module loading failure of large kernel modules")
> Fixes: 7d485f647c1f ("ARM: 8220/1: allow modules outside of bl range")
> Fixes: fd045f6cd98e ("arm64: add support for module PLTs")
> Fixes: ab1ef68e5401 ("RISC-V: Add sections of PLT and GOT for kernel module")
> Fixes: fcdfe9d22bed ("LoongArch: Add ELF and module support")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> ---
> 
> A custom harness for upstream Frama-C 33.0 (Arsenic) Eva found the
> ARM32 instance in a source-identical ARM module_frob_arch_sections()
> slice. Eva reported the out-of-range section-table pointer and sh_flags
> access.
> 
> The analysis and ARM32 A/B test ran at Linux b9b3e33b70b7 ("Merge tag
> 'trace-v7.2-rc6' of
> git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace"). The
> PA-RISC, ARM64, RISC-V, LoongArch and x86_64 A/B tests ran at the
> declared base commit, 28924df2a08f. arch/arm/kernel/module-plts.c and
> the touched loop in kernel/module/main.c are identical between the two
> commits.
> 
> Each A/B test changed only a relocation section's sh_info to
> 0x10000000. The same configurations and modules were used before and
> after the change. All controls loaded before and after the change. The
> fixed kernels rejected the malformed modules with ENOEXEC.
> 
> Original-kernel results with QEMU 10.2.1 TCG:
> 
> - ARM32, virt/Cortex-A15, GCC 15.2.0, multi_v7_defconfig plus
>    VMSPLIT_2G: page fault at module_frob_arch_sections()+0x160.
> - ARM64, virt/Cortex-A57, GCC 15.2.0, defconfig: page fault at
>    module_frob_arch_sections()+0x110.
> - PA-RISC, B160L, hppa-linux-gcc 8.1.0, binutils 2.30,
>    generic-32bit_defconfig: Data TLB miss at
>    module_frob_arch_sections()+0x11c on the stub_entries read for a
>    counted R_PARISC_PCREL17F relocation.
> - RISC-V, virt, GCC 15.2.0, defconfig plus RELOCATABLE with
>    MODULE_SECTIONS enabled: page fault at
>    module_frob_arch_sections()+0xe4.
> - LoongArch, virt/LA464, LLVM 21.1.8, loongson64_defconfig: page fault
>    at module_frob_arch_sections()+0x1b8.
> 
> On x86_64, which has no vulnerable early sh_info access, the original
> kernel loaded both modules. The fixed kernel loaded the control and
> rejected the malformed module with ENOEXEC. The test used pc/qemu64,
> x86_64_defconfig and GCC 15.2.0.

Interesting.
So, this patch helps to prevent loading modules with buggy entries,
even if the module was e.g. loaded on x86-64 before.

For me the patch is OK, but it only adds one random sanitizing check,
and where would we stop to check?

> ---
>   kernel/module/main.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index d0e1e0bd2ad0..30c7a05488bc 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -1933,6 +1933,7 @@ static int elf_validity_ehdr(const struct load_info *info)
>    * * Section array fits in the user provided data
>    * * Section index 0 is NULL
>    * * Section contents are inbounds
> + * * Relocation section target indices are inbounds
>    *
>    * Then updates @info with a &load_info->sechdrs pointer if valid.
>    *
> @@ -1983,6 +1984,12 @@ static int elf_validity_cache_sechdrs(struct load_info *info)
>   	/* Validate contents are inbounds */
>   	for (i = 1; i < info->hdr->e_shnum; i++) {
>   		shdr = &sechdrs[i];
> +		if ((shdr->sh_type == SHT_REL || shdr->sh_type == SHT_RELA) &&
> +		    shdr->sh_info >= info->hdr->e_shnum) {
> +			pr_err("Invalid ELF relocation section target index %u\n",
> +			       shdr->sh_info);

Suggestion:
Print the module name too, otherwise it's hard to find out later which module is broken.

Helge

> +			return -ENOEXEC;
> +		}
>   		switch (shdr->sh_type) {
>   		case SHT_NULL:
>   		case SHT_NOBITS:
> 
> base-commit: 28924df2a08f440c73991b83028032c901de2ae4