[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4ef46874-4dbc-2663-ebad-7adbbbfa3b18@huawei.com>
Date: Sat, 24 Sep 2022 09:11:24 +0800
From: "Leizhen (ThunderTown)" <thunder.leizhen@...wei.com>
To: Josh Poimboeuf <jpoimboe@...nel.org>,
Jiri Kosina <jikos@...nel.org>,
Miroslav Benes <mbenes@...e.cz>,
Petr Mladek <pmladek@...e.com>,
Joe Lawrence <joe.lawrence@...hat.com>,
<live-patching@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
Masahiro Yamada <masahiroy@...nel.org>,
Alexei Starovoitov <ast@...nel.org>,
Jiri Olsa <jolsa@...nel.org>,
Kees Cook <keescook@...omium.org>,
Andrew Morton <akpm@...ux-foundation.org>,
"Luis Chamberlain" <mcgrof@...nel.org>,
<linux-modules@...r.kernel.org>,
"Steven Rostedt" <rostedt@...dmis.org>,
Ingo Molnar <mingo@...hat.com>
Subject: Re: [PATCH v5 08/10] livepatch: Improve the search performance of
module_kallsyms_on_each_symbol()
On 2022/9/23 19:20, Zhen Lei wrote:
> Currently we traverse all symbols of all modules to find the specified
> function for the specified module. But in reality, we just need to find
> the given module and then traverse all the symbols in it.
>
> In order to achieve this purpose, split the call to hook 'fn' into two
> phases:
> 1. Finds the given module. Pass pointer 'mod'. Hook 'fn' directly returns
> the comparison result of the module name without comparing the function
> name.
> 2. Finds the given function in that module. Pass pointer 'mod = NULL'.
> Hook 'fn' skip the comparison of module name and directly compare
> function names.
Sorry, I forgot to change the description. I will fix it in v6, after I've
collected review comments.
>
> Phase1: mod1-->mod2..(subsequent modules do not need to be compared)
> |
> Phase2: -->f1-->f2-->f3
>
> Signed-off-by: Zhen Lei <thunder.leizhen@...wei.com>
> ---
> include/linux/module.h | 4 ++--
> kernel/livepatch/core.c | 13 ++-----------
> kernel/module/kallsyms.c | 15 ++++++++++++---
> 3 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 518296ea7f73af6..6e1a531d78e7e8b 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -879,8 +879,8 @@ static inline bool module_sig_ok(struct module *module)
> }
> #endif /* CONFIG_MODULE_SIG */
>
> -int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
> - struct module *, unsigned long),
> +int module_kallsyms_on_each_symbol(const char *modname,
> + int (*fn)(void *, const char *, unsigned long),
> void *data);
>
> #endif /* _LINUX_MODULE_H */
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 31b57ccf908017e..b02de4cb311c703 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -118,27 +118,19 @@ static struct klp_object *klp_find_object(struct klp_patch *patch,
> }
>
> struct klp_find_arg {
> - const char *objname;
> const char *name;
> unsigned long addr;
> unsigned long count;
> unsigned long pos;
> };
>
> -static int klp_find_callback(void *data, const char *name,
> - struct module *mod, unsigned long addr)
> +static int klp_find_callback(void *data, const char *name, unsigned long addr)
> {
> struct klp_find_arg *args = data;
>
> - if ((mod && !args->objname) || (!mod && args->objname))
> - return 0;
> -
> if (strcmp(args->name, name))
> return 0;
>
> - if (args->objname && strcmp(args->objname, mod->name))
> - return 0;
> -
> args->addr = addr;
> args->count++;
>
> @@ -175,7 +167,6 @@ static int klp_find_object_symbol(const char *objname, const char *name,
> unsigned long sympos, unsigned long *addr)
> {
> struct klp_find_arg args = {
> - .objname = objname,
> .name = name,
> .addr = 0,
> .count = 0,
> @@ -183,7 +174,7 @@ static int klp_find_object_symbol(const char *objname, const char *name,
> };
>
> if (objname)
> - module_kallsyms_on_each_symbol(klp_find_callback, &args);
> + module_kallsyms_on_each_symbol(objname, klp_find_callback, &args);
> else
> kallsyms_on_each_match_symbol(klp_match_callback, name, &args);
>
> diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
> index f5c5c9175333df7..329cef573675d49 100644
> --- a/kernel/module/kallsyms.c
> +++ b/kernel/module/kallsyms.c
> @@ -495,8 +495,8 @@ unsigned long module_kallsyms_lookup_name(const char *name)
> }
>
> #ifdef CONFIG_LIVEPATCH
> -int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
> - struct module *, unsigned long),
> +int module_kallsyms_on_each_symbol(const char *modname,
> + int (*fn)(void *, const char *, unsigned long),
> void *data)
> {
> struct module *mod;
> @@ -510,6 +510,9 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
> if (mod->state == MODULE_STATE_UNFORMED)
> continue;
>
> + if (strcmp(modname, mod->name))
> + continue;
> +
> /* Use rcu_dereference_sched() to remain compliant with the sparse tool */
> preempt_disable();
> kallsyms = rcu_dereference_sched(mod->kallsyms);
> @@ -522,10 +525,16 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
> continue;
>
> ret = fn(data, kallsyms_symbol_name(kallsyms, i),
> - mod, kallsyms_symbol_value(sym));
> + kallsyms_symbol_value(sym));
> if (ret != 0)
> goto out;
> }
> +
> + /*
> + * The given module is found, the subsequent modules do not
> + * need to be compared.
> + */
> + break;
> }
> out:
> mutex_unlock(&module_mutex);
>
--
Regards,
Zhen Lei
Powered by blists - more mailing lists