+++ Petr Mladek [14/03/16 16:06 +0100]:
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@xxxxxxxxxx>
Acked-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
Acked-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
---
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.
Ah, you are right. I think you meant false, though :-) Will fix that.
klp_free_object_loaded(obj);
+ mutex_unlock(&klp_mutex);
+
+ return ret;
}
With the above fix:
Reviewed-by: Petr Mladek <pmladek@xxxxxxx>
Best Regards,
Petr
--
To unsubscribe from this list: send the line "unsubscribe live-patching" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
