linux-kernel - [PATCH v9 8/9] livepatch: Improve dynamic struct 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: <20180302102935.14564-9-pmladek@suse.com>
Date:   Fri,  2 Mar 2018 11:29:34 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     Jiri Kosina <jikos@...nel.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Miroslav Benes <mbenes@...e.cz>
Cc:     Jason Baron <jbaron@...mai.com>,
        Joe Lawrence <joe.lawrence@...hat.com>,
        Jessica Yu <jeyu@...nel.org>,
        Evgenii Shatokhin <eshatokhin@...tuozzo.com>,
        live-patching@...r.kernel.org, linux-kernel@...r.kernel.org,
        Petr Mladek <pmladek@...e.com>
Subject: [PATCH v9 8/9] livepatch: Improve dynamic struct klp_object detection and manipulation

The check for dynamically allocated objects was too optimistic. There
might exist livepatches that modify an object only with callbacks. They
are statically defined and have "funcs" array empty.

A solution would be to check also the callback pointers in
klp_is_object_dynamic(). But it still might be error prone.

Note that we must avoid calling klp_free_object_dynamic() even for useless
structures that were defined statically.

Therefore this patch takes a more safe approach. It adds an extra flag
into struct klp_object. The type is different from a similar flag on
the func level. It is because one object structure might point to func
structures of different types. In general, only two states make sense
on the object level.

This fixed the problem _how_ the structures were freed. But there
was also a bug _when_ this happened. For this we added a check
to keep statically defined structures until the statically defined
function structures are being freed.

Signed-off-by: Petr Mladek <pmladek@...e.com>
---
 include/linux/livepatch.h |  8 +++++++-
 kernel/livepatch/core.c   | 10 ++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index ed598d849029..7222b801d63a 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -45,6 +45,11 @@ enum klp_func_type {
 	KLP_FUNC_NOP,		/* Dynamically allocated NOP function patch */
 };
 
+enum klp_object_type {
+	KLP_OBJECT_STATIC = 0,  /* Original statically defined structure */
+	KLP_OBJECT_DYNAMIC,	/* Dynamically allocated structure. */
+};
+
 /**
  * struct klp_func - function structure for live patching
  * @old_name:	name of the function to be patched
@@ -143,6 +148,7 @@ struct klp_object {
 	struct klp_callbacks callbacks;
 
 	/* internal */
+	enum klp_object_type otype;
 	struct kobject kobj;
 	struct list_head func_list;
 	struct list_head obj_entry;
@@ -198,7 +204,7 @@ struct klp_patch {
 
 static inline bool klp_is_object_dynamic(struct klp_object *obj)
 {
-	return !obj->funcs;
+	return obj->otype == KLP_OBJECT_DYNAMIC;
 }
 
 static inline bool klp_is_func_dynamic(struct klp_func *func)
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index ce9f40a175d9..6fa022cce4bf 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -743,6 +743,7 @@ static struct klp_object *klp_alloc_object_dynamic(const char *name)
 			return ERR_PTR(-ENOMEM);
 		}
 	}
+	obj->otype = KLP_OBJECT_DYNAMIC;
 
 	return obj;
 }
@@ -970,6 +971,15 @@ void klp_free_objects(struct klp_patch *patch, enum klp_func_type ftype)
 		if (!list_empty(&obj->func_list))
 			continue;
 
+		/*
+		 * Keep objects from the original patch initialized until
+		 * the entire patch is being freed.
+		 */
+		if (!klp_is_object_dynamic(obj) &&
+		    ftype != KLP_FUNC_STATIC &&
+		    ftype != KLP_FUNC_ANY)
+			continue;
+
 		/* Avoid freeing the object twice. */
 		list_del(&obj->obj_entry);
 
-- 
2.13.6