| Message ID | 737f952790c96a09ad5e51689918b97ef9b29174.1731148254.git.christophe.leroy@csgroup.eu (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [RFC,1/3] module: Split module_enable_rodata_ro() | expand |
| Context | Check | Description |
|---|---|---|
| mcgrof/vmtest-modules-next-VM_Test-0 | fail | Logs for Run CI tests |
| mcgrof/vmtest-modules-next-VM_Test-1 | fail | Logs for Run CI tests |
| mcgrof/vmtest-modules-next-VM_Test-4 | success | Logs for setup / Setup kdevops environment |
| mcgrof/vmtest-modules-next-VM_Test-5 | success | Logs for setup / Setup kdevops environment |
| mcgrof/vmtest-modules-next-PR | fail | PR summary |
| mcgrof/vmtest-modules-next-VM_Test-2 | success | Logs for cleanup / Archive results and cleanup |
| mcgrof/vmtest-modules-next-VM_Test-3 | success | Logs for cleanup / Archive results and cleanup |
On Sat Nov 9, 2024 at 11:35 AM CET, Christophe Leroy wrote: > module_enable_rodata_ro() is called twice, once before module init > to set rodata sections readonly and once after module init to set > rodata_after_init section readonly. > > The second time, only the rodata_after_init section needs to be > set to read-only, no need to re-apply it to already set rodata. > > Split module_enable_rodata_ro() in two. > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> Tested-by: Daniel Gomez <da.gomez@samsung.com> > --- > kernel/module/internal.h | 3 ++- > kernel/module/main.c | 4 ++-- > kernel/module/strict_rwx.c | 13 +++++++++---- > 3 files changed, 13 insertions(+), 7 deletions(-) > > diff --git a/kernel/module/internal.h b/kernel/module/internal.h > index 2ebece8a789f..994f35a779dc 100644 > --- a/kernel/module/internal.h > +++ b/kernel/module/internal.h > @@ -322,7 +322,8 @@ static inline struct module *mod_find(unsigned long addr, struct mod_tree_root * > } > #endif /* CONFIG_MODULES_TREE_LOOKUP */ > > -int module_enable_rodata_ro(const struct module *mod, bool after_init); > +int module_enable_rodata_ro(const struct module *mod); > +int module_enable_rodata_ro_after_init(const struct module *mod); > int module_enable_data_nx(const struct module *mod); > int module_enable_text_rox(const struct module *mod); > int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs, > diff --git a/kernel/module/main.c b/kernel/module/main.c > index 49b9bca9de12..2de4ad7af335 100644 > --- a/kernel/module/main.c > +++ b/kernel/module/main.c > @@ -2581,7 +2581,7 @@ static noinline int do_init_module(struct module *mod) > /* Switch to core kallsyms now init is done: kallsyms may be walking! */ > rcu_assign_pointer(mod->kallsyms, &mod->core_kallsyms); > #endif > - ret = module_enable_rodata_ro(mod, true); > + ret = module_enable_rodata_ro_after_init(mod); > if (ret) > goto fail_mutex_unlock; > mod_tree_remove_init(mod); > @@ -2751,7 +2751,7 @@ static int complete_formation(struct module *mod, struct load_info *info) > module_bug_finalize(info->hdr, info->sechdrs, mod); > module_cfi_finalize(info->hdr, info->sechdrs, mod); > > - err = module_enable_rodata_ro(mod, false); > + err = module_enable_rodata_ro(mod); > if (err) > goto out_strict_rwx; > err = module_enable_data_nx(mod); > diff --git a/kernel/module/strict_rwx.c b/kernel/module/strict_rwx.c > index c45caa4690e5..f68c59974ae2 100644 > --- a/kernel/module/strict_rwx.c > +++ b/kernel/module/strict_rwx.c > @@ -44,7 +44,7 @@ int module_enable_text_rox(const struct module *mod) > return 0; > } > > -int module_enable_rodata_ro(const struct module *mod, bool after_init) > +int module_enable_rodata_ro(const struct module *mod) > { > int ret; > > @@ -58,12 +58,17 @@ int module_enable_rodata_ro(const struct module *mod, bool after_init) > if (ret) > return ret; > > - if (after_init) > - return module_set_memory(mod, MOD_RO_AFTER_INIT, set_memory_ro); > - > return 0; > } > > +int module_enable_rodata_ro_after_init(const struct module *mod) > +{ > + if (!IS_ENABLED(CONFIG_STRICT_MODULE_RWX) || !rodata_enabled) > + return 0; > + > + return module_set_memory(mod, MOD_RO_AFTER_INIT, set_memory_ro); > +} > + > int module_enable_data_nx(const struct module *mod) > { > if (!IS_ENABLED(CONFIG_STRICT_MODULE_RWX))
On Sat, Nov 09, 2024 at 11:35:35AM +0100, Christophe Leroy wrote: > module_enable_rodata_ro() is called twice, once before module init > to set rodata sections readonly and once after module init to set > rodata_after_init section readonly. > > The second time, only the rodata_after_init section needs to be > set to read-only, no need to re-apply it to already set rodata. > > Split module_enable_rodata_ro() in two. > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> Didn't see a respin so this will have to be a post v6.13-rc1 fix. Luis
Le 26/11/2024 à 20:58, Luis Chamberlain a écrit : > On Sat, Nov 09, 2024 at 11:35:35AM +0100, Christophe Leroy wrote: >> module_enable_rodata_ro() is called twice, once before module init >> to set rodata sections readonly and once after module init to set >> rodata_after_init section readonly. >> >> The second time, only the rodata_after_init section needs to be >> set to read-only, no need to re-apply it to already set rodata. >> >> Split module_enable_rodata_ro() in two. >> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > > Didn't see a respin so this will have to be a post v6.13-rc1 fix. Indeed I was waiting for v6.13-rc1 before sending the non RFC rebased version, but I can send it now if you prefer. I expect it to spend a few days in linux-next before being applied to mainline. Christophe
On Wed, Nov 27, 2024 at 02:37:40PM +0100, Christophe Leroy wrote: > > > Le 26/11/2024 à 20:58, Luis Chamberlain a écrit : > > On Sat, Nov 09, 2024 at 11:35:35AM +0100, Christophe Leroy wrote: > > > module_enable_rodata_ro() is called twice, once before module init > > > to set rodata sections readonly and once after module init to set > > > rodata_after_init section readonly. > > > > > > The second time, only the rodata_after_init section needs to be > > > set to read-only, no need to re-apply it to already set rodata. > > > > > > Split module_enable_rodata_ro() in two. > > > > > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > > > > Didn't see a respin so this will have to be a post v6.13-rc1 fix. > > Indeed I was waiting for v6.13-rc1 before sending the non RFC rebased > version, but I can send it now if you prefer. > > I expect it to spend a few days in linux-next before being applied to > mainline. No rush on my end, let's wait as you noted until v6.13-rc1 is out. Luis
diff --git a/kernel/module/internal.h b/kernel/module/internal.h index 2ebece8a789f..994f35a779dc 100644 --- a/kernel/module/internal.h +++ b/kernel/module/internal.h @@ -322,7 +322,8 @@ static inline struct module *mod_find(unsigned long addr, struct mod_tree_root * } #endif /* CONFIG_MODULES_TREE_LOOKUP */ -int module_enable_rodata_ro(const struct module *mod, bool after_init); +int module_enable_rodata_ro(const struct module *mod); +int module_enable_rodata_ro_after_init(const struct module *mod); int module_enable_data_nx(const struct module *mod); int module_enable_text_rox(const struct module *mod); int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs, diff --git a/kernel/module/main.c b/kernel/module/main.c index 49b9bca9de12..2de4ad7af335 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -2581,7 +2581,7 @@ static noinline int do_init_module(struct module *mod) /* Switch to core kallsyms now init is done: kallsyms may be walking! */ rcu_assign_pointer(mod->kallsyms, &mod->core_kallsyms); #endif - ret = module_enable_rodata_ro(mod, true); + ret = module_enable_rodata_ro_after_init(mod); if (ret) goto fail_mutex_unlock; mod_tree_remove_init(mod); @@ -2751,7 +2751,7 @@ static int complete_formation(struct module *mod, struct load_info *info) module_bug_finalize(info->hdr, info->sechdrs, mod); module_cfi_finalize(info->hdr, info->sechdrs, mod); - err = module_enable_rodata_ro(mod, false); + err = module_enable_rodata_ro(mod); if (err) goto out_strict_rwx; err = module_enable_data_nx(mod); diff --git a/kernel/module/strict_rwx.c b/kernel/module/strict_rwx.c index c45caa4690e5..f68c59974ae2 100644 --- a/kernel/module/strict_rwx.c +++ b/kernel/module/strict_rwx.c @@ -44,7 +44,7 @@ int module_enable_text_rox(const struct module *mod) return 0; } -int module_enable_rodata_ro(const struct module *mod, bool after_init) +int module_enable_rodata_ro(const struct module *mod) { int ret; @@ -58,12 +58,17 @@ int module_enable_rodata_ro(const struct module *mod, bool after_init) if (ret) return ret; - if (after_init) - return module_set_memory(mod, MOD_RO_AFTER_INIT, set_memory_ro); - return 0; } +int module_enable_rodata_ro_after_init(const struct module *mod) +{ + if (!IS_ENABLED(CONFIG_STRICT_MODULE_RWX) || !rodata_enabled) + return 0; + + return module_set_memory(mod, MOD_RO_AFTER_INIT, set_memory_ro); +} + int module_enable_data_nx(const struct module *mod) { if (!IS_ENABLED(CONFIG_STRICT_MODULE_RWX))
module_enable_rodata_ro() is called twice, once before module init to set rodata sections readonly and once after module init to set rodata_after_init section readonly. The second time, only the rodata_after_init section needs to be set to read-only, no need to re-apply it to already set rodata. Split module_enable_rodata_ro() in two. Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> --- kernel/module/internal.h | 3 ++- kernel/module/main.c | 4 ++-- kernel/module/strict_rwx.c | 13 +++++++++---- 3 files changed, 13 insertions(+), 7 deletions(-)