diff mbox series

[2/3] arm64: module: Use module_init_layout_section() to spot init sections

Message ID 20230801145409.8935-3-james.morse@arm.com (mailing list archive)
State New, archived
Headers show
Series ARM/arm64: Fix loading of modules with an exit section | expand

Commit Message

James Morse Aug. 1, 2023, 2:54 p.m. UTC
Today module_frob_arch_sections() spots init sections from their
'init' prefix, and uses this to keep the init PLTs separate from the rest.

module_emit_plt_entry() uses within_module_init() to determine if a
location is in the init text or not, but this depends on whether
core code thought this was an init section.

Naturally the logic is different.

module_init_layout_section() groups the init and exit text together if
module unloading is disabled, as the exit code will never run. The result
is kernels with this configuration can't load all their modules because
there are not enough PLTs for the combined init+exit section.

This results in the following:
| WARNING: CPU: 2 PID: 51 at arch/arm64/kernel/module-plts.c:99 module_emit_plt_entry+0x184/0x1cc
| Modules linked in: crct10dif_common
| CPU: 2 PID: 51 Comm: modprobe Not tainted 6.5.0-rc4-yocto-standard-dirty #15208
| Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
| pstate: 20400005 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
| pc : module_emit_plt_entry+0x184/0x1cc
| lr : module_emit_plt_entry+0x94/0x1cc
| sp : ffffffc0803bba60
[...]
| Call trace:
|  module_emit_plt_entry+0x184/0x1cc
|  apply_relocate_add+0x2bc/0x8e4
|  load_module+0xe34/0x1bd4
|  init_module_from_file+0x84/0xc0
|  __arm64_sys_finit_module+0x1b8/0x27c
|  invoke_syscall.constprop.0+0x5c/0x104
|  do_el0_svc+0x58/0x160
|  el0_svc+0x38/0x110
|  el0t_64_sync_handler+0xc0/0xc4
|  el0t_64_sync+0x190/0x194

A previous patch exposed module_init_layout_section(), use that so the
logic is the same.

Reported-by: Adam Johnston <adam.johnston@arm.com>
Tested-by: Adam Johnston <adam.johnston@arm.com>
Fixes: 055f23b74b20 ("module: check for exit sections in layout_sections() instead of module_init_section()")
Cc: <stable@vger.kernel.org> # 5.15.x: 60a0aab7463ee69 arm64: module-plts: inline linux/moduleloader.h
Cc: <stable@vger.kernel.org> # 5.15.x
Signed-off-by: James Morse <james.morse@arm.com>
---
---
 arch/arm64/kernel/module-plts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Catalin Marinas Aug. 2, 2023, 5 p.m. UTC | #1
On Tue, Aug 01, 2023 at 02:54:08PM +0000, James Morse wrote:
> Today module_frob_arch_sections() spots init sections from their
> 'init' prefix, and uses this to keep the init PLTs separate from the rest.
> 
> module_emit_plt_entry() uses within_module_init() to determine if a
> location is in the init text or not, but this depends on whether
> core code thought this was an init section.
> 
> Naturally the logic is different.
> 
> module_init_layout_section() groups the init and exit text together if
> module unloading is disabled, as the exit code will never run. The result
> is kernels with this configuration can't load all their modules because
> there are not enough PLTs for the combined init+exit section.
> 
> This results in the following:
> | WARNING: CPU: 2 PID: 51 at arch/arm64/kernel/module-plts.c:99 module_emit_plt_entry+0x184/0x1cc
> | Modules linked in: crct10dif_common
> | CPU: 2 PID: 51 Comm: modprobe Not tainted 6.5.0-rc4-yocto-standard-dirty #15208
> | Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
> | pstate: 20400005 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> | pc : module_emit_plt_entry+0x184/0x1cc
> | lr : module_emit_plt_entry+0x94/0x1cc
> | sp : ffffffc0803bba60
> [...]
> | Call trace:
> |  module_emit_plt_entry+0x184/0x1cc
> |  apply_relocate_add+0x2bc/0x8e4
> |  load_module+0xe34/0x1bd4
> |  init_module_from_file+0x84/0xc0
> |  __arm64_sys_finit_module+0x1b8/0x27c
> |  invoke_syscall.constprop.0+0x5c/0x104
> |  do_el0_svc+0x58/0x160
> |  el0_svc+0x38/0x110
> |  el0t_64_sync_handler+0xc0/0xc4
> |  el0t_64_sync+0x190/0x194
> 
> A previous patch exposed module_init_layout_section(), use that so the
> logic is the same.
> 
> Reported-by: Adam Johnston <adam.johnston@arm.com>
> Tested-by: Adam Johnston <adam.johnston@arm.com>
> Fixes: 055f23b74b20 ("module: check for exit sections in layout_sections() instead of module_init_section()")
> Cc: <stable@vger.kernel.org> # 5.15.x: 60a0aab7463ee69 arm64: module-plts: inline linux/moduleloader.h
> Cc: <stable@vger.kernel.org> # 5.15.x
> Signed-off-by: James Morse <james.morse@arm.com>

Acked-by: Catalin Marinas <catalin.marinas@arm.com>
diff mbox series

Patch

diff --git a/arch/arm64/kernel/module-plts.c b/arch/arm64/kernel/module-plts.c
index ad02058756b5..bd69a4e7cd60 100644
--- a/arch/arm64/kernel/module-plts.c
+++ b/arch/arm64/kernel/module-plts.c
@@ -339,7 +339,7 @@  int module_frob_arch_sections(Elf_Ehdr *ehdr, Elf_Shdr *sechdrs,
 		if (nents)
 			sort(rels, nents, sizeof(Elf64_Rela), cmp_rela, NULL);
 
-		if (!str_has_prefix(secstrings + dstsec->sh_name, ".init"))
+		if (!module_init_layout_section(secstrings + dstsec->sh_name))
 			core_plts += count_plts(syms, rels, numrels,
 						sechdrs[i].sh_info, dstsec);
 		else