From 6e6de3dee51a439f76eb73c22ae2ffd2c9384712 Mon Sep 17 00:00:00 2001 From: Prarit Bhargava Date: Wed, 29 May 2019 07:26:25 -0400 Subject: kernel/module.c: Only return -EEXIST for modules that have finished loading Microsoft HyperV disables the X86_FEATURE_SMCA bit on AMD systems, and linux guests boot with repeated errors: amd64_edac_mod: Unknown symbol amd_unregister_ecc_decoder (err -2) amd64_edac_mod: Unknown symbol amd_register_ecc_decoder (err -2) amd64_edac_mod: Unknown symbol amd_report_gart_errors (err -2) amd64_edac_mod: Unknown symbol amd_unregister_ecc_decoder (err -2) amd64_edac_mod: Unknown symbol amd_register_ecc_decoder (err -2) amd64_edac_mod: Unknown symbol amd_report_gart_errors (err -2) The warnings occur because the module code erroneously returns -EEXIST for modules that have failed to load and are in the process of being removed from the module list. module amd64_edac_mod has a dependency on module edac_mce_amd. Using modules.dep, systemd will load edac_mce_amd for every request of amd64_edac_mod. When the edac_mce_amd module loads, the module has state MODULE_STATE_UNFORMED and once the module load fails and the state becomes MODULE_STATE_GOING. Another request for edac_mce_amd module executes and add_unformed_module() will erroneously return -EEXIST even though the previous instance of edac_mce_amd has MODULE_STATE_GOING. Upon receiving -EEXIST, systemd attempts to load amd64_edac_mod, which fails because of unknown symbols from edac_mce_amd. add_unformed_module() must wait to return for any case other than MODULE_STATE_LIVE to prevent a race between multiple loads of dependent modules. Signed-off-by: Prarit Bhargava Signed-off-by: Barret Rhoden Cc: David Arcari Cc: Jessica Yu Cc: Heiko Carstens Signed-off-by: Jessica Yu --- kernel/module.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/kernel/module.c b/kernel/module.c index 6e6712b3aaf5..1e7dcbe527af 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -3397,8 +3397,7 @@ static bool finished_loading(const char *name) sched_annotate_sleep(); mutex_lock(&module_mutex); mod = find_module_all(name, strlen(name), true); - ret = !mod || mod->state == MODULE_STATE_LIVE - || mod->state == MODULE_STATE_GOING; + ret = !mod || mod->state == MODULE_STATE_LIVE; mutex_unlock(&module_mutex); return ret; @@ -3588,8 +3587,7 @@ again: mutex_lock(&module_mutex); old = find_module_all(mod->name, strlen(mod->name), true); if (old != NULL) { - if (old->state == MODULE_STATE_COMING - || old->state == MODULE_STATE_UNFORMED) { + if (old->state != MODULE_STATE_LIVE) { /* Wait in case it fails to load. */ mutex_unlock(&module_mutex); err = wait_event_interruptible(module_wq, -- cgit v1.2.3 From 8d1b73dd25ff92c3fa9807a20c22fa2b44c07336 Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Thu, 6 Jun 2019 13:18:53 -0500 Subject: kernel: module: Use struct_size() helper One of the more common cases of allocation size calculations is finding the size of a structure that has a zero-sized array at the end, along with memory for some number of elements for that array. For example: struct module_sect_attrs { ... struct module_sect_attr attrs[0]; }; Make use of the struct_size() helper instead of an open-coded version in order to avoid any potential type mistakes. So, replace the following form: sizeof(*sect_attrs) + nloaded * sizeof(sect_attrs->attrs[0] with: struct_size(sect_attrs, attrs, nloaded) This code was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva Signed-off-by: Jessica Yu --- kernel/module.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/kernel/module.c b/kernel/module.c index 1e7dcbe527af..1a0d8cab9eb7 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1504,8 +1504,7 @@ static void add_sect_attrs(struct module *mod, const struct load_info *info) for (i = 0; i < info->hdr->e_shnum; i++) if (!sect_empty(&info->sechdrs[i])) nloaded++; - size[0] = ALIGN(sizeof(*sect_attrs) - + nloaded * sizeof(sect_attrs->attrs[0]), + size[0] = ALIGN(struct_size(sect_attrs, attrs, nloaded), sizeof(sect_attrs->grp.attrs[0])); size[1] = (nloaded + 1) * sizeof(sect_attrs->grp.attrs[0]); sect_attrs = kzalloc(size[0] + size[1], GFP_KERNEL); -- cgit v1.2.3 From bc6f2a757d525e001268c3658bd88822e768f8db Mon Sep 17 00:00:00 2001 From: YueHaibing Date: Tue, 11 Jun 2019 23:00:07 +0800 Subject: kernel/module: Fix mem leak in module_add_modinfo_attrs In module_add_modinfo_attrs if sysfs_create_file fails, we forget to free allocated modinfo_attrs and roll back the sysfs files. Fixes: 03e88ae1b13d ("[PATCH] fix module sysfs files reference counting") Reviewed-by: Miroslav Benes Signed-off-by: YueHaibing Signed-off-by: Jessica Yu --- kernel/module.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/kernel/module.c b/kernel/module.c index 1a0d8cab9eb7..c1517053e9d6 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1708,6 +1708,8 @@ static int add_usage_links(struct module *mod) return ret; } +static void module_remove_modinfo_attrs(struct module *mod, int end); + static int module_add_modinfo_attrs(struct module *mod) { struct module_attribute *attr; @@ -1722,24 +1724,34 @@ static int module_add_modinfo_attrs(struct module *mod) return -ENOMEM; temp_attr = mod->modinfo_attrs; - for (i = 0; (attr = modinfo_attrs[i]) && !error; i++) { + for (i = 0; (attr = modinfo_attrs[i]); i++) { if (!attr->test || attr->test(mod)) { memcpy(temp_attr, attr, sizeof(*temp_attr)); sysfs_attr_init(&temp_attr->attr); error = sysfs_create_file(&mod->mkobj.kobj, &temp_attr->attr); + if (error) + goto error_out; ++temp_attr; } } + + return 0; + +error_out: + if (i > 0) + module_remove_modinfo_attrs(mod, --i); return error; } -static void module_remove_modinfo_attrs(struct module *mod) +static void module_remove_modinfo_attrs(struct module *mod, int end) { struct module_attribute *attr; int i; for (i = 0; (attr = &mod->modinfo_attrs[i]); i++) { + if (end >= 0 && i > end) + break; /* pick a field to test for end of list */ if (!attr->attr.name) break; @@ -1827,7 +1839,7 @@ static int mod_sysfs_setup(struct module *mod, return 0; out_unreg_modinfo_attrs: - module_remove_modinfo_attrs(mod); + module_remove_modinfo_attrs(mod, -1); out_unreg_param: module_param_sysfs_remove(mod); out_unreg_holders: @@ -1863,7 +1875,7 @@ static void mod_sysfs_fini(struct module *mod) { } -static void module_remove_modinfo_attrs(struct module *mod) +static void module_remove_modinfo_attrs(struct module *mod, int end) { } @@ -1879,7 +1891,7 @@ static void init_param_lock(struct module *mod) static void mod_sysfs_teardown(struct module *mod) { del_usage_links(mod); - module_remove_modinfo_attrs(mod); + module_remove_modinfo_attrs(mod, -1); module_param_sysfs_remove(mod); kobject_put(mod->mkobj.drivers_dir); kobject_put(mod->holders_dir); -- cgit v1.2.3 From 2eef1399a866c57687962e15142b141a4f8e7862 Mon Sep 17 00:00:00 2001 From: Yang Yingliang Date: Thu, 20 Jun 2019 10:18:14 +0800 Subject: modules: fix BUG when load module with rodata=n When loading a module with rodata=n, it causes an executing NX-protected page BUG. [ 32.379191] kernel tried to execute NX-protected page - exploit attempt? (uid: 0) [ 32.382917] BUG: unable to handle page fault for address: ffffffffc0005000 [ 32.385947] #PF: supervisor instruction fetch in kernel mode [ 32.387662] #PF: error_code(0x0011) - permissions violation [ 32.389352] PGD 240c067 P4D 240c067 PUD 240e067 PMD 421a52067 PTE 8000000421a53063 [ 32.391396] Oops: 0011 [#1] SMP PTI [ 32.392478] CPU: 7 PID: 2697 Comm: insmod Tainted: G O 5.2.0-rc5+ #202 [ 32.394588] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014 [ 32.398157] RIP: 0010:ko_test_init+0x0/0x1000 [ko_test] [ 32.399662] Code: Bad RIP value. [ 32.400621] RSP: 0018:ffffc900029f3ca8 EFLAGS: 00010246 [ 32.402171] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000 [ 32.404332] RDX: 00000000000004c7 RSI: 0000000000000cc0 RDI: ffffffffc0005000 [ 32.406347] RBP: ffffffffc0005000 R08: ffff88842fbebc40 R09: ffffffff810ede4a [ 32.408392] R10: ffffea00108e3480 R11: 0000000000000000 R12: ffff88842bee21a0 [ 32.410472] R13: 0000000000000001 R14: 0000000000000001 R15: ffffc900029f3e78 [ 32.412609] FS: 00007fb4f0c0a700(0000) GS:ffff88842fbc0000(0000) knlGS:0000000000000000 [ 32.414722] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 32.416290] CR2: ffffffffc0004fd6 CR3: 0000000421a90004 CR4: 0000000000020ee0 [ 32.418471] Call Trace: [ 32.419136] do_one_initcall+0x41/0x1df [ 32.420199] ? _cond_resched+0x10/0x40 [ 32.421433] ? kmem_cache_alloc_trace+0x36/0x160 [ 32.422827] do_init_module+0x56/0x1f7 [ 32.423946] load_module+0x1e67/0x2580 [ 32.424947] ? __alloc_pages_nodemask+0x150/0x2c0 [ 32.426413] ? map_vm_area+0x2d/0x40 [ 32.427530] ? __vmalloc_node_range+0x1ef/0x260 [ 32.428850] ? __do_sys_init_module+0x135/0x170 [ 32.430060] ? _cond_resched+0x10/0x40 [ 32.431249] __do_sys_init_module+0x135/0x170 [ 32.432547] do_syscall_64+0x43/0x120 [ 32.433853] entry_SYSCALL_64_after_hwframe+0x44/0xa9 Because if rodata=n, set_memory_x() can't be called, fix this by calling set_memory_x in complete_formation(); Fixes: f2c65fb3221a ("x86/modules: Avoid breaking W^X while loading modules") Suggested-by: Jian Cheng Reviewed-by: Nadav Amit Signed-off-by: Yang Yingliang Signed-off-by: Jessica Yu --- kernel/module.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/kernel/module.c b/kernel/module.c index c1517053e9d6..41258bab24f1 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1972,13 +1972,9 @@ void module_enable_ro(const struct module *mod, bool after_init) set_vm_flush_reset_perms(mod->core_layout.base); set_vm_flush_reset_perms(mod->init_layout.base); frob_text(&mod->core_layout, set_memory_ro); - frob_text(&mod->core_layout, set_memory_x); frob_rodata(&mod->core_layout, set_memory_ro); - frob_text(&mod->init_layout, set_memory_ro); - frob_text(&mod->init_layout, set_memory_x); - frob_rodata(&mod->init_layout, set_memory_ro); if (after_init) @@ -2041,6 +2037,12 @@ void set_all_modules_text_ro(void) static void module_enable_nx(const struct module *mod) { } #endif +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); +} + #ifdef CONFIG_LIVEPATCH /* * Persist Elf information about a module. Copy the Elf header, @@ -3637,6 +3639,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); /* Mark state as coming so strong_try_module_get() ignores us, * but kallsyms etc. can see us. */ -- cgit v1.2.3 From 38b37d631aec80da0c65ac03a7ef680b468c7857 Mon Sep 17 00:00:00 2001 From: Matthias Schiffer Date: Fri, 7 Jun 2019 12:49:11 +0200 Subject: module: allow arch overrides for .exit section names Some archs like ARM store unwind information for .exit.text in sections with unusual names. As this unwind information refers to .exit.text, it must not be loaded when .exit.text is not loaded (when CONFIG_MODULE_UNLOAD is unset); otherwise, loading a module can fail due to relocation failures. Signed-off-by: Matthias Schiffer Signed-off-by: Jessica Yu --- include/linux/moduleloader.h | 5 +++++ kernel/module.c | 7 ++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/include/linux/moduleloader.h b/include/linux/moduleloader.h index 31013c2effd3..5229c18025e9 100644 --- a/include/linux/moduleloader.h +++ b/include/linux/moduleloader.h @@ -29,6 +29,11 @@ void *module_alloc(unsigned long size); /* Free memory returned from module_alloc. */ void module_memfree(void *module_region); +/* Determines if the section name is an exit section (that is only used during + * module unloading) + */ +bool module_exit_section(const char *name); + /* * Apply the given relocation to the (simplified) ELF. Return -error * or 0. diff --git a/kernel/module.c b/kernel/module.c index 41258bab24f1..537c456ce3ee 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2748,6 +2748,11 @@ void * __weak module_alloc(unsigned long size) return vmalloc_exec(size); } +bool __weak module_exit_section(const char *name) +{ + return strstarts(name, ".exit"); +} + #ifdef CONFIG_DEBUG_KMEMLEAK static void kmemleak_load_module(const struct module *mod, const struct load_info *info) @@ -2937,7 +2942,7 @@ static int rewrite_section_headers(struct load_info *info, int flags) #ifndef CONFIG_MODULE_UNLOAD /* Don't load .exit sections */ - if (strstarts(info->secstrings+shdr->sh_name, ".exit")) + if (module_exit_section(info->secstrings+shdr->sh_name)) shdr->sh_flags &= ~(unsigned long)SHF_ALLOC; #endif } -- cgit v1.2.3 From 70bac08d4157fda334fe21ee38a2e93bc434bac4 Mon Sep 17 00:00:00 2001 From: Matthias Schiffer Date: Fri, 7 Jun 2019 12:49:12 +0200 Subject: ARM: module: recognize unwind exit sections In addition to the prefix ".exit", ".ARM.extab.exit" and ".ARM.exidx.exit" must be recognized as exit sections as well. Otherwise, loading modules can fail without CONFIG_MODULE_UNLOAD depending on the memory layout, when relocations for the unwind sections refer to the .exit.text section: imx_sdma: section 16 reloc 0 sym '': relocation 42 out of range (0x7f015260 -> 0xc0f5a5e8) where 0x7F000000 is the module load area and 0xC0000000 is the vmalloc area. Relocation 42 refers to R_ARM_PREL31, which is limited to signed 31bit offsets. Signed-off-by: Matthias Schiffer Signed-off-by: Jessica Yu --- arch/arm/kernel/module.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c index 3ff571c2c71c..692001aabb0f 100644 --- a/arch/arm/kernel/module.c +++ b/arch/arm/kernel/module.c @@ -58,6 +58,13 @@ void *module_alloc(unsigned long size) } #endif +bool module_exit_section(const char *name) +{ + return strstarts(name, ".exit") || + strstarts(name, ".ARM.extab.exit") || + strstarts(name, ".ARM.exidx.exit"); +} + int apply_relocate(Elf32_Shdr *sechdrs, const char *strtab, unsigned int symindex, unsigned int relindex, struct module *module) -- cgit v1.2.3 From 93651f80dcb616b8c9115cdafc8e57a781af22d0 Mon Sep 17 00:00:00 2001 From: Yang Yingliang Date: Tue, 25 Jun 2019 17:40:28 +0800 Subject: modules: fix compile error if don't have strict module rwx 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. Fixes: 2eef1399a866 ("modules: fix BUG when load module with rodata=n") Signed-off-by: Yang Yingliang Signed-off-by: Jessica Yu --- kernel/module.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/kernel/module.c b/kernel/module.c index 537c456ce3ee..b51838325d08 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1898,7 +1898,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 /* * LKM RO/NX protection: protect module's text/ro-data * from modification and any data from execution. @@ -1921,6 +1921,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)) { @@ -2033,15 +2034,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 /* -- cgit v1.2.3