lkml.org 
[lkml]   [2019]   [Dec]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] modules: lockdep: Suppress suspicious RCU usage warning
+++ Masami Hiramatsu [03/12/19 15:14 +0900]:
>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_lock) to the list_for_each_entry_rcu()

s/module_lock/module_mutex/, but you don't have to respin the patch
just for this.

>to suppress this warning, This also fixes similar issue in
>mod_find() and each_symbol_section().
>
>Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>

Thanks Masami! This looks good. I'll queue this up shortly after the
merge window.

Jessica

>---
> kernel/module.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
>diff --git a/kernel/module.c b/kernel/module.c
>index cb6250be6ee9..38e5c6a7451b 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))
>

\
 
 \ /
  Last update: 2019-12-05 20:19    [W:0.037 / U:8.352 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site