public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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	[thread overview]
Message-ID: <20180323120028.31451-6-pmladek@suse.com> (raw)
In-Reply-To: <20180323120028.31451-1-pmladek@suse.com>

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

  parent reply	other threads:[~2018-03-23 12:02 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-23 12:00 [PATCH 0/8] livepatch: Atomic replace feature Petr Mladek
2018-03-23 12:00 ` [PATCH 1/8] livepatch: Use lists to manage patches, objects and functions Petr Mladek
2018-03-23 12:00 ` [PATCH 2/8] livepatch: Free only structures with initialized kobject Petr Mladek
2018-03-23 12:00 ` [PATCH 3/8] livepatch: Add atomic replace Petr Mladek
2018-04-06 22:05   ` Josh Poimboeuf
2018-04-09 13:53     ` Miroslav Benes
2018-04-10  9:31       ` Petr Mladek
2018-03-23 12:00 ` [PATCH 4/8] livepatch: Add an extra flag to distinguish registered patches Petr Mladek
2018-04-06 22:06   ` Josh Poimboeuf
2018-04-09 14:02   ` Miroslav Benes
2018-04-10 10:56     ` Petr Mladek
2018-04-10 17:53       ` Josh Poimboeuf
2018-03-23 12:00 ` Petr Mladek [this message]
2018-03-23 12:00 ` [PATCH 6/8] livepatch: Remove Nop structures when unused Petr Mladek
2018-04-06 22:07   ` Josh Poimboeuf
2018-04-10  9:14   ` Miroslav Benes
2018-04-10 11:09     ` Petr Mladek
2018-03-23 12:00 ` [PATCH 7/8] livepatch: Allow to replace even disabled patches Petr Mladek
2018-03-23 12:00 ` [PATCH 8/8] livepatch: Atomic replace and cumulative patches documentation Petr Mladek
2018-03-23 14:54 ` [PATCH 0/8] livepatch: Atomic replace feature Petr Mladek
2018-04-06 22:10 ` Josh Poimboeuf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180323120028.31451-6-pmladek@suse.com \
    --to=pmladek@suse.com \
    --cc=eshatokhin@virtuozzo.com \
    --cc=jbaron@akamai.com \
    --cc=jeyu@kernel.org \
    --cc=jikos@kernel.org \
    --cc=joe.lawrence@redhat.com \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mbenes@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox