+++ 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@xxxxxxxxxx> --- 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