| Message ID | 20210507121322.6441-1-jeyu@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: module: treat exit sections the same as init sections when !CONFIG_MODULE_UNLOAD | expand |
On Fri, May 07, 2021 at 02:13:22PM +0200, 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. ... > Previously, the module loader never loaded the exit sections in the first > place when CONFIG_MODULE_UNLOAD=n. Commit 33121347fb1c ("module: treat exit > sections the same as init sections when !CONFIG_MODULE_UNLOAD") addressed > the issue by having the module loader load the exit sections and then making > __exit identify as __init for !MODULE_UNLOAD. Then, since they are treated > like init sections, they will be also discarded after init. > > That commit satisfied the above requirements for jump_labels and > static_calls by modifying the checks in the core module_init_section() > function in kernel/module.c to include exit sections. However, ARM > overrides these and implements their own module_{init,exit}_section() > functions. Add a similar check for exit sections to ARM's > module_init_section() function so that all arches are on the same page. Shouldn't the module core code itself be doing: module_init_section(name) || module_exit_section(name) itself when CONFIG_MODULE_UNLOAD is not set, rather than pushing this logic down into every module_init_section() implementation?
+++ Russell King - ARM Linux admin [07/05/21 13:30 +0100]: >On Fri, May 07, 2021 at 02:13:22PM +0200, 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. >... >> Previously, the module loader never loaded the exit sections in the first >> place when CONFIG_MODULE_UNLOAD=n. Commit 33121347fb1c ("module: treat exit >> sections the same as init sections when !CONFIG_MODULE_UNLOAD") addressed >> the issue by having the module loader load the exit sections and then making >> __exit identify as __init for !MODULE_UNLOAD. Then, since they are treated >> like init sections, they will be also discarded after init. >> >> That commit satisfied the above requirements for jump_labels and >> static_calls by modifying the checks in the core module_init_section() >> function in kernel/module.c to include exit sections. However, ARM >> overrides these and implements their own module_{init,exit}_section() >> functions. Add a similar check for exit sections to ARM's >> module_init_section() function so that all arches are on the same page. > >Shouldn't the module core code itself be doing: > > module_init_section(name) || module_exit_section(name) > >itself when CONFIG_MODULE_UNLOAD is not set, rather than pushing this >logic down into every module_init_section() implementation? Yeah, that sounds better. Originally, I had wanted to keep the #ifndef in one place to keep the churn to a minimum. But seeing that we have to patch up ARM too, it's probably the less ugly option now. Let me cook up an alternative patch and resend. Thanks, Jessica
diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c index beac45e89ba6..9d403fc1893b 100644 --- a/arch/arm/kernel/module.c +++ b/arch/arm/kernel/module.c @@ -56,9 +56,16 @@ void *module_alloc(unsigned long size) bool module_init_section(const char *name) { +#ifndef CONFIG_MODULE_UNLOAD + return strstarts(name, ".init") || + strstarts(name, ".ARM.extab.init") || + strstarts(name, ".ARM.exidx.init") || + module_exit_section(name); +#else return strstarts(name, ".init") || strstarts(name, ".ARM.extab.init") || strstarts(name, ".ARM.exidx.init"); +#endif } bool module_exit_section(const char *name)
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. Previously, the module loader never loaded the exit sections in the first place when CONFIG_MODULE_UNLOAD=n. Commit 33121347fb1c ("module: treat exit sections the same as init sections when !CONFIG_MODULE_UNLOAD") addressed the issue by having the module loader load the exit sections and then making __exit identify as __init for !MODULE_UNLOAD. Then, since they are treated like init sections, they will be also discarded after init. That commit satisfied the above requirements for jump_labels and static_calls by modifying the checks in the core module_init_section() function in kernel/module.c to include exit sections. However, ARM overrides these and implements their own module_{init,exit}_section() functions. Add a similar check for exit sections to ARM's module_init_section() function so that all arches are on the same page. Fixes: 33121347fb1c ("module: treat exit sections the same as init sections when !CONFIG_MODULE_UNLOAD") Link: https://lore.kernel.org/lkml/YFiuphGw0RKehWsQ@gunter/ Link: https://lore.kernel.org/r/20210323142756.11443-1-jeyu@kernel.org Signed-off-by: Jessica Yu <jeyu@kernel.org> --- arch/arm/kernel/module.c | 7 +++++++ 1 file changed, 7 insertions(+)