linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: Josh Poimboeuf <jpoimboe@kernel.org>, Miroslav Benes <mbenes@suse.cz>
Cc: Joe Lawrence <joe.lawrence@redhat.com>,
	Nicolai Stange <nstange@suse.de>,
	live-patching@vger.kernel.org, linux-kernel@vger.kernel.org,
	Petr Mladek <pmladek@suse.com>
Subject: [POC 2/7] livepatch: Allow to handle lifetime of shadow variables using the livepatch state
Date: Fri, 10 Nov 2023 18:04:23 +0100	[thread overview]
Message-ID: <20231110170428.6664-3-pmladek@suse.com> (raw)
In-Reply-To: <20231110170428.6664-1-pmladek@suse.com>

The handling of the lifetime of the shadow variables is not easy
when the atomic replace is used. The new patch does not know
if a shadow variable has already been used by the previous livepatch.
Or if there is a shadow variable which will not longer be used.

Shadow variables are almost always used together with callbacks.
At least @post_unpatch callback is used to free not longer used shadow
variables. And sometimes @post_patch and @pre_unpatch callbacks
are used to enable or disable the use of the shadow variables.
It is needed when the shadow variable can be used only when
the entire system is able to handle them.

All this gets even more complicated because the original callbacks
are called only from the new livepatch when atomic replace is used.
Newly created livepatches might be able to handle obsolete shadow
variables so the upgrade would work. But older livepatches do not know
anything about later introduced shadow variables. They might leak
during downgrade. And they might contain outdated information when
another upgrade would start using them again.

All problems are better solved with the new callbacks associated with
a livepatch state. They are called when the state is first introduced
and when it gets obsolete. Also the callbacks are called from the patch
where the state was supported. So that even downgrade might be safe.

Let's make it official. A shadow variable might be associated with
a livepatch state by setting the new "state.is_shadow" flag and
using the same "id" in both struct klp_shadow and struct klp_state.

The shadow variable will then have the same lifetime as the livepatch
state. It allows to free obsolete shadow variables automatically
without the need to add a callback.

The generic callback will free the shadow variables using
state->callbacks.shadow_dtor callback when provided.

Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 include/linux/livepatch.h | 15 ++++++++++-----
 kernel/livepatch/state.c  | 14 ++++++++++----
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index c2a39e5f5b66..189ec7c6a89f 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -132,12 +132,18 @@ struct klp_object {
 struct klp_patch;
 struct klp_state;
 
+typedef int (*klp_shadow_ctor_t)(void *obj,
+				 void *shadow_data,
+				 void *ctor_data);
+typedef void (*klp_shadow_dtor_t)(void *obj, void *shadow_data);
+
 /**
  * struct klp_state_callbacks - callbacks manipulating the state
  * @setup:	executed before code patching when the state is added
  * @enable:	executed after code patching when the state is added
  * @disable:	executed before code unpatching when the state is removed
  * @release:	executed after code unpatching when the state is removed
+ * @shadow_dtor: destructor for the related shadow variable
  * @setup_succeeded: internal state used by a rollback on error
  *
  * All callbacks are optional.
@@ -152,6 +158,7 @@ struct klp_state_callbacks {
 	void (*enable)(struct klp_patch *patch, struct klp_state *state);
 	void (*disable)(struct klp_patch *patch, struct klp_state *state);
 	void (*release)(struct klp_patch *patch, struct klp_state *state);
+	klp_shadow_dtor_t shadow_dtor;
 	bool setup_succeeded;
 };
 
@@ -160,12 +167,15 @@ struct klp_state_callbacks {
  * @id:		system state identifier (non-zero)
  * @version:	version of the change
  * @callbacks:	optional callbacks used when introducing or removing the state
+ * @is_shadow:  the state handles lifetime of a shadow variable
+ *		with the same @id
  * @data:	custom data
  */
 struct klp_state {
 	unsigned long id;
 	unsigned int version;
 	struct klp_state_callbacks callbacks;
+	bool is_shadow;
 	void *data;
 };
 
@@ -240,11 +250,6 @@ static inline bool klp_have_reliable_stack(void)
 	       IS_ENABLED(CONFIG_HAVE_RELIABLE_STACKTRACE);
 }
 
-typedef int (*klp_shadow_ctor_t)(void *obj,
-				 void *shadow_data,
-				 void *ctor_data);
-typedef void (*klp_shadow_dtor_t)(void *obj, void *shadow_data);
-
 void *klp_shadow_get(void *obj, unsigned long id);
 void *klp_shadow_alloc(void *obj, unsigned long id,
 		       size_t size, gfp_t gfp_flags,
diff --git a/kernel/livepatch/state.c b/kernel/livepatch/state.c
index 6693d808106b..4ec65afe3a43 100644
--- a/kernel/livepatch/state.c
+++ b/kernel/livepatch/state.c
@@ -198,11 +198,17 @@ void klp_release_states(struct klp_patch *patch)
 		if (is_state_in_other_patches(patch, state))
 			continue;
 
-		if (!state->callbacks.release)
-			continue;
-
-		if (state->callbacks.setup_succeeded)
+		if (state->callbacks.release && state->callbacks.setup_succeeded)
 			state->callbacks.release(patch, state);
+
+		if (state->is_shadow)
+			klp_shadow_free_all(state->id, state->callbacks.shadow_dtor);
+
+		/*
+		 * The @release callback is supposed to restore the original
+		 * state before the @setup callback was called.
+		 */
+		state->callbacks.setup_succeeded = 0;
 	}
 }
 
-- 
2.35.3


  parent reply	other threads:[~2023-11-10 17:43 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-10 17:04 [POC 0/7] livepatch: Make livepatch states, callbacks, and shadow variables work together Petr Mladek
2023-11-10 17:04 ` [POC 1/7] livepatch: Add callbacks for introducing and removing states Petr Mladek
2023-11-11  0:54   ` kernel test robot
2023-11-11  3:19   ` kernel test robot
2023-11-10 17:04 ` Petr Mladek [this message]
2023-11-10 17:04 ` [POC 3/7] livepatch: Use per-state callbacks in state API tests Petr Mladek
2023-11-10 17:04 ` [POC 4/7] livepatch: Do not use callbacks when testing sysfs interface Petr Mladek
2023-11-10 17:04 ` [POC 5/7] livepatch: Convert klp module callbacks tests into livepatch module tests Petr Mladek
2023-11-11  1:15   ` kernel test robot
2023-11-10 17:04 ` [POC 6/7] livepatch: Remove the obsolete per-object callbacks Petr Mladek
2023-11-10 17:04 ` [POC 7/7] livepatching: Remove per-state version Petr Mladek
2023-11-10 21:33 ` [POC 0/7] livepatch: Make livepatch states, callbacks, and shadow variables work together 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=20231110170428.6664-3-pmladek@suse.com \
    --to=pmladek@suse.com \
    --cc=joe.lawrence@redhat.com \
    --cc=jpoimboe@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mbenes@suse.cz \
    --cc=nstange@suse.de \
    /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;
as well as URLs for NNTP newsgroup(s).