Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Petr Mladek <pmladek@suse.com>
To:     Jiri Kosina <jikos@kernel.org>,
        Josh Poimboeuf <jpoimboe@redhat.com>,
        Miroslav Benes <mbenes@suse.cz>
Cc:     Jason Baron <jbaron@akamai.com>,
        Joe Lawrence <joe.lawrence@redhat.com>,
        Jessica Yu <jeyu@kernel.org>,
        Evgenii Shatokhin <eshatokhin@virtuozzo.com>,
        live-patching@vger.kernel.org, linux-kernel@vger.kernel.org,
        Petr Mladek <pmladek@suse.com>
Subject: [PATCH 5/8] livepatch: Remove replaced patches from the stack
Date:   Fri, 23 Mar 2018 13:00:25 +0100
Message-Id: <20180323120028.31451-6-pmladek@suse.com>
In-Reply-To: <20180323120028.31451-1-pmladek@suse.com>
References: <20180323120028.31451-1-pmladek@suse.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

From: Jason Baron <jbaron@akamai.com>

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>
---
 kernel/livepatch/core.c       | 37 +++++++++++++++++++++++++++++++++++++
 kernel/livepatch/core.h       |  3 +++
 kernel/livepatch/transition.c |  3 +++
 3 files changed, 43 insertions(+)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 70c67a834e9a..35f4bbff395f 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -332,6 +332,43 @@ 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 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;
diff --git a/kernel/livepatch/core.h b/kernel/livepatch/core.h
index 48a83d4364cf..2d2b724d56a4 100644
--- a/kernel/livepatch/core.h
+++ b/kernel/livepatch/core.h
@@ -6,6 +6,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;
diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
index 7c6631e693bc..24daed700ee7 100644
--- 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
-- 
2.13.6