From c05780ef3c190c2dafbf0be8e65d4f01103ad577 Mon Sep 17 00:00:00 2001 From: Palmer Dabbelt Date: Fri, 7 Jul 2023 09:00:51 -0700 Subject: module: Ignore RISC-V mapping symbols too RISC-V has an extended form of mapping symbols that we use to encode the ISA when it changes in the middle of an ELF. This trips up modpost as a build failure, I haven't yet verified it yet but I believe the kallsyms difference should result in stacks looking sane again. Reported-by: Randy Dunlap Closes: https://lore.kernel.org/all/9d9e2902-5489-4bf0-d9cb-556c8e5d71c2@infradead.org/ Signed-off-by: Palmer Dabbelt Reviewed-by: Randy Dunlap Tested-by: Randy Dunlap # build-tested Signed-off-by: Luis Chamberlain --- kernel/module/kallsyms.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'kernel') diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c index ef73ae7c8909..78a1ffc399d9 100644 --- a/kernel/module/kallsyms.c +++ b/kernel/module/kallsyms.c @@ -289,7 +289,7 @@ static const char *find_kallsyms_symbol(struct module *mod, * and inserted at a whim. */ if (*kallsyms_symbol_name(kallsyms, i) == '\0' || - is_mapping_symbol(kallsyms_symbol_name(kallsyms, i))) + is_mapping_symbol(kallsyms_symbol_name(kallsyms, i), IS_ENABLED(CONFIG_RISCV))) continue; if (thisval <= addr && thisval > bestval) { -- cgit v1.2.3 From 9ce170cef66916526dcaa868a67cfcc0c2f0c3d6 Mon Sep 17 00:00:00 2001 From: Li zeming Date: Wed, 12 Jul 2023 03:43:31 +0800 Subject: kernel: params: Remove unnecessary ‘0’ values from err MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit err is assigned first, so it does not need to initialize the assignment. Signed-off-by: Li zeming Reviewed-by: Randy Dunlap Signed-off-by: Luis Chamberlain --- kernel/params.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'kernel') diff --git a/kernel/params.c b/kernel/params.c index 07d01f6ce9a2..2d4a0564697e 100644 --- a/kernel/params.c +++ b/kernel/params.c @@ -331,7 +331,7 @@ EXPORT_SYMBOL(param_ops_bool); int param_set_bool_enable_only(const char *val, const struct kernel_param *kp) { - int err = 0; + int err; bool new_value; bool orig_value = *(bool *)kp->arg; struct kernel_param dummy_kp = *kp; -- cgit v1.2.3 From ff09f6fd297293175eaa0ed492495e36b3eb1a8e Mon Sep 17 00:00:00 2001 From: Palmer Dabbelt Date: Fri, 21 Jul 2023 08:01:48 -0700 Subject: modpost, kallsyms: Treat add '$'-prefixed symbols as mapping symbols Trying to restrict the '$'-prefix change to RISC-V caused some fallout, so let's just treat all those symbols as special. Fixes: c05780ef3c190 ("module: Ignore RISC-V mapping symbols too") Link: https://lore.kernel.org/all/20230712015747.77263-1-wangkefeng.wang@huawei.com/ Signed-off-by: Palmer Dabbelt Reviewed-by: Masahiro Yamada Signed-off-by: Luis Chamberlain --- include/linux/module_symbol.h | 16 ++-------------- kernel/module/kallsyms.c | 2 +- scripts/mod/modpost.c | 2 +- 3 files changed, 4 insertions(+), 16 deletions(-) (limited to 'kernel') diff --git a/include/linux/module_symbol.h b/include/linux/module_symbol.h index 5b799942b243..1269543d0634 100644 --- a/include/linux/module_symbol.h +++ b/include/linux/module_symbol.h @@ -3,25 +3,13 @@ #define _LINUX_MODULE_SYMBOL_H /* This ignores the intensely annoying "mapping symbols" found in ELF files. */ -static inline int is_mapping_symbol(const char *str, int is_riscv) +static inline int is_mapping_symbol(const char *str) { if (str[0] == '.' && str[1] == 'L') return true; if (str[0] == 'L' && str[1] == '0') return true; - /* - * RISC-V defines various special symbols that start with "$".  The - * mapping symbols, which exist to differentiate between incompatible - * instruction encodings when disassembling, show up all over the place - * and are generally not meant to be treated like other symbols.  So - * just ignore any of the special symbols. - */ - if (is_riscv) - return str[0] == '$'; - - return str[0] == '$' && - (str[1] == 'a' || str[1] == 'd' || str[1] == 't' || str[1] == 'x') - && (str[2] == '\0' || str[2] == '.'); + return str[0] == '$'; } #endif /* _LINUX_MODULE_SYMBOL_H */ diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c index 78a1ffc399d9..ef73ae7c8909 100644 --- a/kernel/module/kallsyms.c +++ b/kernel/module/kallsyms.c @@ -289,7 +289,7 @@ static const char *find_kallsyms_symbol(struct module *mod, * and inserted at a whim. */ if (*kallsyms_symbol_name(kallsyms, i) == '\0' || - is_mapping_symbol(kallsyms_symbol_name(kallsyms, i), IS_ENABLED(CONFIG_RISCV))) + is_mapping_symbol(kallsyms_symbol_name(kallsyms, i))) continue; if (thisval <= addr && thisval > bestval) { diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 7c71429d6502..b29b29707f10 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -1052,7 +1052,7 @@ static inline int is_valid_name(struct elf_info *elf, Elf_Sym *sym) if (!name || !strlen(name)) return 0; - return !is_mapping_symbol(name, elf->hdr->e_machine == EM_RISCV); + return !is_mapping_symbol(name); } /* Look up the nearest symbol based on the section and the address */ -- cgit v1.2.3 From 9011e49d54dcc7653ebb8a1e05b5badb5ecfa9f9 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Tue, 1 Aug 2023 19:35:44 +0200 Subject: modules: only allow symbol_get of EXPORT_SYMBOL_GPL modules It has recently come to my attention that nvidia is circumventing the protection added in 262e6ae7081d ("modules: inherit TAINT_PROPRIETARY_MODULE") by importing exports from their proprietary modules into an allegedly GPL licensed module and then rexporting them. Given that symbol_get was only ever intended for tightly cooperating modules using very internal symbols it is logical to restrict it to being used on EXPORT_SYMBOL_GPL and prevent nvidia from costly DMCA Circumvention of Access Controls law suites. All symbols except for four used through symbol_get were already exported as EXPORT_SYMBOL_GPL, and the remaining four ones were switched over in the preparation patches. Fixes: 262e6ae7081d ("modules: inherit TAINT_PROPRIETARY_MODULE") Signed-off-by: Christoph Hellwig Reviewed-by: Greg Kroah-Hartman Signed-off-by: Luis Chamberlain --- kernel/module/main.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) (limited to 'kernel') diff --git a/kernel/module/main.c b/kernel/module/main.c index 59b1d067e528..c395af9eced1 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -1295,12 +1295,20 @@ void *__symbol_get(const char *symbol) }; preempt_disable(); - if (!find_symbol(&fsa) || strong_try_module_get(fsa.owner)) { - preempt_enable(); - return NULL; + if (!find_symbol(&fsa)) + goto fail; + if (fsa.license != GPL_ONLY) { + pr_warn("failing symbol_get of non-GPLONLY symbol %s.\n", + symbol); + goto fail; } + if (strong_try_module_get(fsa.owner)) + goto fail; preempt_enable(); return (void *)kernel_symbol_value(fsa.sym); +fail: + preempt_enable(); + return NULL; } EXPORT_SYMBOL_GPL(__symbol_get); -- cgit v1.2.3 From 2abcc4b5a64a65a2d2287ba0be5c2871c1552416 Mon Sep 17 00:00:00 2001 From: James Morse Date: Tue, 1 Aug 2023 14:54:07 +0000 Subject: module: Expose module_init_layout_section() module_init_layout_section() choses whether the core module loader considers a section as init or not. This affects the placement of the exit section when module unloading is disabled. This code will never run, so it can be free()d once the module has been initialised. arm and arm64 need to count the number of PLTs they need before applying relocations based on the section name. The init PLTs are stored separately so they can be free()d. arm and arm64 both use within_module_init() to decide which list of PLTs to use when applying the relocation. Because within_module_init()'s behaviour changes when module unloading is disabled, both architecture would need to take this into account when counting the PLTs. Today neither architecture does this, meaning when module unloading is disabled there are insufficient PLTs in the init section to load some modules, resulting in warnings: | WARNING: CPU: 2 PID: 51 at arch/arm64/kernel/module-plts.c:99 module_emit_plt_entry+0x184/0x1cc | Modules linked in: crct10dif_common | CPU: 2 PID: 51 Comm: modprobe Not tainted 6.5.0-rc4-yocto-standard-dirty #15208 | Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015 | pstate: 20400005 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) | pc : module_emit_plt_entry+0x184/0x1cc | lr : module_emit_plt_entry+0x94/0x1cc | sp : ffffffc0803bba60 [...] | Call trace: | module_emit_plt_entry+0x184/0x1cc | apply_relocate_add+0x2bc/0x8e4 | load_module+0xe34/0x1bd4 | init_module_from_file+0x84/0xc0 | __arm64_sys_finit_module+0x1b8/0x27c | invoke_syscall.constprop.0+0x5c/0x104 | do_el0_svc+0x58/0x160 | el0_svc+0x38/0x110 | el0t_64_sync_handler+0xc0/0xc4 | el0t_64_sync+0x190/0x194 Instead of duplicating module_init_layout_section()s logic, expose it. Reported-by: Adam Johnston Fixes: 055f23b74b20 ("module: check for exit sections in layout_sections() instead of module_init_section()") Cc: stable@vger.kernel.org Signed-off-by: James Morse Signed-off-by: Luis Chamberlain --- include/linux/moduleloader.h | 5 +++++ kernel/module/main.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) (limited to 'kernel') diff --git a/include/linux/moduleloader.h b/include/linux/moduleloader.h index 03be088fb439..001b2ce83832 100644 --- a/include/linux/moduleloader.h +++ b/include/linux/moduleloader.h @@ -42,6 +42,11 @@ bool module_init_section(const char *name); */ bool module_exit_section(const char *name); +/* Describes whether within_module_init() will consider this an init section + * or not. This behaviour changes with CONFIG_MODULE_UNLOAD. + */ +bool module_init_layout_section(const char *sname); + /* * Apply the given relocation to the (simplified) ELF. Return -error * or 0. diff --git a/kernel/module/main.c b/kernel/module/main.c index c395af9eced1..98fedfdb8db5 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -1492,7 +1492,7 @@ long module_get_offset_and_type(struct module *mod, enum mod_mem_type type, return offset | mask; } -static bool module_init_layout_section(const char *sname) +bool module_init_layout_section(const char *sname) { #ifndef CONFIG_MODULE_UNLOAD if (module_exit_section(sname)) -- cgit v1.2.3 From 33c24bee4b787ef58d8c9a36316ba85363b93617 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 24 Aug 2023 13:58:00 -0700 Subject: kallsyms: Add more debug output for selftest While debugging a recent kallsyms_selftest failure[1], I needed more details on what specifically was failing. This adds those details for each failure state that is checked. [1] https://lore.kernel.org/all/202308232200.1c932a90-oliver.sang@intel.com/ Cc: Luis Chamberlain Cc: Yonghong Song Cc: "Erhard F." Cc: Zhen Lei Cc: kernel test robot Cc: Petr Mladek Cc: Nicholas Piggin Cc: Yang Li Signed-off-by: Kees Cook Signed-off-by: Luis Chamberlain --- kernel/kallsyms_selftest.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) (limited to 'kernel') diff --git a/kernel/kallsyms_selftest.c b/kernel/kallsyms_selftest.c index a2e3745d15c4..232e8faefb46 100644 --- a/kernel/kallsyms_selftest.c +++ b/kernel/kallsyms_selftest.c @@ -341,6 +341,7 @@ static int test_kallsyms_basic_function(void) ret = lookup_symbol_name(addr, namebuf); if (unlikely(ret)) { namebuf[0] = 0; + pr_info("%d: lookup_symbol_name(%lx) failed\n", i, addr); goto failed; } @@ -388,8 +389,11 @@ static int test_kallsyms_basic_function(void) if (stat->addr != stat2->addr || stat->real_cnt != stat2->real_cnt || memcmp(stat->addrs, stat2->addrs, - stat->save_cnt * sizeof(stat->addrs[0]))) + stat->save_cnt * sizeof(stat->addrs[0]))) { + pr_info("%s: mismatch between kallsyms_on_each_symbol() and kallsyms_on_each_match_symbol()\n", + namebuf); goto failed; + } /* * The average of random increments is 128, that is, one of @@ -400,15 +404,23 @@ static int test_kallsyms_basic_function(void) } /* Need to be found at least once */ - if (!stat->real_cnt) + if (!stat->real_cnt) { + pr_info("%s: Never found\n", namebuf); goto failed; + } /* * kallsyms_lookup_name() returns the address of the first * symbol found and cannot be NULL. */ - if (!lookup_addr || lookup_addr != stat->addrs[0]) + if (!lookup_addr) { + pr_info("%s: NULL lookup_addr?!\n", namebuf); + goto failed; + } + if (lookup_addr != stat->addrs[0]) { + pr_info("%s: lookup_addr != stat->addrs[0]\n", namebuf); goto failed; + } /* * If the addresses of all matching symbols are recorded, the @@ -420,8 +432,10 @@ static int test_kallsyms_basic_function(void) break; } - if (j == stat->save_cnt) + if (j == stat->save_cnt) { + pr_info("%s: j == save_cnt?!\n", namebuf); goto failed; + } } } -- cgit v1.2.3 From a419beac4a070aff63c520f36ebf7cb8a76a8ae5 Mon Sep 17 00:00:00 2001 From: Andrea Righi Date: Tue, 29 Aug 2023 14:05:08 +0200 Subject: module/decompress: use vmalloc() for zstd decompression workspace Using kmalloc() to allocate the decompression workspace for zstd may trigger the following warning when large modules are loaded (i.e., xfs): [ 2.961884] WARNING: CPU: 1 PID: 254 at mm/page_alloc.c:4453 __alloc_pages+0x2c3/0x350 ... [ 2.989033] Call Trace: [ 2.989841] [ 2.990614] ? show_regs+0x6d/0x80 [ 2.991573] ? __warn+0x89/0x160 [ 2.992485] ? __alloc_pages+0x2c3/0x350 [ 2.993520] ? report_bug+0x17e/0x1b0 [ 2.994506] ? handle_bug+0x51/0xa0 [ 2.995474] ? exc_invalid_op+0x18/0x80 [ 2.996469] ? asm_exc_invalid_op+0x1b/0x20 [ 2.997530] ? module_zstd_decompress+0xdc/0x2a0 [ 2.998665] ? __alloc_pages+0x2c3/0x350 [ 2.999695] ? module_zstd_decompress+0xdc/0x2a0 [ 3.000821] __kmalloc_large_node+0x7a/0x150 [ 3.001920] __kmalloc+0xdb/0x170 [ 3.002824] module_zstd_decompress+0xdc/0x2a0 [ 3.003857] module_decompress+0x37/0xc0 [ 3.004688] init_module_from_file+0xd0/0x100 [ 3.005668] idempotent_init_module+0x11c/0x2b0 [ 3.006632] __x64_sys_finit_module+0x64/0xd0 [ 3.007568] do_syscall_64+0x59/0x90 [ 3.008373] ? ksys_read+0x73/0x100 [ 3.009395] ? exit_to_user_mode_prepare+0x30/0xb0 [ 3.010531] ? syscall_exit_to_user_mode+0x37/0x60 [ 3.011662] ? do_syscall_64+0x68/0x90 [ 3.012511] ? do_syscall_64+0x68/0x90 [ 3.013364] entry_SYSCALL_64_after_hwframe+0x6e/0xd8 However, continuous physical memory does not seem to be required in module_zstd_decompress(), so use vmalloc() instead, to prevent the warning and avoid potential failures at loading compressed modules. Fixes: 169a58ad824d ("module/decompress: Support zstd in-kernel decompression") Signed-off-by: Andrea Righi Signed-off-by: Luis Chamberlain --- kernel/module/decompress.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'kernel') diff --git a/kernel/module/decompress.c b/kernel/module/decompress.c index 8a5d6d63b06c..87440f714c0c 100644 --- a/kernel/module/decompress.c +++ b/kernel/module/decompress.c @@ -241,7 +241,7 @@ static ssize_t module_zstd_decompress(struct load_info *info, } wksp_size = zstd_dstream_workspace_bound(header.windowSize); - wksp = kmalloc(wksp_size, GFP_KERNEL); + wksp = vmalloc(wksp_size); if (!wksp) { retval = -ENOMEM; goto out; @@ -284,7 +284,7 @@ static ssize_t module_zstd_decompress(struct load_info *info, retval = new_size; out: - kfree(wksp); + vfree(wksp); return retval; } #else -- cgit v1.2.3