--- v4
+++ v3
@@ -25,42 +25,27 @@
counting symmetric (module_put() is in a patch disable path) and to
allow to take a new reference to a disabled module when being enabled.
-Finally, we need to be very careful about possible races between
-klp_unregister_patch(), kobject_put() functions and operations
-on the related sysfs files.
-
-kobject_put(&patch->kobj) must be called without klp_mutex. Otherwise,
-it might be blocked by enabled_store() that needs the mutex as well.
-In addition, enabled_store() must check if the patch was not
-unregisted in the meantime.
-
-There is no need to do the same for other kobject_put() callsites
-at the moment. Their sysfs operations neiter take the lock nor
-they access any data that might be freed in the meantime.
-
-There was an attempt to use kobjects the right way and prevent these
-races by design. But it made the patch definition more complicated
-and opened another can of worms. See
-https://lkml.kernel.org/r/1464018848-4303-1-git-send-email-pmladek@suse.com
-
-[Thanks to Petr Mladek for improving the commit message.]
+Also all kobject_put(&patch->kobj) calls are moved outside of klp_mutex
+lock protection to prevent a deadlock situation when
+klp_unregister_patch is called and sysfs directories are removed. There
+is no need to do the same for other kobject_put() callsites as we
+currently do not have their sysfs counterparts.
Signed-off-by: Miroslav Benes <mbenes@suse.cz>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
-Reviewed-by: Petr Mladek <pmladek@suse.com>
---
- Documentation/livepatch/livepatch.txt | 28 ++++--------
+ Documentation/livepatch/livepatch.txt | 29 ++++---------
include/linux/livepatch.h | 3 ++
kernel/livepatch/core.c | 80 ++++++++++++++++++++++-------------
kernel/livepatch/transition.c | 12 +++++-
samples/livepatch/livepatch-sample.c | 1 -
- 5 files changed, 72 insertions(+), 52 deletions(-)
+ 5 files changed, 72 insertions(+), 53 deletions(-)
diff --git a/Documentation/livepatch/livepatch.txt b/Documentation/livepatch/livepatch.txt
-index fb00d66..9816df3 100644
+index f87e742..b0eaaf8 100644
--- a/Documentation/livepatch/livepatch.txt
+++ b/Documentation/livepatch/livepatch.txt
-@@ -296,8 +296,15 @@ section "Livepatch life-cycle" below for more details about these
+@@ -265,8 +265,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
@@ -78,8 +63,8 @@
5. Livepatch life-cycle
=======================
-@@ -449,23 +456,6 @@ The current Livepatch implementation has several limitations:
- by "notrace".
+@@ -437,24 +444,6 @@ The current Livepatch implementation has several limitations:
+ There is work in progress to remove this limitation.
- + Livepatch modules can not be removed.
@@ -99,11 +84,12 @@
- 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 ed90ad1..194991e 100644
+index 8e06fe5..1959e52 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -23,6 +23,7 @@
@@ -131,7 +117,7 @@
#define klp_for_each_object(patch, obj) \
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
-index 4c5a816..e449fc7 100644
+index 22c0c01..cc44f40 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -29,6 +29,7 @@
@@ -142,7 +128,7 @@
#include <asm/cacheflush.h>
#include "patch.h"
#include "transition.h"
-@@ -352,6 +353,18 @@ static int __klp_enable_patch(struct klp_patch *patch)
+@@ -377,6 +378,18 @@ static int __klp_enable_patch(struct klp_patch *patch)
!list_prev_entry(patch, list)->enabled)
return -EBUSY;
@@ -161,7 +147,7 @@
pr_notice("enabling patch '%s'\n", patch->mod->name);
klp_init_transition(patch, KLP_PATCHED);
-@@ -441,6 +454,15 @@ static ssize_t enabled_store(struct kobject *kobj, struct kobj_attribute *attr,
+@@ -471,6 +484,15 @@ static ssize_t enabled_store(struct kobject *kobj, struct kobj_attribute *attr,
mutex_lock(&klp_mutex);
@@ -177,7 +163,7 @@
if (patch->enabled == enabled) {
/* already in requested state */
ret = -EINVAL;
-@@ -497,10 +519,10 @@ static struct attribute *klp_patch_attrs[] = {
+@@ -528,10 +550,10 @@ static struct attribute *klp_patch_attrs[] = {
static void klp_kobj_release_patch(struct kobject *kobj)
{
@@ -192,7 +178,7 @@
}
static struct kobj_type klp_ktype_patch = {
-@@ -571,7 +593,6 @@ static void klp_free_patch(struct klp_patch *patch)
+@@ -602,7 +624,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);
@@ -200,7 +186,7 @@
}
static int klp_init_func(struct klp_object *obj, struct klp_func *func)
-@@ -694,11 +715,14 @@ static int klp_init_patch(struct klp_patch *patch)
+@@ -725,11 +746,14 @@ static int klp_init_patch(struct klp_patch *patch)
mutex_lock(&klp_mutex);
patch->enabled = false;
@@ -217,7 +203,7 @@
klp_for_each_object(patch, obj) {
ret = klp_init_object(patch, obj);
-@@ -714,9 +738,12 @@ static int klp_init_patch(struct klp_patch *patch)
+@@ -745,9 +769,12 @@ static int klp_init_patch(struct klp_patch *patch)
free:
klp_free_objects_limited(patch, obj);
@@ -232,7 +218,7 @@
return ret;
}
-@@ -730,23 +757,29 @@ static int klp_init_patch(struct klp_patch *patch)
+@@ -761,23 +788,29 @@ static int klp_init_patch(struct klp_patch *patch)
*/
int klp_unregister_patch(struct klp_patch *patch)
{
@@ -266,7 +252,7 @@
mutex_unlock(&klp_mutex);
return ret;
}
-@@ -759,12 +792,13 @@ EXPORT_SYMBOL_GPL(klp_unregister_patch);
+@@ -790,12 +823,13 @@ EXPORT_SYMBOL_GPL(klp_unregister_patch);
* Initializes the data structure associated with the patch and
* creates the sysfs interface.
*
@@ -282,9 +268,9 @@
if (!patch || !patch->mod)
return -EINVAL;
-@@ -787,21 +821,7 @@ int klp_register_patch(struct klp_patch *patch)
+@@ -816,21 +850,7 @@ int klp_register_patch(struct klp_patch *patch)
+ if (!klp_have_reliable_stack() && !patch->immediate)
return -ENOSYS;
- }
- /*
- * A reference is taken on the patch module to prevent it from being
@@ -306,10 +292,10 @@
EXPORT_SYMBOL_GPL(klp_register_patch);
diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
-index 2b87ff9..b5e6f3b 100644
+index 4494fe6..dc950d5 100644
--- a/kernel/livepatch/transition.c
+++ b/kernel/livepatch/transition.c
-@@ -60,13 +60,21 @@ void klp_complete_transition(void)
+@@ -195,13 +195,21 @@ void klp_complete_transition(void)
struct klp_func *func;
struct task_struct *g, *task;
unsigned int cpu;
@@ -331,13 +317,13 @@
+ if (klp_target_state == KLP_UNPATCHED && !is_immediate)
+ module_put(klp_transition_patch->mod);
- /* Prevent klp_ftrace_handler() from seeing KLP_UNDEFINED state */
- if (klp_target_state == KLP_PATCHED)
+ read_lock(&tasklist_lock);
+ for_each_process_thread(g, task) {
diff --git a/samples/livepatch/livepatch-sample.c b/samples/livepatch/livepatch-sample.c
-index 629e0dc..84795223f 100644
+index bb61c65..0625f38 100644
--- a/samples/livepatch/livepatch-sample.c
+++ b/samples/livepatch/livepatch-sample.c
-@@ -99,7 +99,6 @@ static int livepatch_init(void)
+@@ -89,7 +89,6 @@ static int livepatch_init(void)
static void livepatch_exit(void)
{