linux-kernel - [PATCH 3/6] livepatch v5: find and verify the old address in klp_*

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1418148307-21434-4-git-send-email-pmladek@suse.cz>
Date:	Tue,  9 Dec 2014 19:05:04 +0100
From:	Petr Mladek <pmladek@...e.cz>
To:	Seth Jennings <sjenning@...hat.com>,
	Josh Poimboeuf <jpoimboe@...hat.com>,
	Jiri Kosina <jkosina@...e.cz>,
	Vojtech Pavlik <vojtech@...e.cz>,
	Steven Rostedt <rostedt@...dmis.org>,
	Miroslav Benes <mbenes@...e.cz>,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
Cc:	Christoph Hellwig <hch@...radead.org>,
	Greg KH <gregkh@...uxfoundation.org>,
	Andy Lutomirski <luto@...capital.net>,
	live-patching@...r.kernel.org, x86@...nel.org, kpatch@...hat.com,
	linux-kernel@...r.kernel.org, Petr Mladek <pmladek@...e.cz>
Subject: [PATCH 3/6] livepatch v5: find and verify the old address in klp_*_init()

The patched address is found and checked when the patch is enabled.
If there is a failure the patch must be reverted. It might happen,
for example, when there are duplicated symbols or when the patch
and the kernel are incompatible.

It would be better to detect these problems earlier when the structures
are initialized. It will also help to avoid the costly symbol lookup
when the patch is disabled and enabled again.

This patch does the necessary changes. It adds some complexity
to module_coming() and module_going() functions but I think that
the design is cleaner. For example, it removes the strange situation
when klp_find_verify_func_addr() sets func->old_addr but it complains
when even a valid value has already been set before.

Note that I plan to remove the duplicate code by introducing
__klp_init_func().

Signed-off-by: Petr Mladek <pmladek@...e.cz>
---
 kernel/livepatch/core.c | 44 +++++++++++++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 034f79a926af..54bb39d3abb5 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -179,7 +179,8 @@ static int klp_verify_vmlinux_symbol(const char *name, unsigned long addr)
 	return -EINVAL;
 }
 
-static int klp_find_verify_func_addr(struct klp_func *func, const char *objname)
+static int klp_find_verify_func_addr(struct klp_object *obj,
+				     struct klp_func *func)
 {
 	int ret;
 
@@ -188,17 +189,18 @@ static int klp_find_verify_func_addr(struct klp_func *func, const char *objname)
 	func->old_addr = 0;
 #endif
 
-	if (func->old_addr && objname) {
-		pr_err("old address specified for module symbol\n");
-		return -EINVAL;
-	}
-
-	if (func->old_addr)
+	/*
+	 * If the old address is predefined, it might be used to check
+	 * the consistency between the patch and the patched kernel.
+	 * This does not work for kernel modules where the address
+	 * always must be detected.
+	 */
+	if (!func->old_addr || klp_is_module(obj))
+		ret = klp_find_object_symbol(obj->name, func->old_name,
+					     &func->old_addr);
+	else
 		ret = klp_verify_vmlinux_symbol(func->old_name,
 						func->old_addr);
-	else
-		ret = klp_find_object_symbol(objname, func->old_name,
-					     &func->old_addr);
 
 	return ret;
 }
@@ -348,9 +350,6 @@ static int klp_disable_object(struct klp_object *obj)
 		ret = klp_disable_func(func);
 		if (ret)
 			return ret;
-
-		if (klp_is_module(obj))
-			func->old_addr = 0;
 	}
 
 	obj->state = KLP_DISABLED;
@@ -370,10 +369,6 @@ static int klp_enable_object(struct klp_object *obj)
 		return -EINVAL;
 
 	for (func = obj->funcs; func->old_name; func++) {
-		ret = klp_find_verify_func_addr(func, obj->name);
-		if (ret)
-			goto unregister;
-
 		ret = klp_enable_func(func);
 		if (ret)
 			goto unregister;
@@ -512,6 +507,7 @@ EXPORT_SYMBOL_GPL(klp_enable_patch);
 static void klp_module_notify_coming(struct module *pmod,
 				     struct klp_object *obj)
 {
+	struct klp_func *func;
 	struct module *mod = obj->mod;
 	int ret;
 
@@ -524,6 +520,12 @@ static void klp_module_notify_coming(struct module *pmod,
 			goto err;
 	}
 
+	for (func = obj->funcs; func->old_name; func++) {
+		ret = klp_find_verify_func_addr(obj, func);
+		if (ret)
+			goto err;
+	}
+
 	ret = klp_enable_object(obj);
 	if (!ret)
 		return;
@@ -536,6 +538,7 @@ err:
 static void klp_module_notify_going(struct module *pmod,
 				    struct klp_object *obj)
 {
+	struct klp_func *func;
 	struct module *mod = obj->mod;
 	int ret;
 
@@ -547,6 +550,9 @@ static void klp_module_notify_going(struct module *pmod,
 		pr_warn("failed to revert patch '%s' on module '%s' (%d)\n",
 			pmod->name, mod->name, ret);
 
+	for (func = obj->funcs; func->old_name; func++)
+		func->old_addr = 0;
+
 	obj->mod = NULL;
 }
 
@@ -719,6 +725,10 @@ static int klp_init_func(struct klp_object *obj, struct klp_func *func)
 
 	func->state = KLP_DISABLED;
 
+	ret = klp_find_verify_func_addr(obj, func);
+	if (ret)
+		return ret;
+
 	ops = kzalloc(sizeof(*ops), GFP_KERNEL);
 	if (!ops)
 		ret = -ENOMEM;
-- 
1.8.5.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/