public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: Jiri Kosina <jikos@kernel.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Miroslav Benes <mbenes@suse.cz>
Cc: Jason Baron <jbaron@akamai.com>,
	Joe Lawrence <joe.lawrence@redhat.com>,
	Jessica Yu <jeyu@kernel.org>,
	Evgenii Shatokhin <eshatokhin@virtuozzo.com>,
	live-patching@vger.kernel.org, linux-kernel@vger.kernel.org,
	Petr Mladek <pmladek@suse.com>
Subject: [PATCH v8 7/8] livepatch: Correctly handle atomic replace for not yet loaded modules
Date: Wed, 21 Feb 2018 14:29:13 +0100	[thread overview]
Message-ID: <20180221132914.4809-8-pmladek@suse.com> (raw)
In-Reply-To: <20180221132914.4809-1-pmladek@suse.com>

The atomic replace feature uses dynamically allocated struct klp_func to
handle functions that will not longer be patched. These structures are
of the type KLP_FUNC_NOP. They cause the ftrace handler to jump to
the original code. But the address of the original code is not known
until the patched module is loaded.

This patch allows the late initialization. Also it adds a sanity check
into the ftrace handler.

Alternative solution would be to do not set the address at all. The ftrace
handler could just return to the original code when NOP struct klp_func
is used. But this would require another changes. For example, in the stack
checking. Note that NOP structures might be available even when the patch
is being disabled. This would happen when the patch enable transition is
reverted.

Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 kernel/livepatch/core.c  | 16 ++++++++++++++--
 kernel/livepatch/patch.c |  5 +++++
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index ad508a86b2f9..da1438d47d83 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -945,8 +945,12 @@ static void klp_free_object_loaded(struct klp_object *obj)
 
 	obj->mod = NULL;
 
-	klp_for_each_func(obj, func)
+	klp_for_each_func(obj, func) {
 		func->old_addr = 0;
+
+		if (klp_is_func_type(func, KLP_FUNC_NOP))
+			func->new_func = NULL;
+	}
 }
 
 /*
@@ -984,7 +988,12 @@ static void klp_free_patch(struct klp_patch *patch)
 
 static int klp_init_func(struct klp_object *obj, struct klp_func *func)
 {
-	if (!func->old_name || !func->new_func)
+	if (!func->old_name)
+		return -EINVAL;
+
+	/* NOPs do not know the address until the patched module is loaded. */
+	if (!func->new_func &&
+	    (!klp_is_func_type(func, KLP_FUNC_NOP) || klp_is_object_loaded(obj)))
 		return -EINVAL;
 
 	INIT_LIST_HEAD(&func->stack_node);
@@ -1039,6 +1048,9 @@ static int klp_init_object_loaded(struct klp_patch *patch,
 			return -ENOENT;
 		}
 
+		if (klp_is_func_type(func, KLP_FUNC_NOP))
+			func->new_func = (void *)func->old_addr;
+
 		ret = kallsyms_lookup_size_offset((unsigned long)func->new_func,
 						  &func->new_size, NULL);
 		if (!ret) {
diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c
index 54b3c379bb0f..1f5c3eea9ee1 100644
--- a/kernel/livepatch/patch.c
+++ b/kernel/livepatch/patch.c
@@ -118,7 +118,12 @@ static void notrace klp_ftrace_handler(unsigned long ip,
 		}
 	}
 
+	/* Survive ugly mistakes, for example, when handling NOPs. */
+	if (WARN_ON_ONCE(!func->new_func))
+		goto unlock;
+
 	klp_arch_set_pc(regs, (unsigned long)func->new_func);
+
 unlock:
 	preempt_enable_notrace();
 }
-- 
2.13.6

  parent reply	other threads:[~2018-02-21 13:29 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-21 13:29 [PATCH v8 0/8] livepatch: Atomic replace feature Petr Mladek
2018-02-21 13:29 ` [PATCH v8 1/8] livepatch: Use lists to manage patches, objects and functions Petr Mladek
2018-02-21 13:29 ` [PATCH v8 2/8] livepatch: Free only structures with initialized kobject Petr Mladek
2018-02-21 13:29 ` [PATCH v8 3/8] livepatch: Initial support for dynamic structures Petr Mladek
2018-02-21 13:29 ` [PATCH v8 4/8] livepatch: Allow to unpatch only functions of the given type Petr Mladek
2018-02-22 15:25   ` Miroslav Benes
2018-02-21 13:29 ` [PATCH v8 5/8] livepatch: Support separate list for replaced patches Petr Mladek
2018-02-21 13:29 ` [PATCH v8 6/8] livepatch: Add atomic replace Petr Mladek
2018-02-21 13:29 ` Petr Mladek [this message]
2018-02-22 21:00   ` [PATCH v8 7/8] livepatch: Correctly handle atomic replace for not yet loaded modules Miroslav Benes
2018-03-01 10:28     ` Petr Mladek
2018-03-02 22:00       ` Joe Lawrence
2018-03-05  9:54         ` Miroslav Benes
2018-03-05 14:42           ` Josh Poimboeuf
2018-03-06 14:10           ` Petr Mladek
2018-02-21 13:29 ` [PATCH v8 8/8] livepatch: Atomic replace and cumulative patches documentation Petr Mladek
2018-02-23 10:41   ` Miroslav Benes
2018-02-23 16:55     ` Joe Lawrence
2018-02-23 20:40       ` Miroslav Benes
2018-02-22 15:23 ` [PATCH v8 0/8] livepatch: Atomic replace feature Miroslav Benes
2018-03-05 10:56 ` Evgenii Shatokhin
2018-03-05 12:54   ` Miroslav Benes
2018-03-06 15:32     ` Petr Mladek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180221132914.4809-8-pmladek@suse.com \
    --to=pmladek@suse.com \
    --cc=eshatokhin@virtuozzo.com \
    --cc=jbaron@akamai.com \
    --cc=jeyu@kernel.org \
    --cc=jikos@kernel.org \
    --cc=joe.lawrence@redhat.com \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mbenes@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox