From ac88ee7d2b87c1f93b89fd9ce5911c2ab2bda816 Mon Sep 17 00:00:00 2001 From: Christophe Leroy Date: Thu, 21 Dec 2023 08:24:23 +0100 Subject: [PATCH 1/7] module: Use set_memory_rox() A couple of architectures seem concerned about calling set_memory_ro() and set_memory_x() too frequently and have implemented a version of set_memory_rox(), see commit 60463628c9e0 ("x86/mm: Implement native set_memory_rox()") and commit 22e99fa56443 ("s390/mm: implement set_memory_rox()") Use set_memory_rox() in modules when STRICT_MODULES_RWX is set. Signed-off-by: Christophe Leroy Signed-off-by: Luis Chamberlain --- kernel/module/internal.h | 2 +- kernel/module/main.c | 2 +- kernel/module/strict_rwx.c | 12 +++++++----- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/kernel/module/internal.h b/kernel/module/internal.h index c8b7b4dcf782..a647ab17193d 100644 --- a/kernel/module/internal.h +++ b/kernel/module/internal.h @@ -324,7 +324,7 @@ static inline struct module *mod_find(unsigned long addr, struct mod_tree_root * void module_enable_ro(const struct module *mod, bool after_init); void module_enable_nx(const struct module *mod); -void module_enable_x(const struct module *mod); +void module_enable_rox(const struct module *mod); int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs, char *secstrings, struct module *mod); diff --git a/kernel/module/main.c b/kernel/module/main.c index 36681911c05a..2e0187e16669 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -2738,7 +2738,7 @@ static int complete_formation(struct module *mod, struct load_info *info) module_enable_ro(mod, false); module_enable_nx(mod); - module_enable_x(mod); + module_enable_rox(mod); /* * Mark state as coming so strong_try_module_get() ignores us, diff --git a/kernel/module/strict_rwx.c b/kernel/module/strict_rwx.c index a2b656b4e3d2..9345b09f28a5 100644 --- a/kernel/module/strict_rwx.c +++ b/kernel/module/strict_rwx.c @@ -26,10 +26,14 @@ static void module_set_memory(const struct module *mod, enum mod_mem_type type, * CONFIG_STRICT_MODULE_RWX because they are needed regardless of whether we * are strict. */ -void module_enable_x(const struct module *mod) +void module_enable_rox(const struct module *mod) { - for_class_mod_mem_type(type, text) - module_set_memory(mod, type, set_memory_x); + for_class_mod_mem_type(type, text) { + if (IS_ENABLED(CONFIG_STRICT_MODULE_RWX)) + module_set_memory(mod, type, set_memory_rox); + else + module_set_memory(mod, type, set_memory_x); + } } void module_enable_ro(const struct module *mod, bool after_init) @@ -41,8 +45,6 @@ void module_enable_ro(const struct module *mod, bool after_init) return; #endif - module_set_memory(mod, MOD_TEXT, set_memory_ro); - module_set_memory(mod, MOD_INIT_TEXT, set_memory_ro); module_set_memory(mod, MOD_RODATA, set_memory_ro); module_set_memory(mod, MOD_INIT_RODATA, set_memory_ro); From 3559ad395bf02f3dee576dc9acab4ce330ce57b5 Mon Sep 17 00:00:00 2001 From: Christophe Leroy Date: Thu, 21 Dec 2023 08:24:24 +0100 Subject: [PATCH 2/7] module: Change module_enable_{nx/x/ro}() to more explicit names It's a bit puzzling to see a call to module_enable_nx() followed by a call to module_enable_x(). This is because one applies on text while the other applies on data. Change name to make that more clear. Signed-off-by: Christophe Leroy Signed-off-by: Luis Chamberlain --- kernel/module/internal.h | 6 +++--- kernel/module/main.c | 8 ++++---- kernel/module/strict_rwx.c | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/kernel/module/internal.h b/kernel/module/internal.h index a647ab17193d..4f1b98f011da 100644 --- a/kernel/module/internal.h +++ b/kernel/module/internal.h @@ -322,9 +322,9 @@ static inline struct module *mod_find(unsigned long addr, struct mod_tree_root * } #endif /* CONFIG_MODULES_TREE_LOOKUP */ -void module_enable_ro(const struct module *mod, bool after_init); -void module_enable_nx(const struct module *mod); -void module_enable_rox(const struct module *mod); +void module_enable_rodata_ro(const struct module *mod, bool after_init); +void module_enable_data_nx(const struct module *mod); +void module_enable_text_rox(const struct module *mod); int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs, char *secstrings, struct module *mod); diff --git a/kernel/module/main.c b/kernel/module/main.c index 2e0187e16669..a9a4a4885102 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -2571,7 +2571,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 - module_enable_ro(mod, true); + module_enable_rodata_ro(mod, true); mod_tree_remove_init(mod); module_arch_freeing_init(mod); for_class_mod_mem_type(type, init) { @@ -2736,9 +2736,9 @@ 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); - module_enable_ro(mod, false); - module_enable_nx(mod); - module_enable_rox(mod); + module_enable_rodata_ro(mod, false); + module_enable_data_nx(mod); + module_enable_text_rox(mod); /* * Mark state as coming so strong_try_module_get() ignores us, diff --git a/kernel/module/strict_rwx.c b/kernel/module/strict_rwx.c index 9345b09f28a5..9b2d58a8d59d 100644 --- a/kernel/module/strict_rwx.c +++ b/kernel/module/strict_rwx.c @@ -26,7 +26,7 @@ static void module_set_memory(const struct module *mod, enum mod_mem_type type, * CONFIG_STRICT_MODULE_RWX because they are needed regardless of whether we * are strict. */ -void module_enable_rox(const struct module *mod) +void module_enable_text_rox(const struct module *mod) { for_class_mod_mem_type(type, text) { if (IS_ENABLED(CONFIG_STRICT_MODULE_RWX)) @@ -36,7 +36,7 @@ void module_enable_rox(const struct module *mod) } } -void module_enable_ro(const struct module *mod, bool after_init) +void module_enable_rodata_ro(const struct module *mod, bool after_init) { if (!IS_ENABLED(CONFIG_STRICT_MODULE_RWX)) return; @@ -52,7 +52,7 @@ void module_enable_ro(const struct module *mod, bool after_init) module_set_memory(mod, MOD_RO_AFTER_INIT, set_memory_ro); } -void module_enable_nx(const struct module *mod) +void module_enable_data_nx(const struct module *mod) { if (!IS_ENABLED(CONFIG_STRICT_MODULE_RWX)) return; From 398ec3e925eb1c4d5850ec60f7075e0c20199003 Mon Sep 17 00:00:00 2001 From: Christophe Leroy Date: Thu, 21 Dec 2023 10:02:46 +0100 Subject: [PATCH 3/7] init: Declare rodata_enabled and mark_rodata_ro() at all time Declaring rodata_enabled and mark_rodata_ro() at all time helps removing related #ifdefery in C files. Signed-off-by: Christophe Leroy Signed-off-by: Luis Chamberlain --- include/linux/init.h | 4 ---- init/main.c | 21 +++++++-------------- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/include/linux/init.h b/include/linux/init.h index 3fa3f6241350..58cef4c2e59a 100644 --- a/include/linux/init.h +++ b/include/linux/init.h @@ -168,12 +168,8 @@ extern initcall_entry_t __initcall_end[]; extern struct file_system_type rootfs_fs_type; -#if defined(CONFIG_STRICT_KERNEL_RWX) || defined(CONFIG_STRICT_MODULE_RWX) extern bool rodata_enabled; -#endif -#ifdef CONFIG_STRICT_KERNEL_RWX void mark_rodata_ro(void); -#endif extern void (*late_time_init)(void); diff --git a/init/main.c b/init/main.c index e24b0780fdff..807df08c501f 100644 --- a/init/main.c +++ b/init/main.c @@ -1396,10 +1396,9 @@ static int __init set_debug_rodata(char *str) early_param("rodata", set_debug_rodata); #endif -#ifdef CONFIG_STRICT_KERNEL_RWX static void mark_readonly(void) { - if (rodata_enabled) { + if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX) && rodata_enabled) { /* * load_module() results in W+X mappings, which are cleaned * up with call_rcu(). Let's make sure that queued work is @@ -1409,20 +1408,14 @@ static void mark_readonly(void) rcu_barrier(); mark_rodata_ro(); rodata_test(); - } else + } else if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX)) { pr_info("Kernel memory protection disabled.\n"); + } else if (IS_ENABLED(CONFIG_ARCH_HAS_STRICT_KERNEL_RWX)) { + pr_warn("Kernel memory protection not selected by kernel config.\n"); + } else { + pr_warn("This architecture does not have kernel memory protection.\n"); + } } -#elif defined(CONFIG_ARCH_HAS_STRICT_KERNEL_RWX) -static inline void mark_readonly(void) -{ - pr_warn("Kernel memory protection not selected by kernel config.\n"); -} -#else -static inline void mark_readonly(void) -{ - pr_warn("This architecture does not have kernel memory protection.\n"); -} -#endif void __weak free_initmem(void) { From 315df9c476c587af26467f10c6f27414572f3938 Mon Sep 17 00:00:00 2001 From: Christophe Leroy Date: Thu, 21 Dec 2023 10:02:47 +0100 Subject: [PATCH 4/7] modules: Remove #ifdef CONFIG_STRICT_MODULE_RWX around rodata_enabled Now that rodata_enabled is declared at all time, the #ifdef CONFIG_STRICT_MODULE_RWX can be removed. Signed-off-by: Christophe Leroy Signed-off-by: Luis Chamberlain --- kernel/module/strict_rwx.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/kernel/module/strict_rwx.c b/kernel/module/strict_rwx.c index 9b2d58a8d59d..b36d93983465 100644 --- a/kernel/module/strict_rwx.c +++ b/kernel/module/strict_rwx.c @@ -38,12 +38,8 @@ void module_enable_text_rox(const struct module *mod) void module_enable_rodata_ro(const struct module *mod, bool after_init) { - if (!IS_ENABLED(CONFIG_STRICT_MODULE_RWX)) + if (!IS_ENABLED(CONFIG_STRICT_MODULE_RWX) || !rodata_enabled) return; -#ifdef CONFIG_STRICT_MODULE_RWX - if (!rodata_enabled) - return; -#endif module_set_memory(mod, MOD_RODATA, set_memory_ro); module_set_memory(mod, MOD_INIT_RODATA, set_memory_ro); From 79d9f965ecfd765b7e2aaf198516d011cfcaa57e Mon Sep 17 00:00:00 2001 From: Christophe Leroy Date: Thu, 21 Dec 2023 10:02:48 +0100 Subject: [PATCH 5/7] powerpc: Simplify strict_kernel_rwx_enabled() Now that rodata_enabled is always declared, remove #ifdef and define a single version of strict_kernel_rwx_enabled(). Signed-off-by: Christophe Leroy Signed-off-by: Luis Chamberlain --- arch/powerpc/include/asm/mmu.h | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h index d8b7e246a32f..24241995f740 100644 --- a/arch/powerpc/include/asm/mmu.h +++ b/arch/powerpc/include/asm/mmu.h @@ -330,17 +330,10 @@ static __always_inline bool early_radix_enabled(void) return early_mmu_has_feature(MMU_FTR_TYPE_RADIX); } -#ifdef CONFIG_STRICT_KERNEL_RWX static inline bool strict_kernel_rwx_enabled(void) { - return rodata_enabled; + return IS_ENABLED(CONFIG_STRICT_KERNEL_RWX) && rodata_enabled; } -#else -static inline bool strict_kernel_rwx_enabled(void) -{ - return false; -} -#endif static inline bool strict_module_rwx_enabled(void) { From 157285397f6a7b35d8f7f115e1be24f49e947ba3 Mon Sep 17 00:00:00 2001 From: Randy Dunlap Date: Fri, 3 Nov 2023 21:20:44 -0700 Subject: [PATCH 6/7] lib/test_kmod: fix kernel-doc warnings Fix all kernel-doc warnings in test_kmod.c: - Mark some enum values as private so that kernel-doc is not needed for them - s/thread_mutex/thread_lock/ in a struct's kernel-doc comments - add kernel-doc info for @task_sync test_kmod.c:67: warning: Enum value '__TEST_KMOD_INVALID' not described in enum 'kmod_test_case' test_kmod.c:67: warning: Enum value '__TEST_KMOD_MAX' not described in enum 'kmod_test_case' test_kmod.c:100: warning: Function parameter or member 'task_sync' not described in 'kmod_test_device_info' test_kmod.c:134: warning: Function parameter or member 'thread_mutex' not described in 'kmod_test_device' Signed-off-by: Randy Dunlap Cc: Luis Chamberlain Cc: Signed-off-by: Luis Chamberlain --- lib/test_kmod.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/test_kmod.c b/lib/test_kmod.c index 43d9dfd57ab7..1eec3b7ac67c 100644 --- a/lib/test_kmod.c +++ b/lib/test_kmod.c @@ -58,11 +58,14 @@ static int num_test_devs; * @need_mod_put for your tests case. */ enum kmod_test_case { + /* private: */ __TEST_KMOD_INVALID = 0, + /* public: */ TEST_KMOD_DRIVER, TEST_KMOD_FS_TYPE, + /* private: */ __TEST_KMOD_MAX, }; @@ -82,6 +85,7 @@ struct kmod_test_device; * @ret_sync: return value if request_module() is used, sync request for * @TEST_KMOD_DRIVER * @fs_sync: return value of get_fs_type() for @TEST_KMOD_FS_TYPE + * @task_sync: kthread's task_struct or %NULL if not running * @thread_idx: thread ID * @test_dev: test device test is being performed under * @need_mod_put: Some tests (get_fs_type() is one) requires putting the module @@ -108,7 +112,7 @@ struct kmod_test_device_info { * @dev: pointer to misc_dev's own struct device * @config_mutex: protects configuration of test * @trigger_mutex: the test trigger can only be fired once at a time - * @thread_lock: protects @done count, and the @info per each thread + * @thread_mutex: protects @done count, and the @info per each thread * @done: number of threads which have completed or failed * @test_is_oom: when we run out of memory, use this to halt moving forward * @kthreads_done: completion used to signal when all work is done From d1909c0221739356f31c721de4743e7d219a56cc Mon Sep 17 00:00:00 2001 From: Christophe Leroy Date: Fri, 16 Feb 2024 09:14:27 +0100 Subject: [PATCH 7/7] module: Don't ignore errors from set_memory_XX() set_memory_ro(), set_memory_nx(), set_memory_x() and other helpers can fail and return an error. In that case the memory might not be protected as expected and the module loading has to be aborted to avoid security issues. Check return value of all calls to set_memory_XX() and handle error if any. Add a check to not call set_memory_XX() on NULL pointers as some architectures may not like it allthough numpages is always 0 in that case. This also avoid a useless call to set_vm_flush_reset_perms(). Link: https://github.com/KSPP/linux/issues/7 Signed-off-by: Christophe Leroy Tested-by: Marek Szyprowski Reviewed-by: Kees Cook Signed-off-by: Luis Chamberlain --- kernel/module/internal.h | 6 ++--- kernel/module/main.c | 20 +++++++++++--- kernel/module/strict_rwx.c | 53 ++++++++++++++++++++++++++------------ 3 files changed, 56 insertions(+), 23 deletions(-) diff --git a/kernel/module/internal.h b/kernel/module/internal.h index 4f1b98f011da..2ebece8a789f 100644 --- a/kernel/module/internal.h +++ b/kernel/module/internal.h @@ -322,9 +322,9 @@ static inline struct module *mod_find(unsigned long addr, struct mod_tree_root * } #endif /* CONFIG_MODULES_TREE_LOOKUP */ -void module_enable_rodata_ro(const struct module *mod, bool after_init); -void module_enable_data_nx(const struct module *mod); -void module_enable_text_rox(const struct module *mod); +int module_enable_rodata_ro(const struct module *mod, bool after_init); +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, char *secstrings, struct module *mod); diff --git a/kernel/module/main.c b/kernel/module/main.c index a9a4a4885102..689def7676c4 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -2571,7 +2571,9 @@ 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 - module_enable_rodata_ro(mod, true); + ret = module_enable_rodata_ro(mod, true); + if (ret) + goto fail_mutex_unlock; mod_tree_remove_init(mod); module_arch_freeing_init(mod); for_class_mod_mem_type(type, init) { @@ -2609,6 +2611,8 @@ static noinline int do_init_module(struct module *mod) return 0; +fail_mutex_unlock: + mutex_unlock(&module_mutex); fail_free_freeinit: kfree(freeinit); fail: @@ -2736,9 +2740,15 @@ 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); - module_enable_rodata_ro(mod, false); - module_enable_data_nx(mod); - module_enable_text_rox(mod); + err = module_enable_rodata_ro(mod, false); + if (err) + goto out_strict_rwx; + err = module_enable_data_nx(mod); + if (err) + goto out_strict_rwx; + err = module_enable_text_rox(mod); + if (err) + goto out_strict_rwx; /* * Mark state as coming so strong_try_module_get() ignores us, @@ -2749,6 +2759,8 @@ static int complete_formation(struct module *mod, struct load_info *info) return 0; +out_strict_rwx: + module_bug_cleanup(mod); out: mutex_unlock(&module_mutex); return err; diff --git a/kernel/module/strict_rwx.c b/kernel/module/strict_rwx.c index b36d93983465..c45caa4690e5 100644 --- a/kernel/module/strict_rwx.c +++ b/kernel/module/strict_rwx.c @@ -11,13 +11,16 @@ #include #include "internal.h" -static void module_set_memory(const struct module *mod, enum mod_mem_type type, - int (*set_memory)(unsigned long start, int num_pages)) +static int module_set_memory(const struct module *mod, enum mod_mem_type type, + int (*set_memory)(unsigned long start, int num_pages)) { const struct module_memory *mod_mem = &mod->mem[type]; + if (!mod_mem->base) + return 0; + set_vm_flush_reset_perms(mod_mem->base); - set_memory((unsigned long)mod_mem->base, mod_mem->size >> PAGE_SHIFT); + return set_memory((unsigned long)mod_mem->base, mod_mem->size >> PAGE_SHIFT); } /* @@ -26,35 +29,53 @@ static void module_set_memory(const struct module *mod, enum mod_mem_type type, * CONFIG_STRICT_MODULE_RWX because they are needed regardless of whether we * are strict. */ -void module_enable_text_rox(const struct module *mod) +int module_enable_text_rox(const struct module *mod) { for_class_mod_mem_type(type, text) { + int ret; + if (IS_ENABLED(CONFIG_STRICT_MODULE_RWX)) - module_set_memory(mod, type, set_memory_rox); + ret = module_set_memory(mod, type, set_memory_rox); else - module_set_memory(mod, type, set_memory_x); + ret = module_set_memory(mod, type, set_memory_x); + if (ret) + return ret; } + return 0; } -void module_enable_rodata_ro(const struct module *mod, bool after_init) +int module_enable_rodata_ro(const struct module *mod, bool after_init) { - if (!IS_ENABLED(CONFIG_STRICT_MODULE_RWX) || !rodata_enabled) - return; + int ret; - module_set_memory(mod, MOD_RODATA, set_memory_ro); - module_set_memory(mod, MOD_INIT_RODATA, set_memory_ro); + if (!IS_ENABLED(CONFIG_STRICT_MODULE_RWX) || !rodata_enabled) + return 0; + + ret = module_set_memory(mod, MOD_RODATA, set_memory_ro); + if (ret) + return ret; + ret = module_set_memory(mod, MOD_INIT_RODATA, set_memory_ro); + if (ret) + return ret; if (after_init) - module_set_memory(mod, MOD_RO_AFTER_INIT, set_memory_ro); + return module_set_memory(mod, MOD_RO_AFTER_INIT, set_memory_ro); + + return 0; } -void module_enable_data_nx(const struct module *mod) +int module_enable_data_nx(const struct module *mod) { if (!IS_ENABLED(CONFIG_STRICT_MODULE_RWX)) - return; + return 0; - for_class_mod_mem_type(type, data) - module_set_memory(mod, type, set_memory_nx); + for_class_mod_mem_type(type, data) { + int ret = module_set_memory(mod, type, set_memory_nx); + + if (ret) + return ret; + } + return 0; } int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,