From bf08949cc8b98b7d1e20cfbba169a5938d42dae8 Mon Sep 17 00:00:00 2001 From: Masami Hiramatsu Date: Tue, 3 Dec 2019 15:14:04 +0900 Subject: modules: lockdep: Suppress suspicious RCU usage warning While running kprobe module test, find_module_all() caused a suspicious RCU usage warning. ----- ============================= WARNING: suspicious RCU usage 5.4.0-next-20191202+ #63 Not tainted ----------------------------- kernel/module.c:619 RCU-list traversed in non-reader section!! other info that might help us debug this: rcu_scheduler_active = 2, debug_locks = 1 1 lock held by rmmod/642: #0: ffffffff8227da80 (module_mutex){+.+.}, at: __x64_sys_delete_module+0x9a/0x230 stack backtrace: CPU: 0 PID: 642 Comm: rmmod Not tainted 5.4.0-next-20191202+ #63 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014 Call Trace: dump_stack+0x71/0xa0 find_module_all+0xc1/0xd0 __x64_sys_delete_module+0xac/0x230 ? do_syscall_64+0x12/0x1f0 do_syscall_64+0x50/0x1f0 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x4b6d49 ----- This is because list_for_each_entry_rcu(modules) is called without rcu_read_lock(). This is safe because the module_mutex is locked. Pass lockdep_is_held(&module_mutex) to the list_for_each_entry_rcu() to suppress this warning, This also fixes similar issue in mod_find() and each_symbol_section(). Signed-off-by: Masami Hiramatsu Signed-off-by: Jessica Yu --- kernel/module.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'kernel') diff --git a/kernel/module.c b/kernel/module.c index 3a486f826224..155349275b8e 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -214,7 +214,8 @@ static struct module *mod_find(unsigned long addr) { struct module *mod; - list_for_each_entry_rcu(mod, &modules, list) { + list_for_each_entry_rcu(mod, &modules, list, + lockdep_is_held(&module_mutex)) { if (within_module(addr, mod)) return mod; } @@ -448,7 +449,8 @@ bool each_symbol_section(bool (*fn)(const struct symsearch *arr, if (each_symbol_in_section(arr, ARRAY_SIZE(arr), NULL, fn, data)) return true; - list_for_each_entry_rcu(mod, &modules, list) { + list_for_each_entry_rcu(mod, &modules, list, + lockdep_is_held(&module_mutex)) { struct symsearch arr[] = { { mod->syms, mod->syms + mod->num_syms, mod->crcs, NOT_GPL_ONLY, false }, @@ -616,7 +618,8 @@ static struct module *find_module_all(const char *name, size_t len, module_assert_mutex_or_preempt(); - list_for_each_entry_rcu(mod, &modules, list) { + list_for_each_entry_rcu(mod, &modules, list, + lockdep_is_held(&module_mutex)) { if (!even_unformed && mod->state == MODULE_STATE_UNFORMED) continue; if (strlen(mod->name) == len && !memcmp(mod->name, name, len)) -- cgit From f6d061d617124abbd55396a3bc37b9bf7d33233c Mon Sep 17 00:00:00 2001 From: YueHaibing Date: Sat, 28 Dec 2019 19:54:55 +0800 Subject: kernel/module: Fix memleak in module_add_modinfo_attrs() In module_add_modinfo_attrs() if sysfs_create_file() fails on the first iteration of the loop (so i = 0), we forget to free the modinfo_attrs. Fixes: bc6f2a757d52 ("kernel/module: Fix mem leak in module_add_modinfo_attrs") Reviewed-by: Miroslav Benes Signed-off-by: YueHaibing Signed-off-by: Jessica Yu --- kernel/module.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'kernel') diff --git a/kernel/module.c b/kernel/module.c index 155349275b8e..d4d876172e9d 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1784,6 +1784,8 @@ static int module_add_modinfo_attrs(struct module *mod) error_out: if (i > 0) module_remove_modinfo_attrs(mod, --i); + else + kfree(mod->modinfo_attrs); return error; } -- cgit From e9f35f634e099894f4d6c3b039cd3de5281ee637 Mon Sep 17 00:00:00 2001 From: Jessica Yu Date: Wed, 15 Jan 2020 15:49:31 +0100 Subject: modsign: print module name along with error message It is useful to know which module failed signature verification, so print the module name along with the error message. Signed-off-by: Jessica Yu --- kernel/module.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'kernel') diff --git a/kernel/module.c b/kernel/module.c index d4d876172e9d..2b5f9c4748bc 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2882,7 +2882,7 @@ static int module_sig_check(struct load_info *info, int flags) reason = "Loading of module with unavailable key"; decide: if (is_module_sig_enforced()) { - pr_notice("%s is rejected\n", reason); + pr_notice("%s: %s is rejected\n", info->name, reason); return -EKEYREJECTED; } -- cgit From 708e0ada1916be765b7faa58854062f2bc620bbf Mon Sep 17 00:00:00 2001 From: Jessica Yu Date: Fri, 17 Jan 2020 13:32:21 +0100 Subject: module: avoid setting info->name early in case we can fall back to info->mod->name In setup_load_info(), info->name (which contains the name of the module, mostly used for early logging purposes before the module gets set up) gets unconditionally assigned if .modinfo is missing despite the fact that there is an if (!info->name) check near the end of the function. Avoid assigning a placeholder string to info->name if .modinfo doesn't exist, so that we can fall back to info->mod->name later on. Fixes: 5fdc7db6448a ("module: setup load info before module_sig_check()") Reviewed-by: Miroslav Benes Signed-off-by: Jessica Yu --- kernel/module.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'kernel') diff --git a/kernel/module.c b/kernel/module.c index 2b5f9c4748bc..f2379e54fae8 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -3059,9 +3059,7 @@ static int setup_load_info(struct load_info *info, int flags) /* Try to find a name early so we can log errors with a module name */ info->index.info = find_sec(info, ".modinfo"); - if (!info->index.info) - info->name = "(missing .modinfo section)"; - else + if (info->index.info) info->name = get_modinfo(info, "name"); /* Find internal symbols and strings. */ @@ -3076,14 +3074,15 @@ static int setup_load_info(struct load_info *info, int flags) } if (info->index.sym == 0) { - pr_warn("%s: module has no symbols (stripped?)\n", info->name); + pr_warn("%s: module has no symbols (stripped?)\n", + info->name ?: "(missing .modinfo section or name field)"); return -ENOEXEC; } info->index.mod = find_sec(info, ".gnu.linkonce.this_module"); if (!info->index.mod) { pr_warn("%s: No module found in object\n", - info->name ?: "(missing .modinfo name field)"); + info->name ?: "(missing .modinfo section or name field)"); return -ENOEXEC; } /* This is temporary: point mod into copy of data. */ -- cgit