From db991af02f11053558431467102ee5832894d7a4 Mon Sep 17 00:00:00 2001 From: Jessica Yu Date: Wed, 8 Apr 2020 16:31:06 +0200 Subject: module: break nested ARCH_HAS_STRICT_MODULE_RWX and STRICT_MODULE_RWX #ifdefs Various frob_* and module_{enable,disable}_* functions are defined in a CONFIG_ARCH_HAS_STRICT_MODULE_RWX ifdef block which also has a nested CONFIG_STRICT_MODULE_RWX ifdef block within it. This is unecessary and makes things hard to read. Not only that, this construction requires redundant empty stubs for module_enable_nx(). I suspect this was originally done for cosmetic reasons - to keep all the frob_* functions in the same place, and all the module_{enable,disable}_* functions right after, but as a result it made the code harder to read. Make this more readable by unnesting the ifdef blocks and getting rid of the redundant empty stubs. Signed-off-by: Jessica Yu --- kernel/module.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/kernel/module.c b/kernel/module.c index 646f1e2330d2..01d01a489778 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1943,7 +1943,6 @@ static void mod_sysfs_teardown(struct module *mod) mod_sysfs_fini(mod); } -#ifdef CONFIG_ARCH_HAS_STRICT_MODULE_RWX /* * LKM RO/NX protection: protect module's text/ro-data * from modification and any data from execution. @@ -1957,6 +1956,14 @@ static void mod_sysfs_teardown(struct module *mod) * * These values are always page-aligned (as is base) */ + +/* + * Since some arches are moving towards PAGE_KERNEL module allocations instead + * of PAGE_KERNEL_EXEC, keep frob_text() and module_enable_x() outside of the + * CONFIG_STRICT_MODULE_RWX block below because they are needed regardless of + * whether we are strict. + */ +#ifdef CONFIG_ARCH_HAS_STRICT_MODULE_RWX static void frob_text(const struct module_layout *layout, int (*set_memory)(unsigned long start, int num_pages)) { @@ -1966,6 +1973,15 @@ static void frob_text(const struct module_layout *layout, layout->text_size >> PAGE_SHIFT); } +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_x(const struct module *mod) { } +#endif /* CONFIG_ARCH_HAS_STRICT_MODULE_RWX */ + #ifdef CONFIG_STRICT_MODULE_RWX static void frob_rodata(const struct module_layout *layout, int (*set_memory)(unsigned long start, int num_pages)) @@ -2037,18 +2053,9 @@ static void module_enable_nx(const struct module *mod) } #else /* !CONFIG_STRICT_MODULE_RWX */ +/* module_{enable,disable}_ro() stubs are in module.h */ static void module_enable_nx(const struct module *mod) { } #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 /* -- cgit v1.2.3 From 5c3a7db0c7ec4bbd5bd3f48af9be859a8fa3e532 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Fri, 3 Apr 2020 19:13:03 +0200 Subject: module: Harden STRICT_MODULE_RWX We're very close to enforcing W^X memory, refuse to load modules that violate this principle per construction. [jeyu: move module_enforce_rwx_sections under STRICT_MODULE_RWX as per discussion] Link: http://lore.kernel.org/r/20200403171303.GK20760@hirez.programming.kicks-ass.net Acked-by: Kees Cook Signed-off-by: Peter Zijlstra (Intel) Signed-off-by: Jessica Yu --- kernel/module.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/kernel/module.c b/kernel/module.c index 01d01a489778..70fc20583e66 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2052,9 +2052,28 @@ static void module_enable_nx(const struct module *mod) frob_writable_data(&mod->init_layout, set_memory_nx); } +static int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs, + char *secstrings, struct module *mod) +{ + const unsigned long shf_wx = SHF_WRITE|SHF_EXECINSTR; + int i; + + for (i = 0; i < hdr->e_shnum; i++) { + if ((sechdrs[i].sh_flags & shf_wx) == shf_wx) + return -ENOEXEC; + } + + return 0; +} + #else /* !CONFIG_STRICT_MODULE_RWX */ /* module_{enable,disable}_ro() stubs are in module.h */ static void module_enable_nx(const struct module *mod) { } +static int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs, + char *secstrings, struct module *mod) +{ + return 0; +} #endif /* CONFIG_STRICT_MODULE_RWX */ #ifdef CONFIG_LIVEPATCH @@ -3385,6 +3404,11 @@ static struct module *layout_and_allocate(struct load_info *info, int flags) if (err < 0) return ERR_PTR(err); + err = module_enforce_rwx_sections(info->hdr, info->sechdrs, + info->secstrings, info->mod); + if (err < 0) + return ERR_PTR(err); + /* We will do a special allocation for per-cpu sections later. */ info->sechdrs[info->index.pcpu].sh_flags &= ~(unsigned long)SHF_ALLOC; -- cgit v1.2.3