[RFC 0/5] livepatch: new API to track system state changes

[RFC 0/5] livepatch: new API to track system state changes

[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, hopefully, a simple and generic API that
helps to keep and pass information between the livepatches.
It is also usable to prevent loading incompatible livepatches.

There was also a related idea to add a sticky flag. It should be
easy to add it later. It would perfectly fit into the new struct
klp_state.

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        |  80 ++++++++++
 include/linux/livepatch.h                       |  17 +++
 kernel/livepatch/Makefile                       |   2 +-
 kernel/livepatch/core.c                         |  44 ++++--
 kernel/livepatch/core.h                         |   5 +-
 kernel/livepatch/state.c                        | 121 +++++++++++++++
 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, 814 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

[RFC 1/5] livepatch: Keep replaced patches until post_patch callback is called

[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 has 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>
---
 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 2398832947c6..24c4a13bd26c 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -631,7 +631,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);
@@ -676,6 +676,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)
@@ -1016,12 +1033,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.
@@ -1029,18 +1047,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 abb2a4a2cbb2..d6a73dd409ba 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);
 	}
 
@@ -441,14 +441,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

[RFC 2/5] livepatch: Basic API to track system state changes

[Petr Mladek]

This is another step how to help maintaining more livepatches.

One big help was the atomic replace and cumulative livepatches. These
livepatches replaces 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 not 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:

  - 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. There are two functions that allow
to find this structure for a given struct klp_patch or for
already installed (replaced) livepatches.

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 the same logic
as that it is allowed to modify the same function several times.
The livepatch author is responsible for preventing incompatible
changes.

Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 include/linux/livepatch.h | 15 +++++++++
 kernel/livepatch/Makefile |  2 +-
 kernel/livepatch/state.c  | 83 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 99 insertions(+), 1 deletion(-)
 create mode 100644 kernel/livepatch/state.c

diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index eeba421cc671..591abdee30d7 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -132,10 +132,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 {
+	int 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
@@ -150,6 +161,7 @@ struct klp_patch {
 	/* external */
 	struct module *mod;
 	struct klp_object *objs;
+	struct klp_state *states;
 	bool replace;
 
 	/* internal */
@@ -220,6 +232,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, int id);
+struct klp_state *klp_get_prev_state(int 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..f8822b71f96e
--- /dev/null
+++ b/kernel/livepatch/state.c
@@ -0,0 +1,83 @@
+// 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 to 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, int 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(int 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

Re: [RFC 2/5] livepatch: Basic API to track system state changes

[Joe Lawrence]

On Tue, Jun 11, 2019 at 03:56:24PM +0200, Petr Mladek wrote:
> This is another step how to help maintaining more livepatches.
> 
> One big help was the atomic replace and cumulative livepatches. These
> livepatches replaces the already installed ones. Therefore it should

nit: s/replaces/replaces

> 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 not longer safe to

nit: s/not longer safe/no longer safe

> 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:
> 
>   - 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. There are two functions that allow
> to find this structure for a given struct klp_patch or for
> already installed (replaced) livepatches.
> 

suggestion: "Two new functions, klp_get_state() and
klp_get_prev_state(), can find this structure ..." or perhaps drop this
part altogether and let the future reader do a 'git show' or 'git log
-p' to see the code changes and the exact function names.

-- Joe

> 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 the same logic
> as that it is allowed to modify the same function several times.
> The livepatch author is responsible for preventing incompatible
> changes.
> 
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> ---
>  include/linux/livepatch.h | 15 +++++++++
>  kernel/livepatch/Makefile |  2 +-
>  kernel/livepatch/state.c  | 83 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 99 insertions(+), 1 deletion(-)
>  create mode 100644 kernel/livepatch/state.c
> 
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index eeba421cc671..591abdee30d7 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -132,10 +132,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 {
> +	int 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
> @@ -150,6 +161,7 @@ struct klp_patch {
>  	/* external */
>  	struct module *mod;
>  	struct klp_object *objs;
> +	struct klp_state *states;
>  	bool replace;
>  
>  	/* internal */
> @@ -220,6 +232,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, int id);
> +struct klp_state *klp_get_prev_state(int 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..f8822b71f96e
> --- /dev/null
> +++ b/kernel/livepatch/state.c
> @@ -0,0 +1,83 @@
> +// 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 to 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, int 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(int 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
> 

Re: [RFC 2/5] livepatch: Basic API to track system state changes

[Nicolai Stange]

Petr Mladek <pmladek@suse.com> writes:

> ---
>  include/linux/livepatch.h | 15 +++++++++
>  kernel/livepatch/Makefile |  2 +-
>  kernel/livepatch/state.c  | 83 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 99 insertions(+), 1 deletion(-)
>  create mode 100644 kernel/livepatch/state.c
>
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index eeba421cc671..591abdee30d7 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -132,10 +132,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 {
> +	int id;

Can we make this an unsigned long please? It would be consistent with
shadow variable ids and would give more room for encoding bugzilla or
CVE numbers or so.

Nicolai

> +	void *data;
> +};
> +

[RFC 3/5] livepatch: Allow to distinguish different version of system state changes

[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>
---
 include/linux/livepatch.h |  2 ++
 kernel/livepatch/core.c   |  8 ++++++++
 kernel/livepatch/state.c  | 40 +++++++++++++++++++++++++++++++++++++++-
 kernel/livepatch/state.h  |  9 +++++++++
 4 files changed, 58 insertions(+), 1 deletion(-)
 create mode 100644 kernel/livepatch/state.h

diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index 591abdee30d7..8bc4c6cc3f3f 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -135,10 +135,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 {
 	int id;
+	int version;
 	void *data;
 };
 
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 24c4a13bd26c..614642719825 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -21,6 +21,7 @@
 #include <asm/cacheflush.h>
 #include "core.h"
 #include "patch.h"
+#include "state.h"
 #include "transition.h"
 
 /*
@@ -1003,6 +1004,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 f8822b71f96e..b54a69b9e4b4 100644
--- a/kernel/livepatch/state.c
+++ b/kernel/livepatch/state.c
@@ -12,7 +12,9 @@
 #include "transition.h"
 
 #define klp_for_each_state(patch, state)		\
-	for (state = patch->states; state && state->id; state++)
+	for (state = patch->states;			\
+	     state && state->id && state->version;	\
+	     state++)
 
 /**
  * klp_get_state() - get information about system state modified by
@@ -81,3 +83,39 @@ struct klp_state *klp_get_prev_state(int 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 ? false : true;
+
+	/* Cumulative livepatch must handle all already modified states. */
+	return patch->replace ? false : true;
+}
+
+/*
+ * Check if 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

Re: [RFC 3/5] livepatch: Allow to distinguish different version of system state changes

[Miroslav Benes]

> +/* 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 ? false : true;

return new_state->version >= state->version;

?

> +     /* Cumulative livepatch must handle all already modified states. 
*/
> +     return patch->replace ? false : true;

return !patch->replace;

?

> + * Check if 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)

make C=1 kernel/livepatch/state.o

kernel/livepatch/state.c:107:6: warning: symbol 'klp_is_patch_compatible' was not declared. Should it be static?

#include "state.h" in state.c would solve it.

Miroslav

Re: [RFC 3/5] livepatch: Allow to distinguish different version of system state changes

[Joe Lawrence]

On Tue, Jun 11, 2019 at 03:56:25PM +0200, Petr Mladek wrote:
> 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.
> 

More word play: would it be any clearer to drop the use of
"modification" when talking about klp_states?  Sometimes I read
modification to mean a change to a klp_state itself rather than the
system at large.

In my mind, "modification" is implied, but I already know where this
patchset is going, so perhaps I'm just trying to be lazy and not type
the whole thing out :)  I wish I could come up with a nice, succinct
alternative, but "state" or "klp_state" would work for me.  /two cents

>   + 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>
> ---
>  include/linux/livepatch.h |  2 ++
>  kernel/livepatch/core.c   |  8 ++++++++
>  kernel/livepatch/state.c  | 40 +++++++++++++++++++++++++++++++++++++++-
>  kernel/livepatch/state.h  |  9 +++++++++
>  4 files changed, 58 insertions(+), 1 deletion(-)
>  create mode 100644 kernel/livepatch/state.h
> 
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index 591abdee30d7..8bc4c6cc3f3f 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -135,10 +135,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 {
>  	int id;
> +	int version;
>  	void *data;
>  };
>  
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 24c4a13bd26c..614642719825 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -21,6 +21,7 @@
>  #include <asm/cacheflush.h>
>  #include "core.h"
>  #include "patch.h"
> +#include "state.h"
>  #include "transition.h"
>  
>  /*
> @@ -1003,6 +1004,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 f8822b71f96e..b54a69b9e4b4 100644
> --- a/kernel/livepatch/state.c
> +++ b/kernel/livepatch/state.c
> @@ -12,7 +12,9 @@
>  #include "transition.h"
>  
>  #define klp_for_each_state(patch, state)		\
> -	for (state = patch->states; state && state->id; state++)
> +	for (state = patch->states;			\
> +	     state && state->id && state->version;	\
> +	     state++)

Minor git bookkeeping here, but this could be moved to the patch that
introduced the macro.

>  
>  /**
>   * klp_get_state() - get information about system state modified by
> @@ -81,3 +83,39 @@ struct klp_state *klp_get_prev_state(int 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 ? false : true;
> +
> +	/* Cumulative livepatch must handle all already modified states. */
> +	return patch->replace ? false : true;
> +}
> +
> +/*
> + * Check if the new livepatch will not break the existing system states.

suggestion: "Check that the new livepatch will not break" or
            "Check if the new livepatch will break"

-- Joe

Re: [RFC 3/5] livepatch: Allow to distinguish different version of system state changes

[Joe Lawrence]

On Fri, Jun 21, 2019 at 10:09:11AM -0400, Joe Lawrence wrote:
> More word play: would it be any clearer to drop the use of
> "modification" when talking about klp_states?  Sometimes I read
> modification to mean a change to a klp_state itself rather than the
> system at large.

After reading through the rest of the series, maybe I was premature
about this.  "System state modification" is used consistently throughout
the series, so without having any better suggestion, ignore my comment.

-- Joe

Re: [RFC 3/5] livepatch: Allow to distinguish different version of system state changes

[Nicolai Stange]

Petr Mladek <pmladek@suse.com> writes:

> ---
>  include/linux/livepatch.h |  2 ++
>  kernel/livepatch/core.c   |  8 ++++++++
>  kernel/livepatch/state.c  | 40 +++++++++++++++++++++++++++++++++++++++-
>  kernel/livepatch/state.h  |  9 +++++++++
>  4 files changed, 58 insertions(+), 1 deletion(-)
>  create mode 100644 kernel/livepatch/state.h
>
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index 591abdee30d7..8bc4c6cc3f3f 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -135,10 +135,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 {
>  	int id;
> +	int version;
>  	void *data;
>  };
>  
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 24c4a13bd26c..614642719825 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -21,6 +21,7 @@
>  #include <asm/cacheflush.h>
>  #include "core.h"
>  #include "patch.h"
> +#include "state.h"
>  #include "transition.h"
>  
>  /*
> @@ -1003,6 +1004,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);


Just as a remark: klp_reverse_transition() could still transition back
to a !klp_is_patch_compatible() patch.

I don't think it's much of a problem, because for live patches
introducing completely new states to the system, it is reasonable
to assume that they'll start applying incompatible changes only from
their ->post_patch(), I guess.

For state "upgrades" to higher versions, it's not so clear though and
some care will be needed. But I think these could still be handled
safely at the cost of some complexity in the new live patch's
->post_patch().

Another detail is that ->post_unpatch() will be called for the new live
patch which has been unpatched due to transition reversal and one would
have to be careful not to free shared state from under the older, still
active live patch. How would ->post_unpatch() distinguish between
transition reversal and "normal" live patch disabling?  By
klp_get_prev_state() != NULL?

Perhaps transition reversal should be mentioned in the documentation?

Thanks,

Nicolai


-- 
SUSE Linux GmbH, GF: Felix Imendörffer, Mary Higgins, Sri Rasiah, HRB
21284 (AG Nürnberg)

Re: [RFC 3/5] livepatch: Allow to distinguish different version of system state changes

[Petr Mladek]

Hi,

first, I am sorry that I answer this non-trivial mail so late.
I know that it might be hard to remember the context.


On Mon 2019-06-24 12:26:07, Nicolai Stange wrote:
> Petr Mladek <pmladek@suse.com> writes:
> > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> > index 24c4a13bd26c..614642719825 100644
> > --- a/kernel/livepatch/core.c
> > +++ b/kernel/livepatch/core.c
> > @@ -1003,6 +1004,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);
> 
> 
> Just as a remark: klp_reverse_transition() could still transition back
> to a !klp_is_patch_compatible() patch.

I am slightly confused. The new livepatch is enabled only when the new
states have the same or higher version. And only callbacks from
the new livepatch are used, including post_unpatch() when
the transition gets reverted.

The "compatible" livepatch should be able to handle all situations:

    + Modify the system state when it was not modified before.

    + Take over the system state when it has already been modified
      by the previous livepatch.

    + Restore the previous state when the transition is reverted.


> I don't think it's much of a problem, because for live patches
> introducing completely new states to the system, it is reasonable
> to assume that they'll start applying incompatible changes only from
> their ->post_patch(), I guess.
>
> For state "upgrades" to higher versions, it's not so clear though and
> some care will be needed. But I think these could still be handled
> safely at the cost of some complexity in the new live patch's
> ->post_patch().

Just to be sure. The post_unpatch() from the new livepatch
will get called when the transitions is reverted. It should
be able to revert any changes made by its own pre_patch().

You are right that it will need some care. Especially because
the transition revert is not easy to test.

I think that this is the main reason why Joe would like
to introduce the sticky flag. It might be used to block
the transition revert and livepatch disabling when it would
be to complicated, error-prone, or even impossible.


> Another detail is that ->post_unpatch() will be called for the new live
> patch which has been unpatched due to transition reversal and one would
> have to be careful not to free shared state from under the older, still
> active live patch. How would ->post_unpatch() distinguish between
> transition reversal and "normal" live patch disabling?  By
> klp_get_prev_state() != NULL?

Exactly. klp_get_prev_state() != NULL can be used in the
post_unpatch() to restore the original state when
the transition gets reverted.

See restore_console_loglevel() in lib/livepatch/test_klp_state2.c

> Perhaps transition reversal should be mentioned in the documentation?

Good point. I'll mention it in the documentation.

Best Regards,
Petr

Re: [RFC 3/5] livepatch: Allow to distinguish different version of system state changes

[Petr Mladek]

On Tue 2019-06-11 15:56:25, Petr Mladek wrote:
> 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>
> ---
>  include/linux/livepatch.h |  2 ++
>  kernel/livepatch/core.c   |  8 ++++++++
>  kernel/livepatch/state.c  | 40 +++++++++++++++++++++++++++++++++++++++-
>  kernel/livepatch/state.h  |  9 +++++++++
>  4 files changed, 58 insertions(+), 1 deletion(-)
>  create mode 100644 kernel/livepatch/state.h
> 
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index 591abdee30d7..8bc4c6cc3f3f 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -135,10 +135,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 {
>  	int id;

As suggested by Nicolay, there will be in v2:

	unsigned long id;

> +	int version;

It would make sense to make "version" unsigned as well.
I am just unsure about the size:

  + "unsigned long" looks like an overhead to me
  + "u8" might be enough

But I would stay on the safe side and use:

	unsigned int version;

Is anyone against?

Best Regards,
Petr

[RFC 4/5] livepatch: Documentation of the new API for tracking system state changes

[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>
---
 Documentation/livepatch/index.rst        |  1 +
 Documentation/livepatch/system-state.rst | 80 ++++++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+)
 create mode 100644 Documentation/livepatch/system-state.rst

diff --git a/Documentation/livepatch/index.rst b/Documentation/livepatch/index.rst
index edd291d51847..94bbbc2c8993 100644
--- a/Documentation/livepatch/index.rst
+++ b/Documentation/livepatch/index.rst
@@ -9,6 +9,7 @@ Kernel Livepatching
 
     livepatch
     callbacks
+    system-state
     cumulative-patches
     module-elf-format
     shadow-vars
diff --git a/Documentation/livepatch/system-state.rst b/Documentation/livepatch/system-state.rst
new file mode 100644
index 000000000000..3a35073a0b80
--- /dev/null
+++ b/Documentation/livepatch/system-state.rst
@@ -0,0 +1,80 @@
+====================
+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 not 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.
+
+1. 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.
-- 
2.16.4

Re: [RFC 4/5] livepatch: Documentation of the new API for tracking system state changes

[Joe Lawrence]

On Tue, Jun 11, 2019 at 03:56:26PM +0200, Petr Mladek wrote:
> Documentation explaining the motivation, capabilities, and usage
> of the new API for tracking system state changes.
> 
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> ---
>  Documentation/livepatch/index.rst        |  1 +
>  Documentation/livepatch/system-state.rst | 80 ++++++++++++++++++++++++++++++++
>  2 files changed, 81 insertions(+)
>  create mode 100644 Documentation/livepatch/system-state.rst
> 
> diff --git a/Documentation/livepatch/index.rst b/Documentation/livepatch/index.rst
> index edd291d51847..94bbbc2c8993 100644
> --- a/Documentation/livepatch/index.rst
> +++ b/Documentation/livepatch/index.rst
> @@ -9,6 +9,7 @@ Kernel Livepatching
>  
>      livepatch
>      callbacks
> +    system-state
>      cumulative-patches
>      module-elf-format
>      shadow-vars
> diff --git a/Documentation/livepatch/system-state.rst b/Documentation/livepatch/system-state.rst
> new file mode 100644
> index 000000000000..3a35073a0b80
> --- /dev/null
> +++ b/Documentation/livepatch/system-state.rst
> @@ -0,0 +1,80 @@
> [ ... snip ... ]
> +1. Livepatch compatibility

nit: 2. Livepatch compatibility

-- Joe

[RFC 5/5] livepatch: Selftests of the API for tracking system state changes

[Petr Mladek]

Four selftests for the new API.

Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 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..c43dc2f2e01d
--- /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 = 1,
+	}, { }
+};
+
+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("Joe Lawrence <joe.lawrence@redhat.com>");
+MODULE_DESCRIPTION("Livepatch test: livepatch demo");
diff --git a/lib/livepatch/test_klp_state2.c b/lib/livepatch/test_klp_state2.c
new file mode 100644
index 000000000000..2b4968fad60e
--- /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("Joe Lawrence <joe.lawrence@redhat.com>");
+MODULE_DESCRIPTION("Livepatch test: livepatch demo");
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

Re: [RFC 5/5] livepatch: Selftests of the API for tracking system state changes

[Miroslav Benes]

> diff --git a/lib/livepatch/test_klp_state.c b/lib/livepatch/test_klp_state.c
> new file mode 100644
> index 000000000000..c43dc2f2e01d
> --- /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 struct klp_state states[] = {
> +	{
> +		.id = CONSOLE_LOGLEVEL_STATE,
> +		.version = 1,

s/1/CONSOLE_LOGLEVEL_STATE_VERSION/

Miroslav

Re: [RFC 5/5] livepatch: Selftests of the API for tracking system state changes

[Joe Lawrence]

On Tue, Jun 11, 2019 at 03:56:27PM +0200, Petr Mladek wrote:
> 
> [ ... snip ... ]
> 
> diff --git a/lib/livepatch/test_klp_state.c b/lib/livepatch/test_klp_state.c
> new file mode 100644
> index 000000000000..c43dc2f2e01d
> --- /dev/null
> +++ b/lib/livepatch/test_klp_state.c
> @@ -0,0 +1,161 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2019 SUSE
> 
> [ ... snip ... ]
> 
> +MODULE_AUTHOR("Joe Lawrence <joe.lawrence@redhat.com>");

Feel free to update the module author for these.

-- Joe

Re: [RFC 0/5] livepatch: new API to track system state changes

[Josh Poimboeuf]

On Tue, Jun 11, 2019 at 03:56:22PM +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, hopefully, a simple and generic API that
> helps to keep and pass information between the livepatches.
> It is also usable to prevent loading incompatible livepatches.
> 
> There was also a related idea to add a sticky flag. It should be
> easy to add it later. It would perfectly fit into the new struct
> klp_state.
> 
> 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

I confess I missed most of the previous discussion, but from a first
read-through this seems reasonable, and the code looks simple and
self-contained.

-- 
Josh

Re: [RFC 0/5] livepatch: new API to track system state changes

[Joe Lawrence]

On Tue, Jun 11, 2019 at 03:56:22PM +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, hopefully, a simple and generic API that
> helps to keep and pass information between the livepatches.
> It is also usable to prevent loading incompatible livepatches.
>

Thanks for posting, Petr and aplogies for not getting to this RFC
earlier.  I think this strikes a reasonable balance between the (too) 
"simplified" versioning scheme that I posted a few weeks back, and what
I was afraid might have been too complicated callback-state-version
concept.

This RFC reads fairly straightforward and especially easy to review
given the included documentation and self-tests.  I'll add a few
comments per patch, but again, I like how this came out.
 
> There was also a related idea to add a sticky flag. It should be
> easy to add it later. It would perfectly fit into the new struct
> klp_state.

I think so, too.  It would indicate that the patch is introducing a
state which cannot be safely unloaded.  But we can talk about that at a
later time if/when we want to add that wrinkle to klp_state.
 
-- Joe

Re: [RFC 0/5] livepatch: new API to track system state changes

[Nicolai Stange]

Hi Petr,

> 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, hopefully, a simple and generic API that
> helps to keep and pass information between the livepatches.
> It is also usable to prevent loading incompatible livepatches.

I like it a lot, many thanks for doing this!

Minor remarks/questions will follow inline.

Nicolai