Messages in this thread Patch in this message | | | Date | Tue, 25 Jun 2019 21:21:15 +0200 | From | Jessica Yu <> | Subject | Re: [PATCH] modules: fix compile error if don't have strict module rwx |
| |
+++ Yang Yingliang [25/06/19 17:40 +0800]: >If CONFIG_ARCH_HAS_STRICT_MODULE_RWX is not defined, >we need stub for module_enable_nx() and module_enable_x(). > >If CONFIG_ARCH_HAS_STRICT_MODULE_RWX is defined, but >CONFIG_STRICT_MODULE_RWX is disabled, we need stub for >module_enable_nx. > >Move frob_text() outside of the CONFIG_STRICT_MODULE_RWX, >because it is needed anyway.
Maybe include a fixes tag?
Fixes: 2eef1399a866 ("modules: fix BUG when load module with rodata=n")
>Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> >--- > kernel/module.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > >diff --git a/kernel/module.c b/kernel/module.c >index c3ae34c..cfff441 100644 >--- a/kernel/module.c >+++ b/kernel/module.c >@@ -1875,7 +1875,7 @@ static void mod_sysfs_teardown(struct module *mod) > mod_sysfs_fini(mod); > } > >-#ifdef CONFIG_STRICT_MODULE_RWX >+#ifdef CONFIG_ARCH_HAS_STRICT_MODULE_RWX
Could you please explain why you introduced a new CONFIG_ARCH_HAS_STRICT_MODULE_RWX #ifdef block instead of just moving frob_text() and module_enable_x() outside of CONFIG_STRICT_MODULE_RWX?
I do not have anything against it, although the nested #ifdef's are a bit painful to read. But I could not find a better way to do it :/ It's awkward because we need module_enable_x() and frob_text() regardless of of CONFIG_STRICT_MODULE_RWX for x86, but other arches don't need to call module_enable_x(), they usually just call the empty stub.
But I think having the CONFIG_ARCH_HAS_STRICT_MODULE_RWX block is OK, for the reason of limiting the scope of the calls rather than blanketly calling frob_text() andd module_enable_x() for arches that don't need to call them. Was that your reasoning as well?
Thanks,
Jessica
> /* > * LKM RO/NX protection: protect module's text/ro-data > * from modification and any data from execution. >@@ -1898,6 +1898,7 @@ static void frob_text(const struct module_layout *layout, > layout->text_size >> PAGE_SHIFT); > } > >+#ifdef CONFIG_STRICT_MODULE_RWX > static void frob_rodata(const struct module_layout *layout, > int (*set_memory)(unsigned long start, int num_pages)) > { >@@ -2010,15 +2011,19 @@ void set_all_modules_text_ro(void) > } > mutex_unlock(&module_mutex); > } >-#else >+#else /* !CONFIG_STRICT_MODULE_RWX */ > static void module_enable_nx(const struct module *mod) { } >-#endif >- >+#endif /* CONFIG_STRICT_MODULE_RWX */ > static void module_enable_x(const struct module *mod) > { > frob_text(&mod->core_layout, set_memory_x); > frob_text(&mod->init_layout, set_memory_x); > } >+#else /* !CONFIG_ARCH_HAS_STRICT_MODULE_RWX */ >+static void module_enable_nx(const struct module *mod) { } >+static void module_enable_x(const struct module *mod) { } >+#endif /* CONFIG_ARCH_HAS_STRICT_MODULE_RWX */ >+ > > #ifdef CONFIG_LIVEPATCH > /* >-- >1.8.3 >
| |