summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--Documentation/livepatch/livepatch.txt28
-rw-r--r--include/linux/livepatch.h3
-rw-r--r--kernel/livepatch/core.c80
-rw-r--r--kernel/livepatch/transition.c37
-rw-r--r--samples/livepatch/livepatch-sample.c1
5 files changed, 96 insertions, 53 deletions
diff --git a/Documentation/livepatch/livepatch.txt b/Documentation/livepatch/livepatch.txt
index 4f2aec8d4c12..ecdb18104ab0 100644
--- a/Documentation/livepatch/livepatch.txt
+++ b/Documentation/livepatch/livepatch.txt
@@ -316,8 +316,15 @@ section "Livepatch life-cycle" below for more details about these
two operations.
Module removal is only safe when there are no users of the underlying
-functions. The immediate consistency model is not able to detect this;
-therefore livepatch modules cannot be removed. See "Limitations" below.
+functions. The immediate consistency model is not able to detect this. The
+code just redirects the functions at the very beginning and it does not
+check if the functions are in use. In other words, it knows when the
+functions get called but it does not know when the functions return.
+Therefore it cannot be decided when the livepatch module can be safely
+removed. This is solved by a hybrid consistency model. When the system is
+transitioned to a new patch state (patched/unpatched) it is guaranteed that
+no task sleeps or runs in the old code.
+
5. Livepatch life-cycle
=======================
@@ -469,23 +476,6 @@ The current Livepatch implementation has several limitations:
by "notrace".
- + Livepatch modules can not be removed.
-
- The current implementation just redirects the functions at the very
- beginning. It does not check if the functions are in use. In other
- words, it knows when the functions get called but it does not
- know when the functions return. Therefore it can not decide when
- the livepatch module can be safely removed.
-
- This will get most likely solved once a more complex consistency model
- is supported. The idea is that a safe state for patching should also
- mean a safe state for removing the patch.
-
- Note that the patch itself might get disabled by writing zero
- to /sys/kernel/livepatch/<patch>/enabled. It causes that the new
- code will not longer get called. But it does not guarantee
- that anyone is not sleeping anywhere in the new code.
-
+ Livepatch works reliably only when the dynamic ftrace is located at
the very beginning of the function.
diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index ed90ad1605c1..194991ef9347 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -23,6 +23,7 @@
#include <linux/module.h>
#include <linux/ftrace.h>
+#include <linux/completion.h>
#if IS_ENABLED(CONFIG_LIVEPATCH)
@@ -114,6 +115,7 @@ struct klp_object {
* @list: list node for global list of registered patches
* @kobj: kobject for sysfs resources
* @enabled: the patch is enabled (but operation may be incomplete)
+ * @finish: for waiting till it is safe to remove the patch module
*/
struct klp_patch {
/* external */
@@ -125,6 +127,7 @@ struct klp_patch {
struct list_head list;
struct kobject kobj;
bool enabled;
+ struct completion finish;
};
#define klp_for_each_object(patch, obj) \
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 3dc3c9049690..6844c1213df8 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -29,6 +29,7 @@
#include <linux/livepatch.h>
#include <linux/elf.h>
#include <linux/moduleloader.h>
+#include <linux/completion.h>
#include <asm/cacheflush.h>
#include "patch.h"
#include "transition.h"
@@ -354,6 +355,18 @@ static int __klp_enable_patch(struct klp_patch *patch)
!list_prev_entry(patch, list)->enabled)
return -EBUSY;
+ /*
+ * A reference is taken on the patch module to prevent it from being
+ * unloaded.
+ *
+ * Note: For immediate (no consistency model) patches we don't allow
+ * patch modules to unload since there is no safe/sane method to
+ * determine if a thread is still running in the patched code contained
+ * in the patch module once the ftrace registration is successful.
+ */
+ if (!try_module_get(patch->mod))
+ return -ENODEV;
+
pr_notice("enabling patch '%s'\n", patch->mod->name);
klp_init_transition(patch, KLP_PATCHED);
@@ -442,6 +455,15 @@ static ssize_t enabled_store(struct kobject *kobj, struct kobj_attribute *attr,
mutex_lock(&klp_mutex);
+ if (!klp_is_patch_registered(patch)) {
+ /*
+ * Module with the patch could either disappear meanwhile or is
+ * not properly initialized yet.
+ */
+ ret = -EINVAL;
+ goto err;
+ }
+
if (patch->enabled == enabled) {
/* already in requested state */
ret = -EINVAL;
@@ -498,10 +520,10 @@ static struct attribute *klp_patch_attrs[] = {
static void klp_kobj_release_patch(struct kobject *kobj)
{
- /*
- * Once we have a consistency model we'll need to module_put() the
- * patch module here. See klp_register_patch() for more details.
- */
+ struct klp_patch *patch;
+
+ patch = container_of(kobj, struct klp_patch, kobj);
+ complete(&patch->finish);
}
static struct kobj_type klp_ktype_patch = {
@@ -572,7 +594,6 @@ static void klp_free_patch(struct klp_patch *patch)
klp_free_objects_limited(patch, NULL);
if (!list_empty(&patch->list))
list_del(&patch->list);
- kobject_put(&patch->kobj);
}
static int klp_init_func(struct klp_object *obj, struct klp_func *func)
@@ -695,11 +716,14 @@ static int klp_init_patch(struct klp_patch *patch)
mutex_lock(&klp_mutex);
patch->enabled = false;
+ init_completion(&patch->finish);
ret = kobject_init_and_add(&patch->kobj, &klp_ktype_patch,
klp_root_kobj, "%s", patch->mod->name);
- if (ret)
- goto unlock;
+ if (ret) {
+ mutex_unlock(&klp_mutex);
+ return ret;
+ }
klp_for_each_object(patch, obj) {
ret = klp_init_object(patch, obj);
@@ -715,9 +739,12 @@ static int klp_init_patch(struct klp_patch *patch)
free:
klp_free_objects_limited(patch, obj);
- kobject_put(&patch->kobj);
-unlock:
+
mutex_unlock(&klp_mutex);
+
+ kobject_put(&patch->kobj);
+ wait_for_completion(&patch->finish);
+
return ret;
}
@@ -731,23 +758,29 @@ unlock:
*/
int klp_unregister_patch(struct klp_patch *patch)
{
- int ret = 0;
+ int ret;
mutex_lock(&klp_mutex);
if (!klp_is_patch_registered(patch)) {
ret = -EINVAL;
- goto out;
+ goto err;
}
if (patch->enabled) {
ret = -EBUSY;
- goto out;
+ goto err;
}
klp_free_patch(patch);
-out:
+ mutex_unlock(&klp_mutex);
+
+ kobject_put(&patch->kobj);
+ wait_for_completion(&patch->finish);
+
+ return 0;
+err:
mutex_unlock(&klp_mutex);
return ret;
}
@@ -760,12 +793,13 @@ EXPORT_SYMBOL_GPL(klp_unregister_patch);
* Initializes the data structure associated with the patch and
* creates the sysfs interface.
*
+ * There is no need to take the reference on the patch module here. It is done
+ * later when the patch is enabled.
+ *
* Return: 0 on success, otherwise error
*/
int klp_register_patch(struct klp_patch *patch)
{
- int ret;
-
if (!patch || !patch->mod)
return -EINVAL;
@@ -788,21 +822,7 @@ int klp_register_patch(struct klp_patch *patch)
return -ENOSYS;
}
- /*
- * A reference is taken on the patch module to prevent it from being
- * unloaded. Right now, we don't allow patch modules to unload since
- * there is currently no method to determine if a thread is still
- * running in the patched code contained in the patch module once
- * the ftrace registration is successful.
- */
- if (!try_module_get(patch->mod))
- return -ENODEV;
-
- ret = klp_init_patch(patch);
- if (ret)
- module_put(patch->mod);
-
- return ret;
+ return klp_init_patch(patch);
}
EXPORT_SYMBOL_GPL(klp_register_patch);
diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
index 428533ec51b5..0ab7abd53b0b 100644
--- a/kernel/livepatch/transition.c
+++ b/kernel/livepatch/transition.c
@@ -59,6 +59,7 @@ static void klp_complete_transition(void)
struct klp_func *func;
struct task_struct *g, *task;
unsigned int cpu;
+ bool immediate_func = false;
if (klp_target_state == KLP_UNPATCHED) {
/*
@@ -79,9 +80,16 @@ static void klp_complete_transition(void)
if (klp_transition_patch->immediate)
goto done;
- klp_for_each_object(klp_transition_patch, obj)
- klp_for_each_func(obj, func)
+ klp_for_each_object(klp_transition_patch, obj) {
+ klp_for_each_func(obj, func) {
func->transition = false;
+ if (func->immediate)
+ immediate_func = true;
+ }
+ }
+
+ if (klp_target_state == KLP_UNPATCHED && !immediate_func)
+ module_put(klp_transition_patch->mod);
/* Prevent klp_ftrace_handler() from seeing KLP_UNDEFINED state */
if (klp_target_state == KLP_PATCHED)
@@ -113,8 +121,31 @@ done:
*/
void klp_cancel_transition(void)
{
- klp_target_state = !klp_target_state;
+ struct klp_patch *patch = klp_transition_patch;
+ struct klp_object *obj;
+ struct klp_func *func;
+ bool immediate_func = false;
+
+ if (WARN_ON_ONCE(klp_target_state != KLP_PATCHED))
+ return;
+
+ klp_target_state = KLP_UNPATCHED;
klp_complete_transition();
+
+ /*
+ * In the enable error path, even immediate patches can be safely
+ * removed because the transition hasn't been started yet.
+ *
+ * klp_complete_transition() doesn't have a module_put() for immediate
+ * patches, so do it here.
+ */
+ klp_for_each_object(patch, obj)
+ klp_for_each_func(obj, func)
+ if (func->immediate)
+ immediate_func = true;
+
+ if (patch->immediate || immediate_func)
+ module_put(patch->mod);
}
/*
diff --git a/samples/livepatch/livepatch-sample.c b/samples/livepatch/livepatch-sample.c
index 629e0dca0887..84795223f15f 100644
--- a/samples/livepatch/livepatch-sample.c
+++ b/samples/livepatch/livepatch-sample.c
@@ -99,7 +99,6 @@ static int livepatch_init(void)
static void livepatch_exit(void)
{
- WARN_ON(klp_disable_patch(&patch));
WARN_ON(klp_unregister_patch(&patch));
}