[PATCH] module: treat exit sections the same as init sections when !CONFIG_MODULE_UNLOAD

[PATCH] module: treat exit sections the same as init sections when !CONFIG_MODULE_UNLOAD

[Jessica Yu]

Dynamic code patching (alternatives, jump_label and static_call) can
have sites in __exit code, even if __exit is never executed. Therefore
__exit must be present at runtime, at least for as long as __init code
is.

Additionally, for jump_label and static_call, the __exit sites must also
identify as within_module_init(), such that the infrastructure is aware
to never touch them after module init -- alternatives are only ran once
at init and hence don't have this particular constraint.

By making __exit identify as __init for !MODULE_UNLOAD, the above is
satisfied.

So the section ordering should look like the following when
!CONFIG_MODULE_UNLOAD, with the .exit sections moved to the init region of
the module.

Core section allocation order:
 	.text
 	.rodata
 	__ksymtab_gpl
 	__ksymtab_strings
 	.note.* sections
 	.bss
 	.data
 	.gnu.linkonce.this_module
 Init section allocation order:
 	.init.text
 	.exit.text
 	.symtab
 	.strtab

[jeyu: thanks to Peter Zijlstra for most of the changelog]

Link: https://lore.kernel.org/lkml/YFiuphGw0RKehWsQ@gunter/
Signed-off-by: Jessica Yu <jeyu@kernel.org>
---

Do you want to take this patch with the other static_call patches? Or
should I take this through modules-next?

 kernel/module.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index 30479355ab85..173a09175511 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2802,7 +2802,11 @@ void * __weak module_alloc(unsigned long size)
 
 bool __weak module_init_section(const char *name)
 {
+#ifndef CONFIG_MODULE_UNLOAD
+	return strstarts(name, ".init") || module_exit_section(name);
+#else
 	return strstarts(name, ".init");
+#endif
 }
 
 bool __weak module_exit_section(const char *name)
@@ -3116,11 +3120,6 @@ static int rewrite_section_headers(struct load_info *info, int flags)
 		 */
 		shdr->sh_addr = (size_t)info->hdr + shdr->sh_offset;
 
-#ifndef CONFIG_MODULE_UNLOAD
-		/* Don't load .exit sections */
-		if (module_exit_section(info->secstrings+shdr->sh_name))
-			shdr->sh_flags &= ~(unsigned long)SHF_ALLOC;
-#endif
 	}
 
 	/* Track but don't keep modinfo and version sections. */
-- 
2.30.1

Re: [PATCH] module: treat exit sections the same as init sections when !CONFIG_MODULE_UNLOAD

[Peter Zijlstra]

On Tue, Mar 23, 2021 at 03:27:56PM +0100, Jessica Yu wrote:
> Dynamic code patching (alternatives, jump_label and static_call) can
> have sites in __exit code, even if __exit is never executed. Therefore
> __exit must be present at runtime, at least for as long as __init code
> is.
> 
> Additionally, for jump_label and static_call, the __exit sites must also
> identify as within_module_init(), such that the infrastructure is aware
> to never touch them after module init -- alternatives are only ran once
> at init and hence don't have this particular constraint.
> 
> By making __exit identify as __init for !MODULE_UNLOAD, the above is
> satisfied.
> 
> So the section ordering should look like the following when
> !CONFIG_MODULE_UNLOAD, with the .exit sections moved to the init region of
> the module.
> 
> Core section allocation order:
>  	.text
>  	.rodata
>  	__ksymtab_gpl
>  	__ksymtab_strings
>  	.note.* sections
>  	.bss
>  	.data
>  	.gnu.linkonce.this_module
>  Init section allocation order:
>  	.init.text
>  	.exit.text
>  	.symtab
>  	.strtab
> 
> [jeyu: thanks to Peter Zijlstra for most of the changelog]
> 
> Link: https://lore.kernel.org/lkml/YFiuphGw0RKehWsQ@gunter/
> Signed-off-by: Jessica Yu <jeyu@kernel.org>

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

> ---
> 
> Do you want to take this patch with the other static_call patches? Or
> should I take this through modules-next?

Take it through modules-next, I haven't seen an actual report on this
and it's been broken forever afaict. I only found it by accident while
chasing this other problem.