bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next] kallsyms: Disable preemption for find_kallsyms_symbol_value
@ 2023-04-03 22:02 Jiri Olsa
  2023-04-04  1:43 ` Leizhen (ThunderTown)
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Jiri Olsa @ 2023-04-03 22:02 UTC (permalink / raw)
  To: Luis Chamberlain, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: Artem Savkov, bpf, linux-modules, linux-kernel, Martin KaFai Lau,
	Song Liu, Yonghong Song, John Fastabend, KP Singh,
	Stanislav Fomichev, Hao Luo, Petr Mladek, Leizhen (ThunderTown),
	Viktor Malik

Artem reported suspicious RCU usage [1]. The reason is that verifier
calls find_kallsyms_symbol_value with preemption enabled which will
trigger suspicious RCU usage warning in rcu_dereference_sched call.

Disabling preemption in find_kallsyms_symbol_value and adding
__find_kallsyms_symbol_value function.

Fixes: 31bf1dbccfb0 ("bpf: Fix attaching fentry/fexit/fmod_ret/lsm to modules")
[1] https://lore.kernel.org/bpf/ZBrPMkv8YVRiWwCR@samus.usersys.redhat.com/
Reported-by: Artem Savkov <asavkov@redhat.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/module/kallsyms.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index ab2376a1be88..bdc911dbcde5 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -442,7 +442,7 @@ int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
 }
 
 /* Given a module and name of symbol, find and return the symbol's value */
-unsigned long find_kallsyms_symbol_value(struct module *mod, const char *name)
+static unsigned long __find_kallsyms_symbol_value(struct module *mod, const char *name)
 {
 	unsigned int i;
 	struct mod_kallsyms *kallsyms = rcu_dereference_sched(mod->kallsyms);
@@ -466,7 +466,7 @@ static unsigned long __module_kallsyms_lookup_name(const char *name)
 	if (colon) {
 		mod = find_module_all(name, colon - name, false);
 		if (mod)
-			return find_kallsyms_symbol_value(mod, colon + 1);
+			return __find_kallsyms_symbol_value(mod, colon + 1);
 		return 0;
 	}
 
@@ -475,7 +475,7 @@ static unsigned long __module_kallsyms_lookup_name(const char *name)
 
 		if (mod->state == MODULE_STATE_UNFORMED)
 			continue;
-		ret = find_kallsyms_symbol_value(mod, name);
+		ret = __find_kallsyms_symbol_value(mod, name);
 		if (ret)
 			return ret;
 	}
@@ -494,6 +494,16 @@ unsigned long module_kallsyms_lookup_name(const char *name)
 	return ret;
 }
 
+unsigned long find_kallsyms_symbol_value(struct module *mod, const char *name)
+{
+	unsigned long ret;
+
+	preempt_disable();
+	ret = __find_kallsyms_symbol_value(mod, name);
+	preempt_enable();
+	return ret;
+}
+
 int module_kallsyms_on_each_symbol(const char *modname,
 				   int (*fn)(void *, const char *,
 					     struct module *, unsigned long),
-- 
2.39.2


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

* Re: [PATCH bpf-next] kallsyms: Disable preemption for find_kallsyms_symbol_value
  2023-04-03 22:02 [PATCH bpf-next] kallsyms: Disable preemption for find_kallsyms_symbol_value Jiri Olsa
@ 2023-04-04  1:43 ` Leizhen (ThunderTown)
  2023-04-04  6:56 ` Artem Savkov
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Leizhen (ThunderTown) @ 2023-04-04  1:43 UTC (permalink / raw)
  To: Jiri Olsa, Luis Chamberlain, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko
  Cc: Artem Savkov, bpf, linux-modules, linux-kernel, Martin KaFai Lau,
	Song Liu, Yonghong Song, John Fastabend, KP Singh,
	Stanislav Fomichev, Hao Luo, Petr Mladek, Viktor Malik



On 2023/4/4 6:02, Jiri Olsa wrote:
> Artem reported suspicious RCU usage [1]. The reason is that verifier
> calls find_kallsyms_symbol_value with preemption enabled which will
> trigger suspicious RCU usage warning in rcu_dereference_sched call.
> 
> Disabling preemption in find_kallsyms_symbol_value and adding
> __find_kallsyms_symbol_value function.

Reviewed-by: Zhen Lei <thunder.leizhen@huawei.com>

> 
> Fixes: 31bf1dbccfb0 ("bpf: Fix attaching fentry/fexit/fmod_ret/lsm to modules")
> [1] https://lore.kernel.org/bpf/ZBrPMkv8YVRiWwCR@samus.usersys.redhat.com/
> Reported-by: Artem Savkov <asavkov@redhat.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  kernel/module/kallsyms.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
> index ab2376a1be88..bdc911dbcde5 100644
> --- a/kernel/module/kallsyms.c
> +++ b/kernel/module/kallsyms.c
> @@ -442,7 +442,7 @@ int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
>  }
>  
>  /* Given a module and name of symbol, find and return the symbol's value */
> -unsigned long find_kallsyms_symbol_value(struct module *mod, const char *name)
> +static unsigned long __find_kallsyms_symbol_value(struct module *mod, const char *name)
>  {
>  	unsigned int i;
>  	struct mod_kallsyms *kallsyms = rcu_dereference_sched(mod->kallsyms);
> @@ -466,7 +466,7 @@ static unsigned long __module_kallsyms_lookup_name(const char *name)
>  	if (colon) {
>  		mod = find_module_all(name, colon - name, false);
>  		if (mod)
> -			return find_kallsyms_symbol_value(mod, colon + 1);
> +			return __find_kallsyms_symbol_value(mod, colon + 1);
>  		return 0;
>  	}
>  
> @@ -475,7 +475,7 @@ static unsigned long __module_kallsyms_lookup_name(const char *name)
>  
>  		if (mod->state == MODULE_STATE_UNFORMED)
>  			continue;
> -		ret = find_kallsyms_symbol_value(mod, name);
> +		ret = __find_kallsyms_symbol_value(mod, name);
>  		if (ret)
>  			return ret;
>  	}
> @@ -494,6 +494,16 @@ unsigned long module_kallsyms_lookup_name(const char *name)
>  	return ret;
>  }
>  
> +unsigned long find_kallsyms_symbol_value(struct module *mod, const char *name)
> +{
> +	unsigned long ret;
> +
> +	preempt_disable();
> +	ret = __find_kallsyms_symbol_value(mod, name);
> +	preempt_enable();
> +	return ret;
> +}
> +
>  int module_kallsyms_on_each_symbol(const char *modname,
>  				   int (*fn)(void *, const char *,
>  					     struct module *, unsigned long),
> 

-- 
Regards,
  Zhen Lei

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

* Re: [PATCH bpf-next] kallsyms: Disable preemption for find_kallsyms_symbol_value
  2023-04-03 22:02 [PATCH bpf-next] kallsyms: Disable preemption for find_kallsyms_symbol_value Jiri Olsa
  2023-04-04  1:43 ` Leizhen (ThunderTown)
