Blob Blame History Raw
From: Jason Baron <jbaron@akamai.com>
Date: Fri, 23 Mar 2018 13:00:25 +0100
Subject: livepatch: Remove replaced patches from the stack
Patch-mainline: Submitted, for 4.18
References: bsc#1071995 fate#323487

The stack of patches defines an order in which the patches must be enabled.
It has some drawbacks, for example:

  + People could not disable earlier patches without disabling the later
    ones even when they are independent.

  + The order is not clearly visible in /sys/kernel/livepatch. Users might
    get lost.

  + lsmod does not give a clue what patch is the last one and in use.

  + Lots of unused data and code might be locked in memory.

Some of the above problems can already be solved by using cumulative
patches (replace flag) and reasonable patch names.

This is expected. The atomic replace and cumulative patches are supposed
to help managing and maintaining many patches. And they can resolve even
more problems mentioned above when the replaced patches get removed from
the stack. Then the unused patches might be unregistered and the modules
unloaded.

Removing replaced patches will actually help even more. The patch
will become the first on the stack:

  + Nops' structs will not longer be necessary and might be removed.
    This would save memory, restore performance (no ftrace handler),
    allow clear view on what is really patched.

  + Disabling the patch will cause using the original code everywhere.
    Therefore the livepatch callbacks could handle only one scenario.
    Note that the complication is already complex enough when the patch
    gets enabled. It is currently solved by calling callbacks only from
    the new cumulative patch.

Some people actually expected this behavior from the beginning. After all
a cumulative patch is supposed to "completely" replace an existing one.
It is like when a new version of an application replaces an older one.

This patch does the first step. It removes the replaced patches from
the stack. It is safe. The consistency model ensures that they are
not longer used. By other words, each process works only with
the structures from klp_transition_patch.

The removal is done by a special function. It combines actions done by
__disable_patch() and klp_complete_transition(). But it is a fast
track without all the transaction-related stuff.

Signed-off-by: Jason Baron <jbaron@akamai.com>
[pmladek@suse.com: Split and refactored]
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: Miroslav Benes <mbenes@suse.cz>

Acked-by: Miroslav Benes <mbenes@suse.cz>
---
 kernel/livepatch/core.c       |   37 +++++++++++++++++++++++++++++++++++++
 kernel/livepatch/core.h       |    3 +++
 kernel/livepatch/transition.c |    3 +++
 3 files changed, 43 insertions(+)

--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -341,6 +341,43 @@ static void klp_taint_kernel(const struc
 #endif
 }
 
+/*
+ * This function removes replaced patches from both func_stack
+ * and klp_patches stack.
+ *
+ * We could be pretty aggressive here. It is called in the situation where
+ * these structures are no longer accessible. All functions are redirected
+ * by the klp_transition_patch. They use either a new code or they are in
+ * the original code because of the special nop function patches.
+ *
+ * The only exception is when the transition was forced. In this case,
+ * klp_ftrace_handler() might still see the replaced patch on the stack.
+ * Fortunately, it is carefully designed to work with removed functions
+ * thanks to RCU. We only have to keep the patches on the system. This
+ * is handled by @keep_module parameter.
+ */
+void klp_discard_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);
+		old_patch->enabled = false;
+
+		if (!keep_module)
+			module_put(old_patch->mod);
+
+		/*
+		 * Replaced patches could not get re-enabled to keep
+		 * the code sane.
+		 */
+		list_del_init(&old_patch->list);
+	}
+}
+
 static int __klp_disable_patch(struct klp_patch *patch)
 {
 	struct klp_object *obj;
--- a/kernel/livepatch/core.h
+++ b/kernel/livepatch/core.h
@@ -5,6 +5,9 @@
 
 extern struct mutex klp_mutex;
 
+void klp_discard_replaced_patches(struct klp_patch *new_patch,
+				  bool keep_module);
+
 static inline bool klp_is_object_loaded(struct klp_object *obj)
 {
 	return !obj->name || obj->mod;
--- a/kernel/livepatch/transition.c
+++ b/kernel/livepatch/transition.c
@@ -87,6 +87,9 @@ static void klp_complete_transition(void
 		 klp_transition_patch->mod->name,
 		 klp_target_state == KLP_PATCHED ? "patching" : "unpatching");
 
+	if (klp_transition_patch->replace && klp_target_state == KLP_PATCHED)
+		klp_discard_replaced_patches(klp_transition_patch, klp_forced);
+
 	if (klp_target_state == KLP_UNPATCHED) {
 		/*
 		 * All tasks have transitioned to KLP_UNPATCHED so we can now