Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
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 v10 08/10] livepatch: Improve dynamic struct klp_object detection and manipulation
Date:   Wed,  7 Mar 2018 09:20:37 +0100
Message-Id: <20180307082039.10196-9-pmladek@suse.com>
In-Reply-To: <20180307082039.10196-1-pmladek@suse.com>
References: <20180307082039.10196-1-pmladek@suse.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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@suse.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 67aa4ec9e087..73ce3f93e0bc 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