@ 2023-04-04  6:56 ` Artem Savkov
  2023-04-05  0:20 ` patchwork-bot+netdevbpf
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Artem Savkov @ 2023-04-04  6:56 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Luis Chamberlain, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, bpf, linux-modules, linux-kernel,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Petr Mladek,
	Leizhen (ThunderTown),
	Viktor Malik

On Tue, Apr 04, 2023 at 12:02:54AM +0200, Jiri Olsa wrote:
> Artem reported suspicious RCU usage [1]. The reason is that verifier
> calls find_kallsyms_symbol_value with preemption enabled which will
> trigger suspicious RCU usage warning in rcu_dereference_sched call.
> 
> Disabling preemption in find_kallsyms_symbol_value and adding
> __find_kallsyms_symbol_value function.

Tested-by: Artem Savkov <asavkov@redhat.com>

> Fixes: 31bf1dbccfb0 ("bpf: Fix attaching fentry/fexit/fmod_ret/lsm to modules")
> [1] https://lore.kernel.org/bpf/ZBrPMkv8YVRiWwCR@samus.usersys.redhat.com/
> Reported-by: Artem Savkov <asavkov@redhat.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  kernel/module/kallsyms.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
> index ab2376a1be88..bdc911dbcde5 100644
> --- a/kernel/module/kallsyms.c
> +++ b/kernel/module/kallsyms.c
> @@ -442,7 +442,7 @@ int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
>  }
>  
>  /* Given a module and name of symbol, find and return the symbol's value */
> -unsigned long find_kallsyms_symbol_value(struct module *mod, const char *name)
> +static unsigned long __find_kallsyms_symbol_value(struct module *mod, const char *name)
>  {
>  	unsigned int i;
>  	struct mod_kallsyms *kallsyms = rcu_dereference_sched(mod->kallsyms);
> @@ -466,7 +466,7 @@ static unsigned long __module_kallsyms_lookup_name(const char *name)
>  	if (colon) {
>  		mod = find_module_all(name, colon - name, false);
>  		if (mod)
> -			return find_kallsyms_symbol_value(mod, colon + 1);
> +			return __find_kallsyms_symbol_value(mod, colon + 1);
>  		return 0;
>  	}
>  
> @@ -475,7 +475,7 @@ static unsigned long __module_kallsyms_lookup_name(const char *name)
>  
>  		if (mod->state == MODULE_STATE_UNFORMED)
>  			continue;
> -		ret = find_kallsyms_symbol_value(mod, name);
> +		ret = __find_kallsyms_symbol_value(mod, name);
>  		if (ret)
>  			return ret;
>  	}
> @@ -494,6 +494,16 @@ unsigned long module_kallsyms_lookup_name(const char *name)
>  	return ret;
>  }
>  
> +unsigned long find_kallsyms_symbol_value(struct module *mod, const char *name)
> +{
> +	unsigned long ret;
> +
> +	preempt_disable();
> +	ret = __find_kallsyms_symbol_value(mod, name);
> +	preempt_enable();
> +	return ret;
> +}
> +
>  int module_kallsyms_on_each_symbol(const char *modname,
>  				   int (*fn)(void *, const char *,
>  					     struct module *, unsigned long),
> -- 
> 2.39.2
> 

-- 
 Artem


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

* Re: [PATCH bpf-next] kallsyms: Disable preemption for find_kallsyms_symbol_value
  2023-04-03 22:02 [PATCH bpf-next] kallsyms: Disable preemption for find_kallsyms_symbol_value Jiri Olsa
  2023-04-04  1:43 ` Leizhen (ThunderTown)
  2023-04-04  6:56 ` Artem Savkov
@ 2023-04-05  0:20 ` patchwork-bot+netdevbpf
  2023-04-06 14:47 ` Petr Mladek
  2023-04-06 14:56 ` Aaron Tomlin
  4 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-04-05  0:20 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: mcgrof, ast, daniel, andrii, asavkov, bpf, linux-modules,
	linux-kernel, kafai, songliubraving, yhs, john.fastabend,
	kpsingh, sdf, haoluo, pmladek, thunder.leizhen, vmalik

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Tue,  4 Apr 2023 00:02:54 +0200 you wrote:
> Artem reported suspicious RCU usage [1]. The reason is that verifier
> calls find_kallsyms_symbol_value with preemption enabled which will
> trigger suspicious RCU usage warning in rcu_dereference_sched call.
> 
> Disabling preemption in find_kallsyms_symbol_value and adding
> __find_kallsyms_symbol_value function.
> 
> [...]

Here is the summary with links:
  - [bpf-next] kallsyms: Disable preemption for find_kallsyms_symbol_value
    https://git.kernel.org/bpf/bpf-next/c/d099f594ad56

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

* Re: [PATCH bpf-next] kallsyms: Disable preemption for find_kallsyms_symbol_value
  2023-04-03 22:02 [PATCH bpf-next] kallsyms: Disable preemption for find_kallsyms_symbol_value Jiri Olsa
                   ` (2 preceding siblings ...)
  2023-04-05  0:20 ` patchwork-bot+netdevbpf
@ 2023-04-06 14:47 ` Petr Mladek
  2023-04-06 14:56 ` Aaron Tomlin
  4 siblings, 0 replies; 6+ messages in thread
From: Petr Mladek @ 2023-04-06 14:47 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Luis Chamberlain, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Artem Savkov, bpf, linux-modules, linux-kernel,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Leizhen (ThunderTown),
	Viktor Malik

On Tue 2023-04-04 00:02:54, Jiri Olsa wrote:
> Artem reported suspicious RCU usage [1]. The reason is that verifier
> calls find_kallsyms_symbol_value with preemption enabled which will
> trigger suspicious RCU usage warning in rcu_dereference_sched call.
> 
> Disabling preemption in find_kallsyms_symbol_value and adding
> __find_kallsyms_symbol_value function.
> 
> Fixes: 31bf1dbccfb0 ("bpf: Fix attaching fentry/fexit/fmod_ret/lsm to modules")
> [1] https://lore.kernel.org/bpf/ZBrPMkv8YVRiWwCR@samus.usersys.redhat.com/
> Reported-by: Artem Savkov <asavkov@redhat.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>

Just for record:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

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

* Re: [PATCH bpf-next] kallsyms: Disable preemption for find_kallsyms_symbol_value
  2023-04-03 22:02 [PATCH bpf-next] kallsyms: Disable preemption for find_kallsyms_symbol_value Jiri Olsa
                   ` (3 preceding siblings ...)
  2023-04-06 14:47 ` Petr Mladek
@ 2023-04-06 14:56 ` Aaron Tomlin
  4 siblings, 0 replies; 6+ messages in thread
From: Aaron Tomlin @ 2023-04-06 14:56 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Luis Chamberlain, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Artem Savkov, bpf, linux-modules, linux-kernel,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Petr Mladek,
	Leizhen (ThunderTown),
	Viktor Malik

On Tue, Apr 04, 2023 at 12:02:54AM +0200, Jiri Olsa wrote:
> Artem reported suspicious RCU usage [1]. The reason is that verifier
> calls find_kallsyms_symbol_value with preemption enabled which will
> trigger suspicious RCU usage warning in rcu_dereference_sched call.
> 
> Disabling preemption in find_kallsyms_symbol_value and adding
> __find_kallsyms_symbol_value function.
> 
> Fixes: 31bf1dbccfb0 ("bpf: Fix attaching fentry/fexit/fmod_ret/lsm to modules")
> [1] https://lore.kernel.org/bpf/ZBrPMkv8YVRiWwCR@samus.usersys.redhat.com/
> Reported-by: Artem Savkov <asavkov@redhat.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  kernel/module/kallsyms.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
> index ab2376a1be88..bdc911dbcde5 100644
> --- a/kernel/module/kallsyms.c
> +++ b/kernel/module/kallsyms.c
> @@ -442,7 +442,7 @@ int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
>  }
>  
>  /* Given a module and name of symbol, find and return the symbol's value */
> -unsigned long find_kallsyms_symbol_value(struct module *mod, const char *name)
> +static unsigned long __find_kallsyms_symbol_value(struct module *mod, const char *name)
>  {
>  	unsigned int i;
>  	struct mod_kallsyms *kallsyms = rcu_dereference_sched(mod->kallsyms);
> @@ -466,7 +466,7 @@ static unsigned long __module_kallsyms_lookup_name(const char *name)
>  	if (colon) {
>  		mod = find_module_all(name, colon - name, false);
>  		if (mod)
> -			return find_kallsyms_symbol_value(mod, colon + 1);
> +			return __find_kallsyms_symbol_value(mod, colon + 1);
>  		return 0;
>  	}
>  
> @@ -475,7 +475,7 @@ static unsigned long __module_kallsyms_lookup_name(const char *name)
>  
>  		if (mod->state == MODULE_STATE_UNFORMED)
>  			continue;
> -		ret = find_kallsyms_symbol_value(mod, name);
> +		ret = __find_kallsyms_symbol_value(mod, name);
>  		if (ret)
>  			return ret;
>  	}
> @@ -494,6 +494,16 @@ unsigned long module_kallsyms_lookup_name(const char *name)
>  	return ret;
>  }
>  
> +unsigned long find_kallsyms_symbol_value(struct module *mod, const char *name)
> +{
> +	unsigned long ret;
> +
> +	preempt_disable();
> +	ret = __find_kallsyms_symbol_value(mod, name);
> +	preempt_enable();
> +	return ret;
> +}
> +
>  int module_kallsyms_on_each_symbol(const char *modname,
>  				   int (*fn)(void *, const char *,
>  					     struct module *, unsigned long),
> -- 
> 2.39.2
> 

Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>

-- 
Aaron Tomlin

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

end of thread, other threads:[~2023-04-06 14:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-03 22:02 [PATCH bpf-next] kallsyms: Disable preemption for find_kallsyms_symbol_value Jiri Olsa
2023-04-04  1:43 ` Leizhen (ThunderTown)
2023-04-04  6:56 ` Artem Savkov
2023-04-05  0:20 ` patchwork-bot+netdevbpf
2023-04-06 14:47 ` Petr Mladek
2023-04-06 14:56 ` Aaron Tomlin

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