* [PATCH v9 1/9] livepatch: Use lists to manage patches, objects and functions
2018-03-02 10:29 [PATCH v9 0/9] livepatch: Atomic replace feature Petr Mladek
@ 2018-03-02 10:29 ` Petr Mladek
2018-03-02 10:29 ` [PATCH v9 2/9] livepatch: Free only structures with initialized kobject Petr Mladek
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Petr Mladek @ 2018-03-02 10:29 UTC (permalink / raw)
To: Jiri Kosina, Josh Poimboeuf, Miroslav Benes
Cc: Jason Baron, Joe Lawrence, Jessica Yu, Evgenii Shatokhin,
live-patching, linux-kernel, Petr Mladek
From: Jason Baron <jbaron@akamai.com>
Currently klp_patch contains a pointer to a statically allocated array of
struct klp_object and struct klp_objects contains a pointer to a statically
allocated array of klp_func. In order to allow for the dynamic allocation
of objects and functions, link klp_patch, klp_object, and klp_func together
via linked lists. This allows us to more easily allocate new objects and
functions, while having the iterator be a simple linked list walk.
The static structures are added to the lists early. It allows to add
the dynamically allocated objects before klp_init_object() and
klp_init_func() calls. Therefore it reduces the further changes
to the code.
Also klp_init_*_list() functions are split because they will
be used when adding the dynamically allocated structures.
This patch does not change the existing behavior.
Signed-off-by: Jason Baron <jbaron@akamai.com>
[pmladek@suse.com: Initialize lists before init calls]
Signed-off-by: Petr Mladek <pmladek@suse.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Jessica Yu <jeyu@kernel.org>
Cc: Jiri Kosina <jikos@kernel.org>
Acked-by: Miroslav Benes <mbenes@suse.cz>
---
include/linux/livepatch.h | 19 +++++++++++++++++--
kernel/livepatch/core.c | 27 +++++++++++++++++++++++++++
2 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index 4754f01c1abb..e5db2ba7e2a5 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -24,6 +24,7 @@
#include <linux/module.h>
#include <linux/ftrace.h>
#include <linux/completion.h>
+#include <linux/list.h>
#if IS_ENABLED(CONFIG_LIVEPATCH)
@@ -43,6 +44,7 @@
* @old_addr: the address of the function being patched
* @kobj: kobject for sysfs resources
* @stack_node: list node for klp_ops func_stack list
+ * @func_entry: links struct klp_func to struct klp_object
* @old_size: size of the old function
* @new_size: size of the new function
* @patched: the func has been added to the klp_ops list
@@ -80,6 +82,7 @@ struct klp_func {
unsigned long old_addr;
struct kobject kobj;
struct list_head stack_node;
+ struct list_head func_entry;
unsigned long old_size, new_size;
bool patched;
bool transition;
@@ -117,6 +120,8 @@ struct klp_callbacks {
* @kobj: kobject for sysfs resources
* @mod: kernel module associated with the patched object
* (NULL for vmlinux)
+ * @func_list: head of list for struct klp_func
+ * @obj_entry: links struct klp_object to struct klp_patch
* @patched: the object's funcs have been added to the klp_ops list
*/
struct klp_object {
@@ -127,6 +132,8 @@ struct klp_object {
/* internal */
struct kobject kobj;
+ struct list_head func_list;
+ struct list_head obj_entry;
struct module *mod;
bool patched;
};
@@ -137,6 +144,7 @@ struct klp_object {
* @objs: object entries for kernel objects to be patched
* @list: list node for global list of registered patches
* @kobj: kobject for sysfs resources
+ * @obj_list: head of list for struct klp_object
* @enabled: the patch is enabled (but operation may be incomplete)
* @finish: for waiting till it is safe to remove the patch module
*/
@@ -148,18 +156,25 @@ struct klp_patch {
/* internal */
struct list_head list;
struct kobject kobj;
+ struct list_head obj_list;
bool enabled;
struct completion finish;
};
-#define klp_for_each_object(patch, obj) \
+#define klp_for_each_object_static(patch, obj) \
for (obj = patch->objs; obj->funcs || obj->name; obj++)
-#define klp_for_each_func(obj, func) \
+#define klp_for_each_object(patch, obj) \
+ list_for_each_entry(obj, &patch->obj_list, obj_entry)
+
+#define klp_for_each_func_static(obj, func) \
for (func = obj->funcs; \
func->old_name || func->new_func || func->old_sympos; \
func++)
+#define klp_for_each_func(obj, func) \
+ list_for_each_entry(func, &obj->func_list, func_entry)
+
int klp_register_patch(struct klp_patch *);
int klp_unregister_patch(struct klp_patch *);
int klp_enable_patch(struct klp_patch *);
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 3a4656fb7047..1d525f4a270a 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -49,6 +49,32 @@ static LIST_HEAD(klp_patches);
static struct kobject *klp_root_kobj;
+static void klp_init_func_list(struct klp_object *obj, struct klp_func *func)
+{
+ list_add(&func->func_entry, &obj->func_list);
+}
+
+static void klp_init_object_list(struct klp_patch *patch,
+ struct klp_object *obj)
+{
+ struct klp_func *func;
+
+ list_add(&obj->obj_entry, &patch->obj_list);
+
+ INIT_LIST_HEAD(&obj->func_list);
+ klp_for_each_func_static(obj, func)
+ klp_init_func_list(obj, func);
+}
+
+static void klp_init_patch_list(struct klp_patch *patch)
+{
+ struct klp_object *obj;
+
+ INIT_LIST_HEAD(&patch->obj_list);
+ klp_for_each_object_static(patch, obj)
+ klp_init_object_list(patch, obj);
+}
+
static bool klp_is_module(struct klp_object *obj)
{
return obj->name;
@@ -794,6 +820,7 @@ static int klp_init_patch(struct klp_patch *patch)
patch->enabled = false;
init_completion(&patch->finish);
+ klp_init_patch_list(patch);
ret = kobject_init_and_add(&patch->kobj, &klp_ktype_patch,
klp_root_kobj, "%s", patch->mod->name);
--
2.13.6
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v9 2/9] livepatch: Free only structures with initialized kobject
2018-03-02 10:29 [PATCH v9 0/9] livepatch: Atomic replace feature Petr Mladek
2018-03-02 10:29 ` [PATCH v9 1/9] livepatch: Use lists to manage patches, objects and functions Petr Mladek
@ 2018-03-02 10:29 ` Petr Mladek
2018-03-02 10:29 ` [PATCH v9 3/9] livepatch: Initial support for dynamic structures Petr Mladek
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Petr Mladek @ 2018-03-02 10:29 UTC (permalink / raw)
To: Jiri Kosina, Josh Poimboeuf, Miroslav Benes
Cc: Jason Baron, Joe Lawrence, Jessica Yu, Evgenii Shatokhin,
live-patching, linux-kernel, Petr Mladek
We are going to add a feature called atomic replace. It will allow to
create a patch that would replace all already registered patches.
For this, we will need to dynamically create funcs and objects
for functions that are no longer patched.
We will want to reuse the existing init() and free() functions. Up to now,
the free() functions checked a limit and were called only for structures
with initialized kobject. But we will want to call them also for structures
that were allocated but where the kobject was not initialized yet.
This patch removes the limit. It calls klp_free*() functions for all
structures. But only the ones with initialized kobject are freed.
The handling of un-initialized structures will be added later with
the support for dynamic structures.
This patch does not change the existing behavior.
Signed-off-by: Petr Mladek <pmladek@suse.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Jessica Yu <jeyu@kernel.org>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Jason Baron <jbaron@akamai.com>
Acked-by: Miroslav Benes <mbenes@suse.cz>
---
kernel/livepatch/core.c | 44 ++++++++++++++++++--------------------------
1 file changed, 18 insertions(+), 26 deletions(-)
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 1d525f4a270a..69bde95e76f8 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -653,17 +653,15 @@ static struct kobj_type klp_ktype_func = {
.sysfs_ops = &kobj_sysfs_ops,
};
-/*
- * Free all functions' kobjects in the array up to some limit. When limit is
- * NULL, all kobjects are freed.
- */
-static void klp_free_funcs_limited(struct klp_object *obj,
- struct klp_func *limit)
+/* Free all funcs that have the kobject initialized. */
+static void klp_free_funcs(struct klp_object *obj)
{
struct klp_func *func;
- for (func = obj->funcs; func->old_name && func != limit; func++)
- kobject_put(&func->kobj);
+ klp_for_each_func(obj, func) {
+ if (func->kobj.state_initialized)
+ kobject_put(&func->kobj);
+ }
}
/* Clean up when a patched object is unloaded */
@@ -677,24 +675,23 @@ static void klp_free_object_loaded(struct klp_object *obj)
func->old_addr = 0;
}
-/*
- * Free all objects' kobjects in the array up to some limit. When limit is
- * NULL, all kobjects are freed.
- */
-static void klp_free_objects_limited(struct klp_patch *patch,
- struct klp_object *limit)
+/* Free all funcs and objects that have the kobject initialized. */
+static void klp_free_objects(struct klp_patch *patch)
{
struct klp_object *obj;
- for (obj = patch->objs; obj->funcs && obj != limit; obj++) {
- klp_free_funcs_limited(obj, NULL);
- kobject_put(&obj->kobj);
+ klp_for_each_object(patch, obj) {
+ klp_free_funcs(obj);
+
+ if (obj->kobj.state_initialized)
+ kobject_put(&obj->kobj);
}
}
static void klp_free_patch(struct klp_patch *patch)
{
- klp_free_objects_limited(patch, NULL);
+ klp_free_objects(patch);
+
if (!list_empty(&patch->list))
list_del(&patch->list);
}
@@ -791,21 +788,16 @@ static int klp_init_object(struct klp_patch *patch, struct klp_object *obj)
klp_for_each_func(obj, func) {
ret = klp_init_func(obj, func);
if (ret)
- goto free;
+ return ret;
}
if (klp_is_object_loaded(obj)) {
ret = klp_init_object_loaded(patch, obj);
if (ret)
- goto free;
+ return ret;
}
return 0;
-
-free:
- klp_free_funcs_limited(obj, func);
- kobject_put(&obj->kobj);
- return ret;
}
static int klp_init_patch(struct klp_patch *patch)
@@ -842,7 +834,7 @@ static int klp_init_patch(struct klp_patch *patch)
return 0;
free:
- klp_free_objects_limited(patch, obj);
+ klp_free_objects(patch);
mutex_unlock(&klp_mutex);
--
2.13.6
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v9 3/9] livepatch: Initial support for dynamic structures
2018-03-02 10:29 [PATCH v9 0/9] livepatch: Atomic replace feature Petr Mladek
2018-03-02 10:29 ` [PATCH v9 1/9] livepatch: Use lists to manage patches, objects and functions Petr Mladek
2018-03-02 10:29 ` [PATCH v9 2/9] livepatch: Free only structures with initialized kobject Petr Mladek
@ 2018-03-02 10:29 ` Petr Mladek
2018-03-02 10:29 ` [PATCH v9 4/9] livepatch: Allow to unpatch only functions of the given type Petr Mladek
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Petr Mladek @ 2018-03-02 10:29 UTC (permalink / raw)
To: Jiri Kosina, Josh Poimboeuf, Miroslav Benes
Cc: Jason Baron, Joe Lawrence, Jessica Yu, Evgenii Shatokhin,
live-patching, linux-kernel, Petr Mladek
From: Jason Baron <jbaron@akamai.com>
We are going to add a feature called atomic replace. It will allow to
create a patch that would replace all already registered patches.
For this, we will need to dynamically create funcs and objects
for functions that are no longer patched.
This patch adds basic framework to handle such dynamic structures.
It adds enum klp_func_type that allows to distinguish the dynamically
allocated funcs' structures. Note that objects' structures do not have
a clear type. Namely the static objects' structures might list both static
and dynamic funcs' structures.
The function type is then used to limit klp_free() functions. We will
want to free the dynamic structures separately when they are no longer
needed. At the same time, we also want to make our life easier,
and introduce _any_ type that will allow to process all existing
structures in one go.
We need to be careful here. First, objects' structures must be freed
only when all listed funcs' structures are freed. Also we must avoid
double free. Both problems are solved by removing the freed structures
from the list.
Also note that klp_free*() functions are called also in klp_init_patch()
error path when only some kobjects have been initialized. The other
dynamic structures must be freed immediately by calling the respective
klp_free_*_dynamic() functions.
Finally, the dynamic objects' structures are generic. The respective
klp_allocate_object_dynamic() and klp_free_object_dynamic() can
be implemented here. On the other hand, klp_free_func_dynamic()
is empty. It must be updated when a particular dynamic
klp_func_type is introduced.
Signed-off-by: Jason Baron <jbaron@akamai.com>
[pmladek@suse.com: Converted into a generic API]
Signed-off-by: Petr Mladek <pmladek@suse.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Jessica Yu <jeyu@kernel.org>
Cc: Jiri Kosina <jikos@kernel.org>
Acked-by: Miroslav Benes <mbenes@suse.cz>
---
include/linux/livepatch.h | 37 +++++++++++-
kernel/livepatch/core.c | 141 +++++++++++++++++++++++++++++++++++++++++-----
2 files changed, 163 insertions(+), 15 deletions(-)
diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index e5db2ba7e2a5..7fb76e7d2693 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -35,12 +35,22 @@
#define KLP_UNPATCHED 0
#define KLP_PATCHED 1
+/*
+ * Function type is used to distinguish dynamically allocated structures
+ * and limit some operations.
+ */
+enum klp_func_type {
+ KLP_FUNC_ANY = -1, /* Substitute any type */
+ KLP_FUNC_STATIC = 0, /* Original statically defined structure */
+};
+
/**
* struct klp_func - function structure for live patching
* @old_name: name of the function to be patched
* @new_func: pointer to the patched function code
* @old_sympos: a hint indicating which symbol position the old function
* can be found (optional)
+ * @ftype: distinguish static and dynamic structures
* @old_addr: the address of the function being patched
* @kobj: kobject for sysfs resources
* @stack_node: list node for klp_ops func_stack list
@@ -79,6 +89,7 @@ struct klp_func {
unsigned long old_sympos;
/* internal */
+ enum klp_func_type ftype;
unsigned long old_addr;
struct kobject kobj;
struct list_head stack_node;
@@ -164,17 +175,41 @@ struct klp_patch {
#define klp_for_each_object_static(patch, obj) \
for (obj = patch->objs; obj->funcs || obj->name; obj++)
+#define klp_for_each_object_safe(patch, obj, tmp_obj) \
+ list_for_each_entry_safe(obj, tmp_obj, &patch->obj_list, obj_entry)
+
#define klp_for_each_object(patch, obj) \
list_for_each_entry(obj, &patch->obj_list, obj_entry)
+/* Support also dynamically allocated struct klp_object */
#define klp_for_each_func_static(obj, func) \
for (func = obj->funcs; \
- func->old_name || func->new_func || func->old_sympos; \
+ func && (func->old_name || func->new_func || func->old_sympos); \
func++)
+#define klp_for_each_func_safe(obj, func, tmp_func) \
+ list_for_each_entry_safe(func, tmp_func, &obj->func_list, func_entry)
+
#define klp_for_each_func(obj, func) \
list_for_each_entry(func, &obj->func_list, func_entry)
+static inline bool klp_is_object_dynamic(struct klp_object *obj)
+{
+ return !obj->funcs;
+}
+
+static inline bool klp_is_func_dynamic(struct klp_func *func)
+{
+ WARN_ON_ONCE(func->ftype == KLP_FUNC_ANY);
+ return func->ftype != KLP_FUNC_STATIC;
+}
+
+static inline bool klp_is_func_type(struct klp_func *func,
+ enum klp_func_type ftype)
+{
+ return ftype == KLP_FUNC_ANY || ftype == func->ftype;
+}
+
int klp_register_patch(struct klp_patch *);
int klp_unregister_patch(struct klp_patch *);
int klp_enable_patch(struct klp_patch *);
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 69bde95e76f8..a47c26147c17 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -124,6 +124,26 @@ static bool klp_initialized(void)
return !!klp_root_kobj;
}
+static struct klp_object *klp_find_object(struct klp_patch *patch,
+ struct klp_object *old_obj)
+{
+ struct klp_object *obj;
+ bool mod = klp_is_module(old_obj);
+
+ klp_for_each_object(patch, obj) {
+ if (mod) {
+ if (klp_is_module(obj) &&
+ strcmp(old_obj->name, obj->name) == 0) {
+ return obj;
+ }
+ } else if (!klp_is_module(obj)) {
+ return obj;
+ }
+ }
+
+ return NULL;
+}
+
struct klp_find_arg {
const char *objname;
const char *name;
@@ -621,6 +641,66 @@ static struct attribute *klp_patch_attrs[] = {
NULL
};
+/*
+ * Dynamically allocated objects and functions.
+ */
+static void klp_free_func_dynamic(struct klp_func *func)
+{
+}
+
+static void klp_free_object_dynamic(struct klp_object *obj)
+{
+ kfree(obj->name);
+ kfree(obj);
+}
+
+static struct klp_object *klp_alloc_object_dynamic(const char *name)
+{
+ struct klp_object *obj;
+
+ obj = kzalloc(sizeof(*obj), GFP_KERNEL);
+ if (!obj)
+ return ERR_PTR(-ENOMEM);
+
+ if (name) {
+ obj->name = kstrdup(name, GFP_KERNEL);
+ if (!obj->name) {
+ kfree(obj);
+ return ERR_PTR(-ENOMEM);
+ }
+ }
+
+ return obj;
+}
+
+static struct klp_object *klp_get_or_add_object(struct klp_patch *patch,
+ struct klp_object *old_obj)
+{
+ struct klp_object *obj;
+
+ obj = klp_find_object(patch, old_obj);
+ if (obj)
+ return obj;
+
+ obj = klp_alloc_object_dynamic(old_obj->name);
+ if (IS_ERR(obj))
+ return obj;
+
+ klp_init_object_list(patch, obj);
+ return obj;
+}
+
+/*
+ * Patch release framework must support the following scenarios:
+ *
+ * + Asynchonous release is used when kobjects are initialized.
+ *
+ * + Direct release is used in error paths for structures that
+ * have not had kobj initialized yet.
+ *
+ * + Allow to release dynamic structures of the given type when
+ * they are not longer needed.
+ */
static void klp_kobj_release_patch(struct kobject *kobj)
{
struct klp_patch *patch;
@@ -637,6 +717,12 @@ static struct kobj_type klp_ktype_patch = {
static void klp_kobj_release_object(struct kobject *kobj)
{
+ struct klp_object *obj;
+
+ obj = container_of(kobj, struct klp_object, kobj);
+
+ if (klp_is_object_dynamic(obj))
+ klp_free_object_dynamic(obj);
}
static struct kobj_type klp_ktype_object = {
@@ -646,6 +732,12 @@ static struct kobj_type klp_ktype_object = {
static void klp_kobj_release_func(struct kobject *kobj)
{
+ struct klp_func *func;
+
+ func = container_of(kobj, struct klp_func, kobj);
+
+ if (klp_is_func_dynamic(func))
+ klp_free_func_dynamic(func);
}
static struct kobj_type klp_ktype_func = {
@@ -653,14 +745,26 @@ static struct kobj_type klp_ktype_func = {
.sysfs_ops = &kobj_sysfs_ops,
};
-/* Free all funcs that have the kobject initialized. */
-static void klp_free_funcs(struct klp_object *obj)
+/*
+ * Free all funcs of the given ftype. Use the kobject when it has already
+ * been initialized. Otherwise, do it directly.
+ */
+static void klp_free_funcs(struct klp_object *obj,
+ enum klp_func_type ftype)
{
- struct klp_func *func;
+ struct klp_func *func, *tmp_func;
+
+ klp_for_each_func_safe(obj, func, tmp_func) {
+ if (!klp_is_func_type(func, ftype))
+ continue;
+
+ /* Avoid double free and allow to detect empty objects. */
+ list_del(&func->func_entry);
- klp_for_each_func(obj, func) {
if (func->kobj.state_initialized)
kobject_put(&func->kobj);
+ else if (klp_is_func_dynamic(func))
+ klp_free_func_dynamic(func);
}
}
@@ -675,22 +779,34 @@ static void klp_free_object_loaded(struct klp_object *obj)
func->old_addr = 0;
}
-/* Free all funcs and objects that have the kobject initialized. */
-static void klp_free_objects(struct klp_patch *patch)
+/*
+ * Free all linked funcs of the given ftype. Then free empty objects.
+ * Use the kobject when it has already been initialized. Otherwise,
+ * do it directly.
+ */
+static void klp_free_objects(struct klp_patch *patch, enum klp_func_type ftype)
{
- struct klp_object *obj;
+ struct klp_object *obj, *tmp_obj;
- klp_for_each_object(patch, obj) {
- klp_free_funcs(obj);
+ klp_for_each_object_safe(patch, obj, tmp_obj) {
+ klp_free_funcs(obj, ftype);
+
+ if (!list_empty(&obj->func_list))
+ continue;
+
+ /* Avoid freeing the object twice. */
+ list_del(&obj->obj_entry);
if (obj->kobj.state_initialized)
kobject_put(&obj->kobj);
+ else if (klp_is_object_dynamic(obj))
+ klp_free_object_dynamic(obj);
}
}
static void klp_free_patch(struct klp_patch *patch)
{
- klp_free_objects(patch);
+ klp_free_objects(patch, KLP_FUNC_ANY);
if (!list_empty(&patch->list))
list_del(&patch->list);
@@ -771,9 +887,6 @@ static int klp_init_object(struct klp_patch *patch, struct klp_object *obj)
int ret;
const char *name;
- if (!obj->funcs)
- return -EINVAL;
-
obj->patched = false;
obj->mod = NULL;
@@ -834,7 +947,7 @@ static int klp_init_patch(struct klp_patch *patch)
return 0;
free:
- klp_free_objects(patch);
+ klp_free_objects(patch, KLP_FUNC_ANY);
mutex_unlock(&klp_mutex);
--
2.13.6
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v9 4/9] livepatch: Allow to unpatch only functions of the given type
2018-03-02 10:29 [PATCH v9 0/9] livepatch: Atomic replace feature Petr Mladek
` (2 preceding siblings ...)
2018-03-02 10:29 ` [PATCH v9 3/9] livepatch: Initial support for dynamic structures Petr Mladek
@ 2018-03-02 10:29 ` Petr Mladek
2018-03-02 10:29 ` [PATCH v9 5/9] livepatch: Support separate list for replaced patches Petr Mladek
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Petr Mladek @ 2018-03-02 10:29 UTC (permalink / raw)
To: Jiri Kosina, Josh Poimboeuf, Miroslav Benes
Cc: Jason Baron, Joe Lawrence, Jessica Yu, Evgenii Shatokhin,
live-patching, linux-kernel, Petr Mladek
From: Jason Baron <jbaron@akamai.com>
We are going to add a feature called atomic replace. It will allow to
create a patch that would replace all already registered patches.
For this, we will need to dynamically create funcs and objects
for functions that are no longer patched.
The dynamically allocated objects will not longer be needed once
the patch is applied.
This patch allows to unpatch functions of the given type. It might
cause that the obj->patched flag is true even when some listed
functions are not longer patched. This is fine as long as the
unpatched funcs' structures are removed right after. It will
be the case. Anyway, it is safe. In the worst case, it will
not be possible to enable the disabled functions.
Signed-off-by: Jason Baron <jbaron@akamai.com>
[pmladek@suse.com: Split and modified to use the generic ftype]
Signed-off-by: Petr Mladek <pmladek@suse.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Jessica Yu <jeyu@kernel.org>
Cc: Jiri Kosina <jikos@kernel.org>
Acked-by: Miroslav Benes <mbenes@suse.cz>
---
kernel/livepatch/core.c | 2 +-
kernel/livepatch/patch.c | 26 +++++++++++++++++++-------
kernel/livepatch/patch.h | 4 ++--
kernel/livepatch/transition.c | 2 +-
4 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index a47c26147c17..ab1f6a371fc8 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -1060,7 +1060,7 @@ static void klp_cleanup_module_patches_limited(struct module *mod,
pr_notice("reverting patch '%s' on unloading module '%s'\n",
patch->mod->name, obj->mod->name);
- klp_unpatch_object(obj);
+ klp_unpatch_object(obj, KLP_FUNC_ANY);
klp_post_unpatch_callback(obj);
}
diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c
index 82d584225dc6..54b3c379bb0f 100644
--- a/kernel/livepatch/patch.c
+++ b/kernel/livepatch/patch.c
@@ -236,15 +236,27 @@ static int klp_patch_func(struct klp_func *func)
return ret;
}
-void klp_unpatch_object(struct klp_object *obj)
+/*
+ * It keeps obj->patched flag true when any listed function is still patched.
+ * The caller is responsible for removing the unpatched functions to
+ * make the flag clean again.
+ */
+void klp_unpatch_object(struct klp_object *obj, enum klp_func_type ftype)
{
struct klp_func *func;
+ bool patched = false;
- klp_for_each_func(obj, func)
- if (func->patched)
+ klp_for_each_func(obj, func) {
+ if (!func->patched)
+ continue;
+
+ if (klp_is_func_type(func, ftype))
klp_unpatch_func(func);
+ else
+ patched = true;
+ }
- obj->patched = false;
+ obj->patched = patched;
}
int klp_patch_object(struct klp_object *obj)
@@ -258,7 +270,7 @@ int klp_patch_object(struct klp_object *obj)
klp_for_each_func(obj, func) {
ret = klp_patch_func(func);
if (ret) {
- klp_unpatch_object(obj);
+ klp_unpatch_object(obj, KLP_FUNC_ANY);
return ret;
}
}
@@ -267,11 +279,11 @@ int klp_patch_object(struct klp_object *obj)
return 0;
}
-void klp_unpatch_objects(struct klp_patch *patch)
+void klp_unpatch_objects(struct klp_patch *patch, enum klp_func_type ftype)
{
struct klp_object *obj;
klp_for_each_object(patch, obj)
if (obj->patched)
- klp_unpatch_object(obj);
+ klp_unpatch_object(obj, ftype);
}
diff --git a/kernel/livepatch/patch.h b/kernel/livepatch/patch.h
index e72d8250d04b..885f644add4c 100644
--- a/kernel/livepatch/patch.h
+++ b/kernel/livepatch/patch.h
@@ -28,7 +28,7 @@ struct klp_ops {
struct klp_ops *klp_find_ops(unsigned long old_addr);
int klp_patch_object(struct klp_object *obj);
-void klp_unpatch_object(struct klp_object *obj);
-void klp_unpatch_objects(struct klp_patch *patch);
+void klp_unpatch_object(struct klp_object *obj, enum klp_func_type ftype);
+void klp_unpatch_objects(struct klp_patch *patch, enum klp_func_type ftype);
#endif /* _LIVEPATCH_PATCH_H */
diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
index 7c6631e693bc..6917100fbe79 100644
--- a/kernel/livepatch/transition.c
+++ b/kernel/livepatch/transition.c
@@ -92,7 +92,7 @@ static void klp_complete_transition(void)
* All tasks have transitioned to KLP_UNPATCHED so we can now
* remove the new functions from the func_stack.
*/
- klp_unpatch_objects(klp_transition_patch);
+ klp_unpatch_objects(klp_transition_patch, KLP_FUNC_ANY);
/*
* Make sure klp_ftrace_handler() can no longer see functions
--
2.13.6
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v9 5/9] livepatch: Support separate list for replaced patches.
2018-03-02 10:29 [PATCH v9 0/9] livepatch: Atomic replace feature Petr Mladek
` (3 preceding siblings ...)
2018-03-02 10:29 ` [PATCH v9 4/9] livepatch: Allow to unpatch only functions of the given type Petr Mladek
@ 2018-03-02 10:29 ` Petr Mladek
2018-03-02 10:29 ` [PATCH v9 6/9] livepatch: Add atomic replace Petr Mladek
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Petr Mladek @ 2018-03-02 10:29 UTC (permalink / raw)
To: Jiri Kosina, Josh Poimboeuf, Miroslav Benes
Cc: Jason Baron, Joe Lawrence, Jessica Yu, Evgenii Shatokhin,
live-patching, linux-kernel, Petr Mladek
From: Jason Baron <jbaron@akamai.com>
We are going to add a feature called atomic replace. It will allow to
create a patch that would replace all already registered patches.
The replaced patches will stay registered because they are typically
unregistered by some package uninstall scripts. But we will remove
these patches from @klp_patches list to keep the enabled patch
on the bottom of the stack. Otherwise, we would need to implement
rather complex logic for moving the patches on the stack. Also
it would complicate implementation of the atomic replace feature.
It is not worth it.
As a result, we will have patches that are registered but that
are no longer usable. Let's get prepared for this and use
a better descriptive name for klp_is_patch_registered() function.
Also create separate list for the replaced patches and allow to
unregister them. Alternative solution would be to add a flag
into struct klp_patch. Note that patch->kobj.state_initialized
is not safe because it can be cleared outside klp_mutex.
This patch does not change the existing behavior.
Signed-off-by: Jason Baron <jbaron@akamai.com>
[pmladek@suse.com: Split and renamed klp_is_patch_usable()]
Signed-off-by: Petr Mladek <pmladek@suse.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Jessica Yu <jeyu@kernel.org>
Cc: Jiri Kosina <jikos@kernel.org>
Acked-by: Miroslav Benes <mbenes@suse.cz>
---
kernel/livepatch/core.c | 30 ++++++++++++++++++++++++------
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index ab1f6a371fc8..fd0296859ff4 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -47,6 +47,13 @@ DEFINE_MUTEX(klp_mutex);
static LIST_HEAD(klp_patches);
+/*
+ * List of 'replaced' patches that have been replaced by a patch that has the
+ * 'replace' bit set. When they are added to this list, they are disabled and
+ * can not be re-enabled, but they can be unregistered().
+ */
+static LIST_HEAD(klp_replaced_patches);
+
static struct kobject *klp_root_kobj;
static void klp_init_func_list(struct klp_object *obj, struct klp_func *func)
@@ -108,17 +115,28 @@ static void klp_find_object_module(struct klp_object *obj)
mutex_unlock(&module_mutex);
}
-static bool klp_is_patch_registered(struct klp_patch *patch)
+static bool klp_is_patch_in_list(struct klp_patch *patch,
+ struct list_head *head)
{
struct klp_patch *mypatch;
- list_for_each_entry(mypatch, &klp_patches, list)
+ list_for_each_entry(mypatch, head, list)
if (mypatch == patch)
return true;
return false;
}
+static bool klp_is_patch_usable(struct klp_patch *patch)
+{
+ return klp_is_patch_in_list(patch, &klp_patches);
+}
+
+static bool klp_is_patch_replaced(struct klp_patch *patch)
+{
+ return klp_is_patch_in_list(patch, &klp_replaced_patches);
+}
+
static bool klp_initialized(void)
{
return !!klp_root_kobj;
@@ -375,7 +393,7 @@ int klp_disable_patch(struct klp_patch *patch)
mutex_lock(&klp_mutex);
- if (!klp_is_patch_registered(patch)) {
+ if (!klp_is_patch_usable(patch)) {
ret = -EINVAL;
goto err;
}
@@ -475,7 +493,7 @@ int klp_enable_patch(struct klp_patch *patch)
mutex_lock(&klp_mutex);
- if (!klp_is_patch_registered(patch)) {
+ if (!klp_is_patch_usable(patch)) {
ret = -EINVAL;
goto err;
}
@@ -516,7 +534,7 @@ static ssize_t enabled_store(struct kobject *kobj, struct kobj_attribute *attr,
mutex_lock(&klp_mutex);
- if (!klp_is_patch_registered(patch)) {
+ if (!klp_is_patch_usable(patch)) {
/*
* Module with the patch could either disappear meanwhile or is
* not properly initialized yet.
@@ -971,7 +989,7 @@ int klp_unregister_patch(struct klp_patch *patch)
mutex_lock(&klp_mutex);
- if (!klp_is_patch_registered(patch)) {
+ if (!klp_is_patch_usable(patch) && !klp_is_patch_replaced(patch)) {
ret = -EINVAL;
goto err;
}
--
2.13.6
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v9 6/9] livepatch: Add atomic replace
2018-03-02 10:29 [PATCH v9 0/9] livepatch: Atomic replace feature Petr Mladek
` (4 preceding siblings ...)
2018-03-02 10:29 ` [PATCH v9 5/9] livepatch: Support separate list for replaced patches Petr Mladek
@ 2018-03-02 10:29 ` Petr Mladek
2018-03-02 10:29 ` [PATCH v9 7/9] livepatch: Correctly handle atomic replace for not yet loaded modules Petr Mladek
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Petr Mladek @ 2018-03-02 10:29 UTC (permalink / raw)
To: Jiri Kosina, Josh Poimboeuf, Miroslav Benes
Cc: Jason Baron, Joe Lawrence, Jessica Yu, Evgenii Shatokhin,
live-patching, linux-kernel, Petr Mladek
From: Jason Baron <jbaron@akamai.com>
Sometimes we would like to revert a particular fix. Currently, this
is not easy because we want to keep all other fixes active and we
could revert only the last applied patch.
One solution would be to apply new patch that implemented all
the reverted functions like in the original code. It would work
as expected but there will be unnecessary redirections. In addition,
it would also require knowing which functions need to be reverted at
build time.
Another problem is when there are many patches that touch the same
functions. There might be dependencies between patches that are
not enforced on the kernel side. Also it might be pretty hard to
actually prepare the patch and ensure compatibility with
the other patches.
A better solution would be to create cumulative patch and say that
it replaces all older ones.
This patch adds a new "replace" flag to struct klp_patch. When it is
enabled, a set of 'nop' klp_func will be dynamically created for all
functions that are already being patched but that will no longer be
modified by the new patch. They are temporarily used as a new target
during the patch transition.
There are used several simplifications:
+ nops' structures are generated already when the patch is registered.
All registered patches are taken into account, even the disabled ones.
As a result, there might be more nops than are really needed when
the patch is enabled and some disabled patches were removed before.
But we are on the safe side and it simplifies the implementation.
Especially we could reuse the existing init() functions. Also freeing
is easier because the same nops are created and removed only once.
Alternative solution would be to create nops when the patch is enabled.
But then any reusing of the init() functions and error paths would be
complicated. Also it would increase the risk of errors because of
late kobject initialization. Finally, it would need tricky waiting
for freed kobjects when finalizing a reverted enable transaction.
+ The replaced patches are removed from the stack and cannot longer
be enabled directly. Otherwise, we would need to implement a more
complex logic of handling the stack of patches. It might be hard
to come with a reasonable semantic.
A fallback is to remove (rmmod) the replaced patches and register
(insmod) them again.
+ Nops are handled like normal function patches. It reduces changes
in the existing code.
It would be possible to copy internal values when they are allocated
and make short cuts in init() functions. It would be possible to use
the fact that old_func and new_func point to the same function and
do not init new_func and new_size at all. It would be possible to
detect nop func in ftrace handler and just leave. But all these would
just complicate the code and maintenance.
+ The callbacks from the replaced patches are not called. It would be
pretty hard to define a reasonable semantic and implement it.
It might even be counter-productive. The new patch is cumulative.
It is supposed to include most of the changes from older patches.
In most cases, it will not want to call pre_unpatch() post_unpatch()
callbacks from the replaced patches. It would disable/break things
for no good reasons. Also it should be easier to handle various
scenarios in a single script in the new patch than think about
interactions caused by running many scripts from older patches.
No to say that the old scripts even would not expect to be called
in this situation.
Signed-off-by: Jason Baron <jbaron@akamai.com>
[pmladek@suse.com: Split, reuse existing code, simplified]
Signed-off-by: Petr Mladek <pmladek@suse.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Jessica Yu <jeyu@kernel.org>
Cc: Jiri Kosina <jikos@kernel.org>
Acked-by: Miroslav Benes <mbenes@suse.cz>
---
include/linux/livepatch.h | 3 +
kernel/livepatch/core.c | 162 +++++++++++++++++++++++++++++++++++++++++-
kernel/livepatch/core.h | 4 ++
kernel/livepatch/transition.c | 39 ++++++++++
4 files changed, 206 insertions(+), 2 deletions(-)
diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index 7fb76e7d2693..ed598d849029 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -42,6 +42,7 @@
enum klp_func_type {
KLP_FUNC_ANY = -1, /* Substitute any type */
KLP_FUNC_STATIC = 0, /* Original statically defined structure */
+ KLP_FUNC_NOP, /* Dynamically allocated NOP function patch */
};
/**
@@ -153,6 +154,7 @@ struct klp_object {
* struct klp_patch - patch structure for live patching
* @mod: reference to the live patch module
* @objs: object entries for kernel objects to be patched
+ * @replace: replace all already registered patches
* @list: list node for global list of registered patches
* @kobj: kobject for sysfs resources
* @obj_list: head of list for struct klp_object
@@ -163,6 +165,7 @@ struct klp_patch {
/* external */
struct module *mod;
struct klp_object *objs;
+ bool replace;
/* internal */
struct list_head list;
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index fd0296859ff4..ad508a86b2f9 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -142,6 +142,21 @@ static bool klp_initialized(void)
return !!klp_root_kobj;
}
+static struct klp_func *klp_find_func(struct klp_object *obj,
+ struct klp_func *old_func)
+{
+ struct klp_func *func;
+
+ klp_for_each_func(obj, func) {
+ if ((strcmp(old_func->old_name, func->old_name) == 0) &&
+ (old_func->old_sympos == func->old_sympos)) {
+ return func;
+ }
+ }
+
+ return NULL;
+}
+
static struct klp_object *klp_find_object(struct klp_patch *patch,
struct klp_object *old_obj)
{
@@ -342,6 +357,39 @@ static int klp_write_object_relocations(struct module *pmod,
return ret;
}
+/*
+ * This function removes replaced patches from both func_stack
+ * and klp_patches stack.
+ *
+ * We could be pretty aggressive here. It is called in situation
+ * when these structures are no longer accessible. All functions
+ * are redirected using the klp_transition_patch. They use either
+ * a new code or they are in the original code because of the special
+ * nop function patches.
+ */
+void klp_throw_away_replaced_patches(struct klp_patch *new_patch,
+ bool keep_module)
+{
+ struct klp_patch *old_patch, *tmp_patch;
+
+ list_for_each_entry_safe(old_patch, tmp_patch, &klp_patches, list) {
+ if (old_patch == new_patch)
+ return;
+
+ klp_unpatch_objects(old_patch, KLP_FUNC_ANY);
+ old_patch->enabled = false;
+
+ /*
+ * Replaced patches could not get re-enabled to keep
+ * the code sane.
+ */
+ list_move(&old_patch->list, &klp_replaced_patches);
+
+ if (!keep_module)
+ module_put(old_patch->mod);
+ }
+}
+
static int __klp_disable_patch(struct klp_patch *patch)
{
struct klp_object *obj;
@@ -537,7 +585,7 @@ static ssize_t enabled_store(struct kobject *kobj, struct kobj_attribute *attr,
if (!klp_is_patch_usable(patch)) {
/*
* Module with the patch could either disappear meanwhile or is
- * not properly initialized yet.
+ * not properly initialized yet or the patch was just replaced.
*/
ret = -EINVAL;
goto err;
@@ -662,8 +710,16 @@ static struct attribute *klp_patch_attrs[] = {
/*
* Dynamically allocated objects and functions.
*/
+static void klp_free_func_nop(struct klp_func *func)
+{
+ kfree(func->old_name);
+ kfree(func);
+}
+
static void klp_free_func_dynamic(struct klp_func *func)
{
+ if (func->ftype == KLP_FUNC_NOP)
+ klp_free_func_nop(func);
}
static void klp_free_object_dynamic(struct klp_object *obj)
@@ -708,6 +764,102 @@ static struct klp_object *klp_get_or_add_object(struct klp_patch *patch,
return obj;
}
+static struct klp_func *klp_alloc_func_nop(struct klp_func *old_func,
+ struct klp_object *obj)
+{
+ struct klp_func *func;
+
+ func = kzalloc(sizeof(*func), GFP_KERNEL);
+ if (!func)
+ return ERR_PTR(-ENOMEM);
+
+ if (old_func->old_name) {
+ func->old_name = kstrdup(old_func->old_name, GFP_KERNEL);
+ if (!func->old_name) {
+ kfree(func);
+ return ERR_PTR(-ENOMEM);
+ }
+ }
+ func->old_sympos = old_func->old_sympos;
+ /* NOP func is the same as using the original implementation. */
+ func->new_func = (void *)old_func->old_addr;
+ func->ftype = KLP_FUNC_NOP;
+
+ return func;
+}
+
+static int klp_add_func_nop(struct klp_object *obj,
+ struct klp_func *old_func)
+{
+ struct klp_func *func;
+
+ func = klp_find_func(obj, old_func);
+
+ if (func)
+ return 0;
+
+ func = klp_alloc_func_nop(old_func, obj);
+ if (IS_ERR(func))
+ return PTR_ERR(func);
+
+ klp_init_func_list(obj, func);
+
+ return 0;
+}
+
+static int klp_add_object_nops(struct klp_patch *patch,
+ struct klp_object *old_obj)
+{
+ struct klp_object *obj;
+ struct klp_func *old_func;
+ int err = 0;
+
+ obj = klp_get_or_add_object(patch, old_obj);
+ if (IS_ERR(obj))
+ return PTR_ERR(obj);
+
+ klp_for_each_func(old_obj, old_func) {
+ err = klp_add_func_nop(obj, old_func);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
+/*
+ * Add 'nop' functions which simply return to the caller to run
+ * the original function. The 'nop' functions are added to a
+ * patch to facilitate a 'replace' mode
+ *
+ * The nops are generated for all patches on the stack when
+ * the new patch is initialized. It is safe even though some
+ * older patches might get disabled and removed before the
+ * new one is enabled. In the worst case, there might be nops
+ * which will not be really needed. But it does not harm and
+ * simplifies the implementation a lot. Especially we could
+ * use the init functions as is.
+ */
+static int klp_add_nops(struct klp_patch *patch)
+{
+ struct klp_patch *old_patch;
+ struct klp_object *old_obj;
+ int err = 0;
+
+ if (WARN_ON(!patch->replace))
+ return -EINVAL;
+
+ list_for_each_entry(old_patch, &klp_patches, list) {
+ klp_for_each_object(old_patch, old_obj) {
+ err = klp_add_object_nops(patch, old_obj);
+ if (err)
+ return err;
+ }
+ }
+
+ return 0;
+}
+
/*
* Patch release framework must support the following scenarios:
*
@@ -802,7 +954,7 @@ static void klp_free_object_loaded(struct klp_object *obj)
* Use the kobject when it has already been initialized. Otherwise,
* do it directly.
*/
-static void klp_free_objects(struct klp_patch *patch, enum klp_func_type ftype)
+void klp_free_objects(struct klp_patch *patch, enum klp_func_type ftype)
{
struct klp_object *obj, *tmp_obj;
@@ -952,6 +1104,12 @@ static int klp_init_patch(struct klp_patch *patch)
return ret;
}
+ if (patch->replace) {
+ ret = klp_add_nops(patch);
+ if (ret)
+ goto free;
+ }
+
klp_for_each_object(patch, obj) {
ret = klp_init_object(patch, obj);
if (ret)
diff --git a/kernel/livepatch/core.h b/kernel/livepatch/core.h
index 48a83d4364cf..43184a5318d8 100644
--- a/kernel/livepatch/core.h
+++ b/kernel/livepatch/core.h
@@ -6,6 +6,10 @@
extern struct mutex klp_mutex;
+void klp_throw_away_replaced_patches(struct klp_patch *new_patch,
+ bool keep_module);
+void klp_free_objects(struct klp_patch *patch, enum klp_func_type ftype);
+
static inline bool klp_is_object_loaded(struct klp_object *obj)
{
return !obj->name || obj->mod;
diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
index 6917100fbe79..d6af190865d2 100644
--- a/kernel/livepatch/transition.c
+++ b/kernel/livepatch/transition.c
@@ -87,6 +87,36 @@ static void klp_complete_transition(void)
klp_transition_patch->mod->name,
klp_target_state == KLP_PATCHED ? "patching" : "unpatching");
+ /*
+ * For replace patches, we disable all previous patches, and replace
+ * the dynamic no-op functions by removing the ftrace hook.
+ */
+ if (klp_transition_patch->replace && klp_target_state == KLP_PATCHED) {
+ /*
+ * Make sure that no ftrace handler accesses any older patch
+ * on the stack. This might happen when the user forced the
+ * transaction while some running tasks were still falling
+ * back to the old code. There might even still be ftrace
+ * handlers that have not seen the last patch on the stack yet.
+ *
+ * It probably is not necessary because of the rcu-safe access.
+ * But better be safe than sorry.
+ */
+ if (klp_forced)
+ klp_synchronize_transition();
+
+ klp_throw_away_replaced_patches(klp_transition_patch,
+ klp_forced);
+
+ /*
+ * There is no need to synchronize the transition after removing
+ * nops. They must be the last on the func_stack. Ftrace
+ * gurantees that nobody will stay in the trampoline after
+ * the ftrace handler is unregistered.
+ */
+ klp_unpatch_objects(klp_transition_patch, KLP_FUNC_NOP);
+ }
+
if (klp_target_state == KLP_UNPATCHED) {
/*
* All tasks have transitioned to KLP_UNPATCHED so we can now
@@ -143,6 +173,15 @@ static void klp_complete_transition(void)
if (!klp_forced && klp_target_state == KLP_UNPATCHED)
module_put(klp_transition_patch->mod);
+ /*
+ * We do not need to wait until the objects are really freed.
+ * The patch must be on the bottom of the stack. Therefore it
+ * will never replace anything else. The only important thing
+ * is that we wait when the patch is being unregistered.
+ */
+ if (klp_transition_patch->replace && klp_target_state == KLP_PATCHED)
+ klp_free_objects(klp_transition_patch, KLP_FUNC_NOP);
+
klp_target_state = KLP_UNDEFINED;
klp_transition_patch = NULL;
}
--
2.13.6
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v9 7/9] livepatch: Correctly handle atomic replace for not yet loaded modules
2018-03-02 10:29 [PATCH v9 0/9] livepatch: Atomic replace feature Petr Mladek
` (5 preceding siblings ...)
2018-03-02 10:29 ` [PATCH v9 6/9] livepatch: Add atomic replace Petr Mladek
@ 2018-03-02 10:29 ` Petr Mladek
2018-03-02 10:29 ` [PATCH v9 8/9] livepatch: Improve dynamic struct klp_object detection and manipulation Petr Mladek
2018-03-02 10:29 ` [PATCH v9 9/9] livepatch: Atomic replace and cumulative patches documentation Petr Mladek
8 siblings, 0 replies; 10+ messages in thread
From: Petr Mladek @ 2018-03-02 10:29 UTC (permalink / raw)
To: Jiri Kosina, Josh Poimboeuf, Miroslav Benes
Cc: Jason Baron, Joe Lawrence, Jessica Yu, Evgenii Shatokhin,
live-patching, linux-kernel, Petr Mladek
The atomic replace feature uses dynamically allocated struct klp_func to
handle functions that will no 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 not to 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 | 22 ++++++++++++++++++----
kernel/livepatch/patch.c | 5 +++++
2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index ad508a86b2f9..ce9f40a175d9 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -781,8 +781,10 @@ static struct klp_func *klp_alloc_func_nop(struct klp_func *old_func,
}
}
func->old_sympos = old_func->old_sympos;
- /* NOP func is the same as using the original implementation. */
- func->new_func = (void *)old_func->old_addr;
+ /*
+ * func->new_func is same as func->old_addr. These addresses are
+ * set when the object is loaded, see klp_init_object_loaded().
+ */
func->ftype = KLP_FUNC_NOP;
return func;
@@ -945,8 +947,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 +990,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 +1050,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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v9 8/9] livepatch: Improve dynamic struct klp_object detection and manipulation
2018-03-02 10:29 [PATCH v9 0/9] livepatch: Atomic replace feature Petr Mladek
` (6 preceding siblings ...)
2018-03-02 10:29 ` [PATCH v9 7/9] livepatch: Correctly handle atomic replace for not yet loaded modules Petr Mladek
@ 2018-03-02 10:29 ` Petr Mladek
2018-03-02 10:29 ` [PATCH v9 9/9] livepatch: Atomic replace and cumulative patches documentation Petr Mladek
8 siblings, 0 replies; 10+ messages in thread
From: Petr Mladek @ 2018-03-02 10:29 UTC (permalink / raw)
To: Jiri Kosina, Josh Poimboeuf, Miroslav Benes
Cc: Jason Baron, Joe Lawrence, Jessica Yu, Evgenii Shatokhin,
live-patching, linux-kernel, Petr Mladek
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 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v9 9/9] livepatch: Atomic replace and cumulative patches documentation
2018-03-02 10:29 [PATCH v9 0/9] livepatch: Atomic replace feature Petr Mladek
` (7 preceding siblings ...)
2018-03-02 10:29 ` [PATCH v9 8/9] livepatch: Improve dynamic struct klp_object detection and manipulation Petr Mladek
@ 2018-03-02 10:29 ` Petr Mladek
8 siblings, 0 replies; 10+ messages in thread
From: Petr Mladek @ 2018-03-02 10:29 UTC (permalink / raw)
To: Jiri Kosina, Josh Poimboeuf, Miroslav Benes
Cc: Jason Baron, Joe Lawrence, Jessica Yu, Evgenii Shatokhin,
live-patching, linux-kernel, Petr Mladek
User documentation for the atomic replace feature. It makes it easier
to maintain livepatches using so-called cumulative patches.
Signed-off-by: Petr Mladek <pmladek@suse.com>
Acked-by: Miroslav Benes <mbenes@suse.cz>
---
Documentation/livepatch/cumulative-patches.txt | 83 ++++++++++++++++++++++++++
1 file changed, 83 insertions(+)
create mode 100644 Documentation/livepatch/cumulative-patches.txt
diff --git a/Documentation/livepatch/cumulative-patches.txt b/Documentation/livepatch/cumulative-patches.txt
new file mode 100644
index 000000000000..c041fc1bd259
--- /dev/null
+++ b/Documentation/livepatch/cumulative-patches.txt
@@ -0,0 +1,83 @@
+===================================
+Atomic Replace & Cumulative Patches
+===================================
+
+There might be dependencies between livepatches. If multiple patches need
+to do different changes to the same function(s) then we need to define
+an order in which the patches will be installed. And function implementations
+from any newer livepatch must be done on top of the older ones.
+
+This might become a maintenance nightmare. Especially if anyone would want
+to remove a patch that is in the middle of the stack.
+
+An elegant solution comes with the feature called "Atomic Replace". It allows
+to create so called "Cumulative Patches". They include all wanted changes
+from all older livepatches and completely replace them in one transition.
+
+Usage
+-----
+
+The atomic replace can be enabled by setting "replace" flag in struct klp_patch,
+for example:
+
+ static struct klp_patch patch = {
+ .mod = THIS_MODULE,
+ .objs = objs,
+ .replace = true,
+ };
+
+Such a patch is added on top of the livepatch stack when registered. It can
+be enabled even when some earlier patches have not been enabled yet.
+
+All processes are then migrated to use the code only from the new patch.
+Once the transition is finished, all older patches are removed from the stack
+of patches. Even the older not-enabled patches mentioned above.
+
+Ftrace handlers are transparently removed from functions that are no
+longer modified by the new cumulative patch.
+
+As a result, the livepatch author might maintain sources only for one
+cumulative patch. It helps to keep the patch consistent while adding or
+removing various fixes or features.
+
+
+Limitations:
+------------
+
+ + Replaced patches can no longer be enabled. But if the transition
+ to the cumulative patch was not forced, the kernel modules with
+ the older livepatches can be removed and eventually added again.
+
+ A good practice is to set .replace flag in any released livepatch.
+ Then re-adding an older livepatch is equivalent to downgrading
+ to that patch. This is safe as long as the livepatches do _not_ do
+ extra modifications in (un)patching callbacks or in the module_init()
+ or module_exit() functions, see below.
+
+
+ + Only the (un)patching callbacks from the _new_ cumulative livepatch are
+ executed. Any callbacks from the replaced patches are ignored.
+
+ By other words, the cumulative patch is responsible for doing any actions
+ that are necessary to properly replace any older patch.
+
+ As a result, it might be dangerous to replace newer cumulative patches by
+ older ones. The old livepatches might not provide the necessary callbacks.
+
+ This might be seen as a limitation in some scenarios. But it makes the life
+ easier in many others. Only the new cumulative livepatch knows what
+ fixes/features are added/removed and what special actions are necessary
+ for a smooth transition.
+
+ In each case, it would be a nightmare to think about the order of
+ the various callbacks and their interactions if the callbacks from all
+ enabled patches were called.
+
+
+ + There is no special handling of shadow variables. Livepatch authors
+ must create their own rules how to pass them from one cumulative
+ patch to the other. Especially they should not blindly remove them
+ in module_exit() functions.
+
+ A good practice might be to remove shadow variables in the post-unpatch
+ callback. It is called only when the livepatch is properly disabled.
--
2.13.6
^ permalink raw reply related [flat|nested] 10+ messages in thread