Re: [RFC PATCH 8/9] arm64: module: Combine init and core PLT entries again

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The check `s->st_shndx != SHN_UNDEF` incorrectly skips PLT counting for special symbols like `SHN_LIVEPATCH` and `SHN_ABS`, breaking livepatching.
- [High] Combining core and init PLTs breaks module loading if `execmem_split` fails and falls back to an independent memory allocation.
--

--- Patch [8]: [RFC PATCH 8/9] arm64: module: Combine init and core PLT entries again ---
commit 9e8ee18408a9e3b59ec81f693240a4dd9c7669a3
Author: Ard Biesheuvel <ardb@kernel.org>

arm64: module: Combine init and core PLT entries again

This patch consolidates the core and init PLT sections into a single array,
operating on the assumption that .init.text and .text are now always placed
within direct branching range of each other. It also eliminates the need
to allocate PLT entries for intra-module cross-section calls.

> diff --git a/arch/arm64/kernel/module-plts.c b/arch/arm64/kernel/module-plts.c
> index bde32979c06af..e30876b90e306 100644
> --- a/arch/arm64/kernel/module-plts.c
> +++ b/arch/arm64/kernel/module-plts.c
[ ... ]
> @@ -169,17 +167,17 @@ static unsigned int count_plts(Elf64_Sym *syms, Elf64_Rela *rela, int num,
>  		case R_AARCH64_CALL26:
>  			/*
>  			 * We only have to consider branch targets that resolve
> -			 * to symbols that are defined in a different section.
> +			 * to symbols that are not defined in the same module.
>  			 * This is not simply a heuristic, it is a fundamental
>  			 * limitation, since there is no guaranteed way to emit
>  			 * PLT entries sufficiently close to the branch if the
>  			 * section size exceeds the range of a branch
>  			 * instruction. So ignore relocations against defined
> -			 * symbols if they live in the same section as the
> +			 * symbols if they live in the same module as the
>  			 * relocation target.
>  			 */

[Severity: High]
Does this safely account for memory pressure during module loading?

While this patch expects .init.text and .text to be placed within direct 
branching range, kernel/module/main.c:module_memory_alloc() attempts to 
enforce this by using execmem_split(). 

If execmem_split() fails, such as a mas_store_gfp() failure under memory 
pressure, module_memory_alloc() falls back to an independent 
execmem_alloc_rw() allocation. Because MODULES_VSIZE on ARM64 is 2GB, 
couldn't this fallback place the init section more than 128MB away from 
the core section?

If that happens, intra-module calls would exceed the architectural range. 
Since this patch removes the PLT entries for them, wouldn't the module 
load fail with a WARN_ON in module_emit_plt_entry() and abort the load?

>  			s = syms + ELF64_R_SYM(rela[i].r_info);
> -			if (s->st_shndx == dstidx)
> +			if (s->st_shndx != SHN_UNDEF)
>  				break;

[Severity: High]
Will this skip PLT allocations for special symbols like SHN_LIVEPATCH and 
SHN_ABS?

When a livepatch module is loaded, its symbols are marked with SHN_LIVEPATCH 
(0xff20) by objtool. Since SHN_LIVEPATCH is not SHN_UNDEF, this check evaluates
to true, breaking the loop and skipping PLT allocation for these symbols.

Later, when klp_apply_section_relocs() attempts to apply the .klp.rela.* 
relocations against vmlinux, out-of-range branches will need a PLT entry.
Since none were allocated, won't module_emit_plt_entry() hit the WARN_ON 
for exceeding pltsec->plt_max_entries and abort the load with -ENOEXEC?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260822135323.795946-11-ardb+git@google.com?part=8