From: Petr Mladek <pmladek@suse.com>
To: Jessica Yu <jeyu@redhat.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>,
Seth Jennings <sjenning@redhat.com>,
Jiri Kosina <jikos@kernel.org>, Vojtech Pavlik <vojtech@suse.com>,
Miroslav Benes <mbenes@suse.cz>,
Rusty Russell <rusty@rustcorp.com.au>,
Steven Rostedt <rostedt@goodmis.org>,
live-patching@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] livepatch/module: remove livepatch module notifier
Date: Mon, 14 Mar 2016 16:06:43 +0100 [thread overview]
Message-ID: <20160314150643.GU10940@pathway.suse.cz> (raw)
In-Reply-To: <1457726628-9171-3-git-send-email-jeyu@redhat.com>
On Fri 2016-03-11 15:03:48, Jessica Yu wrote:
> Remove the livepatch module notifier in favor of directly enabling and
> disabling patches to modules in the module loader. Hard-coding the
> function calls ensures that ftrace_module_enable() is run before
> klp_module_coming() during module load, and that klp_module_going() is
> run before ftrace_release_mod() during module unload. This way, ftrace
> and livepatch code is run in the correct order during the module
> load/unload sequence without dependence on the module notifier call chain.
>
> Signed-off-by: Jessica Yu <jeyu@redhat.com>
> Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>
> Acked-by: Rusty Russell <rusty@rustcorp.com.au>
> ---
> include/linux/livepatch.h | 13 ++++
> kernel/livepatch/core.c | 147 ++++++++++++++++++++++------------------------
> kernel/module.c | 10 ++++
> 3 files changed, 94 insertions(+), 76 deletions(-)
>
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 780f00c..4fafbae 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -866,103 +866,108 @@ int klp_register_patch(struct klp_patch *patch)
> }
> EXPORT_SYMBOL_GPL(klp_register_patch);
>
> -static int klp_module_notify_coming(struct klp_patch *patch,
> - struct klp_object *obj)
> +int klp_module_coming(struct module *mod)
> {
> - struct module *pmod = patch->mod;
> - struct module *mod = obj->mod;
> int ret;
> + struct klp_patch *patch;
> + struct klp_object *obj;
>
> - ret = klp_init_object_loaded(patch, obj);
> - if (ret) {
> - pr_warn("failed to initialize patch '%s' for module '%s' (%d)\n",
> - pmod->name, mod->name, ret);
> - return ret;
> - }
> + if (WARN_ON(mod->state != MODULE_STATE_COMING))
> + return -EINVAL;
>
> - if (patch->state == KLP_DISABLED)
> - return 0;
> + mutex_lock(&klp_mutex);
> + /*
> + * Each module has to know that klp_module_coming()
> + * has been called. We never know what module will
> + * get patched by a new patch.
> + */
> + mod->klp_alive = true;
>
> - pr_notice("applying patch '%s' to loading module '%s'\n",
> - pmod->name, mod->name);
> + list_for_each_entry(patch, &klp_patches, list) {
> + klp_for_each_object(patch, obj) {
> + if (!klp_is_module(obj) || strcmp(obj->name, mod->name))
> + continue;
>
> - ret = klp_enable_object(obj);
> - if (ret)
> - pr_warn("failed to apply patch '%s' to module '%s' (%d)\n",
> - pmod->name, mod->name, ret);
> - return ret;
> -}
> + obj->mod = mod;
>
> -static void klp_module_notify_going(struct klp_patch *patch,
> - struct klp_object *obj)
> -{
> - struct module *pmod = patch->mod;
> - struct module *mod = obj->mod;
> + ret = klp_init_object_loaded(patch, obj);
> + if (ret) {
> + pr_warn("failed to initialize patch '%s' for module '%s' (%d)\n",
> + patch->mod->name, obj->mod->name, ret);
> + goto err;
> + }
>
> - if (patch->state == KLP_DISABLED)
> - goto disabled;
> + if (patch->state == KLP_DISABLED)
> + break;
> +
> + pr_notice("applying patch '%s' to loading module '%s'\n",
> + patch->mod->name, obj->mod->name);
> +
> + ret = klp_enable_object(obj);
> + if (ret) {
> + pr_warn("failed to apply patch '%s' to module '%s' (%d)\n",
> + patch->mod->name, obj->mod->name, ret);
> + goto err;
> + }
> +
> + break;
> + }
> + }
>
> - pr_notice("reverting patch '%s' on unloading module '%s'\n",
> - pmod->name, mod->name);
> + mutex_unlock(&klp_mutex);
>
> - klp_disable_object(obj);
> + return 0;
>
> -disabled:
> +err:
> + /*
> + * If a patch is unsuccessfully applied, return
> + * error to the module loader.
> + */
> + pr_warn("patch '%s' failed for module '%s', refusing to load module '%s'\n",
> + patch->mod->name, obj->mod->name, obj->mod->name);
One more thing. We should add here:
mod->klp_alive = true;
Otherwise, there is a small race window when a new patch will try to
patch the module.
> klp_free_object_loaded(obj);
> + mutex_unlock(&klp_mutex);
> +
> + return ret;
> }
With the above fix:
Reviewed-by: Petr Mladek <pmladek@suse.cz>
Best Regards,
Petr
next prev parent reply other threads:[~2016-03-14 15:06 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-11 20:03 [PATCH v2 0/2] Livepatch module notifier cleanup Jessica Yu
2016-03-11 20:03 ` [PATCH v2 1/2] modules: split part of complete_formation() into prepare_coming_module() Jessica Yu
2016-03-14 15:12 ` Petr Mladek
2016-03-11 20:03 ` [PATCH v2 2/2] livepatch/module: remove livepatch module notifier Jessica Yu
2016-03-14 15:06 ` Petr Mladek [this message]
2016-03-14 17:50 ` Jessica Yu
2016-03-15 9:10 ` Petr Mladek
2016-03-14 20:01 ` [PATCH v2 2/2] " Josh Poimboeuf
-- strict thread matches above, loose matches on Subject: below --
2016-02-02 1:17 [PATCH v2 0/2] Fix ordering of ftrace/livepatch calls on module load and unload Jessica Yu
2016-02-02 1:17 ` [PATCH v2 2/2] livepatch/module: remove livepatch module notifier Jessica Yu
2016-02-04 14:39 ` Petr Mladek
2016-02-04 14:56 ` Steven Rostedt
2016-02-04 16:47 ` Miroslav Benes
2016-02-04 17:29 ` Miroslav Benes
2016-02-04 20:55 ` Josh Poimboeuf
2016-02-05 8:59 ` Miroslav Benes
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160314150643.GU10940@pathway.suse.cz \
--to=pmladek@suse.com \
--cc=jeyu@redhat.com \
--cc=jikos@kernel.org \
--cc=jpoimboe@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=live-patching@vger.kernel.org \
--cc=mbenes@suse.cz \
--cc=rostedt@goodmis.org \
--cc=rusty@rustcorp.com.au \
--cc=sjenning@redhat.com \
--cc=vojtech@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).