| Message ID | 20230403220254.2191240-1-jolsa@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [bpf-next] kallsyms: Disable preemption for find_kallsyms_symbol_value | expand |
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), >
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 >
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!
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
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>
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),
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(-)