lkml.org 
[lkml]   [2016]   [Mar]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: livepatch/module: remove livepatch module notifier
+++ 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@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.

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@suse.cz>
>
>
>Best Regards,
>Petr

\
 
 \ /
  Last update: 2016-03-14 19:41    [W:0.702 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site