From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932821AbbELOEx (ORCPT ); Tue, 12 May 2015 10:04:53 -0400 Received: from mail-wi0-f179.google.com ([209.85.212.179]:33105 "EHLO mail-wi0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932536AbbELOEv (ORCPT ); Tue, 12 May 2015 10:04:51 -0400 From: Minfei Huang To: mbenes@suse.cz, jpoimboe@redhat.com, sjenning@redhat.com, jkosina@suse.cz, vojtech@suse.cz Cc: live-patching@vger.kernel.org, linux-kernel@vger.kernel.org, mhuang@redhat.com, Minfei Huang Subject: [PATCH v3] livepatch: Prevent to apply the patch once coming module notifier fails Date: Tue, 12 May 2015 22:04:44 +0800 Message-Id: <1431439484-44530-1-git-send-email-mnfhuang@gmail.com> X-Mailer: git-send-email 2.2.2 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org The previous patches can be applied, once the corresponding module is loaded. In general, the patch will do relocation (if necessary) and obtain/verify function address before we start to enable patch. There are three different situations in which the coming module notifier can fail: 1) relocations are not applied for some reason. In this case kallsyms for module symbol is not called at all. The patch is not applied to the module. If the user disable and enable patch again, there is possible bug in klp_enable_func. If the user specified func->old_addr for some function in the module (and he shouldn't do that, but nevertheless) our warning would not catch it, there will be something wrong with the ftrace. 2) relocations are applied successfully, but kallsyms lookup fails. In this case func->old_addr can be correct for all previous lookups, 0 for current failed one, and "unspecified" for the rest. If we undergo the same scenario as in 1, the behaviour differs for three cases, but the patch is not enable anyway. 3) the object is initialized, but klp_enable_object fails in the notifier due to possible ftrace error. Since it is improbable that ftrace would heal itself in the future, we would get those errors everytime the patch is enabled. In order to fix above situations, we can make obj->mod to NULL, if the coming modified notifier fails. Signed-off-by: Minfei Huang --- v2: - add the error message to make it more friendly - modify the commit log, base on the mbenes@suse.cz suggesting v1: - modify the commit log, describe the issue more details --- kernel/livepatch/core.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c index 284e269..e60d7ab 100644 --- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c @@ -883,7 +883,7 @@ int klp_register_patch(struct klp_patch *patch) } EXPORT_SYMBOL_GPL(klp_register_patch); -static void klp_module_notify_coming(struct klp_patch *patch, +static int klp_module_notify_coming(struct klp_patch *patch, struct klp_object *obj) { struct module *pmod = patch->mod; @@ -891,22 +891,24 @@ static void klp_module_notify_coming(struct klp_patch *patch, int ret; ret = klp_init_object_loaded(patch, obj); - if (ret) - goto err; + if (ret) { + pr_warn("failed to initialize the patch '%s' (%d)\n", + pmod->name, ret); + goto out; + } if (patch->state == KLP_DISABLED) - return; + goto out; pr_notice("applying patch '%s' to loading module '%s'\n", pmod->name, mod->name); ret = klp_enable_object(obj); - if (!ret) - return; - -err: - pr_warn("failed to apply patch '%s' to module '%s' (%d)\n", - pmod->name, mod->name, ret); + if (ret) + pr_warn("failed to apply patch '%s' to module '%s' (%d)\n", + pmod->name, mod->name, ret); +out: + return ret; } static void klp_module_notify_going(struct klp_patch *patch, @@ -930,6 +932,7 @@ disabled: static int klp_module_notify(struct notifier_block *nb, unsigned long action, void *data) { + int ret; struct module *mod = data; struct klp_patch *patch; struct klp_object *obj; @@ -955,7 +958,13 @@ static int klp_module_notify(struct notifier_block *nb, unsigned long action, if (action == MODULE_STATE_COMING) { obj->mod = mod; - klp_module_notify_coming(patch, obj); + ret = klp_module_notify_coming(patch, obj); + if (ret) { + obj->mod = NULL; + pr_warn("patch '%s' is dead, remove it " + "or re-install the module '%s'\n", + patch->mod->name, obj->name); + } } else /* MODULE_STATE_GOING */ klp_module_notify_going(patch, obj); -- 2.2.2