* [PATCH v3 0/5] livepatch: new API to track system state changes
@ 2019-10-03 9:01 Petr Mladek
2019-10-03 9:01 ` [PATCH v3 1/5] livepatch: Keep replaced patches until post_patch callback is called Petr Mladek
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: Petr Mladek @ 2019-10-03 9:01 UTC (permalink / raw)
To: Jiri Kosina, Josh Poimboeuf, Miroslav Benes
Cc: Joe Lawrence, Kamalesh Babulal, Nicolai Stange, live-patching,
linux-kernel, Petr Mladek
Hi,
this is another piece in the puzzle that helps to maintain more
livepatches.
Especially pre/post (un)patch callbacks might change a system state.
Any newly installed livepatch has to somehow deal with system state
modifications done be already installed livepatches.
This patchset provides a simple and generic API that
helps to keep and pass information between the livepatches.
It is also usable to prevent loading incompatible livepatches.
Changes since v2:
+ Typo fixes [Miroslav]
+ Move the documentation at the end of the list [Miroslav]
+ Add Miroslav's acks
Changes since v1:
+ Use "unsigned long" instead of "int" for "state.id" [Nicolai]
+ Use "unsigned int" instead of "int" for "state.version [Petr]
+ Include "state.h" to avoid warning about non-static func [Miroslav]
+ Simplify logic in klp_is_state_compatible() [Miroslav]
+ Document how livepatches should handle the state [Nicolai]
+ Fix some typos, formulation, module metadata [Joe, Miroslav]
Petr Mladek (5):
livepatch: Keep replaced patches until post_patch callback is called
livepatch: Basic API to track system state changes
livepatch: Allow to distinguish different version of system state
changes
livepatch: Documentation of the new API for tracking system state
changes
livepatch: Selftests of the API for tracking system state changes
Documentation/livepatch/index.rst | 1 +
Documentation/livepatch/system-state.rst | 167 +++++++++++++++++++++
include/linux/livepatch.h | 17 +++
kernel/livepatch/Makefile | 2 +-
kernel/livepatch/core.c | 44 ++++--
kernel/livepatch/core.h | 5 +-
kernel/livepatch/state.c | 122 +++++++++++++++
kernel/livepatch/state.h | 9 ++
kernel/livepatch/transition.c | 12 +-
lib/livepatch/Makefile | 5 +-
lib/livepatch/test_klp_state.c | 161 ++++++++++++++++++++
lib/livepatch/test_klp_state2.c | 190 ++++++++++++++++++++++++
lib/livepatch/test_klp_state3.c | 5 +
tools/testing/selftests/livepatch/Makefile | 3 +-
tools/testing/selftests/livepatch/test-state.sh | 180 ++++++++++++++++++++++
15 files changed, 902 insertions(+), 21 deletions(-)
create mode 100644 Documentation/livepatch/system-state.rst
create mode 100644 kernel/livepatch/state.c
create mode 100644 kernel/livepatch/state.h
create mode 100644 lib/livepatch/test_klp_state.c
create mode 100644 lib/livepatch/test_klp_state2.c
create mode 100644 lib/livepatch/test_klp_state3.c
create mode 100755 tools/testing/selftests/livepatch/test-state.sh
--
2.16.4
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 1/5] livepatch: Keep replaced patches until post_patch callback is called
2019-10-03 9:01 [PATCH v3 0/5] livepatch: new API to track system state changes Petr Mladek
@ 2019-10-03 9:01 ` Petr Mladek
2019-10-03 9:01 ` [PATCH v3 2/5] livepatch: Basic API to track system state changes Petr Mladek
` (4 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Petr Mladek @ 2019-10-03 9:01 UTC (permalink / raw)
To: Jiri Kosina, Josh Poimboeuf, Miroslav Benes
Cc: Joe Lawrence, Kamalesh Babulal, Nicolai Stange, live-patching,
linux-kernel, Petr Mladek
Pre/post (un)patch callbacks might manipulate the system state. Cumulative
livepatches might need to take over the changes made by the replaced
ones. For this they might need to access some data stored or referenced
by the old livepatches.
Therefore the replaced livepatches have to stay around until post_patch()
callback is called. It is achieved by calling the free functions later.
It is the same location where disabled livepatches have already been
freed.
Signed-off-by: Petr Mladek <pmladek@suse.com>
Acked-by: Miroslav Benes <mbenes@suse.cz>
---
kernel/livepatch/core.c | 36 ++++++++++++++++++++++++++----------
kernel/livepatch/core.h | 5 +++--
kernel/livepatch/transition.c | 12 ++++++------
3 files changed, 35 insertions(+), 18 deletions(-)
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index ab4a4606d19b..1e1d87ead55c 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -632,7 +632,7 @@ static void klp_free_objects_dynamic(struct klp_patch *patch)
* The operation must be completed by calling klp_free_patch_finish()
* outside klp_mutex.
*/
-void klp_free_patch_start(struct klp_patch *patch)
+static void klp_free_patch_start(struct klp_patch *patch)
{
if (!list_empty(&patch->list))
list_del(&patch->list);
@@ -677,6 +677,23 @@ static void klp_free_patch_work_fn(struct work_struct *work)
klp_free_patch_finish(patch);
}
+void klp_free_patch_async(struct klp_patch *patch)
+{
+ klp_free_patch_start(patch);
+ schedule_work(&patch->free_work);
+}
+
+void klp_free_replaced_patches_async(struct klp_patch *new_patch)
+{
+ struct klp_patch *old_patch, *tmp_patch;
+
+ klp_for_each_patch_safe(old_patch, tmp_patch) {
+ if (old_patch == new_patch)
+ return;
+ klp_free_patch_async(old_patch);
+ }
+}
+
static int klp_init_func(struct klp_object *obj, struct klp_func *func)
{
if (!func->old_name)
@@ -1022,12 +1039,13 @@ int klp_enable_patch(struct klp_patch *patch)
EXPORT_SYMBOL_GPL(klp_enable_patch);
/*
- * This function removes replaced patches.
+ * This function unpatches objects from the replaced livepatches.
*
* 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.
+ * these structures are no longer accessed from the ftrace handler.
+ * 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.
@@ -1035,18 +1053,16 @@ EXPORT_SYMBOL_GPL(klp_enable_patch);
* thanks to RCU. We only have to keep the patches on the system. Also
* this is handled transparently by patch->module_put.
*/
-void klp_discard_replaced_patches(struct klp_patch *new_patch)
+void klp_unpatch_replaced_patches(struct klp_patch *new_patch)
{
- struct klp_patch *old_patch, *tmp_patch;
+ struct klp_patch *old_patch;
- klp_for_each_patch_safe(old_patch, tmp_patch) {
+ klp_for_each_patch(old_patch) {
if (old_patch == new_patch)
return;
old_patch->enabled = false;
klp_unpatch_objects(old_patch);
- klp_free_patch_start(old_patch);
- schedule_work(&old_patch->free_work);
}
}
diff --git a/kernel/livepatch/core.h b/kernel/livepatch/core.h
index ec43a40b853f..38209c7361b6 100644
--- a/kernel/livepatch/core.h
+++ b/kernel/livepatch/core.h
@@ -13,8 +13,9 @@ extern struct list_head klp_patches;
#define klp_for_each_patch(patch) \
list_for_each_entry(patch, &klp_patches, list)
-void klp_free_patch_start(struct klp_patch *patch);
-void klp_discard_replaced_patches(struct klp_patch *new_patch);
+void klp_free_patch_async(struct klp_patch *patch);
+void klp_free_replaced_patches_async(struct klp_patch *new_patch);
+void klp_unpatch_replaced_patches(struct klp_patch *new_patch);
void klp_discard_nops(struct klp_patch *new_patch);
static inline bool klp_is_object_loaded(struct klp_object *obj)
diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
index cdf318d86dd6..f6310f848f34 100644
--- a/kernel/livepatch/transition.c
+++ b/kernel/livepatch/transition.c
@@ -78,7 +78,7 @@ static void klp_complete_transition(void)
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_unpatch_replaced_patches(klp_transition_patch);
klp_discard_nops(klp_transition_patch);
}
@@ -446,14 +446,14 @@ void klp_try_complete_transition(void)
klp_complete_transition();
/*
- * It would make more sense to free the patch in
+ * It would make more sense to free the unused patches in
* klp_complete_transition() but it is called also
* from klp_cancel_transition().
*/
- if (!patch->enabled) {
- klp_free_patch_start(patch);
- schedule_work(&patch->free_work);
- }
+ if (!patch->enabled)
+ klp_free_patch_async(patch);
+ else if (patch->replace)
+ klp_free_replaced_patches_async(patch);
}
/*
--
2.16.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v3 2/5] livepatch: Basic API to track system state changes
2019-10-03 9:01 [PATCH v3 0/5] livepatch: new API to track system state changes Petr Mladek
2019-10-03 9:01 ` [PATCH v3 1/5] livepatch: Keep replaced patches until post_patch callback is called Petr Mladek
@ 2019-10-03 9:01 ` Petr Mladek
2019-10-03 9:01 ` [PATCH v3 3/5] livepatch: Allow to distinguish different version of " Petr Mladek
` (3 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Petr Mladek @ 2019-10-03 9:01 UTC (permalink / raw)
To: Jiri Kosina, Josh Poimboeuf, Miroslav Benes
Cc: Joe Lawrence, Kamalesh Babulal, Nicolai Stange, live-patching,
linux-kernel, Petr Mladek
This is another step how to help maintaining more livepatches.
One big help was the atomic replace and cumulative livepatches. These
livepatches replace the already installed ones. Therefore it should
be enough when each cumulative livepatch is consistent.
The problems might come with shadow variables and callbacks. They might
change the system behavior or state so that it is no longer safe to
go back and use an older livepatch or the original kernel code. Also,
a new livepatch must be able to detect changes which were made by
the already installed livepatches.
This is where the livepatch system state tracking gets useful. It
allows to:
- find whether a system state has already been modified by
previous livepatches
- store data needed to manipulate and restore the system state
The information about the manipulated system states is stored in an
array of struct klp_state. It can be searched by two new functions
klp_get_state() and klp_get_prev_state().
The dependencies are going to be solved by a version field added later.
The only important information is that it will be allowed to modify
the same state by more non-cumulative livepatches. It is similar
to allowing to modify the same function several times. The livepatch
author is responsible for preventing incompatible changes.
Signed-off-by: Petr Mladek <pmladek@suse.com>
Acked-by: Miroslav Benes <mbenes@suse.cz>
---
include/linux/livepatch.h | 15 +++++++++
kernel/livepatch/Makefile | 2 +-
kernel/livepatch/state.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 101 insertions(+), 1 deletion(-)
create mode 100644 kernel/livepatch/state.c
diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index 273400814020..726947338fd5 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -130,10 +130,21 @@ struct klp_object {
bool patched;
};
+/**
+ * struct klp_state - state of the system modified by the livepatch
+ * @id: system state identifier (non-zero)
+ * @data: custom data
+ */
+struct klp_state {
+ unsigned long id;
+ void *data;
+};
+
/**
* struct klp_patch - patch structure for live patching
* @mod: reference to the live patch module
* @objs: object entries for kernel objects to be patched
+ * @states: system states that can get modified
* @replace: replace all actively used patches
* @list: list node for global list of actively used patches
* @kobj: kobject for sysfs resources
@@ -147,6 +158,7 @@ struct klp_patch {
/* external */
struct module *mod;
struct klp_object *objs;
+ struct klp_state *states;
bool replace;
/* internal */
@@ -217,6 +229,9 @@ void *klp_shadow_get_or_alloc(void *obj, unsigned long id,
void klp_shadow_free(void *obj, unsigned long id, klp_shadow_dtor_t dtor);
void klp_shadow_free_all(unsigned long id, klp_shadow_dtor_t dtor);
+struct klp_state *klp_get_state(struct klp_patch *patch, unsigned long id);
+struct klp_state *klp_get_prev_state(unsigned long id);
+
#else /* !CONFIG_LIVEPATCH */
static inline int klp_module_coming(struct module *mod) { return 0; }
diff --git a/kernel/livepatch/Makefile b/kernel/livepatch/Makefile
index cf9b5bcdb952..cf03d4bdfc66 100644
--- a/kernel/livepatch/Makefile
+++ b/kernel/livepatch/Makefile
@@ -1,4 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
obj-$(CONFIG_LIVEPATCH) += livepatch.o
-livepatch-objs := core.o patch.o shadow.o transition.o
+livepatch-objs := core.o patch.o shadow.o state.o transition.o
diff --git a/kernel/livepatch/state.c b/kernel/livepatch/state.c
new file mode 100644
index 000000000000..0a7014c57478
--- /dev/null
+++ b/kernel/livepatch/state.c
@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * system_state.c - State of the system modified by livepatches
+ *
+ * Copyright (C) 2019 SUSE
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/livepatch.h>
+#include "core.h"
+#include "transition.h"
+
+#define klp_for_each_state(patch, state) \
+ for (state = patch->states; \
+ state && state->id; \
+ state++)
+
+/**
+ * klp_get_state() - get information about system state modified by
+ * the given patch
+ * @patch: livepatch that modifies the given system state
+ * @id: custom identifier of the modified system state
+ *
+ * Checks whether the given patch modifies the given system state.
+ *
+ * The function can be called either from pre/post (un)patch
+ * callbacks or from the kernel code added by the livepatch.
+ *
+ * Return: pointer to struct klp_state when found, otherwise NULL.
+ */
+struct klp_state *klp_get_state(struct klp_patch *patch, unsigned long id)
+{
+ struct klp_state *state;
+
+ klp_for_each_state(patch, state) {
+ if (state->id == id)
+ return state;
+ }
+
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(klp_get_state);
+
+/**
+ * klp_get_prev_state() - get information about system state modified by
+ * the already installed livepatches
+ * @id: custom identifier of the modified system state
+ *
+ * Checks whether already installed livepatches modify the given
+ * system state.
+ *
+ * The same system state can be modified by more non-cumulative
+ * livepatches. It is expected that the latest livepatch has
+ * the most up-to-date information.
+ *
+ * The function can be called only during transition when a new
+ * livepatch is being enabled or when such a transition is reverted.
+ * It is typically called only from from pre/post (un)patch
+ * callbacks.
+ *
+ * Return: pointer to the latest struct klp_state from already
+ * installed livepatches, NULL when not found.
+ */
+struct klp_state *klp_get_prev_state(unsigned long id)
+{
+ struct klp_patch *patch;
+ struct klp_state *state, *last_state = NULL;
+
+ if (WARN_ON_ONCE(!klp_transition_patch))
+ return NULL;
+
+ klp_for_each_patch(patch) {
+ if (patch == klp_transition_patch)
+ goto out;
+
+ state = klp_get_state(patch, id);
+ if (state)
+ last_state = state;
+ }
+
+out:
+ return last_state;
+}
+EXPORT_SYMBOL_GPL(klp_get_prev_state);
--
2.16.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v3 3/5] livepatch: Allow to distinguish different version of system state changes
2019-10-03 9:01 [PATCH v3 0/5] livepatch: new API to track system state changes Petr Mladek
2019-10-03 9:01 ` [PATCH v3 1/5] livepatch: Keep replaced patches until post_patch callback is called Petr Mladek
2019-10-03 9:01 ` [PATCH v3 2/5] livepatch: Basic API to track system state changes Petr Mladek
@ 2019-10-03 9:01 ` Petr Mladek
2019-10-23 21:15 ` Josh Poimboeuf
2019-10-03 9:01 ` [PATCH v3 4/5] livepatch: Documentation of the new API for tracking " Petr Mladek
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Petr Mladek @ 2019-10-03 9:01 UTC (permalink / raw)
To: Jiri Kosina, Josh Poimboeuf, Miroslav Benes
Cc: Joe Lawrence, Kamalesh Babulal, Nicolai Stange, live-patching,
linux-kernel, Petr Mladek
The atomic replace runs pre/post (un)install callbacks only from the new
livepatch. There are several reasons for this:
+ Simplicity: clear ordering of operations, no interactions between
old and new callbacks.
+ Reliability: only new livepatch knows what changes can already be made
by older livepatches and how to take over the state.
+ Testing: the atomic replace can be properly tested only when a newer
livepatch is available. It might be too late to fix unwanted effect
of callbacks from older livepatches.
It might happen that an older change is not enough and the same system
state has to be modified another way. Different changes need to get
distinguished by a version number added to struct klp_state.
The version can also be used to prevent loading incompatible livepatches.
The check is done when the livepatch is enabled. The rules are:
+ Any completely new system state modification is allowed.
+ System state modifications with the same or higher version are allowed
for already modified system states.
+ Cumulative livepatches must handle all system state modifications from
already installed livepatches.
+ Non-cumulative livepatches are allowed to touch already modified
system states.
Signed-off-by: Petr Mladek <pmladek@suse.com>
Acked-by: Miroslav Benes <mbenes@suse.cz>
---
include/linux/livepatch.h | 2 ++
kernel/livepatch/core.c | 8 ++++++++
kernel/livepatch/state.c | 39 ++++++++++++++++++++++++++++++++++++++-
kernel/livepatch/state.h | 9 +++++++++
4 files changed, 57 insertions(+), 1 deletion(-)
create mode 100644 kernel/livepatch/state.h
diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index 726947338fd5..42907c4a0ce8 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -133,10 +133,12 @@ struct klp_object {
/**
* struct klp_state - state of the system modified by the livepatch
* @id: system state identifier (non-zero)
+ * @version: version of the change (non-zero)
* @data: custom data
*/
struct klp_state {
unsigned long id;
+ unsigned int version;
void *data;
};
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 1e1d87ead55c..495eafd07328 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -22,6 +22,7 @@
#include <asm/cacheflush.h>
#include "core.h"
#include "patch.h"
+#include "state.h"
#include "transition.h"
/*
@@ -1009,6 +1010,13 @@ int klp_enable_patch(struct klp_patch *patch)
mutex_lock(&klp_mutex);
+ if(!klp_is_patch_compatible(patch)) {
+ pr_err("Livepatch patch (%s) is not compatible with the already installed livepatches.\n",
+ patch->mod->name);
+ mutex_unlock(&klp_mutex);
+ return -EINVAL;
+ }
+
ret = klp_init_patch_early(patch);
if (ret) {
mutex_unlock(&klp_mutex);
diff --git a/kernel/livepatch/state.c b/kernel/livepatch/state.c
index 0a7014c57478..54d9587d167f 100644
--- a/kernel/livepatch/state.c
+++ b/kernel/livepatch/state.c
@@ -9,11 +9,12 @@
#include <linux/livepatch.h>
#include "core.h"
+#include "state.h"
#include "transition.h"
#define klp_for_each_state(patch, state) \
for (state = patch->states; \
- state && state->id; \
+ state && state->id && state->version; \
state++)
/**
@@ -83,3 +84,39 @@ struct klp_state *klp_get_prev_state(unsigned long id)
return last_state;
}
EXPORT_SYMBOL_GPL(klp_get_prev_state);
+
+/* Check if the patch is able to deal with the given system state. */
+static bool klp_is_state_compatible(struct klp_patch *patch,
+ struct klp_state *state)
+{
+ struct klp_state *new_state;
+
+ new_state = klp_get_state(patch, state->id);
+
+ if (new_state)
+ return new_state->version >= state->version;
+
+ /* Cumulative livepatch must handle all already modified states. */
+ return !patch->replace;
+}
+
+/*
+ * Check that the new livepatch will not break the existing system states.
+ * Cumulative patches must handle all already modified states.
+ * Non-cumulative patches can touch already modified states.
+ */
+bool klp_is_patch_compatible(struct klp_patch *patch)
+{
+ struct klp_patch *old_patch;
+ struct klp_state *state;
+
+
+ klp_for_each_patch(old_patch) {
+ klp_for_each_state(old_patch, state) {
+ if (!klp_is_state_compatible(patch, state))
+ return false;
+ }
+ }
+
+ return true;
+}
diff --git a/kernel/livepatch/state.h b/kernel/livepatch/state.h
new file mode 100644
index 000000000000..49d9c16e8762
--- /dev/null
+++ b/kernel/livepatch/state.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LIVEPATCH_STATE_H
+#define _LIVEPATCH_STATE_H
+
+#include <linux/livepatch.h>
+
+bool klp_is_patch_compatible(struct klp_patch *patch);
+
+#endif /* _LIVEPATCH_STATE_H */
--
2.16.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v3 4/5] livepatch: Documentation of the new API for tracking system state changes
2019-10-03 9:01 [PATCH v3 0/5] livepatch: new API to track system state changes Petr Mladek
` (2 preceding siblings ...)
2019-10-03 9:01 ` [PATCH v3 3/5] livepatch: Allow to distinguish different version of " Petr Mladek
@ 2019-10-03 9:01 ` Petr Mladek
2019-10-03 9:01 ` [PATCH v3 5/5] livepatch: Selftests of the " Petr Mladek
2019-10-04 14:39 ` [PATCH v3 0/5] livepatch: new API to track " Joe Lawrence
5 siblings, 0 replies; 13+ messages in thread
From: Petr Mladek @ 2019-10-03 9:01 UTC (permalink / raw)
To: Jiri Kosina, Josh Poimboeuf, Miroslav Benes
Cc: Joe Lawrence, Kamalesh Babulal, Nicolai Stange, live-patching,
linux-kernel, Petr Mladek
Documentation explaining the motivation, capabilities, and usage
of the new API for tracking system state changes.
Signed-off-by: Petr Mladek <pmladek@suse.com>
Acked-by: Miroslav Benes <mbenes@suse.cz>
---
Documentation/livepatch/index.rst | 1 +
Documentation/livepatch/system-state.rst | 167 +++++++++++++++++++++++++++++++
2 files changed, 168 insertions(+)
create mode 100644 Documentation/livepatch/system-state.rst
diff --git a/Documentation/livepatch/index.rst b/Documentation/livepatch/index.rst
index 17674a9e21b2..525944063be7 100644
--- a/Documentation/livepatch/index.rst
+++ b/Documentation/livepatch/index.rst
@@ -12,6 +12,7 @@ Kernel Livepatching
cumulative-patches
module-elf-format
shadow-vars
+ system-state
.. only:: subproject and html
diff --git a/Documentation/livepatch/system-state.rst b/Documentation/livepatch/system-state.rst
new file mode 100644
index 000000000000..56a3f4d16e8b
--- /dev/null
+++ b/Documentation/livepatch/system-state.rst
@@ -0,0 +1,167 @@
+====================
+System State Changes
+====================
+
+Some users are really reluctant to reboot a system. This brings the need
+to provide more livepatches and maintain some compatibility between them.
+
+Maintaining more livepatches is much easier with cumulative livepatches.
+Each new livepatch completely replaces any older one. It can keep,
+add, and even remove fixes. And it is typically safe to replace any version
+of the livepatch with any other one thanks to the atomic replace feature.
+
+The problems might come with shadow variables and callbacks. They might
+change the system behavior or state so that it is no longer safe to
+go back and use an older livepatch or the original kernel code. Also
+any new livepatch must be able to detect what changes have already been
+done by the already installed livepatches.
+
+This is where the livepatch system state tracking gets useful. It
+allows to:
+
+ - store data needed to manipulate and restore the system state
+
+ - define compatibility between livepatches using a change id
+ and version
+
+
+1. Livepatch system state API
+=============================
+
+The state of the system might get modified either by several livepatch callbacks
+or by the newly used code. Also it must be possible to find changes done by
+already installed livepatches.
+
+Each modified state is described by struct klp_state, see
+include/linux/livepatch.h.
+
+Each livepatch defines an array of struct klp_states. They mention
+all states that the livepatch modifies.
+
+The livepatch author must define the following two fields for each
+struct klp_state:
+
+ - *id*
+
+ - Non-zero number used to identify the affected system state.
+
+ - *version*
+
+ - Number describing the variant of the system state change that
+ is supported by the given livepatch.
+
+The state can be manipulated using two functions:
+
+ - *klp_get_state(patch, id)*
+
+ - Get struct klp_state associated with the given livepatch
+ and state id.
+
+ - *klp_get_prev_state(id)*
+
+ - Get struct klp_state associated with the given feature id and
+ already installed livepatches.
+
+2. Livepatch compatibility
+==========================
+
+The system state version is used to prevent loading incompatible livepatches.
+The check is done when the livepatch is enabled. The rules are:
+
+ - Any completely new system state modification is allowed.
+
+ - System state modifications with the same or higher version are allowed
+ for already modified system states.
+
+ - Cumulative livepatches must handle all system state modifications from
+ already installed livepatches.
+
+ - Non-cumulative livepatches are allowed to touch already modified
+ system states.
+
+3. Supported scenarios
+======================
+
+Livepatches have their life-cycle and the same is true for the system
+state changes. Every compatible livepatch has to support the following
+scenarios:
+
+ - Modify the system state when the livepatch gets enabled and the state
+ has not been already modified by a livepatches that are being
+ replaced.
+
+ - Take over or update the system state modification when is has already
+ been done by a livepatch that is being replaced.
+
+ - Restore the original state when the livepatch is disabled.
+
+ - Restore the previous state when the transition is reverted.
+ It might be the orignal system state or the state modification
+ done by livepatches that were being replaced.
+
+ - Remove any already made changes when error occurs and the livepatch
+ cannot get enabled.
+
+4. Expected usage
+=================
+
+System states are usually modified by livepatch callbacks. The expected
+role of each callback is as follows:
+
+*pre_patch()*
+
+ - Allocate *state->data* when necessary. The allocation might fail
+ and *pre_patch()* is the only callback that could stop loading
+ of the livepatch. The allocation is not needed when the data
+ are already provided by previously installed livepatches.
+
+ - Do any other preparatory action that is needed by
+ the new code even before the transition gets finished.
+ For example, initialize *state->data*.
+
+ The system state itself is typically modified in *post_patch()*
+ when the entire system is able to handle it.
+
+ - Clean up its own mess in case of error. It might be done by a custom
+ code or by calling *post_unpatch()* explicitly.
+
+*post_patch()*
+
+ - Copy *state->data* from the previous livepatch when they are
+ compatible.
+
+ - Do the actual system state modification. Eventually allow
+ the new code to use it.
+
+ - Make sure that *state->data* has all necessary information.
+
+ - Free *state->data* from replaces livepatches when they are
+ not longer needed.
+
+*pre_unpatch()*
+
+ - Prevent the code, added by the livepatch, relying on the system
+ state change.
+
+ - Revert the system state modification..
+
+*post_unpatch()*
+
+ - Distinguish transition reverse and livepatch disabling by
+ checking *klp_get_prev_state()*.
+
+ - In case of transition reverse, restore the previous system
+ state. It might mean doing nothing.
+
+ - Remove any not longer needed setting or data.
+
+.. note::
+
+ *pre_unpatch()* typically does symmetric operations to *post_patch()*.
+ Except that it is called only when the livepatch is being disabled.
+ Therefore it does not need to care about any previously installed
+ livepatch.
+
+ *post_unpatch()* typically does symmetric operations to *pre_patch()*.
+ It might be called also during the transition reverse. Therefore it
+ has to handle the state of the previously installed livepatches.
--
2.16.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v3 5/5] livepatch: Selftests of the API for tracking system state changes
2019-10-03 9:01 [PATCH v3 0/5] livepatch: new API to track system state changes Petr Mladek
` (3 preceding siblings ...)
2019-10-03 9:01 ` [PATCH v3 4/5] livepatch: Documentation of the new API for tracking " Petr Mladek
@ 2019-10-03 9:01 ` Petr Mladek
2019-10-04 14:47 ` Joe Lawrence
2019-10-08 22:19 ` kbuild test robot
2019-10-04 14:39 ` [PATCH v3 0/5] livepatch: new API to track " Joe Lawrence
5 siblings, 2 replies; 13+ messages in thread
From: Petr Mladek @ 2019-10-03 9:01 UTC (permalink / raw)
To: Jiri Kosina, Josh Poimboeuf, Miroslav Benes
Cc: Joe Lawrence, Kamalesh Babulal, Nicolai Stange, live-patching,
linux-kernel, Petr Mladek
Four selftests for the new API.
Signed-off-by: Petr Mladek <pmladek@suse.com>
Acked-by: Miroslav Benes <mbenes@suse.cz>
---
lib/livepatch/Makefile | 5 +-
lib/livepatch/test_klp_state.c | 161 ++++++++++++++++++++
lib/livepatch/test_klp_state2.c | 190 ++++++++++++++++++++++++
lib/livepatch/test_klp_state3.c | 5 +
tools/testing/selftests/livepatch/Makefile | 3 +-
tools/testing/selftests/livepatch/test-state.sh | 180 ++++++++++++++++++++++
6 files changed, 542 insertions(+), 2 deletions(-)
create mode 100644 lib/livepatch/test_klp_state.c
create mode 100644 lib/livepatch/test_klp_state2.c
create mode 100644 lib/livepatch/test_klp_state3.c
create mode 100755 tools/testing/selftests/livepatch/test-state.sh
diff --git a/lib/livepatch/Makefile b/lib/livepatch/Makefile
index 26900ddaef82..295b94bff370 100644
--- a/lib/livepatch/Makefile
+++ b/lib/livepatch/Makefile
@@ -8,7 +8,10 @@ obj-$(CONFIG_TEST_LIVEPATCH) += test_klp_atomic_replace.o \
test_klp_callbacks_busy.o \
test_klp_callbacks_mod.o \
test_klp_livepatch.o \
- test_klp_shadow_vars.o
+ test_klp_shadow_vars.o \
+ test_klp_state.o \
+ test_klp_state2.o \
+ test_klp_state3.o
# Target modules to be livepatched require CC_FLAGS_FTRACE
CFLAGS_test_klp_callbacks_busy.o += $(CC_FLAGS_FTRACE)
diff --git a/lib/livepatch/test_klp_state.c b/lib/livepatch/test_klp_state.c
new file mode 100644
index 000000000000..634257884e6f
--- /dev/null
+++ b/lib/livepatch/test_klp_state.c
@@ -0,0 +1,161 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2019 SUSE
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/printk.h>
+#include <linux/livepatch.h>
+
+#define CONSOLE_LOGLEVEL_STATE 1
+/* Version 1 does not support migration. */
+#define CONSOLE_LOGLEVEL_STATE_VERSION 1
+
+static const char *const module_state[] = {
+ [MODULE_STATE_LIVE] = "[MODULE_STATE_LIVE] Normal state",
+ [MODULE_STATE_COMING] = "[MODULE_STATE_COMING] Full formed, running module_init",
+ [MODULE_STATE_GOING] = "[MODULE_STATE_GOING] Going away",
+ [MODULE_STATE_UNFORMED] = "[MODULE_STATE_UNFORMED] Still setting it up",
+};
+
+static void callback_info(const char *callback, struct klp_object *obj)
+{
+ if (obj->mod)
+ pr_info("%s: %s -> %s\n", callback, obj->mod->name,
+ module_state[obj->mod->state]);
+ else
+ pr_info("%s: vmlinux\n", callback);
+}
+
+static struct klp_patch patch;
+
+static int allocate_loglevel_state(void)
+{
+ struct klp_state *loglevel_state;
+
+ loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
+ if (!loglevel_state)
+ return -EINVAL;
+
+ loglevel_state->data = kzalloc(sizeof(console_loglevel), GFP_KERNEL);
+ if (!loglevel_state->data)
+ return -ENOMEM;
+
+ pr_info("%s: allocating space to store console_loglevel\n",
+ __func__);
+ return 0;
+}
+
+static void fix_console_loglevel(void)
+{
+ struct klp_state *loglevel_state;
+
+ loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
+ if (!loglevel_state)
+ return;
+
+ pr_info("%s: fixing console_loglevel\n", __func__);
+ *(int *)loglevel_state->data = console_loglevel;
+ console_loglevel = CONSOLE_LOGLEVEL_MOTORMOUTH;
+}
+
+static void restore_console_loglevel(void)
+{
+ struct klp_state *loglevel_state;
+
+ loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
+ if (!loglevel_state)
+ return;
+
+ pr_info("%s: restoring console_loglevel\n", __func__);
+ console_loglevel = *(int *)loglevel_state->data;
+}
+
+static void free_loglevel_state(void)
+{
+ struct klp_state *loglevel_state;
+
+ loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
+ if (!loglevel_state)
+ return;
+
+ pr_info("%s: freeing space for the stored console_loglevel\n",
+ __func__);
+ kfree(loglevel_state->data);
+}
+
+/* Executed on object patching (ie, patch enablement) */
+static int pre_patch_callback(struct klp_object *obj)
+{
+ callback_info(__func__, obj);
+ return allocate_loglevel_state();
+}
+
+/* Executed on object unpatching (ie, patch disablement) */
+static void post_patch_callback(struct klp_object *obj)
+{
+ callback_info(__func__, obj);
+ fix_console_loglevel();
+}
+
+/* Executed on object unpatching (ie, patch disablement) */
+static void pre_unpatch_callback(struct klp_object *obj)
+{
+ callback_info(__func__, obj);
+ restore_console_loglevel();
+}
+
+/* Executed on object unpatching (ie, patch disablement) */
+static void post_unpatch_callback(struct klp_object *obj)
+{
+ callback_info(__func__, obj);
+ free_loglevel_state();
+}
+
+static struct klp_func no_funcs[] = {
+ {}
+};
+
+static struct klp_object objs[] = {
+ {
+ .name = NULL, /* vmlinux */
+ .funcs = no_funcs,
+ .callbacks = {
+ .pre_patch = pre_patch_callback,
+ .post_patch = post_patch_callback,
+ .pre_unpatch = pre_unpatch_callback,
+ .post_unpatch = post_unpatch_callback,
+ },
+ }, { }
+};
+
+static struct klp_state states[] = {
+ {
+ .id = CONSOLE_LOGLEVEL_STATE,
+ .version = CONSOLE_LOGLEVEL_STATE_VERSION,
+ }, { }
+};
+
+static struct klp_patch patch = {
+ .mod = THIS_MODULE,
+ .objs = objs,
+ .states = states,
+ .replace = true,
+};
+
+static int test_klp_callbacks_demo_init(void)
+{
+ return klp_enable_patch(&patch);
+}
+
+static void test_klp_callbacks_demo_exit(void)
+{
+}
+
+module_init(test_klp_callbacks_demo_init);
+module_exit(test_klp_callbacks_demo_exit);
+MODULE_LICENSE("GPL");
+MODULE_INFO(livepatch, "Y");
+MODULE_AUTHOR("Petr Mladek <pmladek@suse.com>");
+MODULE_DESCRIPTION("Livepatch test: system state modification");
diff --git a/lib/livepatch/test_klp_state2.c b/lib/livepatch/test_klp_state2.c
new file mode 100644
index 000000000000..c861848afb8f
--- /dev/null
+++ b/lib/livepatch/test_klp_state2.c
@@ -0,0 +1,190 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2019 SUSE
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/printk.h>
+#include <linux/livepatch.h>
+
+#define CONSOLE_LOGLEVEL_STATE 1
+/* Version 2 supports migration. */
+#define CONSOLE_LOGLEVEL_STATE_VERSION 2
+
+static const char *const module_state[] = {
+ [MODULE_STATE_LIVE] = "[MODULE_STATE_LIVE] Normal state",
+ [MODULE_STATE_COMING] = "[MODULE_STATE_COMING] Full formed, running module_init",
+ [MODULE_STATE_GOING] = "[MODULE_STATE_GOING] Going away",
+ [MODULE_STATE_UNFORMED] = "[MODULE_STATE_UNFORMED] Still setting it up",
+};
+
+static void callback_info(const char *callback, struct klp_object *obj)
+{
+ if (obj->mod)
+ pr_info("%s: %s -> %s\n", callback, obj->mod->name,
+ module_state[obj->mod->state]);
+ else
+ pr_info("%s: vmlinux\n", callback);
+}
+
+static struct klp_patch patch;
+
+static int allocate_loglevel_state(void)
+{
+ struct klp_state *loglevel_state, *prev_loglevel_state;
+
+ prev_loglevel_state = klp_get_prev_state(CONSOLE_LOGLEVEL_STATE);
+ if (prev_loglevel_state) {
+ pr_info("%s: space to store console_loglevel already allocated\n",
+ __func__);
+ return 0;
+ }
+
+ loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
+ if (!loglevel_state)
+ return -EINVAL;
+
+ loglevel_state->data = kzalloc(sizeof(console_loglevel), GFP_KERNEL);
+ if (!loglevel_state->data)
+ return -ENOMEM;
+
+ pr_info("%s: allocating space to store console_loglevel\n",
+ __func__);
+ return 0;
+}
+
+static void fix_console_loglevel(void)
+{
+ struct klp_state *loglevel_state, *prev_loglevel_state;
+
+ loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
+ if (!loglevel_state)
+ return;
+
+ prev_loglevel_state = klp_get_prev_state(CONSOLE_LOGLEVEL_STATE);
+ if (prev_loglevel_state) {
+ pr_info("%s: taking over the console_loglevel change\n",
+ __func__);
+ loglevel_state->data = prev_loglevel_state->data;
+ return;
+ }
+
+ pr_info("%s: fixing console_loglevel\n", __func__);
+ *(int *)loglevel_state->data = console_loglevel;
+ console_loglevel = CONSOLE_LOGLEVEL_MOTORMOUTH;
+}
+
+static void restore_console_loglevel(void)
+{
+ struct klp_state *loglevel_state, *prev_loglevel_state;
+
+ prev_loglevel_state = klp_get_prev_state(CONSOLE_LOGLEVEL_STATE);
+ if (prev_loglevel_state) {
+ pr_info("%s: passing the console_loglevel change back to the old livepatch\n",
+ __func__);
+ return;
+ }
+
+ loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
+ if (!loglevel_state)
+ return;
+
+ pr_info("%s: restoring console_loglevel\n", __func__);
+ console_loglevel = *(int *)loglevel_state->data;
+}
+
+static void free_loglevel_state(void)
+{
+ struct klp_state *loglevel_state, *prev_loglevel_state;
+
+ prev_loglevel_state = klp_get_prev_state(CONSOLE_LOGLEVEL_STATE);
+ if (prev_loglevel_state) {
+ pr_info("%s: keeping space to store console_loglevel\n",
+ __func__);
+ return;
+ }
+
+ loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
+ if (!loglevel_state)
+ return;
+
+ pr_info("%s: freeing space for the stored console_loglevel\n",
+ __func__);
+ kfree(loglevel_state->data);
+}
+
+/* Executed on object patching (ie, patch enablement) */
+static int pre_patch_callback(struct klp_object *obj)
+{
+ callback_info(__func__, obj);
+ return allocate_loglevel_state();
+}
+
+/* Executed on object unpatching (ie, patch disablement) */
+static void post_patch_callback(struct klp_object *obj)
+{
+ callback_info(__func__, obj);
+ fix_console_loglevel();
+}
+
+/* Executed on object unpatching (ie, patch disablement) */
+static void pre_unpatch_callback(struct klp_object *obj)
+{
+ callback_info(__func__, obj);
+ restore_console_loglevel();
+}
+
+/* Executed on object unpatching (ie, patch disablement) */
+static void post_unpatch_callback(struct klp_object *obj)
+{
+ callback_info(__func__, obj);
+ free_loglevel_state();
+}
+
+static struct klp_func no_funcs[] = {
+ {}
+};
+
+static struct klp_object objs[] = {
+ {
+ .name = NULL, /* vmlinux */
+ .funcs = no_funcs,
+ .callbacks = {
+ .pre_patch = pre_patch_callback,
+ .post_patch = post_patch_callback,
+ .pre_unpatch = pre_unpatch_callback,
+ .post_unpatch = post_unpatch_callback,
+ },
+ }, { }
+};
+
+static struct klp_state states[] = {
+ {
+ .id = CONSOLE_LOGLEVEL_STATE,
+ .version = CONSOLE_LOGLEVEL_STATE_VERSION,
+ }, { }
+};
+
+static struct klp_patch patch = {
+ .mod = THIS_MODULE,
+ .objs = objs,
+ .states = states,
+ .replace = true,
+};
+
+static int test_klp_callbacks_demo_init(void)
+{
+ return klp_enable_patch(&patch);
+}
+
+static void test_klp_callbacks_demo_exit(void)
+{
+}
+
+module_init(test_klp_callbacks_demo_init);
+module_exit(test_klp_callbacks_demo_exit);
+MODULE_LICENSE("GPL");
+MODULE_INFO(livepatch, "Y");
+MODULE_AUTHOR("Petr Mladek <pmladek@suse.com>");
+MODULE_DESCRIPTION("Livepatch test: system state modification");
diff --git a/lib/livepatch/test_klp_state3.c b/lib/livepatch/test_klp_state3.c
new file mode 100644
index 000000000000..9226579d10c5
--- /dev/null
+++ b/lib/livepatch/test_klp_state3.c
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2019 SUSE
+
+/* The console loglevel fix is the same in the next cumulative patch. */
+#include "test_klp_state2.c"
diff --git a/tools/testing/selftests/livepatch/Makefile b/tools/testing/selftests/livepatch/Makefile
index fd405402c3ff..1cf40a9e7185 100644
--- a/tools/testing/selftests/livepatch/Makefile
+++ b/tools/testing/selftests/livepatch/Makefile
@@ -4,6 +4,7 @@ TEST_PROGS_EXTENDED := functions.sh
TEST_PROGS := \
test-livepatch.sh \
test-callbacks.sh \
- test-shadow-vars.sh
+ test-shadow-vars.sh \
+ test-state.sh
include ../lib.mk
diff --git a/tools/testing/selftests/livepatch/test-state.sh b/tools/testing/selftests/livepatch/test-state.sh
new file mode 100755
index 000000000000..1139c664c11c
--- /dev/null
+++ b/tools/testing/selftests/livepatch/test-state.sh
@@ -0,0 +1,180 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2018 Joe Lawrence <joe.lawrence@redhat.com>
+
+. $(dirname $0)/functions.sh
+
+MOD_LIVEPATCH=test_klp_state
+MOD_LIVEPATCH2=test_klp_state2
+MOD_LIVEPATCH3=test_klp_state3
+
+set_dynamic_debug
+
+
+# TEST: Loading and removing a module that modifies the system state
+
+echo -n "TEST: system state modification ... "
+dmesg -C
+
+load_lp $MOD_LIVEPATCH
+disable_lp $MOD_LIVEPATCH
+unload_lp $MOD_LIVEPATCH
+
+check_result "% modprobe test_klp_state
+livepatch: enabling patch 'test_klp_state'
+livepatch: 'test_klp_state': initializing patching transition
+test_klp_state: pre_patch_callback: vmlinux
+test_klp_state: allocate_loglevel_state: allocating space to store console_loglevel
+livepatch: 'test_klp_state': starting patching transition
+livepatch: 'test_klp_state': completing patching transition
+test_klp_state: post_patch_callback: vmlinux
+test_klp_state: fix_console_loglevel: fixing console_loglevel
+livepatch: 'test_klp_state': patching complete
+% echo 0 > /sys/kernel/livepatch/test_klp_state/enabled
+livepatch: 'test_klp_state': initializing unpatching transition
+test_klp_state: pre_unpatch_callback: vmlinux
+test_klp_state: restore_console_loglevel: restoring console_loglevel
+livepatch: 'test_klp_state': starting unpatching transition
+livepatch: 'test_klp_state': completing unpatching transition
+test_klp_state: post_unpatch_callback: vmlinux
+test_klp_state: free_loglevel_state: freeing space for the stored console_loglevel
+livepatch: 'test_klp_state': unpatching complete
+% rmmod test_klp_state"
+
+
+# TEST: Take over system state change by a cumulative patch
+
+echo -n "TEST: taking over system state modification ... "
+dmesg -C
+
+load_lp $MOD_LIVEPATCH
+load_lp $MOD_LIVEPATCH2
+unload_lp $MOD_LIVEPATCH
+disable_lp $MOD_LIVEPATCH2
+unload_lp $MOD_LIVEPATCH2
+
+check_result "% modprobe test_klp_state
+livepatch: enabling patch 'test_klp_state'
+livepatch: 'test_klp_state': initializing patching transition
+test_klp_state: pre_patch_callback: vmlinux
+test_klp_state: allocate_loglevel_state: allocating space to store console_loglevel
+livepatch: 'test_klp_state': starting patching transition
+livepatch: 'test_klp_state': completing patching transition
+test_klp_state: post_patch_callback: vmlinux
+test_klp_state: fix_console_loglevel: fixing console_loglevel
+livepatch: 'test_klp_state': patching complete
+% modprobe test_klp_state2
+livepatch: enabling patch 'test_klp_state2'
+livepatch: 'test_klp_state2': initializing patching transition
+test_klp_state2: pre_patch_callback: vmlinux
+test_klp_state2: allocate_loglevel_state: space to store console_loglevel already allocated
+livepatch: 'test_klp_state2': starting patching transition
+livepatch: 'test_klp_state2': completing patching transition
+test_klp_state2: post_patch_callback: vmlinux
+test_klp_state2: fix_console_loglevel: taking over the console_loglevel change
+livepatch: 'test_klp_state2': patching complete
+% rmmod test_klp_state
+% echo 0 > /sys/kernel/livepatch/test_klp_state2/enabled
+livepatch: 'test_klp_state2': initializing unpatching transition
+test_klp_state2: pre_unpatch_callback: vmlinux
+test_klp_state2: restore_console_loglevel: restoring console_loglevel
+livepatch: 'test_klp_state2': starting unpatching transition
+livepatch: 'test_klp_state2': completing unpatching transition
+test_klp_state2: post_unpatch_callback: vmlinux
+test_klp_state2: free_loglevel_state: freeing space for the stored console_loglevel
+livepatch: 'test_klp_state2': unpatching complete
+% rmmod test_klp_state2"
+
+
+# TEST: Take over system state change by a cumulative patch
+
+echo -n "TEST: compatible cumulative livepatches ... "
+dmesg -C
+
+load_lp $MOD_LIVEPATCH2
+load_lp $MOD_LIVEPATCH3
+unload_lp $MOD_LIVEPATCH2
+load_lp $MOD_LIVEPATCH2
+disable_lp $MOD_LIVEPATCH2
+unload_lp $MOD_LIVEPATCH2
+unload_lp $MOD_LIVEPATCH3
+
+check_result "% modprobe test_klp_state2
+livepatch: enabling patch 'test_klp_state2'
+livepatch: 'test_klp_state2': initializing patching transition
+test_klp_state2: pre_patch_callback: vmlinux
+test_klp_state2: allocate_loglevel_state: allocating space to store console_loglevel
+livepatch: 'test_klp_state2': starting patching transition
+livepatch: 'test_klp_state2': completing patching transition
+test_klp_state2: post_patch_callback: vmlinux
+test_klp_state2: fix_console_loglevel: fixing console_loglevel
+livepatch: 'test_klp_state2': patching complete
+% modprobe test_klp_state3
+livepatch: enabling patch 'test_klp_state3'
+livepatch: 'test_klp_state3': initializing patching transition
+test_klp_state3: pre_patch_callback: vmlinux
+test_klp_state3: allocate_loglevel_state: space to store console_loglevel already allocated
+livepatch: 'test_klp_state3': starting patching transition
+livepatch: 'test_klp_state3': completing patching transition
+test_klp_state3: post_patch_callback: vmlinux
+test_klp_state3: fix_console_loglevel: taking over the console_loglevel change
+livepatch: 'test_klp_state3': patching complete
+% rmmod test_klp_state2
+% modprobe test_klp_state2
+livepatch: enabling patch 'test_klp_state2'
+livepatch: 'test_klp_state2': initializing patching transition
+test_klp_state2: pre_patch_callback: vmlinux
+test_klp_state2: allocate_loglevel_state: space to store console_loglevel already allocated
+livepatch: 'test_klp_state2': starting patching transition
+livepatch: 'test_klp_state2': completing patching transition
+test_klp_state2: post_patch_callback: vmlinux
+test_klp_state2: fix_console_loglevel: taking over the console_loglevel change
+livepatch: 'test_klp_state2': patching complete
+% echo 0 > /sys/kernel/livepatch/test_klp_state2/enabled
+livepatch: 'test_klp_state2': initializing unpatching transition
+test_klp_state2: pre_unpatch_callback: vmlinux
+test_klp_state2: restore_console_loglevel: restoring console_loglevel
+livepatch: 'test_klp_state2': starting unpatching transition
+livepatch: 'test_klp_state2': completing unpatching transition
+test_klp_state2: post_unpatch_callback: vmlinux
+test_klp_state2: free_loglevel_state: freeing space for the stored console_loglevel
+livepatch: 'test_klp_state2': unpatching complete
+% rmmod test_klp_state2
+% rmmod test_klp_state3"
+
+
+# TEST: Failure caused by incompatible cumulative livepatches
+
+echo -n "TEST: incompatible cumulative livepatches ... "
+dmesg -C
+
+load_lp $MOD_LIVEPATCH2
+load_failing_mod $MOD_LIVEPATCH
+disable_lp $MOD_LIVEPATCH2
+unload_lp $MOD_LIVEPATCH2
+
+check_result "% modprobe test_klp_state2
+livepatch: enabling patch 'test_klp_state2'
+livepatch: 'test_klp_state2': initializing patching transition
+test_klp_state2: pre_patch_callback: vmlinux
+test_klp_state2: allocate_loglevel_state: allocating space to store console_loglevel
+livepatch: 'test_klp_state2': starting patching transition
+livepatch: 'test_klp_state2': completing patching transition
+test_klp_state2: post_patch_callback: vmlinux
+test_klp_state2: fix_console_loglevel: fixing console_loglevel
+livepatch: 'test_klp_state2': patching complete
+% modprobe test_klp_state
+livepatch: Livepatch patch (test_klp_state) is not compatible with the already installed livepatches.
+modprobe: ERROR: could not insert 'test_klp_state': Invalid argument
+% echo 0 > /sys/kernel/livepatch/test_klp_state2/enabled
+livepatch: 'test_klp_state2': initializing unpatching transition
+test_klp_state2: pre_unpatch_callback: vmlinux
+test_klp_state2: restore_console_loglevel: restoring console_loglevel
+livepatch: 'test_klp_state2': starting unpatching transition
+livepatch: 'test_klp_state2': completing unpatching transition
+test_klp_state2: post_unpatch_callback: vmlinux
+test_klp_state2: free_loglevel_state: freeing space for the stored console_loglevel
+livepatch: 'test_klp_state2': unpatching complete
+% rmmod test_klp_state2"
+
+exit 0
--
2.16.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 0/5] livepatch: new API to track system state changes
2019-10-03 9:01 [PATCH v3 0/5] livepatch: new API to track system state changes Petr Mladek
` (4 preceding siblings ...)
2019-10-03 9:01 ` [PATCH v3 5/5] livepatch: Selftests of the " Petr Mladek
@ 2019-10-04 14:39 ` Joe Lawrence
5 siblings, 0 replies; 13+ messages in thread
From: Joe Lawrence @ 2019-10-04 14:39 UTC (permalink / raw)
To: Petr Mladek
Cc: Jiri Kosina, Josh Poimboeuf, Miroslav Benes, Kamalesh Babulal,
Nicolai Stange, live-patching, linux-kernel
On Thu, Oct 03, 2019 at 11:01:32AM +0200, Petr Mladek wrote:
> Hi,
>
> this is another piece in the puzzle that helps to maintain more
> livepatches.
>
> Especially pre/post (un)patch callbacks might change a system state.
> Any newly installed livepatch has to somehow deal with system state
> modifications done be already installed livepatches.
>
> This patchset provides a simple and generic API that
> helps to keep and pass information between the livepatches.
> It is also usable to prevent loading incompatible livepatches.
>
> Changes since v2:
>
> + Typo fixes [Miroslav]
> + Move the documentation at the end of the list [Miroslav]
> + Add Miroslav's acks
>
> Changes since v1:
>
> + Use "unsigned long" instead of "int" for "state.id" [Nicolai]
> + Use "unsigned int" instead of "int" for "state.version [Petr]
> + Include "state.h" to avoid warning about non-static func [Miroslav]
> + Simplify logic in klp_is_state_compatible() [Miroslav]
> + Document how livepatches should handle the state [Nicolai]
> + Fix some typos, formulation, module metadata [Joe, Miroslav]
>
> Petr Mladek (5):
> livepatch: Keep replaced patches until post_patch callback is called
> livepatch: Basic API to track system state changes
> livepatch: Allow to distinguish different version of system state
> changes
> livepatch: Documentation of the new API for tracking system state
> changes
> livepatch: Selftests of the API for tracking system state changes
>
> Documentation/livepatch/index.rst | 1 +
> Documentation/livepatch/system-state.rst | 167 +++++++++++++++++++++
> include/linux/livepatch.h | 17 +++
> kernel/livepatch/Makefile | 2 +-
> kernel/livepatch/core.c | 44 ++++--
> kernel/livepatch/core.h | 5 +-
> kernel/livepatch/state.c | 122 +++++++++++++++
> kernel/livepatch/state.h | 9 ++
> kernel/livepatch/transition.c | 12 +-
> lib/livepatch/Makefile | 5 +-
> lib/livepatch/test_klp_state.c | 161 ++++++++++++++++++++
> lib/livepatch/test_klp_state2.c | 190 ++++++++++++++++++++++++
> lib/livepatch/test_klp_state3.c | 5 +
> tools/testing/selftests/livepatch/Makefile | 3 +-
> tools/testing/selftests/livepatch/test-state.sh | 180 ++++++++++++++++++++++
> 15 files changed, 902 insertions(+), 21 deletions(-)
> create mode 100644 Documentation/livepatch/system-state.rst
> create mode 100644 kernel/livepatch/state.c
> create mode 100644 kernel/livepatch/state.h
> create mode 100644 lib/livepatch/test_klp_state.c
> create mode 100644 lib/livepatch/test_klp_state2.c
> create mode 100644 lib/livepatch/test_klp_state3.c
> create mode 100755 tools/testing/selftests/livepatch/test-state.sh
>
> --
> 2.16.4
>
Hi Petr,
Thanks for respinning this one with the latest updates. The
implementation looks fine to me. I have two really minor nits for the
selftest (I'll reply to that commit), but I wouldn't hold up the series
for them.
Acked-by: Joe Lawrence <joe.lawrence@redhat.com>
-- Joe
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 5/5] livepatch: Selftests of the API for tracking system state changes
2019-10-03 9:01 ` [PATCH v3 5/5] livepatch: Selftests of the " Petr Mladek
@ 2019-10-04 14:47 ` Joe Lawrence
2019-10-09 14:18 ` Petr Mladek
2019-10-08 22:19 ` kbuild test robot
1 sibling, 1 reply; 13+ messages in thread
From: Joe Lawrence @ 2019-10-04 14:47 UTC (permalink / raw)
To: Petr Mladek
Cc: Jiri Kosina, Josh Poimboeuf, Miroslav Benes, Kamalesh Babulal,
Nicolai Stange, live-patching, linux-kernel
On Thu, Oct 03, 2019 at 11:01:37AM +0200, Petr Mladek wrote:
> Four selftests for the new API.
>
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> Acked-by: Miroslav Benes <mbenes@suse.cz>
> ---
> lib/livepatch/Makefile | 5 +-
> lib/livepatch/test_klp_state.c | 161 ++++++++++++++++++++
> lib/livepatch/test_klp_state2.c | 190 ++++++++++++++++++++++++
> lib/livepatch/test_klp_state3.c | 5 +
> tools/testing/selftests/livepatch/Makefile | 3 +-
> tools/testing/selftests/livepatch/test-state.sh | 180 ++++++++++++++++++++++
> 6 files changed, 542 insertions(+), 2 deletions(-)
> create mode 100644 lib/livepatch/test_klp_state.c
> create mode 100644 lib/livepatch/test_klp_state2.c
> create mode 100644 lib/livepatch/test_klp_state3.c
> create mode 100755 tools/testing/selftests/livepatch/test-state.sh
>
> [ ... snip ... ]
> diff --git a/tools/testing/selftests/livepatch/test-state.sh b/tools/testing/selftests/livepatch/test-state.sh
> new file mode 100755
> index 000000000000..1139c664c11c
> --- /dev/null
> +++ b/tools/testing/selftests/livepatch/test-state.sh
> @@ -0,0 +1,180 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2018 Joe Lawrence <joe.lawrence@redhat.com>
> +
nit: this should probably read as:
# Copyright (C) 2019 Petr Mladek <pmladek@suse.com>
> +. $(dirname $0)/functions.sh
> +
> +MOD_LIVEPATCH=test_klp_state
> +MOD_LIVEPATCH2=test_klp_state2
> +MOD_LIVEPATCH3=test_klp_state3
> +
> +set_dynamic_debug
> +
> +
> +# TEST: Loading and removing a module that modifies the system state
> +
> +echo -n "TEST: system state modification ... "
> +dmesg -C
> +
> +load_lp $MOD_LIVEPATCH
> +disable_lp $MOD_LIVEPATCH
> +unload_lp $MOD_LIVEPATCH
> +
> +check_result "% modprobe test_klp_state
> +livepatch: enabling patch 'test_klp_state'
> +livepatch: 'test_klp_state': initializing patching transition
> +test_klp_state: pre_patch_callback: vmlinux
> +test_klp_state: allocate_loglevel_state: allocating space to store console_loglevel
> +livepatch: 'test_klp_state': starting patching transition
> +livepatch: 'test_klp_state': completing patching transition
> +test_klp_state: post_patch_callback: vmlinux
> +test_klp_state: fix_console_loglevel: fixing console_loglevel
> +livepatch: 'test_klp_state': patching complete
> +% echo 0 > /sys/kernel/livepatch/test_klp_state/enabled
> +livepatch: 'test_klp_state': initializing unpatching transition
> +test_klp_state: pre_unpatch_callback: vmlinux
> +test_klp_state: restore_console_loglevel: restoring console_loglevel
> +livepatch: 'test_klp_state': starting unpatching transition
> +livepatch: 'test_klp_state': completing unpatching transition
> +test_klp_state: post_unpatch_callback: vmlinux
> +test_klp_state: free_loglevel_state: freeing space for the stored console_loglevel
> +livepatch: 'test_klp_state': unpatching complete
> +% rmmod test_klp_state"
> +
nit: a matter of style, and I don't mind either, but the other test
scripts used $MOD_LIVEPATCH{2,3} variable replacement in the
check_result string. I think I originally did that when we were
reviewing the first self-test patchset and the filenames may or may not
have changed. Those names are stable now, so I don't have a strong
opinion either way.
FWIW, if someone wants to make the copyright change on merge, I'm cool
with this version.
-- Joe
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 5/5] livepatch: Selftests of the API for tracking system state changes
2019-10-03 9:01 ` [PATCH v3 5/5] livepatch: Selftests of the " Petr Mladek
2019-10-04 14:47 ` Joe Lawrence
@ 2019-10-08 22:19 ` kbuild test robot
1 sibling, 0 replies; 13+ messages in thread
From: kbuild test robot @ 2019-10-08 22:19 UTC (permalink / raw)
To: Petr Mladek
Cc: kbuild-all, Jiri Kosina, Josh Poimboeuf, Miroslav Benes,
Joe Lawrence, Kamalesh Babulal, Nicolai Stange, live-patching,
linux-kernel, Petr Mladek
[-- Attachment #1: Type: text/plain, Size: 4612 bytes --]
Hi Petr,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to v5.4-rc2 next-20191008]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Petr-Mladek/livepatch-Keep-replaced-patches-until-post_patch-callback-is-called/20191003-171833
config: x86_64-randconfig-e004-201940 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-13) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
lib/livepatch/test_klp_state.c: In function 'allocate_loglevel_state':
>> lib/livepatch/test_klp_state.c:41:25: error: implicit declaration of function 'kzalloc'; did you mean 'kvzalloc'? [-Werror=implicit-function-declaration]
loglevel_state->data = kzalloc(sizeof(console_loglevel), GFP_KERNEL);
^~~~~~~
kvzalloc
>> lib/livepatch/test_klp_state.c:41:23: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
loglevel_state->data = kzalloc(sizeof(console_loglevel), GFP_KERNEL);
^
lib/livepatch/test_klp_state.c: In function 'free_loglevel_state':
>> lib/livepatch/test_klp_state.c:85:2: error: implicit declaration of function 'kfree'; did you mean 'kvfree'? [-Werror=implicit-function-declaration]
kfree(loglevel_state->data);
^~~~~
kvfree
cc1: some warnings being treated as errors
--
lib/livepatch/test_klp_state2.c: In function 'allocate_loglevel_state':
>> lib/livepatch/test_klp_state2.c:48:25: error: implicit declaration of function 'kzalloc'; did you mean 'kvzalloc'? [-Werror=implicit-function-declaration]
loglevel_state->data = kzalloc(sizeof(console_loglevel), GFP_KERNEL);
^~~~~~~
kvzalloc
>> lib/livepatch/test_klp_state2.c:48:23: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
loglevel_state->data = kzalloc(sizeof(console_loglevel), GFP_KERNEL);
^
lib/livepatch/test_klp_state2.c: In function 'free_loglevel_state':
>> lib/livepatch/test_klp_state2.c:114:2: error: implicit declaration of function 'kfree'; did you mean 'kvfree'? [-Werror=implicit-function-declaration]
kfree(loglevel_state->data);
^~~~~
kvfree
cc1: some warnings being treated as errors
vim +41 lib/livepatch/test_klp_state.c
32
33 static int allocate_loglevel_state(void)
34 {
35 struct klp_state *loglevel_state;
36
37 loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
38 if (!loglevel_state)
39 return -EINVAL;
40
> 41 loglevel_state->data = kzalloc(sizeof(console_loglevel), GFP_KERNEL);
42 if (!loglevel_state->data)
43 return -ENOMEM;
44
45 pr_info("%s: allocating space to store console_loglevel\n",
46 __func__);
47 return 0;
48 }
49
50 static void fix_console_loglevel(void)
51 {
52 struct klp_state *loglevel_state;
53
54 loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
55 if (!loglevel_state)
56 return;
57
58 pr_info("%s: fixing console_loglevel\n", __func__);
59 *(int *)loglevel_state->data = console_loglevel;
60 console_loglevel = CONSOLE_LOGLEVEL_MOTORMOUTH;
61 }
62
63 static void restore_console_loglevel(void)
64 {
65 struct klp_state *loglevel_state;
66
67 loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
68 if (!loglevel_state)
69 return;
70
71 pr_info("%s: restoring console_loglevel\n", __func__);
72 console_loglevel = *(int *)loglevel_state->data;
73 }
74
75 static void free_loglevel_state(void)
76 {
77 struct klp_state *loglevel_state;
78
79 loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
80 if (!loglevel_state)
81 return;
82
83 pr_info("%s: freeing space for the stored console_loglevel\n",
84 __func__);
> 85 kfree(loglevel_state->data);
86 }
87
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28477 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 5/5] livepatch: Selftests of the API for tracking system state changes
2019-10-04 14:47 ` Joe Lawrence
@ 2019-10-09 14:18 ` Petr Mladek
2019-10-09 14:27 ` Joe Lawrence
0 siblings, 1 reply; 13+ messages in thread
From: Petr Mladek @ 2019-10-09 14:18 UTC (permalink / raw)
To: Joe Lawrence
Cc: Jiri Kosina, Kamalesh Babulal, Josh Poimboeuf, Miroslav Benes,
Nicolai Stange, linux-kernel, live-patching
On Fri 2019-10-04 10:47:42, Joe Lawrence wrote:
> On Thu, Oct 03, 2019 at 11:01:37AM +0200, Petr Mladek wrote:
> > Four selftests for the new API.
> >
> > --- /dev/null
> > +++ b/tools/testing/selftests/livepatch/test-state.sh
> > @@ -0,0 +1,180 @@
> > +#!/bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (C) 2018 Joe Lawrence <joe.lawrence@redhat.com>
> > +
>
> nit: this should probably read as:
> # Copyright (C) 2019 Petr Mladek <pmladek@suse.com>
>
> > +
> > +load_lp $MOD_LIVEPATCH
> > +disable_lp $MOD_LIVEPATCH
> > +unload_lp $MOD_LIVEPATCH
> > +
> > +check_result "% modprobe test_klp_state
> > +livepatch: enabling patch 'test_klp_state'
> > +livepatch: 'test_klp_state': initializing patching transition
> > +test_klp_state: pre_patch_callback: vmlinux
> > +test_klp_state: allocate_loglevel_state: allocating space to store console_loglevel
> > +livepatch: 'test_klp_state': starting patching transition
> > +livepatch: 'test_klp_state': completing patching transition
> > +test_klp_state: post_patch_callback: vmlinux
> > +test_klp_state: fix_console_loglevel: fixing console_loglevel
> > +livepatch: 'test_klp_state': patching complete
> > +% echo 0 > /sys/kernel/livepatch/test_klp_state/enabled
> > +livepatch: 'test_klp_state': initializing unpatching transition
> > +test_klp_state: pre_unpatch_callback: vmlinux
> > +test_klp_state: restore_console_loglevel: restoring console_loglevel
> > +livepatch: 'test_klp_state': starting unpatching transition
> > +livepatch: 'test_klp_state': completing unpatching transition
> > +test_klp_state: post_unpatch_callback: vmlinux
> > +test_klp_state: free_loglevel_state: freeing space for the stored console_loglevel
> > +livepatch: 'test_klp_state': unpatching complete
> > +% rmmod test_klp_state"
> > +
>
> nit: a matter of style, and I don't mind either, but the other test
> scripts used $MOD_LIVEPATCH{2,3} variable replacement in the
> check_result string. I think I originally did that when we were
> reviewing the first self-test patchset and the filenames may or may not
> have changed. Those names are stable now, so I don't have a strong
> opinion either way.
Please, find below and updated version of the patch that fixes
the two above problems and also the kbuild robot failure.
The build failure was fixed by #include <linux/slab.h>
Here we go:
From 206e0a53de25ddb042e5947c2a736e33ca4a2b4c Mon Sep 17 00:00:00 2001
From: Petr Mladek <pmladek@suse.com>
Date: Mon, 10 Jun 2019 14:02:42 +0200
Subject: [PATCH v3.1 5/5] livepatch: Selftests of the API for tracking system state
changes
Four selftests for the new API.
Signed-off-by: Petr Mladek <pmladek@suse.com>
Acked-by: Miroslav Benes <mbenes@suse.cz>
---
lib/livepatch/Makefile | 5 +-
lib/livepatch/test_klp_state.c | 162 ++++++++++++++++++++
lib/livepatch/test_klp_state2.c | 191 ++++++++++++++++++++++++
lib/livepatch/test_klp_state3.c | 5 +
tools/testing/selftests/livepatch/Makefile | 3 +-
tools/testing/selftests/livepatch/test-state.sh | 180 ++++++++++++++++++++++
6 files changed, 544 insertions(+), 2 deletions(-)
create mode 100644 lib/livepatch/test_klp_state.c
create mode 100644 lib/livepatch/test_klp_state2.c
create mode 100644 lib/livepatch/test_klp_state3.c
create mode 100755 tools/testing/selftests/livepatch/test-state.sh
diff --git a/lib/livepatch/Makefile b/lib/livepatch/Makefile
index 26900ddaef82..295b94bff370 100644
--- a/lib/livepatch/Makefile
+++ b/lib/livepatch/Makefile
@@ -8,7 +8,10 @@ obj-$(CONFIG_TEST_LIVEPATCH) += test_klp_atomic_replace.o \
test_klp_callbacks_busy.o \
test_klp_callbacks_mod.o \
test_klp_livepatch.o \
- test_klp_shadow_vars.o
+ test_klp_shadow_vars.o \
+ test_klp_state.o \
+ test_klp_state2.o \
+ test_klp_state3.o
# Target modules to be livepatched require CC_FLAGS_FTRACE
CFLAGS_test_klp_callbacks_busy.o += $(CC_FLAGS_FTRACE)
diff --git a/lib/livepatch/test_klp_state.c b/lib/livepatch/test_klp_state.c
new file mode 100644
index 000000000000..57a4253acb01
--- /dev/null
+++ b/lib/livepatch/test_klp_state.c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2019 SUSE
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/printk.h>
+#include <linux/livepatch.h>
+
+#define CONSOLE_LOGLEVEL_STATE 1
+/* Version 1 does not support migration. */
+#define CONSOLE_LOGLEVEL_STATE_VERSION 1
+
+static const char *const module_state[] = {
+ [MODULE_STATE_LIVE] = "[MODULE_STATE_LIVE] Normal state",
+ [MODULE_STATE_COMING] = "[MODULE_STATE_COMING] Full formed, running module_init",
+ [MODULE_STATE_GOING] = "[MODULE_STATE_GOING] Going away",
+ [MODULE_STATE_UNFORMED] = "[MODULE_STATE_UNFORMED] Still setting it up",
+};
+
+static void callback_info(const char *callback, struct klp_object *obj)
+{
+ if (obj->mod)
+ pr_info("%s: %s -> %s\n", callback, obj->mod->name,
+ module_state[obj->mod->state]);
+ else
+ pr_info("%s: vmlinux\n", callback);
+}
+
+static struct klp_patch patch;
+
+static int allocate_loglevel_state(void)
+{
+ struct klp_state *loglevel_state;
+
+ loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
+ if (!loglevel_state)
+ return -EINVAL;
+
+ loglevel_state->data = kzalloc(sizeof(console_loglevel), GFP_KERNEL);
+ if (!loglevel_state->data)
+ return -ENOMEM;
+
+ pr_info("%s: allocating space to store console_loglevel\n",
+ __func__);
+ return 0;
+}
+
+static void fix_console_loglevel(void)
+{
+ struct klp_state *loglevel_state;
+
+ loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
+ if (!loglevel_state)
+ return;
+
+ pr_info("%s: fixing console_loglevel\n", __func__);
+ *(int *)loglevel_state->data = console_loglevel;
+ console_loglevel = CONSOLE_LOGLEVEL_MOTORMOUTH;
+}
+
+static void restore_console_loglevel(void)
+{
+ struct klp_state *loglevel_state;
+
+ loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
+ if (!loglevel_state)
+ return;
+
+ pr_info("%s: restoring console_loglevel\n", __func__);
+ console_loglevel = *(int *)loglevel_state->data;
+}
+
+static void free_loglevel_state(void)
+{
+ struct klp_state *loglevel_state;
+
+ loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
+ if (!loglevel_state)
+ return;
+
+ pr_info("%s: freeing space for the stored console_loglevel\n",
+ __func__);
+ kfree(loglevel_state->data);
+}
+
+/* Executed on object patching (ie, patch enablement) */
+static int pre_patch_callback(struct klp_object *obj)
+{
+ callback_info(__func__, obj);
+ return allocate_loglevel_state();
+}
+
+/* Executed on object unpatching (ie, patch disablement) */
+static void post_patch_callback(struct klp_object *obj)
+{
+ callback_info(__func__, obj);
+ fix_console_loglevel();
+}
+
+/* Executed on object unpatching (ie, patch disablement) */
+static void pre_unpatch_callback(struct klp_object *obj)
+{
+ callback_info(__func__, obj);
+ restore_console_loglevel();
+}
+
+/* Executed on object unpatching (ie, patch disablement) */
+static void post_unpatch_callback(struct klp_object *obj)
+{
+ callback_info(__func__, obj);
+ free_loglevel_state();
+}
+
+static struct klp_func no_funcs[] = {
+ {}
+};
+
+static struct klp_object objs[] = {
+ {
+ .name = NULL, /* vmlinux */
+ .funcs = no_funcs,
+ .callbacks = {
+ .pre_patch = pre_patch_callback,
+ .post_patch = post_patch_callback,
+ .pre_unpatch = pre_unpatch_callback,
+ .post_unpatch = post_unpatch_callback,
+ },
+ }, { }
+};
+
+static struct klp_state states[] = {
+ {
+ .id = CONSOLE_LOGLEVEL_STATE,
+ .version = CONSOLE_LOGLEVEL_STATE_VERSION,
+ }, { }
+};
+
+static struct klp_patch patch = {
+ .mod = THIS_MODULE,
+ .objs = objs,
+ .states = states,
+ .replace = true,
+};
+
+static int test_klp_callbacks_demo_init(void)
+{
+ return klp_enable_patch(&patch);
+}
+
+static void test_klp_callbacks_demo_exit(void)
+{
+}
+
+module_init(test_klp_callbacks_demo_init);
+module_exit(test_klp_callbacks_demo_exit);
+MODULE_LICENSE("GPL");
+MODULE_INFO(livepatch, "Y");
+MODULE_AUTHOR("Petr Mladek <pmladek@suse.com>");
+MODULE_DESCRIPTION("Livepatch test: system state modification");
diff --git a/lib/livepatch/test_klp_state2.c b/lib/livepatch/test_klp_state2.c
new file mode 100644
index 000000000000..c978ea4d5e67
--- /dev/null
+++ b/lib/livepatch/test_klp_state2.c
@@ -0,0 +1,191 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2019 SUSE
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/printk.h>
+#include <linux/livepatch.h>
+
+#define CONSOLE_LOGLEVEL_STATE 1
+/* Version 2 supports migration. */
+#define CONSOLE_LOGLEVEL_STATE_VERSION 2
+
+static const char *const module_state[] = {
+ [MODULE_STATE_LIVE] = "[MODULE_STATE_LIVE] Normal state",
+ [MODULE_STATE_COMING] = "[MODULE_STATE_COMING] Full formed, running module_init",
+ [MODULE_STATE_GOING] = "[MODULE_STATE_GOING] Going away",
+ [MODULE_STATE_UNFORMED] = "[MODULE_STATE_UNFORMED] Still setting it up",
+};
+
+static void callback_info(const char *callback, struct klp_object *obj)
+{
+ if (obj->mod)
+ pr_info("%s: %s -> %s\n", callback, obj->mod->name,
+ module_state[obj->mod->state]);
+ else
+ pr_info("%s: vmlinux\n", callback);
+}
+
+static struct klp_patch patch;
+
+static int allocate_loglevel_state(void)
+{
+ struct klp_state *loglevel_state, *prev_loglevel_state;
+
+ prev_loglevel_state = klp_get_prev_state(CONSOLE_LOGLEVEL_STATE);
+ if (prev_loglevel_state) {
+ pr_info("%s: space to store console_loglevel already allocated\n",
+ __func__);
+ return 0;
+ }
+
+ loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
+ if (!loglevel_state)
+ return -EINVAL;
+
+ loglevel_state->data = kzalloc(sizeof(console_loglevel), GFP_KERNEL);
+ if (!loglevel_state->data)
+ return -ENOMEM;
+
+ pr_info("%s: allocating space to store console_loglevel\n",
+ __func__);
+ return 0;
+}
+
+static void fix_console_loglevel(void)
+{
+ struct klp_state *loglevel_state, *prev_loglevel_state;
+
+ loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
+ if (!loglevel_state)
+ return;
+
+ prev_loglevel_state = klp_get_prev_state(CONSOLE_LOGLEVEL_STATE);
+ if (prev_loglevel_state) {
+ pr_info("%s: taking over the console_loglevel change\n",
+ __func__);
+ loglevel_state->data = prev_loglevel_state->data;
+ return;
+ }
+
+ pr_info("%s: fixing console_loglevel\n", __func__);
+ *(int *)loglevel_state->data = console_loglevel;
+ console_loglevel = CONSOLE_LOGLEVEL_MOTORMOUTH;
+}
+
+static void restore_console_loglevel(void)
+{
+ struct klp_state *loglevel_state, *prev_loglevel_state;
+
+ prev_loglevel_state = klp_get_prev_state(CONSOLE_LOGLEVEL_STATE);
+ if (prev_loglevel_state) {
+ pr_info("%s: passing the console_loglevel change back to the old livepatch\n",
+ __func__);
+ return;
+ }
+
+ loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
+ if (!loglevel_state)
+ return;
+
+ pr_info("%s: restoring console_loglevel\n", __func__);
+ console_loglevel = *(int *)loglevel_state->data;
+}
+
+static void free_loglevel_state(void)
+{
+ struct klp_state *loglevel_state, *prev_loglevel_state;
+
+ prev_loglevel_state = klp_get_prev_state(CONSOLE_LOGLEVEL_STATE);
+ if (prev_loglevel_state) {
+ pr_info("%s: keeping space to store console_loglevel\n",
+ __func__);
+ return;
+ }
+
+ loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
+ if (!loglevel_state)
+ return;
+
+ pr_info("%s: freeing space for the stored console_loglevel\n",
+ __func__);
+ kfree(loglevel_state->data);
+}
+
+/* Executed on object patching (ie, patch enablement) */
+static int pre_patch_callback(struct klp_object *obj)
+{
+ callback_info(__func__, obj);
+ return allocate_loglevel_state();
+}
+
+/* Executed on object unpatching (ie, patch disablement) */
+static void post_patch_callback(struct klp_object *obj)
+{
+ callback_info(__func__, obj);
+ fix_console_loglevel();
+}
+
+/* Executed on object unpatching (ie, patch disablement) */
+static void pre_unpatch_callback(struct klp_object *obj)
+{
+ callback_info(__func__, obj);
+ restore_console_loglevel();
+}
+
+/* Executed on object unpatching (ie, patch disablement) */
+static void post_unpatch_callback(struct klp_object *obj)
+{
+ callback_info(__func__, obj);
+ free_loglevel_state();
+}
+
+static struct klp_func no_funcs[] = {
+ {}
+};
+
+static struct klp_object objs[] = {
+ {
+ .name = NULL, /* vmlinux */
+ .funcs = no_funcs,
+ .callbacks = {
+ .pre_patch = pre_patch_callback,
+ .post_patch = post_patch_callback,
+ .pre_unpatch = pre_unpatch_callback,
+ .post_unpatch = post_unpatch_callback,
+ },
+ }, { }
+};
+
+static struct klp_state states[] = {
+ {
+ .id = CONSOLE_LOGLEVEL_STATE,
+ .version = CONSOLE_LOGLEVEL_STATE_VERSION,
+ }, { }
+};
+
+static struct klp_patch patch = {
+ .mod = THIS_MODULE,
+ .objs = objs,
+ .states = states,
+ .replace = true,
+};
+
+static int test_klp_callbacks_demo_init(void)
+{
+ return klp_enable_patch(&patch);
+}
+
+static void test_klp_callbacks_demo_exit(void)
+{
+}
+
+module_init(test_klp_callbacks_demo_init);
+module_exit(test_klp_callbacks_demo_exit);
+MODULE_LICENSE("GPL");
+MODULE_INFO(livepatch, "Y");
+MODULE_AUTHOR("Petr Mladek <pmladek@suse.com>");
+MODULE_DESCRIPTION("Livepatch test: system state modification");
diff --git a/lib/livepatch/test_klp_state3.c b/lib/livepatch/test_klp_state3.c
new file mode 100644
index 000000000000..9226579d10c5
--- /dev/null
+++ b/lib/livepatch/test_klp_state3.c
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2019 SUSE
+
+/* The console loglevel fix is the same in the next cumulative patch. */
+#include "test_klp_state2.c"
diff --git a/tools/testing/selftests/livepatch/Makefile b/tools/testing/selftests/livepatch/Makefile
index fd405402c3ff..1cf40a9e7185 100644
--- a/tools/testing/selftests/livepatch/Makefile
+++ b/tools/testing/selftests/livepatch/Makefile
@@ -4,6 +4,7 @@ TEST_PROGS_EXTENDED := functions.sh
TEST_PROGS := \
test-livepatch.sh \
test-callbacks.sh \
- test-shadow-vars.sh
+ test-shadow-vars.sh \
+ test-state.sh
include ../lib.mk
diff --git a/tools/testing/selftests/livepatch/test-state.sh b/tools/testing/selftests/livepatch/test-state.sh
new file mode 100755
index 000000000000..dc2908c22c26
--- /dev/null
+++ b/tools/testing/selftests/livepatch/test-state.sh
@@ -0,0 +1,180 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2019 SUSE
+
+. $(dirname $0)/functions.sh
+
+MOD_LIVEPATCH=test_klp_state
+MOD_LIVEPATCH2=test_klp_state2
+MOD_LIVEPATCH3=test_klp_state3
+
+set_dynamic_debug
+
+
+# TEST: Loading and removing a module that modifies the system state
+
+echo -n "TEST: system state modification ... "
+dmesg -C
+
+load_lp $MOD_LIVEPATCH
+disable_lp $MOD_LIVEPATCH
+unload_lp $MOD_LIVEPATCH
+
+check_result "% modprobe $MOD_LIVEPATCH
+livepatch: enabling patch '$MOD_LIVEPATCH'
+livepatch: '$MOD_LIVEPATCH': initializing patching transition
+$MOD_LIVEPATCH: pre_patch_callback: vmlinux
+$MOD_LIVEPATCH: allocate_loglevel_state: allocating space to store console_loglevel
+livepatch: '$MOD_LIVEPATCH': starting patching transition
+livepatch: '$MOD_LIVEPATCH': completing patching transition
+$MOD_LIVEPATCH: post_patch_callback: vmlinux
+$MOD_LIVEPATCH: fix_console_loglevel: fixing console_loglevel
+livepatch: '$MOD_LIVEPATCH': patching complete
+% echo 0 > /sys/kernel/livepatch/$MOD_LIVEPATCH/enabled
+livepatch: '$MOD_LIVEPATCH': initializing unpatching transition
+$MOD_LIVEPATCH: pre_unpatch_callback: vmlinux
+$MOD_LIVEPATCH: restore_console_loglevel: restoring console_loglevel
+livepatch: '$MOD_LIVEPATCH': starting unpatching transition
+livepatch: '$MOD_LIVEPATCH': completing unpatching transition
+$MOD_LIVEPATCH: post_unpatch_callback: vmlinux
+$MOD_LIVEPATCH: free_loglevel_state: freeing space for the stored console_loglevel
+livepatch: '$MOD_LIVEPATCH': unpatching complete
+% rmmod $MOD_LIVEPATCH"
+
+
+# TEST: Take over system state change by a cumulative patch
+
+echo -n "TEST: taking over system state modification ... "
+dmesg -C
+
+load_lp $MOD_LIVEPATCH
+load_lp $MOD_LIVEPATCH2
+unload_lp $MOD_LIVEPATCH
+disable_lp $MOD_LIVEPATCH2
+unload_lp $MOD_LIVEPATCH2
+
+check_result "% modprobe $MOD_LIVEPATCH
+livepatch: enabling patch '$MOD_LIVEPATCH'
+livepatch: '$MOD_LIVEPATCH': initializing patching transition
+$MOD_LIVEPATCH: pre_patch_callback: vmlinux
+$MOD_LIVEPATCH: allocate_loglevel_state: allocating space to store console_loglevel
+livepatch: '$MOD_LIVEPATCH': starting patching transition
+livepatch: '$MOD_LIVEPATCH': completing patching transition
+$MOD_LIVEPATCH: post_patch_callback: vmlinux
+$MOD_LIVEPATCH: fix_console_loglevel: fixing console_loglevel
+livepatch: '$MOD_LIVEPATCH': patching complete
+% modprobe $MOD_LIVEPATCH2
+livepatch: enabling patch '$MOD_LIVEPATCH2'
+livepatch: '$MOD_LIVEPATCH2': initializing patching transition
+$MOD_LIVEPATCH2: pre_patch_callback: vmlinux
+$MOD_LIVEPATCH2: allocate_loglevel_state: space to store console_loglevel already allocated
+livepatch: '$MOD_LIVEPATCH2': starting patching transition
+livepatch: '$MOD_LIVEPATCH2': completing patching transition
+$MOD_LIVEPATCH2: post_patch_callback: vmlinux
+$MOD_LIVEPATCH2: fix_console_loglevel: taking over the console_loglevel change
+livepatch: '$MOD_LIVEPATCH2': patching complete
+% rmmod $MOD_LIVEPATCH
+% echo 0 > /sys/kernel/livepatch/$MOD_LIVEPATCH2/enabled
+livepatch: '$MOD_LIVEPATCH2': initializing unpatching transition
+$MOD_LIVEPATCH2: pre_unpatch_callback: vmlinux
+$MOD_LIVEPATCH2: restore_console_loglevel: restoring console_loglevel
+livepatch: '$MOD_LIVEPATCH2': starting unpatching transition
+livepatch: '$MOD_LIVEPATCH2': completing unpatching transition
+$MOD_LIVEPATCH2: post_unpatch_callback: vmlinux
+$MOD_LIVEPATCH2: free_loglevel_state: freeing space for the stored console_loglevel
+livepatch: '$MOD_LIVEPATCH2': unpatching complete
+% rmmod $MOD_LIVEPATCH2"
+
+
+# TEST: Take over system state change by a cumulative patch
+
+echo -n "TEST: compatible cumulative livepatches ... "
+dmesg -C
+
+load_lp $MOD_LIVEPATCH2
+load_lp $MOD_LIVEPATCH3
+unload_lp $MOD_LIVEPATCH2
+load_lp $MOD_LIVEPATCH2
+disable_lp $MOD_LIVEPATCH2
+unload_lp $MOD_LIVEPATCH2
+unload_lp $MOD_LIVEPATCH3
+
+check_result "% modprobe $MOD_LIVEPATCH2
+livepatch: enabling patch '$MOD_LIVEPATCH2'
+livepatch: '$MOD_LIVEPATCH2': initializing patching transition
+$MOD_LIVEPATCH2: pre_patch_callback: vmlinux
+$MOD_LIVEPATCH2: allocate_loglevel_state: allocating space to store console_loglevel
+livepatch: '$MOD_LIVEPATCH2': starting patching transition
+livepatch: '$MOD_LIVEPATCH2': completing patching transition
+$MOD_LIVEPATCH2: post_patch_callback: vmlinux
+$MOD_LIVEPATCH2: fix_console_loglevel: fixing console_loglevel
+livepatch: '$MOD_LIVEPATCH2': patching complete
+% modprobe $MOD_LIVEPATCH3
+livepatch: enabling patch '$MOD_LIVEPATCH3'
+livepatch: '$MOD_LIVEPATCH3': initializing patching transition
+$MOD_LIVEPATCH3: pre_patch_callback: vmlinux
+$MOD_LIVEPATCH3: allocate_loglevel_state: space to store console_loglevel already allocated
+livepatch: '$MOD_LIVEPATCH3': starting patching transition
+livepatch: '$MOD_LIVEPATCH3': completing patching transition
+$MOD_LIVEPATCH3: post_patch_callback: vmlinux
+$MOD_LIVEPATCH3: fix_console_loglevel: taking over the console_loglevel change
+livepatch: '$MOD_LIVEPATCH3': patching complete
+% rmmod $MOD_LIVEPATCH2
+% modprobe $MOD_LIVEPATCH2
+livepatch: enabling patch '$MOD_LIVEPATCH2'
+livepatch: '$MOD_LIVEPATCH2': initializing patching transition
+$MOD_LIVEPATCH2: pre_patch_callback: vmlinux
+$MOD_LIVEPATCH2: allocate_loglevel_state: space to store console_loglevel already allocated
+livepatch: '$MOD_LIVEPATCH2': starting patching transition
+livepatch: '$MOD_LIVEPATCH2': completing patching transition
+$MOD_LIVEPATCH2: post_patch_callback: vmlinux
+$MOD_LIVEPATCH2: fix_console_loglevel: taking over the console_loglevel change
+livepatch: '$MOD_LIVEPATCH2': patching complete
+% echo 0 > /sys/kernel/livepatch/$MOD_LIVEPATCH2/enabled
+livepatch: '$MOD_LIVEPATCH2': initializing unpatching transition
+$MOD_LIVEPATCH2: pre_unpatch_callback: vmlinux
+$MOD_LIVEPATCH2: restore_console_loglevel: restoring console_loglevel
+livepatch: '$MOD_LIVEPATCH2': starting unpatching transition
+livepatch: '$MOD_LIVEPATCH2': completing unpatching transition
+$MOD_LIVEPATCH2: post_unpatch_callback: vmlinux
+$MOD_LIVEPATCH2: free_loglevel_state: freeing space for the stored console_loglevel
+livepatch: '$MOD_LIVEPATCH2': unpatching complete
+% rmmod $MOD_LIVEPATCH2
+% rmmod $MOD_LIVEPATCH3"
+
+
+# TEST: Failure caused by incompatible cumulative livepatches
+
+echo -n "TEST: incompatible cumulative livepatches ... "
+dmesg -C
+
+load_lp $MOD_LIVEPATCH2
+load_failing_mod $MOD_LIVEPATCH
+disable_lp $MOD_LIVEPATCH2
+unload_lp $MOD_LIVEPATCH2
+
+check_result "% modprobe $MOD_LIVEPATCH2
+livepatch: enabling patch '$MOD_LIVEPATCH2'
+livepatch: '$MOD_LIVEPATCH2': initializing patching transition
+$MOD_LIVEPATCH2: pre_patch_callback: vmlinux
+$MOD_LIVEPATCH2: allocate_loglevel_state: allocating space to store console_loglevel
+livepatch: '$MOD_LIVEPATCH2': starting patching transition
+livepatch: '$MOD_LIVEPATCH2': completing patching transition
+$MOD_LIVEPATCH2: post_patch_callback: vmlinux
+$MOD_LIVEPATCH2: fix_console_loglevel: fixing console_loglevel
+livepatch: '$MOD_LIVEPATCH2': patching complete
+% modprobe $MOD_LIVEPATCH
+livepatch: Livepatch patch ($MOD_LIVEPATCH) is not compatible with the already installed livepatches.
+modprobe: ERROR: could not insert '$MOD_LIVEPATCH': Invalid argument
+% echo 0 > /sys/kernel/livepatch/$MOD_LIVEPATCH2/enabled
+livepatch: '$MOD_LIVEPATCH2': initializing unpatching transition
+$MOD_LIVEPATCH2: pre_unpatch_callback: vmlinux
+$MOD_LIVEPATCH2: restore_console_loglevel: restoring console_loglevel
+livepatch: '$MOD_LIVEPATCH2': starting unpatching transition
+livepatch: '$MOD_LIVEPATCH2': completing unpatching transition
+$MOD_LIVEPATCH2: post_unpatch_callback: vmlinux
+$MOD_LIVEPATCH2: free_loglevel_state: freeing space for the stored console_loglevel
+livepatch: '$MOD_LIVEPATCH2': unpatching complete
+% rmmod $MOD_LIVEPATCH2"
+
+exit 0
--
2.16.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 5/5] livepatch: Selftests of the API for tracking system state changes
2019-10-09 14:18 ` Petr Mladek
@ 2019-10-09 14:27 ` Joe Lawrence
0 siblings, 0 replies; 13+ messages in thread
From: Joe Lawrence @ 2019-10-09 14:27 UTC (permalink / raw)
To: Petr Mladek
Cc: Jiri Kosina, Kamalesh Babulal, Josh Poimboeuf, Miroslav Benes,
Nicolai Stange, linux-kernel, live-patching
On Wed, Oct 09, 2019 at 04:18:59PM +0200, Petr Mladek wrote:
> On Fri 2019-10-04 10:47:42, Joe Lawrence wrote:
> > On Thu, Oct 03, 2019 at 11:01:37AM +0200, Petr Mladek wrote:
> > > Four selftests for the new API.
> > >
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/livepatch/test-state.sh
> > > @@ -0,0 +1,180 @@
> > > +#!/bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (C) 2018 Joe Lawrence <joe.lawrence@redhat.com>
> > > +
> >
> > nit: this should probably read as:
> > # Copyright (C) 2019 Petr Mladek <pmladek@suse.com>
> >
> > > +
> > > +load_lp $MOD_LIVEPATCH
> > > +disable_lp $MOD_LIVEPATCH
> > > +unload_lp $MOD_LIVEPATCH
> > > +
> > > +check_result "% modprobe test_klp_state
> > > +livepatch: enabling patch 'test_klp_state'
> > > +livepatch: 'test_klp_state': initializing patching transition
> > > +test_klp_state: pre_patch_callback: vmlinux
> > > +test_klp_state: allocate_loglevel_state: allocating space to store console_loglevel
> > > +livepatch: 'test_klp_state': starting patching transition
> > > +livepatch: 'test_klp_state': completing patching transition
> > > +test_klp_state: post_patch_callback: vmlinux
> > > +test_klp_state: fix_console_loglevel: fixing console_loglevel
> > > +livepatch: 'test_klp_state': patching complete
> > > +% echo 0 > /sys/kernel/livepatch/test_klp_state/enabled
> > > +livepatch: 'test_klp_state': initializing unpatching transition
> > > +test_klp_state: pre_unpatch_callback: vmlinux
> > > +test_klp_state: restore_console_loglevel: restoring console_loglevel
> > > +livepatch: 'test_klp_state': starting unpatching transition
> > > +livepatch: 'test_klp_state': completing unpatching transition
> > > +test_klp_state: post_unpatch_callback: vmlinux
> > > +test_klp_state: free_loglevel_state: freeing space for the stored console_loglevel
> > > +livepatch: 'test_klp_state': unpatching complete
> > > +% rmmod test_klp_state"
> > > +
> >
> > nit: a matter of style, and I don't mind either, but the other test
> > scripts used $MOD_LIVEPATCH{2,3} variable replacement in the
> > check_result string. I think I originally did that when we were
> > reviewing the first self-test patchset and the filenames may or may not
> > have changed. Those names are stable now, so I don't have a strong
> > opinion either way.
>
> Please, find below and updated version of the patch that fixes
> the two above problems and also the kbuild robot failure.
>
> The build failure was fixed by #include <linux/slab.h>
>
> Here we go:
>
> From 206e0a53de25ddb042e5947c2a736e33ca4a2b4c Mon Sep 17 00:00:00 2001
> From: Petr Mladek <pmladek@suse.com>
> Date: Mon, 10 Jun 2019 14:02:42 +0200
> Subject: [PATCH v3.1 5/5] livepatch: Selftests of the API for tracking system state
> changes
>
> Four selftests for the new API.
>
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> Acked-by: Miroslav Benes <mbenes@suse.cz>
> ---
> lib/livepatch/Makefile | 5 +-
> lib/livepatch/test_klp_state.c | 162 ++++++++++++++++++++
> lib/livepatch/test_klp_state2.c | 191 ++++++++++++++++++++++++
> lib/livepatch/test_klp_state3.c | 5 +
> tools/testing/selftests/livepatch/Makefile | 3 +-
> tools/testing/selftests/livepatch/test-state.sh | 180 ++++++++++++++++++++++
> 6 files changed, 544 insertions(+), 2 deletions(-)
> create mode 100644 lib/livepatch/test_klp_state.c
> create mode 100644 lib/livepatch/test_klp_state2.c
> create mode 100644 lib/livepatch/test_klp_state3.c
> create mode 100755 tools/testing/selftests/livepatch/test-state.sh
>
> diff --git a/lib/livepatch/Makefile b/lib/livepatch/Makefile
> index 26900ddaef82..295b94bff370 100644
> --- a/lib/livepatch/Makefile
> +++ b/lib/livepatch/Makefile
> @@ -8,7 +8,10 @@ obj-$(CONFIG_TEST_LIVEPATCH) += test_klp_atomic_replace.o \
> test_klp_callbacks_busy.o \
> test_klp_callbacks_mod.o \
> test_klp_livepatch.o \
> - test_klp_shadow_vars.o
> + test_klp_shadow_vars.o \
> + test_klp_state.o \
> + test_klp_state2.o \
> + test_klp_state3.o
>
> # Target modules to be livepatched require CC_FLAGS_FTRACE
> CFLAGS_test_klp_callbacks_busy.o += $(CC_FLAGS_FTRACE)
> diff --git a/lib/livepatch/test_klp_state.c b/lib/livepatch/test_klp_state.c
> new file mode 100644
> index 000000000000..57a4253acb01
> --- /dev/null
> +++ b/lib/livepatch/test_klp_state.c
> @@ -0,0 +1,162 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2019 SUSE
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/slab.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/printk.h>
> +#include <linux/livepatch.h>
> +
> +#define CONSOLE_LOGLEVEL_STATE 1
> +/* Version 1 does not support migration. */
> +#define CONSOLE_LOGLEVEL_STATE_VERSION 1
> +
> +static const char *const module_state[] = {
> + [MODULE_STATE_LIVE] = "[MODULE_STATE_LIVE] Normal state",
> + [MODULE_STATE_COMING] = "[MODULE_STATE_COMING] Full formed, running module_init",
> + [MODULE_STATE_GOING] = "[MODULE_STATE_GOING] Going away",
> + [MODULE_STATE_UNFORMED] = "[MODULE_STATE_UNFORMED] Still setting it up",
> +};
> +
> +static void callback_info(const char *callback, struct klp_object *obj)
> +{
> + if (obj->mod)
> + pr_info("%s: %s -> %s\n", callback, obj->mod->name,
> + module_state[obj->mod->state]);
> + else
> + pr_info("%s: vmlinux\n", callback);
> +}
> +
> +static struct klp_patch patch;
> +
> +static int allocate_loglevel_state(void)
> +{
> + struct klp_state *loglevel_state;
> +
> + loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
> + if (!loglevel_state)
> + return -EINVAL;
> +
> + loglevel_state->data = kzalloc(sizeof(console_loglevel), GFP_KERNEL);
> + if (!loglevel_state->data)
> + return -ENOMEM;
> +
> + pr_info("%s: allocating space to store console_loglevel\n",
> + __func__);
> + return 0;
> +}
> +
> +static void fix_console_loglevel(void)
> +{
> + struct klp_state *loglevel_state;
> +
> + loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
> + if (!loglevel_state)
> + return;
> +
> + pr_info("%s: fixing console_loglevel\n", __func__);
> + *(int *)loglevel_state->data = console_loglevel;
> + console_loglevel = CONSOLE_LOGLEVEL_MOTORMOUTH;
> +}
> +
> +static void restore_console_loglevel(void)
> +{
> + struct klp_state *loglevel_state;
> +
> + loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
> + if (!loglevel_state)
> + return;
> +
> + pr_info("%s: restoring console_loglevel\n", __func__);
> + console_loglevel = *(int *)loglevel_state->data;
> +}
> +
> +static void free_loglevel_state(void)
> +{
> + struct klp_state *loglevel_state;
> +
> + loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
> + if (!loglevel_state)
> + return;
> +
> + pr_info("%s: freeing space for the stored console_loglevel\n",
> + __func__);
> + kfree(loglevel_state->data);
> +}
> +
> +/* Executed on object patching (ie, patch enablement) */
> +static int pre_patch_callback(struct klp_object *obj)
> +{
> + callback_info(__func__, obj);
> + return allocate_loglevel_state();
> +}
> +
> +/* Executed on object unpatching (ie, patch disablement) */
> +static void post_patch_callback(struct klp_object *obj)
> +{
> + callback_info(__func__, obj);
> + fix_console_loglevel();
> +}
> +
> +/* Executed on object unpatching (ie, patch disablement) */
> +static void pre_unpatch_callback(struct klp_object *obj)
> +{
> + callback_info(__func__, obj);
> + restore_console_loglevel();
> +}
> +
> +/* Executed on object unpatching (ie, patch disablement) */
> +static void post_unpatch_callback(struct klp_object *obj)
> +{
> + callback_info(__func__, obj);
> + free_loglevel_state();
> +}
> +
> +static struct klp_func no_funcs[] = {
> + {}
> +};
> +
> +static struct klp_object objs[] = {
> + {
> + .name = NULL, /* vmlinux */
> + .funcs = no_funcs,
> + .callbacks = {
> + .pre_patch = pre_patch_callback,
> + .post_patch = post_patch_callback,
> + .pre_unpatch = pre_unpatch_callback,
> + .post_unpatch = post_unpatch_callback,
> + },
> + }, { }
> +};
> +
> +static struct klp_state states[] = {
> + {
> + .id = CONSOLE_LOGLEVEL_STATE,
> + .version = CONSOLE_LOGLEVEL_STATE_VERSION,
> + }, { }
> +};
> +
> +static struct klp_patch patch = {
> + .mod = THIS_MODULE,
> + .objs = objs,
> + .states = states,
> + .replace = true,
> +};
> +
> +static int test_klp_callbacks_demo_init(void)
> +{
> + return klp_enable_patch(&patch);
> +}
> +
> +static void test_klp_callbacks_demo_exit(void)
> +{
> +}
> +
> +module_init(test_klp_callbacks_demo_init);
> +module_exit(test_klp_callbacks_demo_exit);
> +MODULE_LICENSE("GPL");
> +MODULE_INFO(livepatch, "Y");
> +MODULE_AUTHOR("Petr Mladek <pmladek@suse.com>");
> +MODULE_DESCRIPTION("Livepatch test: system state modification");
> diff --git a/lib/livepatch/test_klp_state2.c b/lib/livepatch/test_klp_state2.c
> new file mode 100644
> index 000000000000..c978ea4d5e67
> --- /dev/null
> +++ b/lib/livepatch/test_klp_state2.c
> @@ -0,0 +1,191 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2019 SUSE
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/slab.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/printk.h>
> +#include <linux/livepatch.h>
> +
> +#define CONSOLE_LOGLEVEL_STATE 1
> +/* Version 2 supports migration. */
> +#define CONSOLE_LOGLEVEL_STATE_VERSION 2
> +
> +static const char *const module_state[] = {
> + [MODULE_STATE_LIVE] = "[MODULE_STATE_LIVE] Normal state",
> + [MODULE_STATE_COMING] = "[MODULE_STATE_COMING] Full formed, running module_init",
> + [MODULE_STATE_GOING] = "[MODULE_STATE_GOING] Going away",
> + [MODULE_STATE_UNFORMED] = "[MODULE_STATE_UNFORMED] Still setting it up",
> +};
> +
> +static void callback_info(const char *callback, struct klp_object *obj)
> +{
> + if (obj->mod)
> + pr_info("%s: %s -> %s\n", callback, obj->mod->name,
> + module_state[obj->mod->state]);
> + else
> + pr_info("%s: vmlinux\n", callback);
> +}
> +
> +static struct klp_patch patch;
> +
> +static int allocate_loglevel_state(void)
> +{
> + struct klp_state *loglevel_state, *prev_loglevel_state;
> +
> + prev_loglevel_state = klp_get_prev_state(CONSOLE_LOGLEVEL_STATE);
> + if (prev_loglevel_state) {
> + pr_info("%s: space to store console_loglevel already allocated\n",
> + __func__);
> + return 0;
> + }
> +
> + loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
> + if (!loglevel_state)
> + return -EINVAL;
> +
> + loglevel_state->data = kzalloc(sizeof(console_loglevel), GFP_KERNEL);
> + if (!loglevel_state->data)
> + return -ENOMEM;
> +
> + pr_info("%s: allocating space to store console_loglevel\n",
> + __func__);
> + return 0;
> +}
> +
> +static void fix_console_loglevel(void)
> +{
> + struct klp_state *loglevel_state, *prev_loglevel_state;
> +
> + loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
> + if (!loglevel_state)
> + return;
> +
> + prev_loglevel_state = klp_get_prev_state(CONSOLE_LOGLEVEL_STATE);
> + if (prev_loglevel_state) {
> + pr_info("%s: taking over the console_loglevel change\n",
> + __func__);
> + loglevel_state->data = prev_loglevel_state->data;
> + return;
> + }
> +
> + pr_info("%s: fixing console_loglevel\n", __func__);
> + *(int *)loglevel_state->data = console_loglevel;
> + console_loglevel = CONSOLE_LOGLEVEL_MOTORMOUTH;
> +}
> +
> +static void restore_console_loglevel(void)
> +{
> + struct klp_state *loglevel_state, *prev_loglevel_state;
> +
> + prev_loglevel_state = klp_get_prev_state(CONSOLE_LOGLEVEL_STATE);
> + if (prev_loglevel_state) {
> + pr_info("%s: passing the console_loglevel change back to the old livepatch\n",
> + __func__);
> + return;
> + }
> +
> + loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
> + if (!loglevel_state)
> + return;
> +
> + pr_info("%s: restoring console_loglevel\n", __func__);
> + console_loglevel = *(int *)loglevel_state->data;
> +}
> +
> +static void free_loglevel_state(void)
> +{
> + struct klp_state *loglevel_state, *prev_loglevel_state;
> +
> + prev_loglevel_state = klp_get_prev_state(CONSOLE_LOGLEVEL_STATE);
> + if (prev_loglevel_state) {
> + pr_info("%s: keeping space to store console_loglevel\n",
> + __func__);
> + return;
> + }
> +
> + loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
> + if (!loglevel_state)
> + return;
> +
> + pr_info("%s: freeing space for the stored console_loglevel\n",
> + __func__);
> + kfree(loglevel_state->data);
> +}
> +
> +/* Executed on object patching (ie, patch enablement) */
> +static int pre_patch_callback(struct klp_object *obj)
> +{
> + callback_info(__func__, obj);
> + return allocate_loglevel_state();
> +}
> +
> +/* Executed on object unpatching (ie, patch disablement) */
> +static void post_patch_callback(struct klp_object *obj)
> +{
> + callback_info(__func__, obj);
> + fix_console_loglevel();
> +}
> +
> +/* Executed on object unpatching (ie, patch disablement) */
> +static void pre_unpatch_callback(struct klp_object *obj)
> +{
> + callback_info(__func__, obj);
> + restore_console_loglevel();
> +}
> +
> +/* Executed on object unpatching (ie, patch disablement) */
> +static void post_unpatch_callback(struct klp_object *obj)
> +{
> + callback_info(__func__, obj);
> + free_loglevel_state();
> +}
> +
> +static struct klp_func no_funcs[] = {
> + {}
> +};
> +
> +static struct klp_object objs[] = {
> + {
> + .name = NULL, /* vmlinux */
> + .funcs = no_funcs,
> + .callbacks = {
> + .pre_patch = pre_patch_callback,
> + .post_patch = post_patch_callback,
> + .pre_unpatch = pre_unpatch_callback,
> + .post_unpatch = post_unpatch_callback,
> + },
> + }, { }
> +};
> +
> +static struct klp_state states[] = {
> + {
> + .id = CONSOLE_LOGLEVEL_STATE,
> + .version = CONSOLE_LOGLEVEL_STATE_VERSION,
> + }, { }
> +};
> +
> +static struct klp_patch patch = {
> + .mod = THIS_MODULE,
> + .objs = objs,
> + .states = states,
> + .replace = true,
> +};
> +
> +static int test_klp_callbacks_demo_init(void)
> +{
> + return klp_enable_patch(&patch);
> +}
> +
> +static void test_klp_callbacks_demo_exit(void)
> +{
> +}
> +
> +module_init(test_klp_callbacks_demo_init);
> +module_exit(test_klp_callbacks_demo_exit);
> +MODULE_LICENSE("GPL");
> +MODULE_INFO(livepatch, "Y");
> +MODULE_AUTHOR("Petr Mladek <pmladek@suse.com>");
> +MODULE_DESCRIPTION("Livepatch test: system state modification");
> diff --git a/lib/livepatch/test_klp_state3.c b/lib/livepatch/test_klp_state3.c
> new file mode 100644
> index 000000000000..9226579d10c5
> --- /dev/null
> +++ b/lib/livepatch/test_klp_state3.c
> @@ -0,0 +1,5 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2019 SUSE
> +
> +/* The console loglevel fix is the same in the next cumulative patch. */
> +#include "test_klp_state2.c"
> diff --git a/tools/testing/selftests/livepatch/Makefile b/tools/testing/selftests/livepatch/Makefile
> index fd405402c3ff..1cf40a9e7185 100644
> --- a/tools/testing/selftests/livepatch/Makefile
> +++ b/tools/testing/selftests/livepatch/Makefile
> @@ -4,6 +4,7 @@ TEST_PROGS_EXTENDED := functions.sh
> TEST_PROGS := \
> test-livepatch.sh \
> test-callbacks.sh \
> - test-shadow-vars.sh
> + test-shadow-vars.sh \
> + test-state.sh
>
> include ../lib.mk
> diff --git a/tools/testing/selftests/livepatch/test-state.sh b/tools/testing/selftests/livepatch/test-state.sh
> new file mode 100755
> index 000000000000..dc2908c22c26
> --- /dev/null
> +++ b/tools/testing/selftests/livepatch/test-state.sh
> @@ -0,0 +1,180 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2019 SUSE
> +
> +. $(dirname $0)/functions.sh
> +
> +MOD_LIVEPATCH=test_klp_state
> +MOD_LIVEPATCH2=test_klp_state2
> +MOD_LIVEPATCH3=test_klp_state3
> +
> +set_dynamic_debug
> +
> +
> +# TEST: Loading and removing a module that modifies the system state
> +
> +echo -n "TEST: system state modification ... "
> +dmesg -C
> +
> +load_lp $MOD_LIVEPATCH
> +disable_lp $MOD_LIVEPATCH
> +unload_lp $MOD_LIVEPATCH
> +
> +check_result "% modprobe $MOD_LIVEPATCH
> +livepatch: enabling patch '$MOD_LIVEPATCH'
> +livepatch: '$MOD_LIVEPATCH': initializing patching transition
> +$MOD_LIVEPATCH: pre_patch_callback: vmlinux
> +$MOD_LIVEPATCH: allocate_loglevel_state: allocating space to store console_loglevel
> +livepatch: '$MOD_LIVEPATCH': starting patching transition
> +livepatch: '$MOD_LIVEPATCH': completing patching transition
> +$MOD_LIVEPATCH: post_patch_callback: vmlinux
> +$MOD_LIVEPATCH: fix_console_loglevel: fixing console_loglevel
> +livepatch: '$MOD_LIVEPATCH': patching complete
> +% echo 0 > /sys/kernel/livepatch/$MOD_LIVEPATCH/enabled
> +livepatch: '$MOD_LIVEPATCH': initializing unpatching transition
> +$MOD_LIVEPATCH: pre_unpatch_callback: vmlinux
> +$MOD_LIVEPATCH: restore_console_loglevel: restoring console_loglevel
> +livepatch: '$MOD_LIVEPATCH': starting unpatching transition
> +livepatch: '$MOD_LIVEPATCH': completing unpatching transition
> +$MOD_LIVEPATCH: post_unpatch_callback: vmlinux
> +$MOD_LIVEPATCH: free_loglevel_state: freeing space for the stored console_loglevel
> +livepatch: '$MOD_LIVEPATCH': unpatching complete
> +% rmmod $MOD_LIVEPATCH"
> +
> +
> +# TEST: Take over system state change by a cumulative patch
> +
> +echo -n "TEST: taking over system state modification ... "
> +dmesg -C
> +
> +load_lp $MOD_LIVEPATCH
> +load_lp $MOD_LIVEPATCH2
> +unload_lp $MOD_LIVEPATCH
> +disable_lp $MOD_LIVEPATCH2
> +unload_lp $MOD_LIVEPATCH2
> +
> +check_result "% modprobe $MOD_LIVEPATCH
> +livepatch: enabling patch '$MOD_LIVEPATCH'
> +livepatch: '$MOD_LIVEPATCH': initializing patching transition
> +$MOD_LIVEPATCH: pre_patch_callback: vmlinux
> +$MOD_LIVEPATCH: allocate_loglevel_state: allocating space to store console_loglevel
> +livepatch: '$MOD_LIVEPATCH': starting patching transition
> +livepatch: '$MOD_LIVEPATCH': completing patching transition
> +$MOD_LIVEPATCH: post_patch_callback: vmlinux
> +$MOD_LIVEPATCH: fix_console_loglevel: fixing console_loglevel
> +livepatch: '$MOD_LIVEPATCH': patching complete
> +% modprobe $MOD_LIVEPATCH2
> +livepatch: enabling patch '$MOD_LIVEPATCH2'
> +livepatch: '$MOD_LIVEPATCH2': initializing patching transition
> +$MOD_LIVEPATCH2: pre_patch_callback: vmlinux
> +$MOD_LIVEPATCH2: allocate_loglevel_state: space to store console_loglevel already allocated
> +livepatch: '$MOD_LIVEPATCH2': starting patching transition
> +livepatch: '$MOD_LIVEPATCH2': completing patching transition
> +$MOD_LIVEPATCH2: post_patch_callback: vmlinux
> +$MOD_LIVEPATCH2: fix_console_loglevel: taking over the console_loglevel change
> +livepatch: '$MOD_LIVEPATCH2': patching complete
> +% rmmod $MOD_LIVEPATCH
> +% echo 0 > /sys/kernel/livepatch/$MOD_LIVEPATCH2/enabled
> +livepatch: '$MOD_LIVEPATCH2': initializing unpatching transition
> +$MOD_LIVEPATCH2: pre_unpatch_callback: vmlinux
> +$MOD_LIVEPATCH2: restore_console_loglevel: restoring console_loglevel
> +livepatch: '$MOD_LIVEPATCH2': starting unpatching transition
> +livepatch: '$MOD_LIVEPATCH2': completing unpatching transition
> +$MOD_LIVEPATCH2: post_unpatch_callback: vmlinux
> +$MOD_LIVEPATCH2: free_loglevel_state: freeing space for the stored console_loglevel
> +livepatch: '$MOD_LIVEPATCH2': unpatching complete
> +% rmmod $MOD_LIVEPATCH2"
> +
> +
> +# TEST: Take over system state change by a cumulative patch
> +
> +echo -n "TEST: compatible cumulative livepatches ... "
> +dmesg -C
> +
> +load_lp $MOD_LIVEPATCH2
> +load_lp $MOD_LIVEPATCH3
> +unload_lp $MOD_LIVEPATCH2
> +load_lp $MOD_LIVEPATCH2
> +disable_lp $MOD_LIVEPATCH2
> +unload_lp $MOD_LIVEPATCH2
> +unload_lp $MOD_LIVEPATCH3
> +
> +check_result "% modprobe $MOD_LIVEPATCH2
> +livepatch: enabling patch '$MOD_LIVEPATCH2'
> +livepatch: '$MOD_LIVEPATCH2': initializing patching transition
> +$MOD_LIVEPATCH2: pre_patch_callback: vmlinux
> +$MOD_LIVEPATCH2: allocate_loglevel_state: allocating space to store console_loglevel
> +livepatch: '$MOD_LIVEPATCH2': starting patching transition
> +livepatch: '$MOD_LIVEPATCH2': completing patching transition
> +$MOD_LIVEPATCH2: post_patch_callback: vmlinux
> +$MOD_LIVEPATCH2: fix_console_loglevel: fixing console_loglevel
> +livepatch: '$MOD_LIVEPATCH2': patching complete
> +% modprobe $MOD_LIVEPATCH3
> +livepatch: enabling patch '$MOD_LIVEPATCH3'
> +livepatch: '$MOD_LIVEPATCH3': initializing patching transition
> +$MOD_LIVEPATCH3: pre_patch_callback: vmlinux
> +$MOD_LIVEPATCH3: allocate_loglevel_state: space to store console_loglevel already allocated
> +livepatch: '$MOD_LIVEPATCH3': starting patching transition
> +livepatch: '$MOD_LIVEPATCH3': completing patching transition
> +$MOD_LIVEPATCH3: post_patch_callback: vmlinux
> +$MOD_LIVEPATCH3: fix_console_loglevel: taking over the console_loglevel change
> +livepatch: '$MOD_LIVEPATCH3': patching complete
> +% rmmod $MOD_LIVEPATCH2
> +% modprobe $MOD_LIVEPATCH2
> +livepatch: enabling patch '$MOD_LIVEPATCH2'
> +livepatch: '$MOD_LIVEPATCH2': initializing patching transition
> +$MOD_LIVEPATCH2: pre_patch_callback: vmlinux
> +$MOD_LIVEPATCH2: allocate_loglevel_state: space to store console_loglevel already allocated
> +livepatch: '$MOD_LIVEPATCH2': starting patching transition
> +livepatch: '$MOD_LIVEPATCH2': completing patching transition
> +$MOD_LIVEPATCH2: post_patch_callback: vmlinux
> +$MOD_LIVEPATCH2: fix_console_loglevel: taking over the console_loglevel change
> +livepatch: '$MOD_LIVEPATCH2': patching complete
> +% echo 0 > /sys/kernel/livepatch/$MOD_LIVEPATCH2/enabled
> +livepatch: '$MOD_LIVEPATCH2': initializing unpatching transition
> +$MOD_LIVEPATCH2: pre_unpatch_callback: vmlinux
> +$MOD_LIVEPATCH2: restore_console_loglevel: restoring console_loglevel
> +livepatch: '$MOD_LIVEPATCH2': starting unpatching transition
> +livepatch: '$MOD_LIVEPATCH2': completing unpatching transition
> +$MOD_LIVEPATCH2: post_unpatch_callback: vmlinux
> +$MOD_LIVEPATCH2: free_loglevel_state: freeing space for the stored console_loglevel
> +livepatch: '$MOD_LIVEPATCH2': unpatching complete
> +% rmmod $MOD_LIVEPATCH2
> +% rmmod $MOD_LIVEPATCH3"
> +
> +
> +# TEST: Failure caused by incompatible cumulative livepatches
> +
> +echo -n "TEST: incompatible cumulative livepatches ... "
> +dmesg -C
> +
> +load_lp $MOD_LIVEPATCH2
> +load_failing_mod $MOD_LIVEPATCH
> +disable_lp $MOD_LIVEPATCH2
> +unload_lp $MOD_LIVEPATCH2
> +
> +check_result "% modprobe $MOD_LIVEPATCH2
> +livepatch: enabling patch '$MOD_LIVEPATCH2'
> +livepatch: '$MOD_LIVEPATCH2': initializing patching transition
> +$MOD_LIVEPATCH2: pre_patch_callback: vmlinux
> +$MOD_LIVEPATCH2: allocate_loglevel_state: allocating space to store console_loglevel
> +livepatch: '$MOD_LIVEPATCH2': starting patching transition
> +livepatch: '$MOD_LIVEPATCH2': completing patching transition
> +$MOD_LIVEPATCH2: post_patch_callback: vmlinux
> +$MOD_LIVEPATCH2: fix_console_loglevel: fixing console_loglevel
> +livepatch: '$MOD_LIVEPATCH2': patching complete
> +% modprobe $MOD_LIVEPATCH
> +livepatch: Livepatch patch ($MOD_LIVEPATCH) is not compatible with the already installed livepatches.
> +modprobe: ERROR: could not insert '$MOD_LIVEPATCH': Invalid argument
> +% echo 0 > /sys/kernel/livepatch/$MOD_LIVEPATCH2/enabled
> +livepatch: '$MOD_LIVEPATCH2': initializing unpatching transition
> +$MOD_LIVEPATCH2: pre_unpatch_callback: vmlinux
> +$MOD_LIVEPATCH2: restore_console_loglevel: restoring console_loglevel
> +livepatch: '$MOD_LIVEPATCH2': starting unpatching transition
> +livepatch: '$MOD_LIVEPATCH2': completing unpatching transition
> +$MOD_LIVEPATCH2: post_unpatch_callback: vmlinux
> +$MOD_LIVEPATCH2: free_loglevel_state: freeing space for the stored console_loglevel
> +livepatch: '$MOD_LIVEPATCH2': unpatching complete
> +% rmmod $MOD_LIVEPATCH2"
> +
> +exit 0
> --
> 2.16.4
>
>
Looks good to me...
Acked-by: Joe Lawrence <joe.lawrence@redhat.com>
-- Joe
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/5] livepatch: Allow to distinguish different version of system state changes
2019-10-03 9:01 ` [PATCH v3 3/5] livepatch: Allow to distinguish different version of " Petr Mladek
@ 2019-10-23 21:15 ` Josh Poimboeuf
2019-10-24 12:13 ` Petr Mladek
0 siblings, 1 reply; 13+ messages in thread
From: Josh Poimboeuf @ 2019-10-23 21:15 UTC (permalink / raw)
To: Petr Mladek
Cc: Jiri Kosina, Miroslav Benes, Joe Lawrence, Kamalesh Babulal,
Nicolai Stange, live-patching, linux-kernel
Hi Petr,
Sorry for taking so long...
On Thu, Oct 03, 2019 at 11:01:35AM +0200, Petr Mladek wrote:
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index 726947338fd5..42907c4a0ce8 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -133,10 +133,12 @@ struct klp_object {
> /**
> * struct klp_state - state of the system modified by the livepatch
> * @id: system state identifier (non-zero)
> + * @version: version of the change (non-zero)
Is it necessary to assume that 'version' is non-zero? It would be easy
for a user to not realize that and start with version 0. Then the patch
state would be silently ignored.
I have the same concern about 'id', but I guess at least one of them has
to be non-zero to differentiate valid entries from the array terminator.
> +/* Check if the patch is able to deal with the given system state. */
> +static bool klp_is_state_compatible(struct klp_patch *patch,
> + struct klp_state *state)
> +{
> + struct klp_state *new_state;
> +
> + new_state = klp_get_state(patch, state->id);
> +
> + if (new_state)
> + return new_state->version >= state->version;
> +
> + /* Cumulative livepatch must handle all already modified states. */
> + return !patch->replace;
> +}
From my perspective I view '!new_state' as an error condition. I'd find
it easier to read if the ordering were changed to check for the error
first:
if (!new_state) {
/*
* A cumulative livepatch must handle all already
* modified states.
*/
return !patch->replace;
}
return new_state->version >= state->version;
> +
> +/*
> + * Check that the new livepatch will not break the existing system states.
> + * Cumulative patches must handle all already modified states.
> + * Non-cumulative patches can touch already modified states.
> + */
> +bool klp_is_patch_compatible(struct klp_patch *patch)
> +{
> + struct klp_patch *old_patch;
> + struct klp_state *state;
> +
> +
> + klp_for_each_patch(old_patch) {
Extra newline above.
> + klp_for_each_state(old_patch, state) {
> + if (!klp_is_state_compatible(patch, state))
> + return false;
> + }
> + }
I think renaming 'state' to 'old_state' would make the intention a
little clearer, and would be consistent with 'old_patch'.
--
Josh
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/5] livepatch: Allow to distinguish different version of system state changes
2019-10-23 21:15 ` Josh Poimboeuf
@ 2019-10-24 12:13 ` Petr Mladek
0 siblings, 0 replies; 13+ messages in thread
From: Petr Mladek @ 2019-10-24 12:13 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Jiri Kosina, Miroslav Benes, Joe Lawrence, Kamalesh Babulal,
Nicolai Stange, live-patching, linux-kernel
On Wed 2019-10-23 16:15:28, Josh Poimboeuf wrote:
> Hi Petr,
>
> Sorry for taking so long...
>
> On Thu, Oct 03, 2019 at 11:01:35AM +0200, Petr Mladek wrote:
> > diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> > index 726947338fd5..42907c4a0ce8 100644
> > --- a/include/linux/livepatch.h
> > +++ b/include/linux/livepatch.h
> > @@ -133,10 +133,12 @@ struct klp_object {
> > /**
> > * struct klp_state - state of the system modified by the livepatch
> > * @id: system state identifier (non-zero)
> > + * @version: version of the change (non-zero)
>
> Is it necessary to assume that 'version' is non-zero? It would be easy
> for a user to not realize that and start with version 0. Then the patch
> state would be silently ignored.
>
> I have the same concern about 'id', but I guess at least one of them has
> to be non-zero to differentiate valid entries from the array terminator.
Exactly. At least one struct member must be non-zero to differentiate
the array terminator.
I do not mind to allow zero version. Will do so in v4.
> > +/* Check if the patch is able to deal with the given system state. */
> > +static bool klp_is_state_compatible(struct klp_patch *patch,
> > + struct klp_state *state)
> > +{
> > + struct klp_state *new_state;
> > +
> > + new_state = klp_get_state(patch, state->id);
> > +
> > + if (new_state)
> > + return new_state->version >= state->version;
> > +
> > + /* Cumulative livepatch must handle all already modified states. */
> > + return !patch->replace;
> > +}
>
> >From my perspective I view '!new_state' as an error condition. I'd find
> it easier to read if the ordering were changed to check for the error
> first:
>
> if (!new_state) {
> /*
> * A cumulative livepatch must handle all already
> * modified states.
> */
> return !patch->replace;
> }
>
> return new_state->version >= state->version;
-> v4
> > +
> > +/*
> > + * Check that the new livepatch will not break the existing system states.
> > + * Cumulative patches must handle all already modified states.
> > + * Non-cumulative patches can touch already modified states.
> > + */
> > +bool klp_is_patch_compatible(struct klp_patch *patch)
> > +{
> > + struct klp_patch *old_patch;
> > + struct klp_state *state;
> > +
> > +
> > + klp_for_each_patch(old_patch) {
>
> Extra newline above.
>
> > + klp_for_each_state(old_patch, state) {
> > + if (!klp_is_state_compatible(patch, state))
> > + return false;
> > + }
> > + }
>
> I think renaming 'state' to 'old_state' would make the intention a
> little clearer, and would be consistent with 'old_patch'.
Makes sense. I'll make the names consistent also in klp_is_state_compatible():
/* Check if the patch is able to deal with the given system state. */
static bool klp_is_state_compatible(struct klp_patch *patch,
struct klp_state *old_state)
{
struct klp_state *state = klp_get_state(patch, state->id);
if (!state) {
/*
* A cumulative livepatch must handle all already
* modified states.
*/
return !patch->replace;
}
return state->version >= old_state->version;
}
Best Regards,
Petr
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-10-24 12:13 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-03 9:01 [PATCH v3 0/5] livepatch: new API to track system state changes Petr Mladek
2019-10-03 9:01 ` [PATCH v3 1/5] livepatch: Keep replaced patches until post_patch callback is called Petr Mladek
2019-10-03 9:01 ` [PATCH v3 2/5] livepatch: Basic API to track system state changes Petr Mladek
2019-10-03 9:01 ` [PATCH v3 3/5] livepatch: Allow to distinguish different version of " Petr Mladek
2019-10-23 21:15 ` Josh Poimboeuf
2019-10-24 12:13 ` Petr Mladek
2019-10-03 9:01 ` [PATCH v3 4/5] livepatch: Documentation of the new API for tracking " Petr Mladek
2019-10-03 9:01 ` [PATCH v3 5/5] livepatch: Selftests of the " Petr Mladek
2019-10-04 14:47 ` Joe Lawrence
2019-10-09 14:18 ` Petr Mladek
2019-10-09 14:27 ` Joe Lawrence
2019-10-08 22:19 ` kbuild test robot
2019-10-04 14:39 ` [PATCH v3 0/5] livepatch: new API to track " Joe Lawrence
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).