linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] modules: lockdep: Suppress suspicious RCU usage warning
@ 2019-12-03  6:14 Masami Hiramatsu
  2019-12-05 19:17 ` Jessica Yu
  0 siblings, 1 reply; 3+ messages in thread
From: Masami Hiramatsu @ 2019-12-03  6:14 UTC (permalink / raw)
  To: Jessica Yu
  Cc: Joel Fernandes, Ingo Molnar, Anders Roxell, paulmck,
	Naveen N . Rao, Anil S Keshavamurthy, David Miller,
	Linux Kernel Mailing List, mhiramat

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()
to suppress this warning, This also fixes similar issue in
mod_find() and each_symbol_section().

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 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))


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] modules: lockdep: Suppress suspicious RCU usage warning
  2019-12-03  6:14 [PATCH] modules: lockdep: Suppress suspicious RCU usage warning Masami Hiramatsu
@ 2019-12-05 19:17 ` Jessica Yu
  2019-12-06  1:21   ` Masami Hiramatsu
  0 siblings, 1 reply; 3+ messages in thread
From: Jessica Yu @ 2019-12-05 19:17 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Joel Fernandes, Ingo Molnar, Anders Roxell, paulmck,
	Naveen N . Rao, Anil S Keshavamurthy, David Miller,
	Linux Kernel Mailing List

+++ 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))
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] modules: lockdep: Suppress suspicious RCU usage warning
  2019-12-05 19:17 ` Jessica Yu
@ 2019-12-06  1:21   ` Masami Hiramatsu
  0 siblings, 0 replies; 3+ messages in thread
From: Masami Hiramatsu @ 2019-12-06  1:21 UTC (permalink / raw)
  To: Jessica Yu
  Cc: Joel Fernandes, Ingo Molnar, Anders Roxell, paulmck,
	Naveen N . Rao, Anil S Keshavamurthy, David Miller,
	Linux Kernel Mailing List

On Thu, 5 Dec 2019 20:17:59 +0100
Jessica Yu <jeyu@kernel.org> wrote:

> +++ 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.

Oops.

> 
> >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.

Thank you for merging :)


> 
> 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))
> >


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-12-06  1:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-03  6:14 [PATCH] modules: lockdep: Suppress suspicious RCU usage warning Masami Hiramatsu
2019-12-05 19:17 ` Jessica Yu
2019-12-06  1:21   ` Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).