We already warn when RCU is not watching and the system consistency
is not guaranteed.
One problem is that tasks might use incompatible implementations
of the modified functions. We print the warning and could not
do much more about it.
Second problem is that we could not reliably detect unused code
after the patch was disabled. Here we could disable removing
patches to avoid possible damage.
The solution is sub-optimal. First, it might be too late to disable
the patch removal. Second, it affects all patches even though some
of them might be safe. But the question is if a better solution is
worth the effort. Most of the affected functions could not be patched
anyway. They are used by the ftrace handler and patching would
cause an infinite recursion.
Signed-off-by: Petr Mladek <pmladek@xxxxxxxx>
---
Documentation/livepatch/livepatch.txt | 4 ++++
kernel/livepatch/patch.c | 4 +++-
kernel/livepatch/transition.c | 7 ++++++-
kernel/livepatch/transition.h | 2 ++
4 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/Documentation/livepatch/livepatch.txt b/Documentation/livepatch/livepatch.txt
index 92619f7d86fd..87069ab46121 100644
--- a/Documentation/livepatch/livepatch.txt
+++ b/Documentation/livepatch/livepatch.txt
@@ -491,6 +491,10 @@ The current Livepatch implementation has several limitations:
functions using immediate patches. But this is hard to detect properly
in the ftrace handler, so the warning is always printed.
+ Also we are not able to detect when the code is not longer used in
+ the problematic context. We rather disable removing of livepatches
+ to avoid more possible damage.
+
+ Livepatch works reliably only when the dynamic ftrace is located at
the very beginning of the function.
diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c
index ffdf5fa8005b..c670a45d32f4 100644
--- a/kernel/livepatch/patch.c
+++ b/kernel/livepatch/patch.c
@@ -67,8 +67,10 @@ static void notrace klp_ftrace_handler(unsigned long ip,
* the RCU infrastructure. Then we might see wrong state of
* func->stack and other flags.
*/
- if (unlikely(!rcu_is_watching()))
+ if (unlikely(!rcu_is_watching())) {
+ klp_block_patch_removal = true;
WARN_ONCE(1, "Livepatch modified a function that can not be handled a safe way.!");
+ }
rcu_read_lock();
diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
index adc0cc64aa4b..0669517ea6a8 100644
--- a/kernel/livepatch/transition.c
+++ b/kernel/livepatch/transition.c
@@ -31,6 +31,8 @@
struct klp_patch *klp_transition_patch;
+bool klp_block_patch_removal;
+
static int klp_target_state = KLP_UNDEFINED;
/*
@@ -87,8 +89,11 @@ static void klp_complete_transition(void)
}
}
- if (klp_target_state == KLP_UNPATCHED && !immediate_func)
+ if (klp_target_state == KLP_UNPATCHED &&
+ !immediate_func &&
+ !klp_block_patch_removal) {
module_put(klp_transition_patch->mod);
+ }
/* Prevent klp_ftrace_handler() from seeing KLP_UNDEFINED state */
if (klp_target_state == KLP_PATCHED)
diff --git a/kernel/livepatch/transition.h b/kernel/livepatch/transition.h
index ce09b326546c..cc20b19d8707 100644
--- a/kernel/livepatch/transition.h
+++ b/kernel/livepatch/transition.h
@@ -5,6 +5,8 @@
extern struct klp_patch *klp_transition_patch;
+extern bool klp_block_patch_removal;
+
void klp_init_transition(struct klp_patch *patch, int state);
void klp_cancel_transition(void);
void klp_start_transition(void);
--
1.8.5.6
--
To unsubscribe from this list: send the line "unsubscribe live-patching" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
