From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752041AbdJTO5D (ORCPT ); Fri, 20 Oct 2017 10:57:03 -0400 Received: from mx2.suse.de ([195.135.220.15]:52386 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751753AbdJTO5A (ORCPT ); Fri, 20 Oct 2017 10:57:00 -0400 From: Petr Mladek To: Jiri Kosina , Joe Lawrence Cc: Josh Poimboeuf , Jessica Yu , Miroslav Benes , Chris J Arges , live-patching@vger.kernel.org, linux-kernel@vger.kernel.org, Petr Mladek Subject: [PATCH 1/2] livepatch: Correctly call klp_post_unpatch_callback() in error paths Date: Fri, 20 Oct 2017 16:56:50 +0200 Message-Id: <1508511411-4189-2-git-send-email-pmladek@suse.com> X-Mailer: git-send-email 1.8.5.6 In-Reply-To: <1508511411-4189-1-git-send-email-pmladek@suse.com> References: <1508511411-4189-1-git-send-email-pmladek@suse.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org The flag post_unpatch_enabled in struct klp_callbacks is need in the error paths. We need to call the post_unpatch callback only when the pre_patch one was called. We should clear the flag in klp_post_unpatch_callback() to make sure that the callback is not called twice. It makes the API more safe. Note that we actually would call the callback twice in klp_module_coming() when klp_patch_object() fails. In this case, we explicitly call klp_post_unpatch_callback() for the failed object. And we are going to call it once again when reverting operations for all the patches by reusing the klp_module_going() code. There is a patch doing this in the queue. There was another mistake in the error path in klp_comming_module(). It called klp_post_unpatch_callback() only when patch != klp_transition_patch. But klp_pre_patch_callback() was called even for this patch. In fact, we could remove klp_post_unpatch_callback() from the error path at all because it will be covered by the reuse of the klp_module_going() code. But I think that it is cleaner this way. For example, someone might later decide to call the callback only when obj->patched flag is set. Finally, I used this opportunity to make klp_pre_patch_callback() more readable. Signed-off-by: Petr Mladek --- kernel/livepatch/core.c | 4 +--- kernel/livepatch/core.h | 8 +++++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c index cafb5a84417d..eb134479c394 100644 --- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c @@ -894,9 +894,7 @@ int klp_module_coming(struct module *mod) pr_warn("failed to apply patch '%s' to module '%s' (%d)\n", patch->mod->name, obj->mod->name, ret); - if (patch != klp_transition_patch) - klp_post_unpatch_callback(obj); - + klp_post_unpatch_callback(obj); goto err; } diff --git a/kernel/livepatch/core.h b/kernel/livepatch/core.h index 6fc907b54e71..cc3aa708e0b4 100644 --- a/kernel/livepatch/core.h +++ b/kernel/livepatch/core.h @@ -12,10 +12,10 @@ static inline bool klp_is_object_loaded(struct klp_object *obj) static inline int klp_pre_patch_callback(struct klp_object *obj) { - int ret; + int ret = 0; - ret = (obj->callbacks.pre_patch) ? - (*obj->callbacks.pre_patch)(obj) : 0; + if (obj->callbacks.pre_patch) + ret = (*obj->callbacks.pre_patch)(obj); obj->callbacks.post_unpatch_enabled = !ret; @@ -39,6 +39,8 @@ static inline void klp_post_unpatch_callback(struct klp_object *obj) if (obj->callbacks.post_unpatch_enabled && obj->callbacks.post_unpatch) (*obj->callbacks.post_unpatch)(obj); + + obj->callbacks.post_unpatch_enabled = false; } #endif /* _LIVEPATCH_CORE_H */ -- 1.8.5.6