From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757346AbbCEPo4 (ORCPT ); Thu, 5 Mar 2015 10:44:56 -0500 Received: from cantor2.suse.de ([195.135.220.15]:34847 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751208AbbCEPou (ORCPT ); Thu, 5 Mar 2015 10:44:50 -0500 From: Petr Mladek To: Seth Jennings , Josh Poimboeuf , Jiri Kosina , Rusty Russell Cc: Miroslav Benes , Masami Hiramatsu , mingo@kernel.org, mathieu.desnoyers@efficios.com, oleg@redhat.com, paulmck@linux.vnet.ibm.com, live-patching@vger.kernel.org, linux-kernel@vger.kernel.org, andi@firstfloor.org, rostedt@goodmis.org, tglx@linutronix.de, Petr Mladek Subject: [PATCH v2 1/2] livepatch/module: Apply patch when loaded module is unformed Date: Thu, 5 Mar 2015 16:45:13 +0100 Message-Id: <1425570314-23675-2-git-send-email-pmladek@suse.cz> X-Mailer: git-send-email 1.8.5.6 In-Reply-To: <1425570314-23675-1-git-send-email-pmladek@suse.cz> References: <1425570314-23675-1-git-send-email-pmladek@suse.cz> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Existing live patches are applied to loaded modules using a notify handler. There are two problems with this approach. First, errors from module notifiers are ignored and could not stop the module from being loaded. But we will need to refuse the module when there are semantics dependencies between functions and there are some problems to apply the patch to the module. Otherwise, the system might become into an inconsistent state. Second, the module notifiers are called when the module is in STATE_MODULE_COMING. It means that it is visible by find_module() and can be detected by klp_find_object_module() when a new patch is registered. Now, the timing is important. If the new patch is registered after the module notifier has been called, it has to initialize the module object for the new patch. Note that, in this case, the new patch has to see the module as loaded even when it is still in the COMING state. But when the new patch is registered before the module notifier, it _should_ not initialize the module object, see below for detailed explanation. This patch solves both problems by calling klp_module_init() directly in load_module(). We could handle the error there. Also it is called in MODULE_STATE_UNFORMED and therefore before the module is visible via find_module(). The implementation creates three functions for module init and three functions for going modules. We need to revert already initialized patches when something fails and thus need to be able to call the code for going modules without leaving klp_mutex. Detailed explanation of the last problem: Why should not we initialize the module object for a new patch when the related module coming notifier has not been called yet? Note that the notifier could _not_ _simply_ ignore already initialized module objects. The notifier initializes the module object for all existing patches. If the new patch is registered and enabled before, it would crate wrong order of patches in fops->func_stack. For example, let's have three patches (P1, P2, P3) for the functions a() and b() where a() is from vmcore and b() is from a module M. Something like: a() b() P1 a1() b1() P2 a2() b2() P3 a3() b3(3) If you load the module M after all patches are registered and enabled. The ftrace ops for function a() and b() has listed the functions in this order ops_a->func_stack -> list(a3,a2,a1) ops_b->func_stack -> list(b3,b2,b1) , so the pointer to b3() is the first and will be used. Then you might have the following scenario. Let's start with state when patches P1 and P2 are registered and enabled but the module M is not loaded. Then ftrace ops for b() does not exist. Then we get into the following race: CPU0 CPU1 load_module(M) complete_formation() mod->state = MODULE_STATE_COMING; mutex_unlock(&module_mutex); klp_register_patch(P3); klp_enable_patch(P3); # STATE 1 klp_module_notify(M) klp_module_notify_coming(P1); klp_module_notify_coming(P2); klp_module_notify_coming(P3); # STATE 2 The ftrace ops for a() and b() then looks: STATE1: ops_a->func_stack -> list(a3,a2,a1); ops_b->func_stack -> list(b3); STATE2: ops_a->func_stack -> list(a3,a2,a1); ops_b->func_stack -> list(b2,b1,b3); therefore, b2() is used for the module but a3() is used for vmcore because they were the last added. Signed-off-by: Petr Mladek --- include/linux/livepatch.h | 10 +++++ kernel/livepatch/core.c | 94 +++++++++++++++++++++++++++++++++++------------ kernel/module.c | 9 +++++ 3 files changed, 89 insertions(+), 24 deletions(-) diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h index ee6dbb39a809..78ac10546160 100644 --- a/include/linux/livepatch.h +++ b/include/linux/livepatch.h @@ -128,6 +128,16 @@ int klp_unregister_patch(struct klp_patch *); int klp_enable_patch(struct klp_patch *); int klp_disable_patch(struct klp_patch *); +int klp_module_init(struct module *mod); + +#else /* CONFIG_LIVEPATCH */ + +inline int klp_module_init(struct module *mod) +{ + return 0; +} + #endif /* CONFIG_LIVEPATCH */ + #endif /* _LINUX_LIVEPATCH_H_ */ diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c index 7e0c83dc7215..198f7733604b 100644 --- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c @@ -869,8 +869,8 @@ int klp_register_patch(struct klp_patch *patch) } EXPORT_SYMBOL_GPL(klp_register_patch); -static void klp_module_notify_coming(struct klp_patch *patch, - struct klp_object *obj) +static int klp_module_coming_update_patch(struct klp_patch *patch, + struct klp_object *obj) { struct module *pmod = patch->mod; struct module *mod = obj->mod; @@ -881,22 +881,62 @@ static void klp_module_notify_coming(struct klp_patch *patch, goto err; if (patch->state == KLP_DISABLED) - return; + return 0; pr_notice("applying patch '%s' to loading module '%s'\n", pmod->name, mod->name); ret = klp_enable_object(obj); if (!ret) - return; + return 0; err: pr_warn("failed to apply patch '%s' to module '%s' (%d)\n", pmod->name, mod->name, ret); + return ret; } -static void klp_module_notify_going(struct klp_patch *patch, - struct klp_object *obj) +static void klp_module_going(struct module *mod); + +int klp_module_coming(struct module *mod) +{ + struct klp_patch *patch; + struct klp_object *obj; + int ret = 0; + + list_for_each_entry(patch, &klp_patches, list) { + for (obj = patch->objs; obj->funcs; obj++) { + if (!klp_is_module(obj) || strcmp(obj->name, mod->name)) + continue; + + obj->mod = mod; + ret = klp_module_coming_update_patch(patch, obj); + if (ret) + goto err; + } + } + + return 0; + +err: + klp_module_going(mod); + return ret; +} + + +int klp_module_init(struct module *mod) +{ + int ret = 0; + + mutex_lock(&klp_mutex); + ret = klp_module_coming(mod); + mutex_unlock(&klp_mutex); + + return ret; +} + +static void klp_module_going_update_patch(struct klp_patch *patch, + struct klp_object *obj) { struct module *pmod = patch->mod; struct module *mod = obj->mod; @@ -913,40 +953,46 @@ disabled: klp_free_object_loaded(obj); } -static int klp_module_notify(struct notifier_block *nb, unsigned long action, - void *data) +static void klp_module_going(struct module *mod) { - struct module *mod = data; struct klp_patch *patch; struct klp_object *obj; - if (action != MODULE_STATE_COMING && action != MODULE_STATE_GOING) - return 0; - - mutex_lock(&klp_mutex); - list_for_each_entry(patch, &klp_patches, list) { for (obj = patch->objs; obj->funcs; obj++) { - if (!klp_is_module(obj) || strcmp(obj->name, mod->name)) + /* + * Handle only loaded (initialized) modules. + * This is needed when used in an error path. + */ + if (!klp_is_object_loaded(obj) || + strcmp(obj->name, mod->name)) continue; - if (action == MODULE_STATE_COMING) { - obj->mod = mod; - klp_module_notify_coming(patch, obj); - } else /* MODULE_STATE_GOING */ - klp_module_notify_going(patch, obj); - - break; + klp_module_going_update_patch(patch, obj); } } - mutex_unlock(&klp_mutex); + return; +} + +static int klp_module_notify_going(struct notifier_block *nb, + unsigned long action, + void *data) +{ + struct module *mod = data; + + if (action != MODULE_STATE_GOING) + return 0; + + mutex_lock(&klp_mutex); + klp_module_going(mod); + mutex_lock(&klp_mutex); return 0; } static struct notifier_block klp_module_nb = { - .notifier_call = klp_module_notify, + .notifier_call = klp_module_notify_going, .priority = INT_MIN+1, /* called late but before ftrace notifier */ }; diff --git a/kernel/module.c b/kernel/module.c index d856e96a3cce..f744a639460d 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -53,6 +53,7 @@ #include #include #include +#include #include #include #include @@ -3321,6 +3322,14 @@ static int load_module(struct load_info *info, const char __user *uargs, /* Ftrace init must be called in the MODULE_STATE_UNFORMED state */ ftrace_module_init(mod); + /* + * LivePatch init must be called in the MODULE_STATE_UNFORMED state + * and it might reject the module to avoid a system inconsistency. + */ + err = klp_module_init(mod); + if (err) + goto ddebug_cleanup; + /* Finally it's fully formed, ready to start executing. */ err = complete_formation(mod, info); if (err) -- 1.8.5.6