livepatch: Better integrate callbacks and shadow variables with the states API

Hi there,

this patchset is based on POC which I have sent before Linus Plumbers 2013 and presented it there, see
https://lore.kernel.org/r/[email protected]

The aim is to help maintaining lifetime of changes made by livepatch callbacks and shadow variables.

Key changes include:

- Per-state callbacks replace per-object callbacks, invoked only when a livepatch introduces or removes a state.
- Shadow variable lifetime is now tied to the corresponding livepatch state lifetime.
- The "version" field in struct klp_state has been replaced with the "block_disable" flag for improved compatibility handling.
- The "data" field has been removed from struct klp_state; shadow variables are now the recommended way to store state-related data.

Open question:

Shadow variables are connected with the state only when state.is_shadow is set. It might make sense to always connect shadow variables with the state using the same "id".

It would simplify the semantic and use. But it might be error prone when shadow variables are connected with a state by mistake.

It might get improved when we allow to create a shadow variable only when a patch supporting the state with the same "id" is (being) registered.

Motivation:

The basic livepatch functionality is to redirect problematic functions to a fixed or improved variants. In addition, there are two features helping with more problematic situations:

+ pre_patch(), post_patch(), pre_unpatch(), post_unpatch() callbacks might be called before and after the respective transitions. For example, post_patch() callback might enable some functionality at the end of the transition when the entire system is using the new code.

+ Shadow variables allow to add new items into structures or other data objects.

The practice has shown that these features were hard to use with the atomic replace feature. The new livepatch usually just adds more fixes. But it might also remove problematic ones.

Originally, any version of the livepatch was allowed to replace any older or newer version of the patch. It was not clear how to handle the extra features. The new patch did not know whether to run the callbacks or if the changes were already done by the current livepatch. Or if it has to revert some changes or free shadow variables whey they would no longer be supported.

It was even more complicated because only the callbacks from the newly installed livepatch were called. It means that older livepatch might not be able to revert changes supported only by newer livepatches.

The above problems were supposed to be solved by adding livepatch states. Each livepatch might define which states are supported. The states are versioned. The livepatch core checks if the newly installed livepatch is able to handle all states used by the currently installed livepatch.

Though the practice has shown that the states API was not easy to use either. It was not well connected with the callbacks and shadow variables. The states are per-patch. The callbacks are per-object. The livepatch does not know about the supported shadow variables at all.

Changes against POC:

+ Renamed callbacks back to pre_patch, post_patch, pre_unpatch, and post_unpatch [Josh]
+ Remove .version field completely [Miroslav]
+ Removed .data field
+ Fixed the use of "atomic replace" vs. "cumulative livepatch" terms [Miroslav]
+ Set .is_shadow in the test module modifying console_loglevel [Miroslav]
+ Revert pre_unpatch() callbacks when the transition is reverted. [Petr]
+ Improved and added selftests. Split into many patches [Petr]
+ Updated documentation [Petr]

[POC] https://lore.kernel.org/r/https://lore.kernel.org/r/[email protected]

Base commit: c45323b7560ec87c37c729b703c86ee65f136d75 (6.13-rc7+)

Petr Mladek (19):
livepatch: Add callbacks for introducing and removing states
livepatch: Allow to handle lifetime of shadow variables using the
livepatch state
selftests/livepatch: Use per-state callbacks in state API tests
livepatch: Add "block_disable" flag to per-state API and remove
versioning
livepatch: Remove "data" from struct klp_state selftests/livepatch: Remove callbacks from sysfs interface testing selftests/livepatch: Substitute hard-coded /sys/module path selftests/livepatch: Move basic tests for livepatching modules selftests/livepatch: Convert testing of multiple target modules selftests/livepatch: Create a simple selftest for state callbacks selftests/livepatch: Convert selftests for failing pre_patch callback selftests/livepatch: Convert selftest with blocked transition selftests/livepatch: Add more tests for state callbacks with blocked
transitions
selftests/livepatch: Convert selftests for testing callbacks with more
livepatches
selftests/livepatch: Do not use a livepatch with the obsolete
per-object callbacks in the basic selftests
selftests/livepatch: Remove obsolete test modules for per-object
callbacks
samples/livepatch: Replace sample module with obsolete per-object
callbacks
Documentation/livepatch: Update documentation for state, callbacks,
and shadow variables
livepatch: Remove obsolete per-object callbacks

 Documentation/livepatch/api.rst               |   2 +-
 Documentation/livepatch/callbacks.rst         | 166 +++---
 Documentation/livepatch/index.rst             |   4 +-
 Documentation/livepatch/shadow-vars.rst       |  47 +-
 Documentation/livepatch/system-state.rst      | 208 +++----
 include/linux/livepatch.h                     |  81 +--
 kernel/livepatch/core.c                       |  43 +-
 kernel/livepatch/core.h                       |  33 --
 kernel/livepatch/state.c                      | 179 +++++-
 kernel/livepatch/state.h                      |   9 +
 kernel/livepatch/transition.c                 |  20 +-
 samples/livepatch/Makefile                    |   5 +-
 .../livepatch/livepatch-callbacks-busymod.c   |  60 --
 samples/livepatch/livepatch-callbacks-demo.c  | 196 -------
 samples/livepatch/livepatch-callbacks-mod.c   |  41 --
 samples/livepatch/livepatch-speaker-fix.c     | 376 ++++++++++++
 samples/livepatch/livepatch-speaker-mod.c     | 203 +++++++
 samples/livepatch/livepatch-speaker.h         |  15 +
 tools/testing/selftests/livepatch/Makefile    |   2 +
 .../testing/selftests/livepatch/functions.sh  |  75 ++-
 .../selftests/livepatch/test-callbacks.sh     | 553 ------------------
 .../selftests/livepatch/test-livepatch.sh     |   4 +-
 .../testing/selftests/livepatch/test-order.sh | 295 ++++++++++
 .../livepatch/test-state-callbacks.sh         | 451 ++++++++++++++
 .../testing/selftests/livepatch/test-state.sh |  86 ++-
 .../testing/selftests/livepatch/test-sysfs.sh |  48 +-
 .../selftests/livepatch/test_modules/Makefile |   8 +-
 .../test_modules/test_klp_callbacks_busy.c    |  70 ---
 .../test_modules/test_klp_callbacks_demo.c    | 121 ----
 .../test_modules/test_klp_callbacks_demo2.c   |  93 ---
 .../test_modules/test_klp_callbacks_mod.c     |  24 -
 .../livepatch/test_modules/test_klp_speaker.c | 203 +++++++
 .../livepatch/test_modules/test_klp_speaker.h |  15 +
 .../test_modules/test_klp_speaker2.c          |   8 +
 .../test_modules/test_klp_speaker_livepatch.c | 407 +++++++++++++
 .../test_klp_speaker_livepatch2.c             |   5 +
 .../livepatch/test_modules/test_klp_state.c   | 143 ++---
 .../livepatch/test_modules/test_klp_state2.c  | 190 +-----
 .../livepatch/test_modules/test_klp_state3.c  |   2 +-
 39 files changed, 2664 insertions(+), 1827 deletions(-)
 delete mode 100644 samples/livepatch/livepatch-callbacks-busymod.c
 delete mode 100644 samples/livepatch/livepatch-callbacks-demo.c
 delete mode 100644 samples/livepatch/livepatch-callbacks-mod.c
 create mode 100644 samples/livepatch/livepatch-speaker-fix.c
 create mode 100644 samples/livepatch/livepatch-speaker-mod.c
 create mode 100644 samples/livepatch/livepatch-speaker.h
 delete mode 100755 tools/testing/selftests/livepatch/test-callbacks.sh
 create mode 100755 tools/testing/selftests/livepatch/test-order.sh
 create mode 100755 tools/testing/selftests/livepatch/test-state-callbacks.sh
 delete mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_callbacks_busy.c
 delete mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_callbacks_demo.c
 delete mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_callbacks_demo2.c
 delete mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_callbacks_mod.c
 create mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_speaker.c
 create mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_speaker.h
 create mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_speaker2.c
 create mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_speaker_livepatch.c
 create mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_speaker_livepatch2.c

2.47.1

The basic livepatch functionality is to redirect problematic functions to a fixed or improved variants. In addition, there are two features helping with more problematic situations:

+ pre_patch(), post_patch(), pre_unpatch(), post_unpatch() callbacks might be called before and after the respective transitions. For example, post_patch() callback might enable some functionality at the end of the transition when the entire system is using the new code.

+ Shadow variables allow to add new items into structures or other data objects.

The practice has shown that these features were hard to use with the atomic replace feature. The new livepatch usually just adds more fixes. But it might also remove problematic ones.

Originally, any version of the livepatch was allowed to replace any older or newer version of the patch. It was not clear how to handle the extra features. The new patch did not know whether to run the callbacks or if the changes were already done by the current livepatch. Or if it has to revert some changes or free shadow variables whey they would no longer be supported.

It was even more complicated because only the callbacks from the newly installed livepatch were called. It means that older livepatch might not be able to revert changes supported only by newer livepatches.

The above problems were supposed to be solved by adding livepatch states. Each livepatch might define which states are supported. The states are versioned. The livepatch core checks if the newly installed livepatch is able to handle all states used by the currently installed livepatch.

Though the practice has shown that the states API was not easy to use either. It was not well connected with the callbacks and shadow variables. The states are per-patch. The callbacks are per-object. The livepatch does not know about the supported shadow variables at all.

As a first step, new per-state callbacks are introduced:

+ "pre_patch" is called before the livepatch is applied but only when the state is new.

It might be used to allocate some memory. Or it might check if the state change is safe on the running system.

If it fails, the patch will not be enabled.

+ "post_patch" is called after the livepatch is applied but only when the state is new.

It might be used to enable using some functionality provided by the livepatch after the entire system is livepatched.

+ "pre_unpatch" is called before the livepatch is disabled or replaced.

When using the atomic replace, the callback is called only when the new livepatch does not support the related state. And it uses the implementation from the to-be-replaced livepatch.

The to-be-replaced livepatch needed the callback to allow disabling the livepatch anyway. The new livepatch does not need to know anything about the state.

It might be used to disable some functionality which will no longer be supported after the livepatch gets disabled.

+ "post_unpatch" is called after the livepatch was disabled or replaced. There are the same rules for the atomic replace replacement as for "pre_patch" callback.

It might be used for freeing some memory or unused shadow variables.

These callbacks are going to replace the existing ones. It would cause some changes:

+ The new callbacks are not called when a livepatched object is loaded or removed later.

The practice shows that per-object callbacks are not worth supporting. In a rare case, when a per-object callback is needed. the livepatch might register a custom module notifier.

+ The new callbacks are called only when the state is introduced or removed. It does not handle the situation when the newly installed livepatch continues using an existing state.

The practice shows that this is exactly what is needed. In the rare case when this is not enough, an extra takeover might be done in the module->init() callback.

The per-state callbacks are called in similar code paths as the per-object ones. Especially, the ordering against the other operations is the same. Though, there are some obvious and less obvious changes:

+ The per-state callbacks are called for the entire patch instead of loaded object. It means that they called outside the for-each-object cycle.

+ The per-state callbacks are called when a state is introduced or obsoleted. Both variants might happen when the atomic replace is used.

+ In __klp_enable_patch(), the per-state callbacks are called before the smp_wmb() while the per-object ones are called later.

The new location makes more sense. The setup of the state should be ready before the system processes start being transitioned.

The per-object callbacks were called after the barrier. They were using and already existing for-cycle. Nobody though about the potential ordering problem when it was implemented.

---
 include/linux/livepatch.h     |  34 ++++++++
 kernel/livepatch/core.c       |   9 +++
 kernel/livepatch/state.c      | 141 ++++++++++++++++++++++++++++++++++
 kernel/livepatch/state.h      |   8 ++
 kernel/livepatch/transition.c |  15 ++++
 5 files changed, 207 insertions(+)

index 51a258c24ff5..79dddf3dbd52 100644
@@ -129,15 +129,49 @@ struct klp_object {
 	bool patched;
 };
 
+struct klp_patch;
+struct klp_state;
+
+/**
+ * struct klp_state_callbacks - callbacks manipulating the state
+ * @pre_patch:		 executed only when the state is being enabled
+ *			 before code patching
+ * @post_patch:		 executed only when the state is being enabled
+ *			 after code patching
+ * @pre_unpatch:	 executed only when the state is being disabled
+ *			 before code unpatching
+ * @post_unpatch:	 executed only when the state is being disabled
+ *			 after code unpatching
+ * @pre_patch_succeeded: internal state used by a rollback on error
+ *
+ * All callbacks are optional.
+ *
+ * @pre_patch callback returns 0 on success and an error code otherwise.
+ *
+ * Any error prevents enabling the livepatch. @post_unpatch() callbacks are
+ * then called to rollback @pre_patch callbacks which has already succeeded
+ * before. Also @post_patch callbacks are called for to-be-removed states
+ * to rollback pre_unpatch() callbacks when they were called.
+ */
+struct klp_state_callbacks {
+	int (*pre_patch)(struct klp_patch *patch, struct klp_state *state);
+	void (*post_patch)(struct klp_patch *patch, struct klp_state *state);
+	void (*pre_unpatch)(struct klp_patch *patch, struct klp_state *state);
+	void (*post_unpatch)(struct klp_patch *patch, struct klp_state *state);
+	bool pre_patch_succeeded;
+};
+
 /**
  * struct klp_state - state of the system modified by the livepatch
  * @id:		system state identifier (non-zero)
  * @version:	version of the change
+ * @callbacks:	optional callbacks used when enabling or disabling the state
  * @data:	custom data
  */
 struct klp_state {
 	unsigned long id;
 	unsigned int version;
+	struct klp_state_callbacks callbacks;
 	void *data;
 };
 

index 3c21c31796db..527fdb0a6b0a 100644
@@ -986,6 +986,8 @@ static int __klp_disable_patch(struct klp_patch *patch)
 
 	klp_init_transition(patch, KLP_TRANSITION_UNPATCHED);
 
+	klp_states_pre_unpatch(patch);
+
 	klp_for_each_object(patch, obj)
 		if (obj->patched)
 			klp_pre_unpatch_callback(obj);

@@ -1021,6 +1023,13 @@ static int __klp_enable_patch(struct klp_patch *patch)
 
 	klp_init_transition(patch, KLP_TRANSITION_PATCHED);
 
+	ret = klp_states_pre_patch(patch);
+	if (ret)
+		goto err;
+
+	if (patch->replace)
+		klp_states_pre_unpatch_replaced(patch);
+
 	/*
 	 * Enforce the order of the func->transition writes in
 	 * klp_init_transition() and the ops->func_stack writes in

index 49d9c16e8762..65c0c2cde04c 100644
@@ -5,5 +5,13 @@
 #include <linux/livepatch.h>
 
 bool klp_is_patch_compatible(struct klp_patch *patch);
+int klp_states_pre_patch(struct klp_patch *patch);
+void klp_states_post_patch(struct klp_patch *patch);
+void klp_states_pre_unpatch(struct klp_patch *patch);
+void klp_states_post_unpatch(struct klp_patch *patch);
+
+void klp_states_pre_unpatch_replaced(struct klp_patch *patch);
+void klp_states_post_unpatch_replaced(struct klp_patch *patch);
+void klp_states_post_patch_replaced(struct klp_patch *patch);
 
 #endif /* _LIVEPATCH_STATE_H */

index ba069459c101..f3dce9fe9897 100644
@@ -12,6 +12,7 @@
 #include <linux/static_call.h>
 #include "core.h"
 #include "patch.h"
+#include "state.h"
 #include "transition.h"
 
 #define MAX_STACK_ENTRIES  100

@@ -101,6 +102,7 @@ static void klp_complete_transition(void)
 	if (klp_transition_patch->replace && klp_target_state == KLP_TRANSITION_PATCHED) {
 		klp_unpatch_replaced_patches(klp_transition_patch);
 		klp_discard_nops(klp_transition_patch);
+		klp_states_post_unpatch_replaced(klp_transition_patch);
 	}
 
 	if (klp_target_state == KLP_TRANSITION_UNPATCHED) {

@@ -140,6 +142,19 @@ static void klp_complete_transition(void)
 		task->patch_state = KLP_TRANSITION_IDLE;
 	}
 
+	if (klp_target_state == KLP_TRANSITION_PATCHED) {
+		klp_states_post_patch(klp_transition_patch);
+	} else if (klp_target_state == KLP_TRANSITION_UNPATCHED) {
+		/*
+		 * Re-enable states which should have been replaced but
+		 * the transition was cancelled or reverted.
+		 */
+		if (klp_transition_patch->replace)
+			klp_states_post_patch_replaced(klp_transition_patch);
+
+		klp_states_post_unpatch(klp_transition_patch);
+	}
+
 	klp_for_each_object(klp_transition_patch, obj) {
 		if (!klp_is_object_loaded(obj))
 			continue;

2.47.1

Managing the lifetime of shadow variables becomes challenging when atomic replace is used. The new patch cannot determine whether a shadow variable has already been used by a previous live patch or if there is a shadow variable that is no longer in use.

Shadow variables are typically used alongside callbacks. At a minimum, the @post_unpatch callback is called to free shadow variables that are no longer needed. Additionally, @post_patch and @pre_unpatch callbacks are sometimes used to enable or disable the use of shadow variables. This is necessary when the shadow variable can only be used when the entire system is capable of handling it.

The complexity increases when using the atomic replace feature, as only the callbacks from the new live patch are executed. Newly created live patches might manage obsolete shadow variables, ensuring the upgrade functions correctly. However, older live patches are unaware of shadow variables introduced later, which could lead to leaks during a downgrade. Additionally, these leaked variables might retain outdated information, potentially causing issues if those variables are reused in a subsequent upgrade.

These issues are better addressed with the new callbacks associated with a live patch state. These callbacks are triggered both when the states are first introduced and when they become obsolete. Additionally, the callbacks are invoked from the patch that originally supported the state, ensuring that even downgrades are handled safely.

Let’s formalize the process: Associate a shadow variable with a live patch state by setting the "state.is_shadow" flag and using the same "id" in both struct klp_shadow and struct klp_state.

The shadow variable will then share the same lifetime as the livepatch state, allowing obsolete shadow variables to be automatically freed without requiring an additional callback.

A generic callback will free the shadow variables using the state->callbacks.shadow_dtor callback, if provided.

---
 include/linux/livepatch.h | 15 ++++++++++-----
 kernel/livepatch/state.c  |  3 +++
 2 files changed, 13 insertions(+), 5 deletions(-)

index 79dddf3dbd52..c624f1105663 100644
@@ -132,6 +132,11 @@ struct klp_object {
 struct klp_patch;
 struct klp_state;
 
+typedef int (*klp_shadow_ctor_t)(void *obj,
+				 void *shadow_data,
+				 void *ctor_data);
+typedef void (*klp_shadow_dtor_t)(void *obj, void *shadow_data);
+
 /**
  * struct klp_state_callbacks - callbacks manipulating the state
  * @pre_patch:		 executed only when the state is being enabled

@@ -142,6 +147,7 @@ struct klp_state;
  *			 before code unpatching
  * @post_unpatch:	 executed only when the state is being disabled
  *			 after code unpatching
+ * @shadow_dtor:	 destructor for the related shadow variable
  * @pre_patch_succeeded: internal state used by a rollback on error
  *
  * All callbacks are optional.

@@ -158,6 +164,7 @@ struct klp_state_callbacks {
 	void (*post_patch)(struct klp_patch *patch, struct klp_state *state);
 	void (*pre_unpatch)(struct klp_patch *patch, struct klp_state *state);
 	void (*post_unpatch)(struct klp_patch *patch, struct klp_state *state);
+	klp_shadow_dtor_t shadow_dtor;
 	bool pre_patch_succeeded;
 };
 

@@ -166,12 +173,15 @@ struct klp_state_callbacks {
  * @id:		system state identifier (non-zero)
  * @version:	version of the change
  * @callbacks:	optional callbacks used when enabling or disabling the state
+ * @is_shadow:	the state handles lifetime of a shadow variable with
+ *		the same @id
  * @data:	custom data
  */
 struct klp_state {
 	unsigned long id;
 	unsigned int version;
 	struct klp_state_callbacks callbacks;
+	bool is_shadow;
 	void *data;
 };
 

@@ -246,11 +256,6 @@ static inline bool klp_have_reliable_stack(void)
 	       IS_ENABLED(CONFIG_HAVE_RELIABLE_STACKTRACE);
 }
 
-typedef int (*klp_shadow_ctor_t)(void *obj,
-				 void *shadow_data,
-				 void *ctor_data);
-typedef void (*klp_shadow_dtor_t)(void *obj, void *shadow_data);
-
 void *klp_shadow_get(void *obj, unsigned long id);
 void *klp_shadow_alloc(void *obj, unsigned long id,
 		       size_t size, gfp_t gfp_flags,

index bf7ed988d2bb..16ad695b1e88 100644
@@ -201,6 +201,9 @@ void klp_states_post_unpatch(struct klp_patch *patch)
 		if (state->callbacks.post_unpatch)
 			state->callbacks.post_unpatch(patch, state);
 
+		if (state->is_shadow)
+			klp_shadow_free_all(state->id, state->callbacks.shadow_dtor);
+
 		state->callbacks.pre_patch_succeeded = 0;
 	}
 }

2.47.1

Recent updates to the live patch core have enabled the integration of states, shadow variables, and callbacks. Utilize these new features in the state tests.

Use the shadow variable API to store the original log level, as it is better suited for this purpose than directly accessing the .data pointer in klp_state.

A key advantage is that the shadow variable is preserved when the current patch is replaced by a new version, eliminating the need to copy the pointer.

Additionally, the lifetime of the shadow variable is now tied to the lifetime of the state and will be automatically freed when it is no longer supported.

This results into the following changes in the code:

+ Rename CONSOLE_LOGLEVEL_STATE -> CONSOLE_LOGLEVEL_FIX_ID because it will be used also the for shadow variable

+ Remove the extra code for module coming and going states because the new callbacks are per-state.

+ Remove obsolete callbacks which used to be needed to transfer the shadow state data pointer.

+ The post_unpatch_callback() is retained solely for testing purposes. Its primary function is to print a message confirming that it has been called. The shadow variable is automatically freed by the core kernel code when the state is no longer supported by any live patch.

+ The shadow_console_loglevel_dtor() function is added solely for testing purposes. It simply prints a message to confirm that it has been called. Since the shadow variable is straightforward, its memory is automatically freed along with the associated struct klp_shadow.

The versioning of the state still prevents downgrades, but this issue is now largely mitigated because callbacks are no longer required to transfer or free shadow variables. The versioning is going to be removed in a followup commit.

---
 .../testing/selftests/livepatch/test-state.sh |  52 ++---
 .../livepatch/test_modules/test_klp_state.c   | 138 +++++++------
 .../livepatch/test_modules/test_klp_state2.c  | 190 +-----------------
 3 files changed, 95 insertions(+), 285 deletions(-)

index 04b66380f8a0..2f4a5109fdf5 100755
@@ -22,21 +22,21 @@ unload_lp $MOD_LIVEPATCH
 check_result "% insmod test_modules/$MOD_LIVEPATCH.ko
 livepatch: enabling patch '$MOD_LIVEPATCH'
 livepatch: '$MOD_LIVEPATCH': initializing patching transition
-$MOD_LIVEPATCH: pre_patch_callback: vmlinux
+$MOD_LIVEPATCH: pre_patch_callback: state 1
 $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: post_patch_callback: state 1
 $MOD_LIVEPATCH: fix_console_loglevel: fixing console_loglevel
 livepatch: '$MOD_LIVEPATCH': patching complete
 % echo 0 > $SYSFS_KLP_DIR/$MOD_LIVEPATCH/enabled
 livepatch: '$MOD_LIVEPATCH': initializing unpatching transition
-$MOD_LIVEPATCH: pre_unpatch_callback: vmlinux
+$MOD_LIVEPATCH: pre_unpatch_callback: state 1
 $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
+$MOD_LIVEPATCH: post_unpatch_callback: state 1 (nope)
+$MOD_LIVEPATCH: shadow_conosle_loglevel_dtor: freeing space for the stored console_loglevel
 livepatch: '$MOD_LIVEPATCH': unpatching complete
 % rmmod $MOD_LIVEPATCH"
 

@@ -54,32 +54,28 @@ unload_lp $MOD_LIVEPATCH2
 check_result "% insmod test_modules/$MOD_LIVEPATCH.ko
 livepatch: enabling patch '$MOD_LIVEPATCH'
 livepatch: '$MOD_LIVEPATCH': initializing patching transition
-$MOD_LIVEPATCH: pre_patch_callback: vmlinux
+$MOD_LIVEPATCH: pre_patch_callback: state 1
 $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: post_patch_callback: state 1
 $MOD_LIVEPATCH: fix_console_loglevel: fixing console_loglevel
 livepatch: '$MOD_LIVEPATCH': patching complete
 % insmod test_modules/$MOD_LIVEPATCH2.ko
 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 > $SYSFS_KLP_DIR/$MOD_LIVEPATCH2/enabled
 livepatch: '$MOD_LIVEPATCH2': initializing unpatching transition
-$MOD_LIVEPATCH2: pre_unpatch_callback: vmlinux
+$MOD_LIVEPATCH2: pre_unpatch_callback: state 1
 $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
+$MOD_LIVEPATCH2: post_unpatch_callback: state 1 (nope)
+$MOD_LIVEPATCH2: shadow_conosle_loglevel_dtor: freeing space for the stored console_loglevel
 livepatch: '$MOD_LIVEPATCH2': unpatching complete
 % rmmod $MOD_LIVEPATCH2"
 

@@ -99,42 +95,34 @@ unload_lp $MOD_LIVEPATCH3
 check_result "% insmod test_modules/$MOD_LIVEPATCH2.ko
 livepatch: enabling patch '$MOD_LIVEPATCH2'
 livepatch: '$MOD_LIVEPATCH2': initializing patching transition
-$MOD_LIVEPATCH2: pre_patch_callback: vmlinux
+$MOD_LIVEPATCH2: pre_patch_callback: state 1
 $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: post_patch_callback: state 1
 $MOD_LIVEPATCH2: fix_console_loglevel: fixing console_loglevel
 livepatch: '$MOD_LIVEPATCH2': patching complete
 % insmod test_modules/$MOD_LIVEPATCH3.ko
 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
 % insmod test_modules/$MOD_LIVEPATCH2.ko
 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 > $SYSFS_KLP_DIR/$MOD_LIVEPATCH2/enabled
 livepatch: '$MOD_LIVEPATCH2': initializing unpatching transition
-$MOD_LIVEPATCH2: pre_unpatch_callback: vmlinux
+$MOD_LIVEPATCH2: pre_unpatch_callback: state 1
 $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
+$MOD_LIVEPATCH2: post_unpatch_callback: state 1 (nope)
+$MOD_LIVEPATCH2: shadow_conosle_loglevel_dtor: freeing space for the stored console_loglevel
 livepatch: '$MOD_LIVEPATCH2': unpatching complete
 % rmmod $MOD_LIVEPATCH2
 % rmmod $MOD_LIVEPATCH3"

@@ -152,11 +140,11 @@ unload_lp $MOD_LIVEPATCH2
 check_result "% insmod test_modules/$MOD_LIVEPATCH2.ko
 livepatch: enabling patch '$MOD_LIVEPATCH2'
 livepatch: '$MOD_LIVEPATCH2': initializing patching transition
-$MOD_LIVEPATCH2: pre_patch_callback: vmlinux
+$MOD_LIVEPATCH2: pre_patch_callback: state 1
 $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: post_patch_callback: state 1
 $MOD_LIVEPATCH2: fix_console_loglevel: fixing console_loglevel
 livepatch: '$MOD_LIVEPATCH2': patching complete
 % insmod test_modules/$MOD_LIVEPATCH.ko

@@ -164,12 +152,12 @@ livepatch: Livepatch patch ($MOD_LIVEPATCH) is not compatible with the already i
 insmod: ERROR: could not insert module test_modules/$MOD_LIVEPATCH.ko: Invalid parameters
 % echo 0 > $SYSFS_KLP_DIR/$MOD_LIVEPATCH2/enabled
 livepatch: '$MOD_LIVEPATCH2': initializing unpatching transition
-$MOD_LIVEPATCH2: pre_unpatch_callback: vmlinux
+$MOD_LIVEPATCH2: pre_unpatch_callback: state 1
 $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
+$MOD_LIVEPATCH2: post_unpatch_callback: state 1 (nope)
+$MOD_LIVEPATCH2: shadow_conosle_loglevel_dtor: freeing space for the stored console_loglevel
 livepatch: '$MOD_LIVEPATCH2': unpatching complete
 % rmmod $MOD_LIVEPATCH2"
 

2.47.1

This commit enhances the livepatch state API with a new flag, "block_disable", and removes the previously introduced per-state versioning mechanism.

The "block_disable" flag addresses scenarios where reverting certain system changes introduced by a livepatch is undesirable due to complexity, risk, or other factors. When set, this flag prevents:

- Disabling the livepatch entirely.

- Replacing the livepatch with an older version that doesn't support the blocked state.

This ensures that critical system modifications remain in effect, even when updating or reverting livepatches.

The per-state versioning mechanism, intended to facilitate modifications to existing states, has proven unnecessary in practice and is therefore removed. Alternative approaches, such as introducing new states, can achieve the same results if needed.

These changes introduce a new approach to compatibility between livepatches. It is no longer defined by versioning, which lacked specific semantics. Instead, compatibility is determined by whether a livepatch can disable the system state changes caused by the use of callbacks and shadow variables.

Consequently, compatibility is now only checked for livepatches using atomic replacement. This is because only these livepatches can disable an existing state when enabled.

The related self-test has been updated to reflect these changes. It utilizes three livepatches built from the same source, with new command-line options to specify whether a livepatch supports and can disable a given state change.

---
 include/linux/livepatch.h                     |  4 +-
 kernel/livepatch/core.c                       |  7 ++++
 kernel/livepatch/state.c                      | 35 ++++++++++++----
 kernel/livepatch/state.h                      |  1 +
 .../testing/selftests/livepatch/test-state.sh | 40 ++++++++++++-------
 .../livepatch/test_modules/test_klp_state.c   | 27 +++++++------
 .../livepatch/test_modules/test_klp_state2.c  |  2 -
 .../livepatch/test_modules/test_klp_state3.c  |  2 +-
 8 files changed, 78 insertions(+), 40 deletions(-)

index c624f1105663..56e71d488e71 100644
@@ -171,16 +171,16 @@ struct klp_state_callbacks {
 /**
  * struct klp_state - state of the system modified by the livepatch
  * @id:		system state identifier (non-zero)
- * @version:	version of the change
  * @callbacks:	optional callbacks used when enabling or disabling the state
+ * @block_disable: the state disablement is not supported
  * @is_shadow:	the state handles lifetime of a shadow variable with
  *		the same @id
  * @data:	custom data
  */
 struct klp_state {
 	unsigned long id;
-	unsigned int version;
 	struct klp_state_callbacks callbacks;
+	bool block_disable;
 	bool is_shadow;
 	void *data;
 };

index 527fdb0a6b0a..4d244eef0e53 100644
@@ -374,6 +374,13 @@ static ssize_t enabled_store(struct kobject *kobj, struct kobj_attribute *attr,
 		goto out;
 	}
 
+	if (patch->enabled && klp_patch_disable_blocked(patch)) {
+		pr_err("The livepatch '%s' does not support disable\n",
+		       patch->mod->name);
+		ret = -EINVAL;
+		goto out;
+	}
+
 	/*
 	 * Allow to reverse a pending transition in both ways. It might be
 	 * necessary to complete the transition without forcing and breaking

index 16ad695b1e88..a9af511e5d9e 100644
@@ -83,7 +83,12 @@ struct klp_state *klp_get_prev_state(unsigned long id)
 }
 EXPORT_SYMBOL_GPL(klp_get_prev_state);
 
-/* Check if the patch is able to deal with the existing system state. */
+/*
+ * Check if the new patch is able to deal with the existing system state.
+ * Used only for livepatches with the atomic replace enabled. The patch either
+ * has to support the existing state or the existing patch must be able
+ * to disable it.
+ */
 static bool klp_is_state_compatible(struct klp_patch *patch,
 				    struct klp_state *old_state)
 {

@@ -91,23 +96,25 @@ static bool klp_is_state_compatible(struct klp_patch *patch,
 
 	state = klp_get_state(patch, old_state->id);
 
-	/* A cumulative livepatch must handle all already modified states. */
-	if (!state)
-		return !patch->replace;
+	if (!state && old_state->block_disable)
+		return false;
 
-	return state->version >= old_state->version;
+	return true;
 }
 
 /*
- * 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.
+ * Check if the new livepatch could atomically replace existing ones.
+ * It must either support the existing states. Or the existing livepatches
+ * must be able to disable the obsolete states.
  */
 bool klp_is_patch_compatible(struct klp_patch *patch)
 {
 	struct klp_patch *old_patch;
 	struct klp_state *old_state;
 
+	if (!patch->replace)
+		return true;
+
 	klp_for_each_patch(old_patch) {
 		klp_for_each_state(old_patch, old_state) {
 			if (!klp_is_state_compatible(patch, old_state))

@@ -118,6 +125,18 @@ bool klp_is_patch_compatible(struct klp_patch *patch)
 	return true;
 }
 
+bool klp_patch_disable_blocked(struct klp_patch *patch)
+{
+	struct klp_state *state;
+
+	klp_for_each_state(patch, state) {
+		if (state->block_disable)
+			return true;
+	}
+
+	return false;
+}
+
 static bool is_state_in_other_patches(struct klp_patch *patch,
 				      struct klp_state *state)
 {

index 65c0c2cde04c..0620fd692820 100644
@@ -5,6 +5,7 @@
 #include <linux/livepatch.h>
 
 bool klp_is_patch_compatible(struct klp_patch *patch);
+bool klp_patch_disable_blocked(struct klp_patch *patch);
 int klp_states_pre_patch(struct klp_patch *patch);
 void klp_states_post_patch(struct klp_patch *patch);
 void klp_states_pre_unpatch(struct klp_patch *patch);

index 2f4a5109fdf5..72e9b4f0b629 100755
@@ -132,12 +132,15 @@ livepatch: '$MOD_LIVEPATCH2': unpatching complete
 
 start_test "incompatible cumulative livepatches"
 
-load_lp $MOD_LIVEPATCH2
-load_failing_mod $MOD_LIVEPATCH
-disable_lp $MOD_LIVEPATCH2
+load_lp $MOD_LIVEPATCH2 state_block_disable=1
+load_failing_mod $MOD_LIVEPATCH no_state=1
+# load the livepatch again with default features (state and disable supported)
+load_lp $MOD_LIVEPATCH
 unload_lp $MOD_LIVEPATCH2
+disable_lp $MOD_LIVEPATCH
+unload_lp $MOD_LIVEPATCH
 
-check_result "% insmod test_modules/$MOD_LIVEPATCH2.ko
+check_result "% insmod test_modules/$MOD_LIVEPATCH2.ko state_block_disable=1
 livepatch: enabling patch '$MOD_LIVEPATCH2'
 livepatch: '$MOD_LIVEPATCH2': initializing patching transition
 $MOD_LIVEPATCH2: pre_patch_callback: state 1

@@ -147,18 +150,25 @@ livepatch: '$MOD_LIVEPATCH2': completing patching transition
 $MOD_LIVEPATCH2: post_patch_callback: state 1
 $MOD_LIVEPATCH2: fix_console_loglevel: fixing console_loglevel
 livepatch: '$MOD_LIVEPATCH2': patching complete
-% insmod test_modules/$MOD_LIVEPATCH.ko
+% insmod test_modules/$MOD_LIVEPATCH.ko no_state=1
 livepatch: Livepatch patch ($MOD_LIVEPATCH) is not compatible with the already installed livepatches.
 insmod: ERROR: could not insert module test_modules/$MOD_LIVEPATCH.ko: Invalid parameters
-% echo 0 > $SYSFS_KLP_DIR/$MOD_LIVEPATCH2/enabled
-livepatch: '$MOD_LIVEPATCH2': initializing unpatching transition
-$MOD_LIVEPATCH2: pre_unpatch_callback: state 1
-$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: state 1 (nope)
-$MOD_LIVEPATCH2: shadow_conosle_loglevel_dtor: freeing space for the stored console_loglevel
-livepatch: '$MOD_LIVEPATCH2': unpatching complete
-% rmmod $MOD_LIVEPATCH2"
+% insmod test_modules/$MOD_LIVEPATCH.ko
+livepatch: enabling patch '$MOD_LIVEPATCH'
+livepatch: '$MOD_LIVEPATCH': initializing patching transition
+livepatch: '$MOD_LIVEPATCH': starting patching transition
+livepatch: '$MOD_LIVEPATCH': completing patching transition
+livepatch: '$MOD_LIVEPATCH': patching complete
+% rmmod $MOD_LIVEPATCH2
+% echo 0 > $SYSFS_KLP_DIR/$MOD_LIVEPATCH/enabled
+livepatch: '$MOD_LIVEPATCH': initializing unpatching transition
+$MOD_LIVEPATCH: pre_unpatch_callback: state 1
+$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: state 1 (nope)
+$MOD_LIVEPATCH: shadow_conosle_loglevel_dtor: freeing space for the stored console_loglevel
+livepatch: '$MOD_LIVEPATCH': unpatching complete
+% rmmod $MOD_LIVEPATCH"
 
 exit 0

index 7f601898ef7c..518229815449 100644
@@ -11,17 +11,6 @@
 
 #define CONSOLE_LOGLEVEL_FIX_ID 1
 
-/*
- * Version of the state which defines compatibility of livepaches.
- * The value is artificial. It set just for testing the compatibility
- * checks. In reality, all versions are compatible because all
- * the callbacks do nothing and the shadow variable clean up
- * is done by the core.
- */
-#ifndef CONSOLE_LOGLEVEL_FIX_VERSION
-#define CONSOLE_LOGLEVEL_FIX_VERSION 1
-#endif
-
 static struct klp_patch patch;
 
 static int allocate_loglevel_state(void)

@@ -132,7 +121,6 @@ static struct klp_object objs[] = {
 static struct klp_state states[] = {
 	{
 		.id = CONSOLE_LOGLEVEL_FIX_ID,
-		.version = CONSOLE_LOGLEVEL_FIX_VERSION,
 		.is_shadow = true,
 		.callbacks = {
 			.pre_patch = pre_patch_callback,

@@ -151,8 +139,23 @@ static struct klp_patch patch = {
 	.replace = true,
 };
 
+static bool state_block_disable;
+
+module_param(state_block_disable, bool, 0600);
+MODULE_PARM_DESC(state_block_disable, "Set to 1 to pretend that the livepatch is not capable of disabling the state (default = 0).");
+
+static bool no_state;
+
+module_param(no_state, bool, 0600);
+MODULE_PARM_DESC(no_state, "Set to 1 when the livepatch should not support the state (default = 0).");
+
 static int test_klp_callbacks_demo_init(void)
 {
+	states[0].block_disable = state_block_disable;
+
+	if (no_state)
+		patch.states = NULL;
+
 	return klp_enable_patch(&patch);
 }
 

index 128855764bf8..b8fe1ca2d802 100644
@@ -1,7 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0
 // Copyright (C) 2019 SUSE
 
-#define CONSOLE_LOGLEVEL_FIX_VERSION 2
-
 /* The console loglevel fix is the same in the next cumulative patch. */
 #include "test_klp_state.c"

index 9226579d10c5..b8fe1ca2d802 100644
@@ -2,4 +2,4 @@
 // Copyright (C) 2019 SUSE
 
 /* The console loglevel fix is the same in the next cumulative patch. */
-#include "test_klp_state2.c"
+#include "test_klp_state.c"

2.47.1

The "data" pointer in "struct klp_state" is associated with the lifetime of the livepatch module, not the livepatch state. This means it's lost when a livepatch is replaced, even if the new livepatch supports the same state.

Shadow variables provide a more reliable way to attach data to a livepatch state. Their lifetime can be tied to the state's lifetime by:

- Sharing the same "id"
- Setting "is_shadow" in "struct klp_state"

Removing the "data" pointer prevents potential issues once per-object callbacks are removed, as it cannot be used securely in that context.

---
 include/linux/livepatch.h | 2 --
 1 file changed, 2 deletions(-)

index 56e71d488e71..d02d7a616338 100644
@@ -175,14 +175,12 @@ struct klp_state_callbacks {
  * @block_disable: the state disablement is not supported
  * @is_shadow:	the state handles lifetime of a shadow variable with
  *		the same @id
- * @data:	custom data
  */
 struct klp_state {
 	unsigned long id;
 	struct klp_state_callbacks callbacks;
 	bool block_disable;
 	bool is_shadow;
-	void *data;
 };
 
 /**

2.47.1

This commit removes the use of callbacks in the sysfs interface testing. The callbacks are not necessary for this testing and create unnecessary noise.

This commit replaces the test modules that use the obsolete per-object callbacks with a simple test module in a "Hello World" style and a corresponding livepatch.

These new modules will be extended in the future to include an optional integration of livepatch states, per-state callbacks, and shadow variables. They will be used for testing all the new features.

---
 .../testing/selftests/livepatch/test-sysfs.sh | 48 ++++++---------
 .../selftests/livepatch/test_modules/Makefile |  2 +
 .../livepatch/test_modules/test_klp_speaker.c | 38 ++++++++++++
 .../test_modules/test_klp_speaker_livepatch.c | 61 +++++++++++++++++++
 4 files changed, 121 insertions(+), 28 deletions(-)
 create mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_speaker.c
 create mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_speaker_livepatch.c

index 2c91428d2997..c565e6005710 100755
@@ -43,8 +43,8 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete
 
 start_test "sysfs test object/patched"
 
-MOD_LIVEPATCH=test_klp_callbacks_demo
-MOD_TARGET=test_klp_callbacks_mod
+MOD_LIVEPATCH=test_klp_speaker_livepatch
+MOD_TARGET=test_klp_speaker
 load_lp $MOD_LIVEPATCH
 
 # check the "patch" file changes as target module loads/unloads

@@ -57,32 +57,24 @@ check_sysfs_value  "$MOD_LIVEPATCH" "$MOD_TARGET/patched" "0"
 disable_lp $MOD_LIVEPATCH
 unload_lp $MOD_LIVEPATCH
 
-check_result "% insmod test_modules/test_klp_callbacks_demo.ko
-livepatch: enabling patch 'test_klp_callbacks_demo'
-livepatch: 'test_klp_callbacks_demo': initializing patching transition
-test_klp_callbacks_demo: pre_patch_callback: vmlinux
-livepatch: 'test_klp_callbacks_demo': starting patching transition
-livepatch: 'test_klp_callbacks_demo': completing patching transition
-test_klp_callbacks_demo: post_patch_callback: vmlinux
-livepatch: 'test_klp_callbacks_demo': patching complete
-% insmod test_modules/test_klp_callbacks_mod.ko
-livepatch: applying patch 'test_klp_callbacks_demo' to loading module 'test_klp_callbacks_mod'
-test_klp_callbacks_demo: pre_patch_callback: test_klp_callbacks_mod -> [MODULE_STATE_COMING] Full formed, running module_init
-test_klp_callbacks_demo: post_patch_callback: test_klp_callbacks_mod -> [MODULE_STATE_COMING] Full formed, running module_init
-test_klp_callbacks_mod: test_klp_callbacks_mod_init
-% rmmod test_klp_callbacks_mod
-test_klp_callbacks_mod: test_klp_callbacks_mod_exit
-test_klp_callbacks_demo: pre_unpatch_callback: test_klp_callbacks_mod -> [MODULE_STATE_GOING] Going away
-livepatch: reverting patch 'test_klp_callbacks_demo' on unloading module 'test_klp_callbacks_mod'
-test_klp_callbacks_demo: post_unpatch_callback: test_klp_callbacks_mod -> [MODULE_STATE_GOING] Going away
-% echo 0 > $SYSFS_KLP_DIR/test_klp_callbacks_demo/enabled
-livepatch: 'test_klp_callbacks_demo': initializing unpatching transition
-test_klp_callbacks_demo: pre_unpatch_callback: vmlinux
-livepatch: 'test_klp_callbacks_demo': starting unpatching transition
-livepatch: 'test_klp_callbacks_demo': completing unpatching transition
-test_klp_callbacks_demo: post_unpatch_callback: vmlinux
-livepatch: 'test_klp_callbacks_demo': unpatching complete
-% rmmod test_klp_callbacks_demo"
+check_result "% insmod test_modules/$MOD_LIVEPATCH.ko
+livepatch: enabling patch '$MOD_LIVEPATCH'
+livepatch: '$MOD_LIVEPATCH': initializing patching transition
+livepatch: '$MOD_LIVEPATCH': starting patching transition
+livepatch: '$MOD_LIVEPATCH': completing patching transition
+livepatch: '$MOD_LIVEPATCH': patching complete
+% insmod test_modules/$MOD_TARGET.ko
+livepatch: applying patch '$MOD_LIVEPATCH' to loading module '$MOD_TARGET'
+$MOD_TARGET: ${MOD_TARGET}_init
+% rmmod $MOD_TARGET
+$MOD_TARGET: ${MOD_TARGET}_exit
+livepatch: reverting patch '$MOD_LIVEPATCH' on unloading module '$MOD_TARGET'
+% echo 0 > $SYSFS_KLP_DIR/$MOD_LIVEPATCH/enabled
+livepatch: '$MOD_LIVEPATCH': initializing unpatching transition
+livepatch: '$MOD_LIVEPATCH': starting unpatching transition
+livepatch: '$MOD_LIVEPATCH': completing unpatching transition
+livepatch: '$MOD_LIVEPATCH': unpatching complete
+% rmmod $MOD_LIVEPATCH"
 
 start_test "sysfs test replace enabled"
 

index 939230e571f5..0978c489a67a 100644
@@ -9,6 +9,8 @@ obj-m += test_klp_atomic_replace.o \
 	test_klp_kprobe.o \
 	test_klp_livepatch.o \
 	test_klp_shadow_vars.o \
+	test_klp_speaker.o \
+	test_klp_speaker_livepatch.o \
 	test_klp_state.o \
 	test_klp_state2.o \
 	test_klp_state3.o \

new file mode 100644
index 000000000000..b1fb135820b0
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2024 SUSE
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/printk.h>
+
+/**
+ * test_klp_speaker - test module for testing misc livepatching features
+ *
+ * The module provides a virtual speaker who can do:
+ *
+ *    - Start a show with a greeting, see speaker_welcome().
+ */
+
+noinline
+static void __always_used speaker_welcome(void)
+{
+	pr_info("%s: Hello, World!\n", __func__);
+}
+
+static int test_klp_speaker_init(void)
+{
+	pr_info("%s\n", __func__);
+
+	return 0;
+}
+
+static void test_klp_speaker_exit(void)
+{
+	pr_info("%s\n", __func__);
+}
+
+module_init(test_klp_speaker_init);
+module_exit(test_klp_speaker_exit);
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Livepatch test: test functions");

2.47.1

Substitute hard-coded /sys/module path with $SYSFS_MODULE_DIR. It is going to be used even more often in the upcoming selftests.

---
 tools/testing/selftests/livepatch/functions.sh | 18 ++++++++++--------
 .../selftests/livepatch/test-callbacks.sh      |  2 +-
 2 files changed, 11 insertions(+), 9 deletions(-)

index e5d06fb40233..3f86b89e6ea7 100644
@@ -6,7 +6,9 @@
 
 MAX_RETRIES=600
 RETRY_INTERVAL=".1"	# seconds
-SYSFS_KERNEL_DIR="/sys/kernel"
+SYSFS_DIR="/sys"
+SYSFS_KERNEL_DIR="$SYSFS_DIR/kernel"
+SYSFS_MODULE_DIR="$SYSFS_DIR/module"
 SYSFS_KLP_DIR="$SYSFS_KERNEL_DIR/livepatch"
 SYSFS_DEBUG_DIR="$SYSFS_KERNEL_DIR/debug"
 SYSFS_KPROBES_DIR="$SYSFS_DEBUG_DIR/kprobes"

@@ -160,7 +162,7 @@ function __load_mod() {
 	fi
 
 	# Wait for module in sysfs ...
-	loop_until '[[ -e "/sys/module/$mod" ]]' ||
+	loop_until '[[ -e "$SYSFS_MODULE_DIR/$mod" ]]' ||
 		die "failed to load module $mod"
 }
 

@@ -228,7 +230,7 @@ function unload_mod() {
 	local mod="$1"
 
 	# Wait for module reference count to clear ...
-	loop_until '[[ $(cat "/sys/module/$mod/refcnt") == "0" ]]' ||
+	loop_until '[[ $(cat "$SYSFS_MODULE_DIR/$mod/refcnt") == "0" ]]' ||
 		die "failed to unload module $mod (refcnt)"
 
 	log "% rmmod $mod"

@@ -238,8 +240,8 @@ function unload_mod() {
 	fi
 
 	# Wait for module in sysfs ...
-	loop_until '[[ ! -e "/sys/module/$mod" ]]' ||
-		die "failed to unload module $mod (/sys/module)"
+	loop_until '[[ ! -e "$SYSFS_MODULE_DIR/$mod" ]]' ||
+		die "failed to unload module $mod ($SYSFS_MODULE_DIR)"
 }
 
 # unload_lp(modname) - unload a kernel module with a livepatch

@@ -269,11 +271,11 @@ function set_pre_patch_ret {
 	local mod="$1"; shift
 	local ret="$1"
 
-	log "% echo $ret > /sys/module/$mod/parameters/pre_patch_ret"
-	echo "$ret" > /sys/module/"$mod"/parameters/pre_patch_ret
+	log "% echo $ret > $SYSFS_MODULE_DIR/$mod/parameters/pre_patch_ret"
+	echo "$ret" > $SYSFS_MODULE_DIR/"$mod"/parameters/pre_patch_ret
 
 	# Wait for sysfs value to hold ...
-	loop_until '[[ $(cat "/sys/module/$mod/parameters/pre_patch_ret") == "$ret" ]]' ||
+	loop_until '[[ $(cat "$SYSFS_MODULE_DIR/$mod/parameters/pre_patch_ret") == "$ret" ]]' ||
 		die "failed to set pre_patch_ret parameter for $mod module"
 }
 

index 37bbc3fb2780..a65fd860662e 100755
@@ -303,7 +303,7 @@ livepatch: '$MOD_LIVEPATCH': starting patching transition
 livepatch: '$MOD_LIVEPATCH': completing patching transition
 $MOD_LIVEPATCH: post_patch_callback: vmlinux
 livepatch: '$MOD_LIVEPATCH': patching complete
-% echo -19 > /sys/module/$MOD_LIVEPATCH/parameters/pre_patch_ret
+% echo -19 > $SYSFS_MODULE_DIR/$MOD_LIVEPATCH/parameters/pre_patch_ret
 % insmod test_modules/$MOD_TARGET.ko
 livepatch: applying patch '$MOD_LIVEPATCH' to loading module '$MOD_TARGET'
 $MOD_LIVEPATCH: pre_patch_callback: $MOD_TARGET -> [MODULE_STATE_COMING] Full formed, running module_init

2.47.1

The per-object callbacks have been replaced by per-state callbacks and will be removed soon.

There are self-tests that attempt to load a live patch with per-object callbacks and the live-patched module (object) in all possible order variations.

These order variations are less relevant with per-state callbacks because they are only invoked when the live patch is enabled or disabled.

However, it is still important to check all possible order variations to ensure the module is live-patched correctly. The expected state is validated using two approaches.

First, the "patched" sysfs attribute is checked the same way is in the sysfs tests.

Second, a new "welcome" parameter has been added to the speaker test module. By reading this parameter's state, the live-patched function can be invoked.

Instead of displaying the parameter value via the sysfs interface, the .read() callback writes a welcome message to the kernel log. This message is compared with the expected value at the end of the test. This approach ensures that the self-test attempts to unload the test modules even if something goes wrong.

---
 tools/testing/selftests/livepatch/Makefile    |   1 +
 .../testing/selftests/livepatch/functions.sh  |  29 +++
 .../selftests/livepatch/test-callbacks.sh     | 226 ------------------
 .../testing/selftests/livepatch/test-order.sh | 226 ++++++++++++++++++
 .../livepatch/test_modules/test_klp_speaker.c |  19 +-
 5 files changed, 274 insertions(+), 227 deletions(-)
 create mode 100755 tools/testing/selftests/livepatch/test-order.sh

index a080eb54a215..971d0c6f8059 100644
@@ -5,6 +5,7 @@ TEST_GEN_MODS_DIR := test_modules
 TEST_PROGS_EXTENDED := functions.sh
 TEST_PROGS := \
 	test-livepatch.sh \
+	test-order.sh \
 	test-callbacks.sh \
 	test-shadow-vars.sh \
 	test-state.sh \

index 3f86b89e6ea7..bc1e100e47a7 100644
@@ -279,6 +279,23 @@ function set_pre_patch_ret {
 		die "failed to set pre_patch_ret parameter for $mod module"
 }
 
+# read_module_param(modname, param)
+#	modname - module name which provides the given parameter
+#	param - parameter name to be read
+function read_module_param {
+	local mod="$1"; shift
+	local param="$1"
+
+	log "% cat $SYSFS_MODULE_DIR/$mod/parameters/$param"
+	val=$(cat $SYSFS_MODULE_DIR/$mod/parameters/$param 2>&1)
+	# Log only non-empty values. Some test modules write a message
+	# to the log on its own when reading the parameter, for example,
+	# the "welcome" parameter of the "test_klp_speaker" module.
+	if [[ "$val" != "" ]]; then
+		log "$mod:$param: $ret"
+	fi
+}
+
 function start_test {
 	local test="$1"
 

@@ -353,3 +370,15 @@ function check_sysfs_value() {
 		die "Unexpected value in $path: $expected_value vs. $value"
 	fi
 }
+
+# check_object_patched(livepatch_module, objname, expected_value)
+#	livepatch_module - livepatch module creating the sysfs interface
+#	objname - livepatched object to be checked
+#	expected_value - expected value read from the file
+function check_object_patched() {
+	local livepatch_module="$1"; shift
+	local objname="$1"; shift
+	local expected_value="$1"; shift
+
+	check_sysfs_value "$livepatch_module" "$objname/patched" "$expected_value"
+}

index b1fb135820b0..22f6e5fcb009 100644
@@ -12,14 +12,31 @@
  * The module provides a virtual speaker who can do:
  *
  *    - Start a show with a greeting, see speaker_welcome().
+ *
+ *    - Log the greeting by reading the "welcome" module parameter, see
+ *	welcome_get().
  */
 
 noinline
-static void __always_used speaker_welcome(void)
+static void speaker_welcome(void)
 {
 	pr_info("%s: Hello, World!\n", __func__);
 }
 
+static int welcome_get(char *buffer, const struct kernel_param *kp)
+{
+	speaker_welcome();
+
+	return 0;
+}
+
+static const struct kernel_param_ops welcome_ops = {
+	.get	= welcome_get,
+};
+
+module_param_cb(welcome, &welcome_ops, NULL, 0400);
+MODULE_PARM_DESC(welcome, "Print speaker's welcome message into the kernel log when reading the value.");
+
 static int test_klp_speaker_init(void)
 {
 	pr_info("%s\n", __func__);

2.47.1

The per-object callbacks have been replaced by per-state callbacks and will be removed soon.

A selftest previously loaded two livepatched modules to ensure that the per-object callbacks were called for both.

Although per-state callbacks are only invoked when the live patch is enabled or disabled, it is still important to verify that multiple objects can be live-patched.

Convert the test into a generic one by reusing the speaker test module to create the second target module.

In the second variant, speaker_welcome() will only print "(2)" after the function name to distinguish it, while keeping the real function name the same to maintain simplicity.

---
 .../selftests/livepatch/test-callbacks.sh     | 59 ----------------
 .../testing/selftests/livepatch/test-order.sh | 69 +++++++++++++++++++
 .../selftests/livepatch/test_modules/Makefile |  1 +
 .../livepatch/test_modules/test_klp_speaker.c |  8 ++-
 .../test_modules/test_klp_speaker2.c          |  8 +++
 .../test_modules/test_klp_speaker_livepatch.c | 26 ++++++-
 6 files changed, 110 insertions(+), 61 deletions(-)
 create mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_speaker2.c

index 614ed0aa2e40..a9bb90920c0a 100755
@@ -93,65 +93,6 @@ $MOD_LIVEPATCH: post_unpatch_callback: vmlinux
 livepatch: '$MOD_LIVEPATCH': unpatching complete
 % rmmod $MOD_LIVEPATCH"
 
-
-# Test loading multiple targeted kernel modules.  This test-case is
-# mainly for comparing with the next test-case.
-#
-# - Load a target "busy" kernel module which kicks off a worker function
-#   that immediately exits.
-#
-# - Proceed with loading the livepatch and another ordinary target
-#   module.  Post-patch callbacks are executed and the transition
-#   completes quickly.
-
-start_test "multiple target modules"
-
-load_mod $MOD_TARGET_BUSY block_transition=N
-load_lp $MOD_LIVEPATCH
-load_mod $MOD_TARGET
-unload_mod $MOD_TARGET
-disable_lp $MOD_LIVEPATCH
-unload_lp $MOD_LIVEPATCH
-unload_mod $MOD_TARGET_BUSY
-
-check_result "% insmod test_modules/$MOD_TARGET_BUSY.ko block_transition=N
-$MOD_TARGET_BUSY: ${MOD_TARGET_BUSY}_init
-$MOD_TARGET_BUSY: busymod_work_func enter
-$MOD_TARGET_BUSY: busymod_work_func exit
-% insmod test_modules/$MOD_LIVEPATCH.ko
-livepatch: enabling patch '$MOD_LIVEPATCH'
-livepatch: '$MOD_LIVEPATCH': initializing patching transition
-$MOD_LIVEPATCH: pre_patch_callback: vmlinux
-$MOD_LIVEPATCH: pre_patch_callback: $MOD_TARGET_BUSY -> [MODULE_STATE_LIVE] Normal state
-livepatch: '$MOD_LIVEPATCH': starting patching transition
-livepatch: '$MOD_LIVEPATCH': completing patching transition
-$MOD_LIVEPATCH: post_patch_callback: vmlinux
-$MOD_LIVEPATCH: post_patch_callback: $MOD_TARGET_BUSY -> [MODULE_STATE_LIVE] Normal state
-livepatch: '$MOD_LIVEPATCH': patching complete
-% insmod test_modules/$MOD_TARGET.ko
-livepatch: applying patch '$MOD_LIVEPATCH' to loading module '$MOD_TARGET'
-$MOD_LIVEPATCH: pre_patch_callback: $MOD_TARGET -> [MODULE_STATE_COMING] Full formed, running module_init
-$MOD_LIVEPATCH: post_patch_callback: $MOD_TARGET -> [MODULE_STATE_COMING] Full formed, running module_init
-$MOD_TARGET: ${MOD_TARGET}_init
-% rmmod $MOD_TARGET
-$MOD_TARGET: ${MOD_TARGET}_exit
-$MOD_LIVEPATCH: pre_unpatch_callback: $MOD_TARGET -> [MODULE_STATE_GOING] Going away
-livepatch: reverting patch '$MOD_LIVEPATCH' on unloading module '$MOD_TARGET'
-$MOD_LIVEPATCH: post_unpatch_callback: $MOD_TARGET -> [MODULE_STATE_GOING] Going away
-% echo 0 > $SYSFS_KLP_DIR/$MOD_LIVEPATCH/enabled
-livepatch: '$MOD_LIVEPATCH': initializing unpatching transition
-$MOD_LIVEPATCH: pre_unpatch_callback: vmlinux
-$MOD_LIVEPATCH: pre_unpatch_callback: $MOD_TARGET_BUSY -> [MODULE_STATE_LIVE] Normal state
-livepatch: '$MOD_LIVEPATCH': starting unpatching transition
-livepatch: '$MOD_LIVEPATCH': completing unpatching transition
-$MOD_LIVEPATCH: post_unpatch_callback: vmlinux
-$MOD_LIVEPATCH: post_unpatch_callback: $MOD_TARGET_BUSY -> [MODULE_STATE_LIVE] Normal state
-livepatch: '$MOD_LIVEPATCH': unpatching complete
-% rmmod $MOD_LIVEPATCH
-% rmmod $MOD_TARGET_BUSY
-$MOD_TARGET_BUSY: ${MOD_TARGET_BUSY}_exit"
-
-
 # A similar test as the previous one, but force the "busy" kernel module
 # to block the livepatch transition.
 #

index 0978c489a67a..72a817d1ddd9 100644
@@ -10,6 +10,7 @@ obj-m += test_klp_atomic_replace.o \
 	test_klp_livepatch.o \
 	test_klp_shadow_vars.o \
 	test_klp_speaker.o \
+	test_klp_speaker2.o \
 	test_klp_speaker_livepatch.o \
 	test_klp_state.o \
 	test_klp_state2.o \

index 22f6e5fcb009..92c577addb8e 100644
@@ -6,6 +6,10 @@
 #include <linux/module.h>
 #include <linux/printk.h>
 
+#ifndef SPEAKER_ID
+#define SPEAKER_ID ""
+#endif
+
 /**
  * test_klp_speaker - test module for testing misc livepatching features
  *

@@ -15,12 +19,14 @@
  *
  *    - Log the greeting by reading the "welcome" module parameter, see
  *	welcome_get().
+ *
+ *    - Reuse the module source for more speakers, see SPEAKER_ID.
  */
 
 noinline
 static void speaker_welcome(void)
 {
-	pr_info("%s: Hello, World!\n", __func__);
+	pr_info("%s%s: Hello, World!\n", __func__, SPEAKER_ID);
 }
 
 static int welcome_get(char *buffer, const struct kernel_param *kp)

new file mode 100644
index 000000000000..d38ab51414bf
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2023 SUSE
+
+/* Use versioned function name for livepatched functions */
+#define SPEAKER_ID "(2)"
+
+/* Same module with the same features. */
+#include "test_klp_speaker.c"

index 26a8dd15f723..8d7e74a69a5d 100644
@@ -17,11 +17,23 @@
  *
  *    - Improve the speaker's greeting from "Hello, World!" to
  *	"Ladies and gentleman, ..."
+ *
+ *    - Support more speaker modules, see __lp_speaker_welcome().
  */
 
+static void __lp_speaker_welcome(const char *caller_func, const char *speaker_id)
+{
+	pr_info("%s%s: Ladies and gentleman, ...\n", caller_func, speaker_id);
+}
+
 static void lp_speaker_welcome(void)
 {
-	pr_info("%s: Ladies and gentleman, ...\n", __func__);
+	__lp_speaker_welcome(__func__, "");
+}
+
+static void lp_speaker2_welcome(void)
+{
+	__lp_speaker_welcome(__func__, "(2)");
 }
 
 static struct klp_func test_klp_speaker_funcs[] = {

@@ -32,11 +44,23 @@ static struct klp_func test_klp_speaker_funcs[] = {
 	{ }
 };
 
+static struct klp_func test_klp_speaker2_funcs[] = {
+	{
+		.old_name = "speaker_welcome",
+		.new_func = lp_speaker2_welcome,
+	},
+	{ }
+};
+
 static struct klp_object objs[] = {
 	{
 		.name = "test_klp_speaker",
 		.funcs = test_klp_speaker_funcs,
 	},
+	{
+		.name = "test_klp_speaker2",
+		.funcs = test_klp_speaker2_funcs,
+	},
 	{ }
 };
 

2.47.1

The per-object callbacks have been deprecated in favor of per-state callbacks and will be removed soon. The behavior of these two types of callbacks is completely different.

Per-object callbacks are triggered when the livepatching of the related object is enabled or disabled. When using the atomic replace feature, only the callbacks from the new livepatch are called.

Per-state callbacks, on the other hand, are triggered when the state is being added or removed. When using the atomic replace feature, the callbacks that remove the state are called from the old livepatch that is being replaced.

For testing purposes, the state represents a part of an audience in the livepatch for the speaker module. The audience applauds when the speaker delivers the welcome message.

All four callbacks are used as follows:

+ pre_patch() allocates a shadow variable with a string and fills

		it with "[]".

+ post_patch() fills the string with "[APPLAUSE]".
+ pre_unpatch() reverts the string back to "[]".
+ post_unpatch() releases the shadow variable.

The welcome message printed by the livepatched function allows us to distinguish between the transition and the completed transition. Specifically, the speaker's welcome message appears as:

+ Not patched system: "Hello, World!"
+ Transition (unpatched task): "[] Hello, World!"
+ Transition (patched task): "[] Ladies and gentlemen, ..."
+ Patched system: "[APPLAUSE] Ladies and gentlemen, ..."

Create an initial self-test that uses a single livepatch and a single state. This serves as a base for more complex self-tests.

---
 tools/testing/selftests/livepatch/Makefile    |   1 +
 .../livepatch/test-state-callbacks.sh         |  58 +++++++
 .../test_modules/test_klp_speaker_livepatch.c | 154 +++++++++++++++++-
 3 files changed, 212 insertions(+), 1 deletion(-)
 create mode 100755 tools/testing/selftests/livepatch/test-state-callbacks.sh

index 971d0c6f8059..ae00e98ba79f 100644
@@ -9,6 +9,7 @@ TEST_PROGS := \
 	test-callbacks.sh \
 	test-shadow-vars.sh \
 	test-state.sh \
+	test-state-callbacks.sh \
 	test-ftrace.sh \
 	test-sysfs.sh \
 	test-syscall.sh \

2.47.1

The per-object callbacks have been deprecated in favor of per-state callbacks and will be removed soon. The behavior of these two types of callbacks is completely different.

Per-object callbacks are triggered when the livepatching of the related object is enabled or disabled. Per-state callbacks, on the other hand, are triggered when the state is being added or removed.

In other words, it does not matter whether the livepatched object is loaded before or after the livepatch. Therefore, it makes sense to preserve only one variant of the self-test for the failing pre_patch callback.

Use the new speaker module for testing. Simply add a new parameter, "pre_patch_ret", similar to the one in the "test_klp_callbacks_demo" livepatch module.

---
 .../selftests/livepatch/test-callbacks.sh     | 82 -------------------
 .../livepatch/test-state-callbacks.sh         | 35 ++++++++
 .../test_modules/test_klp_speaker_livepatch.c | 10 +++
 3 files changed, 45 insertions(+), 82 deletions(-)

index a9bb90920c0a..1ecd8f08a613 100755
@@ -11,88 +11,6 @@ MOD_TARGET_BUSY=test_klp_callbacks_busy
 
 setup_config
 
-# Test a scenario where a vmlinux pre-patch callback returns a non-zero
-# status (ie, failure).
-#
-# - First load a target module.
-#
-# - Load the livepatch module, setting its 'pre_patch_ret' value to -19
-#   (-ENODEV).  When its vmlinux pre-patch callback executes, this
-#   status code will propagate back to the module-loading subsystem.
-#   The result is that the insmod command refuses to load the livepatch
-#   module.
-
-start_test "pre-patch callback -ENODEV"
-
-load_mod $MOD_TARGET
-load_failing_mod $MOD_LIVEPATCH pre_patch_ret=-19
-unload_mod $MOD_TARGET
-
-check_result "% insmod test_modules/$MOD_TARGET.ko
-$MOD_TARGET: ${MOD_TARGET}_init
-% insmod test_modules/$MOD_LIVEPATCH.ko pre_patch_ret=-19
-livepatch: enabling patch '$MOD_LIVEPATCH'
-livepatch: '$MOD_LIVEPATCH': initializing patching transition
-test_klp_callbacks_demo: pre_patch_callback: vmlinux
-livepatch: pre-patch callback failed for object 'vmlinux'
-livepatch: failed to enable patch '$MOD_LIVEPATCH'
-livepatch: '$MOD_LIVEPATCH': canceling patching transition, going to unpatch
-livepatch: '$MOD_LIVEPATCH': completing unpatching transition
-livepatch: '$MOD_LIVEPATCH': unpatching complete
-insmod: ERROR: could not insert module test_modules/$MOD_LIVEPATCH.ko: No such device
-% rmmod $MOD_TARGET
-$MOD_TARGET: ${MOD_TARGET}_exit"
-
-
-# Similar to the previous test, setup a livepatch such that its vmlinux
-# pre-patch callback returns success.  However, when a targeted kernel
-# module is later loaded, have the livepatch return a failing status
-# code.
-#
-# - Load the livepatch, vmlinux pre-patch callback succeeds.
-#
-# - Set a trap so subsequent pre-patch callbacks to this livepatch will
-#   return -ENODEV.
-#
-# - The livepatch pre-patch callback for subsequently loaded target
-#   modules will return failure, so the module loader refuses to load
-#   the kernel module.  No post-patch or pre/post-unpatch callbacks are
-#   executed for this klp_object.
-#
-# - Pre/post-unpatch callbacks are run for the vmlinux klp_object.
-
-start_test "module_coming + pre-patch callback -ENODEV"
-
-load_lp $MOD_LIVEPATCH
-set_pre_patch_ret $MOD_LIVEPATCH -19
-load_failing_mod $MOD_TARGET
-disable_lp $MOD_LIVEPATCH
-unload_lp $MOD_LIVEPATCH
-
-check_result "% insmod test_modules/$MOD_LIVEPATCH.ko
-livepatch: enabling patch '$MOD_LIVEPATCH'
-livepatch: '$MOD_LIVEPATCH': initializing patching transition
-$MOD_LIVEPATCH: pre_patch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH': starting patching transition
-livepatch: '$MOD_LIVEPATCH': completing patching transition
-$MOD_LIVEPATCH: post_patch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH': patching complete
-% echo -19 > $SYSFS_MODULE_DIR/$MOD_LIVEPATCH/parameters/pre_patch_ret
-% insmod test_modules/$MOD_TARGET.ko
-livepatch: applying patch '$MOD_LIVEPATCH' to loading module '$MOD_TARGET'
-$MOD_LIVEPATCH: pre_patch_callback: $MOD_TARGET -> [MODULE_STATE_COMING] Full formed, running module_init
-livepatch: pre-patch callback failed for object '$MOD_TARGET'
-livepatch: patch '$MOD_LIVEPATCH' failed for module '$MOD_TARGET', refusing to load module '$MOD_TARGET'
-insmod: ERROR: could not insert module test_modules/$MOD_TARGET.ko: No such device
-% echo 0 > $SYSFS_KLP_DIR/$MOD_LIVEPATCH/enabled
-livepatch: '$MOD_LIVEPATCH': initializing unpatching transition
-$MOD_LIVEPATCH: pre_unpatch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH': starting unpatching transition
-livepatch: '$MOD_LIVEPATCH': completing unpatching transition
-$MOD_LIVEPATCH: post_unpatch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH': unpatching complete
-% rmmod $MOD_LIVEPATCH"
-
 # A similar test as the previous one, but force the "busy" kernel module
 # to block the livepatch transition.
 #

index 1183b9d15782..28ef88a2dfc3 100755
@@ -55,4 +55,39 @@ $MOD_TARGET: speaker_welcome: Hello, World!
 % rmmod $MOD_TARGET
 $MOD_TARGET: ${MOD_TARGET}_exit"
 
+# Test failure of the "pre_patch" state callback.
+#
+# The livepatch should not get loaded. The test module should
+# should stay unpatched which is checked by reading the "welcome"
+# parameter.
+
+start_test "failing pre_patch callback with -ENODEV"
+
+load_mod $MOD_TARGET
+read_module_param $MOD_TARGET welcome
+
+load_failing_mod $MOD_LIVEPATCH applause=1 pre_patch_ret=-19
+read_module_param $MOD_TARGET welcome
+
+unload_mod $MOD_TARGET
+
+check_result "% insmod test_modules/$MOD_TARGET.ko
+$MOD_TARGET: ${MOD_TARGET}_init
+% cat $SYSFS_MODULE_DIR/$MOD_TARGET/parameters/welcome
+$MOD_TARGET: speaker_welcome: Hello, World!
+% insmod test_modules/$MOD_LIVEPATCH.ko applause=1 pre_patch_ret=-19
+livepatch: enabling patch '$MOD_LIVEPATCH'
+livepatch: '$MOD_LIVEPATCH': initializing patching transition
+$MOD_LIVEPATCH: applause_pre_patch_callback: state 10
+$MOD_LIVEPATCH: applause_pre_patch_callback: forcing err: -ENODEV
+livepatch: failed to enable patch '$MOD_LIVEPATCH'
+livepatch: '$MOD_LIVEPATCH': canceling patching transition, going to unpatch
+livepatch: '$MOD_LIVEPATCH': completing unpatching transition
+livepatch: '$MOD_LIVEPATCH': unpatching complete
+insmod: ERROR: could not insert module test_modules/$MOD_LIVEPATCH.ko: No such device
+% cat $SYSFS_MODULE_DIR/$MOD_TARGET/parameters/welcome
+$MOD_TARGET: speaker_welcome: Hello, World!
+% rmmod $MOD_TARGET
+$MOD_TARGET: ${MOD_TARGET}_exit"
+
 exit 0

index ab409df0b0e3..c46c98a3c1e6 100644
@@ -31,6 +31,10 @@ static bool add_applause;
 module_param_named(applause, add_applause, bool, 0400);
 MODULE_PARM_DESC(applause, "Use shadow variable to add applause (default=false)");
 
+static int pre_patch_ret;
+module_param(pre_patch_ret, int, 0400);
+MODULE_PARM_DESC(pre_patch_ret, "Allow to force failure for the pre_patch callback (default=0)");
+
 static void __lp_speaker_welcome(const char *caller_func, const char *speaker_id)
 {
 	char entire_applause[APPLAUSE_STR_SIZE + 1] = "";

@@ -123,6 +127,12 @@ static void check_applause(unsigned long id)
 static int applause_pre_patch_callback(struct klp_patch *patch, struct klp_state *state)
 {
 	pr_info("%s: state %lu\n", __func__, state->id);
+
+	if (pre_patch_ret) {
+		pr_err("%s: forcing err: %pe\n", __func__, ERR_PTR(pre_patch_ret));
+		return pre_patch_ret;
+	}
+
 	return allocate_applause(state->id);
 }
 

2.47.1

The per-object callbacks have been deprecated in favor of per-state callbacks and will be removed soon.

For the conversion of the self-test of the callbacks with a blocked transition, use the new module with a virtual speaker.

Implementation Details:

+ The implementation of the blocked function is inspired by the module "test_klp_callbacks_busy". Specifically, the blocked function is called in a workqueue, synchronization is done using a completion, and the replacement of the blocked function is not fully functional[*].

+ The blocked function is queued and called only if the parameter "block_doors" is set.

Important Note on Sibling Call Optimization:

Sibling call optimization must be disabled for functions designed to block transitions. Otherwise, they won't appear on the stack, leading to test failure. These functions can be livepatched because they are called with the call instruction. But when an optimized version just jumps to a nested then the jump instruction obviously doesn't store any return address on the stack.

[*] The livepatch variant of the blocked function is never called
because the transition is reverted. It is going to change in a followup patch.

---
 .../selftests/livepatch/test-callbacks.sh     | 73 -------------------
 .../livepatch/test-state-callbacks.sh         | 56 ++++++++++++++
 .../livepatch/test_modules/test_klp_speaker.c | 61 ++++++++++++++++
 .../test_modules/test_klp_speaker_livepatch.c | 28 +++++++
 4 files changed, 145 insertions(+), 73 deletions(-)

index 1ecd8f08a613..858e8f0b14d5 100755
@@ -11,79 +11,6 @@ MOD_TARGET_BUSY=test_klp_callbacks_busy
 
 setup_config
 
-# A similar test as the previous one, but force the "busy" kernel module
-# to block the livepatch transition.
-#
-# The livepatching core will refuse to patch a task that is currently
-# executing a to-be-patched function -- the consistency model stalls the
-# current patch transition until this safety-check is met.  Test a
-# scenario where one of a livepatch's target klp_objects sits on such a
-# function for a long time.  Meanwhile, load and unload other target
-# kernel modules while the livepatch transition is in progress.
-#
-# - Load the "busy" kernel module, this time make its work function loop
-#
-# - Meanwhile, the livepatch is loaded.  Notice that the patch
-#   transition does not complete as the targeted "busy" module is
-#   sitting on a to-be-patched function.
-#
-# - Load a second target module (this one is an ordinary idle kernel
-#   module).  Note that *no* post-patch callbacks will be executed while
-#   the livepatch is still in transition.
-#
-# - Request an unload of the simple kernel module.  The patch is still
-#   transitioning, so its pre-unpatch callbacks are skipped.
-#
-# - Finally the livepatch is disabled.  Since none of the patch's
-#   klp_object's post-patch callbacks executed, the remaining
-#   klp_object's pre-unpatch callbacks are skipped.
-
-start_test "busy target module"
-
-load_mod $MOD_TARGET_BUSY block_transition=Y
-load_lp_nowait $MOD_LIVEPATCH
-
-# Wait until the livepatch reports in-transition state, i.e. that it's
-# stalled on $MOD_TARGET_BUSY::busymod_work_func()
-loop_until 'grep -q '^1$' $SYSFS_KLP_DIR/$MOD_LIVEPATCH/transition' ||
-	die "failed to stall transition"
-
-load_mod $MOD_TARGET
-unload_mod $MOD_TARGET
-disable_lp $MOD_LIVEPATCH
-unload_lp $MOD_LIVEPATCH
-unload_mod $MOD_TARGET_BUSY
-
-check_result "% insmod test_modules/$MOD_TARGET_BUSY.ko block_transition=Y
-$MOD_TARGET_BUSY: ${MOD_TARGET_BUSY}_init
-$MOD_TARGET_BUSY: busymod_work_func enter
-% insmod test_modules/$MOD_LIVEPATCH.ko
-livepatch: enabling patch '$MOD_LIVEPATCH'
-livepatch: '$MOD_LIVEPATCH': initializing patching transition
-$MOD_LIVEPATCH: pre_patch_callback: vmlinux
-$MOD_LIVEPATCH: pre_patch_callback: $MOD_TARGET_BUSY -> [MODULE_STATE_LIVE] Normal state
-livepatch: '$MOD_LIVEPATCH': starting patching transition
-% insmod test_modules/$MOD_TARGET.ko
-livepatch: applying patch '$MOD_LIVEPATCH' to loading module '$MOD_TARGET'
-$MOD_LIVEPATCH: pre_patch_callback: $MOD_TARGET -> [MODULE_STATE_COMING] Full formed, running module_init
-$MOD_TARGET: ${MOD_TARGET}_init
-% rmmod $MOD_TARGET
-$MOD_TARGET: ${MOD_TARGET}_exit
-livepatch: reverting patch '$MOD_LIVEPATCH' on unloading module '$MOD_TARGET'
-$MOD_LIVEPATCH: post_unpatch_callback: $MOD_TARGET -> [MODULE_STATE_GOING] Going away
-% echo 0 > $SYSFS_KLP_DIR/$MOD_LIVEPATCH/enabled
-livepatch: '$MOD_LIVEPATCH': reversing transition from patching to unpatching
-livepatch: '$MOD_LIVEPATCH': starting unpatching transition
-livepatch: '$MOD_LIVEPATCH': completing unpatching transition
-$MOD_LIVEPATCH: post_unpatch_callback: vmlinux
-$MOD_LIVEPATCH: post_unpatch_callback: $MOD_TARGET_BUSY -> [MODULE_STATE_LIVE] Normal state
-livepatch: '$MOD_LIVEPATCH': unpatching complete
-% rmmod $MOD_LIVEPATCH
-% rmmod $MOD_TARGET_BUSY
-$MOD_TARGET_BUSY: busymod_work_func exit
-$MOD_TARGET_BUSY: ${MOD_TARGET_BUSY}_exit"
-
-
 # Test loading multiple livepatches.  This test-case is mainly for comparing
 # with the next test-case.
 #

index c46c98a3c1e6..76402947c789 100644
@@ -19,6 +19,8 @@
  *	"Ladies and gentleman, ..."
  *
  *    - Support more speaker modules, see __lp_speaker_welcome().
+ *
+ *    - Livepatch block_doors_func() which can block the transition.
  */
 
 #define APPLAUSE_ID 10

@@ -180,11 +182,33 @@ static void applause_shadow_dtor(void *obj, void *shadow_data)
 		__func__, applause);
 }
 
+static void __lp_block_doors_func(struct work_struct *work, const char *caller_func,
+		       const char *speaker_id)
+{
+	/* Just print the message. Never really used. */
+	pr_info("%s: Going to block doors%s (this should never happen).\n",
+		caller_func, speaker_id);
+}
+
+static void lp_block_doors_func(struct work_struct *work)
+{
+	__lp_block_doors_func(work, __func__, "");
+}
+
+static void lp_block_doors_func2(struct work_struct *work)
+{
+	__lp_block_doors_func(work, __func__, "(2)");
+}
+
 static struct klp_func test_klp_speaker_funcs[] = {
 	{
 		.old_name = "speaker_welcome",
 		.new_func = lp_speaker_welcome,
 	},
+	{
+		.old_name = "block_doors_func",
+		.new_func = lp_block_doors_func,
+	},
 	{ }
 };
 

@@ -193,6 +217,10 @@ static struct klp_func test_klp_speaker2_funcs[] = {
 		.old_name = "speaker_welcome",
 		.new_func = lp_speaker2_welcome,
 	},
+	{
+		.old_name = "block_doors_func",
+		.new_func = lp_block_doors_func2,
+	},
 	{ }
 };
 

2.47.1

This commit adds two new self tests to the livepatch state callback testing, specifically focusing on scenarios with blocked transitions. Previously, only the transition reversion scenario was tested.

New Test Scenarios:

+ Transition Unblocking: Verifies the behavior when a blocked transition is unblocked.

+ Disable Operation Blocking: Tests the scenario where a function blocks the disable operation.

Implementation Details:

+ To ensure the blocking function remains fully functional within the livepatch, its pointer is bundled with the associated struct work.

+ The 'block_doors' parameter now controls both enabling and disabling the blocking function. This necessitates a specific initialization of the work struct to guarantee it's flushed in the module's .exit() callbacks. Note that if 'block_doors' is defined on the command line, its .set() callback is called before the module's .init() callback.

+ The livepatched speaker_welcome() function is also called from the blocking function, enabling verification of separate process transitions.

+ A suffix is added to the welcome message when printed via the blocked process (controlled by 'block_doors') for better process differentiation.

Important Note on Sibling Call Optimization:

Sibling call optimization must be disabled for functions designed to block transitions. Otherwise, they won't appear on the stack, leading to test failure. These functions can be livepatched because they are called with the call instruction. But when an optimized version just jumps to a nested then the jump instruction obviously doesn't store any return address on the stack.

---
 .../testing/selftests/livepatch/functions.sh  |  28 +++-
 .../livepatch/test-state-callbacks.sh         | 143 +++++++++++++++++-
 .../livepatch/test_modules/test_klp_speaker.c | 125 ++++++++++++---
 .../livepatch/test_modules/test_klp_speaker.h |  15 ++
 .../test_modules/test_klp_speaker_livepatch.c |  35 +++--
 5 files changed, 310 insertions(+), 36 deletions(-)
 create mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_speaker.h

index bc1e100e47a7..e10a55427b71 100644
@@ -250,13 +250,22 @@ function unload_lp() {
 	unload_mod "$1"
 }
 
-# disable_lp(modname) - disable a livepatch
-#	modname - module name to unload
-function disable_lp() {
+# disable_lp_nowait(modname) - disable a livepatch but do not wait
+#			       until the transition finishes
+#	modname - module name of the livepatch to disable
+function disable_lp_nowait() {
 	local mod="$1"
 
 	log "% echo 0 > $SYSFS_KLP_DIR/$mod/enabled"
 	echo 0 > "$SYSFS_KLP_DIR/$mod/enabled"
+}
+
+# disable_lp(modname) - disable a livepatch
+#	modname - module name of the livepatch to disable
+function disable_lp() {
+	local mod="$1"; shift
+
+	disable_lp_nowait "$mod" "$@"
 
 	# Wait until the transition finishes and the livepatch gets
 	# removed from sysfs...

@@ -279,6 +288,19 @@ function set_pre_patch_ret {
 		die "failed to set pre_patch_ret parameter for $mod module"
 }
 
+# write_module_param(modname, param, val)
+#	modname - module name which provides the given parameter
+#	param - parameter name to be written
+#	val = value to be written
+function write_module_param {
+	local mod="$1"; shift
+	local param="$1"; shift
+	local val="$1"
+
+	log "% echo $val > $SYSFS_MODULE_DIR/$mod/parameters/$param"
+	echo "$val" > $SYSFS_MODULE_DIR/"$mod"/parameters/"$param"
+}
+
 # read_module_param(modname, param)
 #	modname - module name which provides the given parameter
 #	param - parameter name to be read

new file mode 100644
index 000000000000..89309a3acfde
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _TEST_KLP_SPEAKER_H_
+#define _TEST_KLP_SPEAKER_H_
+
+#include <linux/workqueue.h>
+
+typedef void (*do_block_doors_t)(void);
+
+struct hall {
+	struct work_struct block_doors_work;
+	do_block_doors_t do_block_doors;
+};
+
+#endif //  _TEST_KLP_SPEAKER_H_

index 76402947c789..6b82c5636845 100644
@@ -8,6 +8,8 @@
 #include <linux/livepatch.h>
 #include <linux/init.h>
 
+#include "test_klp_speaker.h"
+
 /**
  * test_klp_speaker_livepatch - test livepatch for testing various livepatching
  *	features.

@@ -37,7 +39,9 @@ static int pre_patch_ret;
 module_param(pre_patch_ret, int, 0400);
 MODULE_PARM_DESC(pre_patch_ret, "Allow to force failure for the pre_patch callback (default=0)");
 
-static void __lp_speaker_welcome(const char *caller_func, const char *speaker_id)
+static void __lp_speaker_welcome(const char *caller_func,
+				 const char *speaker_id,
+				 const char *context)
 {
 	char entire_applause[APPLAUSE_STR_SIZE + 1] = "";
 	const char *applause;

@@ -46,18 +50,18 @@ static void __lp_speaker_welcome(const char *caller_func, const char *speaker_id
 	if (applause)
 		snprintf(entire_applause, sizeof(entire_applause), "%s ", applause);
 
-	pr_info("%s%s: %sLadies and gentleman, ...\n", caller_func, speaker_id,
-		entire_applause);
+	pr_info("%s%s: %sLadies and gentleman, ...%s\n",
+		caller_func, speaker_id, entire_applause, context);
 }
 
-static void lp_speaker_welcome(void)
+static void lp_speaker_welcome(const char *context)
 {
-	__lp_speaker_welcome(__func__, "");
+	__lp_speaker_welcome(__func__, "", context);
 }
 
-static void lp_speaker2_welcome(void)
+static void lp_speaker2_welcome(const char *context)
 {
-	__lp_speaker_welcome(__func__, "(2)");
+	__lp_speaker_welcome(__func__, "(2)", context);
 }
 
 static int allocate_applause(unsigned long id)

@@ -185,16 +189,27 @@ static void applause_shadow_dtor(void *obj, void *shadow_data)
 static void __lp_block_doors_func(struct work_struct *work, const char *caller_func,
 		       const char *speaker_id)
 {
-	/* Just print the message. Never really used. */
-	pr_info("%s: Going to block doors%s (this should never happen).\n",
-		caller_func, speaker_id);
+	struct hall *hall = container_of(work, struct hall, block_doors_work);
+
+	pr_info("%s: Going to block doors%s (fixed).\n", caller_func, speaker_id);
+	hall->do_block_doors();
 }
 
+/*
+ * Prevent tail call optimizations to make sure that this function
+ * appears in the backtrace and can block the disable transition.
+ */
+__attribute__((__optimize__("no-optimize-sibling-calls")))
 static void lp_block_doors_func(struct work_struct *work)
 {
 	__lp_block_doors_func(work, __func__, "");
 }
 
+/*
+ * Prevent tail call optimizations to make sure that this function
+ * appears in the backtrace and can block the disable transition.
+ */
+__attribute__((__optimize__("no-optimize-sibling-calls")))
 static void lp_block_doors_func2(struct work_struct *work)
 {
 	__lp_block_doors_func(work, __func__, "(2)");

2.47.1

This patch converts selftests for the obsolete per-object callbacks and more livepatches.

The new tests for the per-state callbacks use the new speaker test module. The second livepatch simply reuses the sources from the existing one.

For greater variability, the livepatch is extended to support more shadow variables. The second shadow variable can be enabled using the "add_applause2" module parameter. It appears in the speaker's welcome message as follows:

+ not patched system: "Hello, World!"
+ transition (unpatched task): "[2] Hello, World!"
+ transition (patched task): "[2] Ladies and gentleman, ..."
+ patched system: "[APPLAUSE2] Ladies and gentleman, ..."

For backward compatibility, the first shadow variable is enabled using the non-versioned parameter "add_applause" and appears as the non-versioned "[APPLAUSE]" string.

Both shadow variables can be enabled together. They will appear next to each other in the speaker's welcome message. For example,
on the patched system:

"[APPLAUSE][APPLAUSE2] Ladies and gentlemen, ..."

Finally, the livepatch will get installed in parallel with other livepatches by default. The atomic replace feature can be enabled by a new module parameter "replace".

---
 .../selftests/livepatch/test-callbacks.sh     | 113 ------------
 .../livepatch/test-state-callbacks.sh         | 161 ++++++++++++++++
 .../selftests/livepatch/test_modules/Makefile |   1 +
 .../test_modules/test_klp_speaker_livepatch.c | 173 +++++++++++++++---
 .../test_klp_speaker_livepatch2.c             |   5 +
 5 files changed, 312 insertions(+), 141 deletions(-)
 delete mode 100755 tools/testing/selftests/livepatch/test-callbacks.sh
 create mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_speaker_livepatch2.c

deleted file mode 100755
index 858e8f0b14d5..000000000000
@@ -1,113 +0,0 @@
-#!/bin/bash
-# SPDX-License-Identifier: GPL-2.0
-# Copyright (C) 2018 Joe Lawrence <[email protected]>
-
-. $(dirname $0)/functions.sh
-
-MOD_LIVEPATCH=test_klp_callbacks_demo
-MOD_LIVEPATCH2=test_klp_callbacks_demo2
-MOD_TARGET=test_klp_callbacks_mod
-MOD_TARGET_BUSY=test_klp_callbacks_busy
-
-setup_config
-
-# Test loading multiple livepatches.  This test-case is mainly for comparing
-# with the next test-case.
-#
-# - Load and unload two livepatches, pre and post (un)patch callbacks
-#   execute as each patch progresses through its (un)patching
-#   transition.
-
-start_test "multiple livepatches"
-
-load_lp $MOD_LIVEPATCH
-load_lp $MOD_LIVEPATCH2
-disable_lp $MOD_LIVEPATCH2
-disable_lp $MOD_LIVEPATCH
-unload_lp $MOD_LIVEPATCH2
-unload_lp $MOD_LIVEPATCH
-
-check_result "% insmod test_modules/$MOD_LIVEPATCH.ko
-livepatch: enabling patch '$MOD_LIVEPATCH'
-livepatch: '$MOD_LIVEPATCH': initializing patching transition
-$MOD_LIVEPATCH: pre_patch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH': starting patching transition
-livepatch: '$MOD_LIVEPATCH': completing patching transition
-$MOD_LIVEPATCH: post_patch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH': patching complete
-% insmod test_modules/$MOD_LIVEPATCH2.ko
-livepatch: enabling patch '$MOD_LIVEPATCH2'
-livepatch: '$MOD_LIVEPATCH2': initializing patching transition
-$MOD_LIVEPATCH2: pre_patch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH2': starting patching transition
-livepatch: '$MOD_LIVEPATCH2': completing patching transition
-$MOD_LIVEPATCH2: post_patch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH2': patching complete
-% echo 0 > $SYSFS_KLP_DIR/$MOD_LIVEPATCH2/enabled
-livepatch: '$MOD_LIVEPATCH2': initializing unpatching transition
-$MOD_LIVEPATCH2: pre_unpatch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH2': starting unpatching transition
-livepatch: '$MOD_LIVEPATCH2': completing unpatching transition
-$MOD_LIVEPATCH2: post_unpatch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH2': unpatching complete
-% echo 0 > $SYSFS_KLP_DIR/$MOD_LIVEPATCH/enabled
-livepatch: '$MOD_LIVEPATCH': initializing unpatching transition
-$MOD_LIVEPATCH: pre_unpatch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH': starting unpatching transition
-livepatch: '$MOD_LIVEPATCH': completing unpatching transition
-$MOD_LIVEPATCH: post_unpatch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH': unpatching complete
-% rmmod $MOD_LIVEPATCH2
-% rmmod $MOD_LIVEPATCH"
-
-
-# Load multiple livepatches, but the second as an 'atomic-replace'
-# patch.  When the latter loads, the original livepatch should be
-# disabled and *none* of its pre/post-unpatch callbacks executed.  On
-# the other hand, when the atomic-replace livepatch is disabled, its
-# pre/post-unpatch callbacks *should* be executed.
-#
-# - Load and unload two livepatches, the second of which has its
-#   .replace flag set true.
-#
-# - Pre and post patch callbacks are executed for both livepatches.
-#
-# - Once the atomic replace module is loaded, only its pre and post
-#   unpatch callbacks are executed.
-
-start_test "atomic replace"
-
-load_lp $MOD_LIVEPATCH
-load_lp $MOD_LIVEPATCH2 replace=1
-disable_lp $MOD_LIVEPATCH2
-unload_lp $MOD_LIVEPATCH2
-unload_lp $MOD_LIVEPATCH
-
-check_result "% insmod test_modules/$MOD_LIVEPATCH.ko
-livepatch: enabling patch '$MOD_LIVEPATCH'
-livepatch: '$MOD_LIVEPATCH': initializing patching transition
-$MOD_LIVEPATCH: pre_patch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH': starting patching transition
-livepatch: '$MOD_LIVEPATCH': completing patching transition
-$MOD_LIVEPATCH: post_patch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH': patching complete
-% insmod test_modules/$MOD_LIVEPATCH2.ko replace=1
-livepatch: enabling patch '$MOD_LIVEPATCH2'
-livepatch: '$MOD_LIVEPATCH2': initializing patching transition
-$MOD_LIVEPATCH2: pre_patch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH2': starting patching transition
-livepatch: '$MOD_LIVEPATCH2': completing patching transition
-$MOD_LIVEPATCH2: post_patch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH2': patching complete
-% echo 0 > $SYSFS_KLP_DIR/$MOD_LIVEPATCH2/enabled
-livepatch: '$MOD_LIVEPATCH2': initializing unpatching transition
-$MOD_LIVEPATCH2: pre_unpatch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH2': starting unpatching transition
-livepatch: '$MOD_LIVEPATCH2': completing unpatching transition
-$MOD_LIVEPATCH2: post_unpatch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH2': unpatching complete
-% rmmod $MOD_LIVEPATCH2
-% rmmod $MOD_LIVEPATCH"
-
-
-exit 0

index 72a817d1ddd9..f1e7b9d64c8e 100644
@@ -12,6 +12,7 @@ obj-m += test_klp_atomic_replace.o \
 	test_klp_speaker.o \
 	test_klp_speaker2.o \
 	test_klp_speaker_livepatch.o \
+	test_klp_speaker_livepatch2.o \
 	test_klp_state.o \
 	test_klp_state2.o \
 	test_klp_state3.o \

new file mode 100644
index 000000000000..c011d2ee8301
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2024 SUSE
+
+/* Same livepatch with the same features. */
+#include "test_klp_speaker_livepatch.c"

2.47.1

The per-object callbacks have been deprecated in favor of per-state callbacks and will be removed soon.

Do not use the test livepatch with the obsolete callbacks in the basic livepatch tests. Replace it with the new generic livepatch, which does not call any callbacks by default.

---
 tools/testing/selftests/livepatch/test-livepatch.sh | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

index 6673023d2b66..5e53adb47c58 100755
@@ -6,7 +6,7 @@
 
 MOD_LIVEPATCH1=test_klp_livepatch
 MOD_LIVEPATCH2=test_klp_syscall
-MOD_LIVEPATCH3=test_klp_callbacks_demo
+MOD_LIVEPATCH3=test_klp_speaker_livepatch
 MOD_REPLACE=test_klp_atomic_replace
 
 setup_config

@@ -172,10 +172,8 @@ livepatch: '$MOD_LIVEPATCH2': patching complete
 % insmod test_modules/$MOD_LIVEPATCH3.ko
 livepatch: enabling patch '$MOD_LIVEPATCH3'
 livepatch: '$MOD_LIVEPATCH3': initializing patching transition
-$MOD_LIVEPATCH3: pre_patch_callback: vmlinux
 livepatch: '$MOD_LIVEPATCH3': starting patching transition
 livepatch: '$MOD_LIVEPATCH3': completing patching transition
-$MOD_LIVEPATCH3: post_patch_callback: vmlinux
 livepatch: '$MOD_LIVEPATCH3': patching complete
 % insmod test_modules/$MOD_REPLACE.ko replace=1
 livepatch: enabling patch '$MOD_REPLACE'

2.47.1

The per-object callbacks have been deprecated in favor of per-state callbacks and are scheduled for removal. This commit removes the corresponding test modules that are no longer needed.

These test modules have been superseded by new tests that exercise the per-state callback functionality.

---
 .../selftests/livepatch/test_modules/Makefile |   4 -
 .../test_modules/test_klp_callbacks_busy.c    |  70 ----------
 .../test_modules/test_klp_callbacks_demo.c    | 121 ------------------
 .../test_modules/test_klp_callbacks_demo2.c   |  93 --------------
 .../test_modules/test_klp_callbacks_mod.c     |  24 ----
 5 files changed, 312 deletions(-)
 delete mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_callbacks_busy.c
 delete mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_callbacks_demo.c
 delete mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_callbacks_demo2.c
 delete mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_callbacks_mod.c

index f1e7b9d64c8e..18b0c23ef656 100644
@@ -2,10 +2,6 @@ TESTMODS_DIR := $(realpath $(dir $(abspath $(lastword $(MAKEFILE_LIST)))))
 KDIR ?= /lib/modules/$(shell uname -r)/build
 
 obj-m += test_klp_atomic_replace.o \
-	test_klp_callbacks_busy.o \
-	test_klp_callbacks_demo.o \
-	test_klp_callbacks_demo2.o \
-	test_klp_callbacks_mod.o \
 	test_klp_kprobe.o \
 	test_klp_livepatch.o \
 	test_klp_shadow_vars.o \

deleted file mode 100644
index 8fbe645b1c2c..000000000000
@@ -1,24 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-// Copyright (C) 2018 Joe Lawrence <[email protected]>
-
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
-#include <linux/module.h>
-#include <linux/kernel.h>
-
-static int test_klp_callbacks_mod_init(void)
-{
-	pr_info("%s\n", __func__);
-	return 0;
-}
-
-static void test_klp_callbacks_mod_exit(void)
-{
-	pr_info("%s\n", __func__);
-}
-
-module_init(test_klp_callbacks_mod_init);
-module_exit(test_klp_callbacks_mod_exit);
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Joe Lawrence <[email protected]>");
-MODULE_DESCRIPTION("Livepatch test: target module");

2.47.1

The per-object callbacks have been deprecated in favor of per-state callbacks and are scheduled for removal.

Remove the sample modules with the obsolete per-object callbacks. Instead add new sample modules based on the new selftests for per-state callbacks.

The test modules are just renamed to follow the naming scheme of the existing sample modules. Also only one version of the test module and one version of the livepatch module is built.

---
 samples/livepatch/Makefile                    |   5 +-
 .../livepatch/livepatch-callbacks-busymod.c   |  60 ---
 samples/livepatch/livepatch-callbacks-demo.c  | 196 ---------
 samples/livepatch/livepatch-callbacks-mod.c   |  41 --
 samples/livepatch/livepatch-speaker-fix.c     | 376 ++++++++++++++++++
 samples/livepatch/livepatch-speaker-mod.c     | 203 ++++++++++
 samples/livepatch/livepatch-speaker.h         |  15 +
 7 files changed, 596 insertions(+), 300 deletions(-)
 delete mode 100644 samples/livepatch/livepatch-callbacks-busymod.c
 delete mode 100644 samples/livepatch/livepatch-callbacks-demo.c
 delete mode 100644 samples/livepatch/livepatch-callbacks-mod.c
 create mode 100644 samples/livepatch/livepatch-speaker-fix.c
 create mode 100644 samples/livepatch/livepatch-speaker-mod.c
 create mode 100644 samples/livepatch/livepatch-speaker.h

index 9f853eeb6140..b3e2bc965a0c 100644
@@ -3,6 +3,5 @@ obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-sample.o
 obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-shadow-mod.o
 obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-shadow-fix1.o
 obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-shadow-fix2.o
-obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-callbacks-demo.o
-obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-callbacks-mod.o
-obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-callbacks-busymod.o
+obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-speaker-mod.o
+obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-speaker-fix.o

new file mode 100644
index 000000000000..a0e65c27ea56
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _LIVEPATCH_SPEAKER_H_
+#define _LIVEPATCH_SPEAKER_H_
+
+#include <linux/workqueue.h>
+
+typedef void (*do_block_doors_t)(void);
+
+struct hall {
+	struct work_struct block_doors_work;
+	do_block_doors_t do_block_doors;
+};
+
+#endif //  _LIVEPATCH_SPEAKER_H_

2.47.1

This commit updates the livepatch documentation to reflect recent changes in the behavior of states, callbacks, and shadow variables.

Key changes include:

- Per-state callbacks replace per-object callbacks, invoked only when a livepatch introduces or removes a state.
- Shadow variable lifetime is now tied to the corresponding livepatch state lifetime.
- The "version" field in struct klp_state has been replaced with the "block_disable" flag for improved compatibility handling.
- The "data" field has been removed from struct klp_state; shadow variables are now the recommended way to store state-related data.

This update ensures the documentation accurately describes the current livepatch functionality.

---
 Documentation/livepatch/api.rst          |   2 +-
 Documentation/livepatch/callbacks.rst    | 166 ++++++++++--------
 Documentation/livepatch/index.rst        |   4 +-
 Documentation/livepatch/shadow-vars.rst  |  47 ++++-
 Documentation/livepatch/system-state.rst | 208 ++++++++---------------
 5 files changed, 218 insertions(+), 209 deletions(-)

index 78944b63d74b..9535138bd52d 100644
@@ -27,4 +27,4 @@ Object Types
 ============
 
 .. kernel-doc:: include/linux/livepatch.h
-   :identifiers: klp_patch klp_object klp_func klp_callbacks klp_state
+   :identifiers: klp_patch klp_object klp_func klp_state_callbacks klp_state

index cebf1c71d4a5..8f799c0b85f3 100644
@@ -8,11 +8,11 @@ Kernel Livepatching
     :maxdepth: 1
 
     livepatch
-    callbacks
     cumulative-patches
     module-elf-format
-    shadow-vars
     system-state
+    callbacks
+    shadow-vars
     reliable-stacktrace
     api
 

2.47.1

Hi Petr, I'm finally getting around to looking through these patches. I've made it as far as the selftest cleanups, then skipped ahead to the Documentation here. Before diving into code review, I just wanted to clarify some things for my own understanding. Please correct anything below that is incorrect. IMHO it is easy to step off the intended path here :D

The original livepatch system states operated with a numeric .version field. New livepatches could only be loaded if providing a new set of states, or an equal-or-better version of states already loaded by existing livepatches.

With that in mind, a livepatch state could be thought of as an indication of "a context needing special handling in a (versioned) way".
Livepatches claiming to deal with a particular state, needed to do so
with its latest or current versioning. A livepatch without a particular state was not bound to any restriction on that state, so nothing prevented subsequent atomic replace patches from blowing away existing states (those patches cleaned up their states on their disabling), or subsequent non-atomic replace patches from adding to the collective livepatch state.

This patchset does away with .version and adds .block_disable. This is very different from versioning as prevents the associated state from ever going away. In an atomic-replace series of livepatches, this means once a state is introduced, all following patches must contain that state. In non-atomic-replace series, there is no restriction on subsequent patches, but the original patch introducing the state cannot ever be disabled/unloaded. (I'm not going to consider future hybrid mixed atomic/not use cases as my brain is already full.)

Finally, the patchset adds .is_shadow and .callbacks. A short sequence of livepatches may look like:

  klp_patch A               |  klp_patch B
    .states[x]              |    .states[y]
      .id            = 42   |      .id            = 42
      .callbacks            |      .callbacks
      .block_disable        |      .block_disable
      .is_shadow            |      .is_shadow

is there any harm or confusion if the two patches' state 42 contained disparate .callbacks, .block_disable, or .is_shadow contents?

I /think/ this is allowed by the patchset (though I haven't gotten too deep in my understanding), but I feel that I'm starting to stretch my intuitive understanding of these livepatching states. Applying them to a series of atomic-replace livepatches is fairly straightforward. But then for a non-atomic-replace series, it starts getting weird as multiple callback sets will exist in multiple patches.

In a perfect world, we could describe livepatching states absent callbacks and shadow variables. The documentation is a bit circular as one needs to understand them before fully grasping the purpose of the states. But to use them, you will first need to understand how to set them up in the states. :) Maybe there is no better way, but first I need to correct my understanding.

Thanks,

Joe

Great, thanks a lot for jumping on it.

I am not sure about the word "context". But it might be because I am not a native speaker.

In my view, a state represents a side effect, made by loading the livepatch module (enabling livepatch), which would need a special handling when the livepatch module gets disabled or replaced.

There are currently two types of these side effects:

+ Changes made by callbacks which have to get reverted when the livepatch gets disabled.

+ Shadow variables which have to be freed when the livepatch gets disabled.

The states API was added to handle compatibility between more livepatches. I primary had the atomic replace mode in mind.

Changes made by callbacks, and shadow variables added, by the current livepatch usually should get preserved during the atomic replace.

The original approach was affected by the behavior of per-object callbacks during the atomic replace. Note that:

Only per-object callbacks from the _new_ livepatch were called during the atomic replace.

As a result, previously, new livepatch, using the atomic replace, had to be able to revert changes made by the older livepatches when it did not want to keep them.

This shows nicely that the original semantic was broken! There was no obvious way how to revert obsolete states using the atomic replace.

The new livepatch needed to provide callbacks which were able to revert the obsoleted state. A workaround was to define it as the same state with a higher version. The same state and higher version made the livepatch compatible. The higher version signalized that it was a new variant (a revert) so that newer livepatches did not do the revert again...

I am confused here. Is the talking about the original or new semantic?

In the original semantic, only callbacks from the new livepatch were called during the atomic replace. Livepatches were compatible only when the new livepatch supported all existing states.

In the new semantic, the callbacks from the obsolete livepatch are called for obsolete states. The new livepatch does not need to know about the state. By other words, the replaced livepatch can clean up its own mess.

Honestly, I do not think much about the non-atomic livepatches. It would require input from people who use this approach. It looks like a wild word to me ;-)

I allowed to use the same states for more non-atomic livepatches because it might make sense to share shadow variables. Also more livepatches might depend on the same change made by callbacks and it need not matter which one is installed first.

Yes, this describes the old behavior very well. And the impossibility to remove existing states using the atomic replace was one of the problems.

The API solves this elegantly because it calls callbacks from the replaced livepatch for the obsolete states. The livepatch needed to implement these callbacks anyway to support the disable livepatch operation.

And there is still the option to do not implement the reverse operation when it is not easy or safe. The author could set the new .block_disable flag. It blocks disabling the state. Which blocks disabling the livepatch or replacing it with a livepatch which does not support, aka is not compatible with, the state.

Yes, two incompatible states with the same .id would break things. The callbacks won't be called and the old shadow variables won't get freed during an atomic replace.

It is responsibility of the author of the livepatches to use different .id for different states.

I am not sure if we could prevent mistakes. Hmm, we might add a check that every .id is there only once in the patch.states[] array. Also we could add a human readable .name of the state and ensure that it is the same. Or something like this.

Yeah, as I said. The non-atomic-replace world is a kind of jungle. It would require some real life users which might define some sane rules.

Yeah, the documentation would deserve some bigger refactoring. We created separate file for each feature but it is not easy to get the full picture.

I hope that my answer helped a bit to understand the change. Feel free to ask.

Best Regards,
Petr

Ok, yeah "side effect" works well here.

Right, these are livepatch-native side effects as we're providing the API to introduce them. Nothing would stop a livepatch author from using states to handle their own interesting side effects though. (I'm trying to initially think of states in the abstract and then callbacks/shadow variables are just implementations that tie together in the livepatching core.)

Ah right ...

... so nevermind about this.

Yes, the new model means that the last livepatch providing the state also handles its cleanup on its way out. This is much more intuitive than the previous semantic (for which I missed the step above :)

IMHO the non-atomic world only made sense with cumulative patches. I see some folks reporting that separate operating groups are layering non-atomic patches across subsystems and my head spins.

But we are going to need to consider the use cases (or perhaps prevent them) for the eventual documentation.

Well, providing the same state twice in the same klp_patch seems highly likely a bug by livepatch author. That's worth a WARN?

I'm not sure what to think about the same state id provided by two klp_patches. For a atomic-replace series of patches, if the state content is the same, it's effectively like handing off cleanup responsibility for that state to the incoming patch, right? If the state content changes, that would mean that the incoming patch is redefining the state... which could be ok?

Yup. I can kinda grok things with cumulative non-replace, but it gets a lot harder when considering other use cases cases. I'll put non-replace aside for now and continue diving into the patchset. Thanks for the explanations!

Joe

Hi,

I am sorry for the late reply. I have read the mail on Friday and then forgot to come back to it last Monday...

Yes, I agree. I'll add the check and warning in the next revision of the patch set.

Exactly. And I could imagine an usage of the same state even without the atomic replace. For example, more livepatches could share the same shadow variable. Or they might need the same semantic change of a data structure which would require updating the data by the state callbacks.

Using the same state .id for different purpose is _not_ ok.

We could also imagine the state as a reference count of its users. The pre_patch/post_patch callbacks are called when it is introduced (refcount goes from 0 -> 1). And the pre_unpatch/post_unpatch callbacks are called when the state is being removed (refcount drops from 1 -> 0). [*]

This won't work when two different states share the same .id. The callbacks won't be called when the 2nd one is added or when the 1st one is removed.

That said, I do not know how to check that two states have different semantic when the atomic replace is _not_ used. We could prohibit it. But I think that there are valid use-cases, especially when using cumulative livepatches. So, I would keep it allowed.

[*] Note that the current code does not count to refcount number.
It just checks whether the state is used in other enabled livepatches, see is_state_in_other_patches().

Best Regards,
Petr

This commit removes the obsolete per-object callbacks from the livepatch framework. All selftests have been migrated to the new per-state callbacks, making the per-object callbacks redundant.

Instead, use the new per-state callbacks. They offer improved semantics by associating callbacks and shadow variables with a specific state, enabling better lifetime management of changes.

---
 include/linux/livepatch.h     | 26 --------------------------
 kernel/livepatch/core.c       | 29 -----------------------------
 kernel/livepatch/core.h       | 33 ---------------------------------
 kernel/livepatch/transition.c |  9 ---------
 4 files changed, 97 deletions(-)

index d02d7a616338..428300181af3 100644
@@ -77,35 +77,10 @@ struct klp_func {
 	bool transition;
 };
 
-struct klp_object;
-
-/**
- * struct klp_callbacks - pre/post live-(un)patch callback structure
- * @pre_patch:		executed before code patching
- * @post_patch:		executed after code patching
- * @pre_unpatch:	executed before code unpatching
- * @post_unpatch:	executed after code unpatching
- * @post_unpatch_enabled:	flag indicating if post-unpatch callback
- * 				should run
- *
- * All callbacks are optional.  Only the pre-patch callback, if provided,
- * will be unconditionally executed.  If the parent klp_object fails to
- * patch for any reason, including a non-zero error status returned from
- * the pre-patch callback, no further callbacks will be executed.
- */
-struct klp_callbacks {
-	int (*pre_patch)(struct klp_object *obj);
-	void (*post_patch)(struct klp_object *obj);
-	void (*pre_unpatch)(struct klp_object *obj);
-	void (*post_unpatch)(struct klp_object *obj);
-	bool post_unpatch_enabled;
-};
-
 /**
  * struct klp_object - kernel object structure for live patching
  * @name:	module name (or NULL for vmlinux)
  * @funcs:	function entries for functions to be patched in the object
- * @callbacks:	functions to be executed pre/post (un)patching
  * @kobj:	kobject for sysfs resources
  * @func_list:	dynamic list of the function entries
  * @node:	list node for klp_patch obj_list

@@ -118,7 +93,6 @@ struct klp_object {
 	/* external */
 	const char *name;
 	struct klp_func *funcs;
-	struct klp_callbacks callbacks;
 
 	/* internal */
 	struct kobject kobj;

index 4d244eef0e53..3d6b8edb3e2b 100644
@@ -983,8 +983,6 @@ static int klp_init_patch(struct klp_patch *patch)
 
 static int __klp_disable_patch(struct klp_patch *patch)
 {
-	struct klp_object *obj;
-
 	if (WARN_ON(!patch->enabled))
 		return -EINVAL;
 

@@ -995,10 +993,6 @@ static int __klp_disable_patch(struct klp_patch *patch)
 
 	klp_states_pre_unpatch(patch);
 
-	klp_for_each_object(patch, obj)
-		if (obj->patched)
-			klp_pre_unpatch_callback(obj);
-
 	/*
 	 * Enforce the order of the func->transition writes in
 	 * klp_init_transition() and the TIF_PATCH_PENDING writes in

@@ -1050,13 +1044,6 @@ static int __klp_enable_patch(struct klp_patch *patch)
 		if (!klp_is_object_loaded(obj))
 			continue;
 
-		ret = klp_pre_patch_callback(obj);
-		if (ret) {
-			pr_warn("pre-patch callback failed for object '%s'\n",
-				klp_is_module(obj) ? obj->name : "vmlinux");
-			goto err;
-		}
-
 		ret = klp_patch_object(obj);
 		if (ret) {
 			pr_warn("failed to patch object '%s'\n",

@@ -1226,14 +1213,10 @@ static void klp_cleanup_module_patches_limited(struct module *mod,
 			if (!klp_is_module(obj) || strcmp(obj->name, mod->name))
 				continue;
 
-			if (patch != klp_transition_patch)
-				klp_pre_unpatch_callback(obj);
-
 			pr_notice("reverting patch '%s' on unloading module '%s'\n",
 				  patch->mod->name, obj->mod->name);
 			klp_unpatch_object(obj);
 
-			klp_post_unpatch_callback(obj);
 			klp_clear_object_relocs(patch, obj);
 			klp_free_object_loaded(obj);
 			break;

@@ -1280,25 +1263,13 @@ int klp_module_coming(struct module *mod)
 			pr_notice("applying patch '%s' to loading module '%s'\n",
 				  patch->mod->name, obj->mod->name);
 
-			ret = klp_pre_patch_callback(obj);
-			if (ret) {
-				pr_warn("pre-patch callback failed for object '%s'\n",
-					obj->name);
-				goto err;
-			}
-
 			ret = klp_patch_object(obj);
 			if (ret) {
 				pr_warn("failed to apply patch '%s' to module '%s' (%d)\n",
 					patch->mod->name, obj->mod->name, ret);
-
-				klp_post_unpatch_callback(obj);
 				goto err;
 			}
 
-			if (patch != klp_transition_patch)
-				klp_post_patch_callback(obj);
-
 			break;
 		}
 	}

index 38209c7361b6..02b8364f6779 100644
@@ -23,37 +23,4 @@ static inline bool klp_is_object_loaded(struct klp_object *obj)
 	return !obj->name || obj->mod;
 }
 
-static inline int klp_pre_patch_callback(struct klp_object *obj)
-{
-	int ret = 0;
-
-	if (obj->callbacks.pre_patch)
-		ret = (*obj->callbacks.pre_patch)(obj);
-
-	obj->callbacks.post_unpatch_enabled = !ret;
-
-	return ret;
-}
-
-static inline void klp_post_patch_callback(struct klp_object *obj)
-{
-	if (obj->callbacks.post_patch)
-		(*obj->callbacks.post_patch)(obj);
-}
-
-static inline void klp_pre_unpatch_callback(struct klp_object *obj)
-{
-	if (obj->callbacks.pre_unpatch)
-		(*obj->callbacks.pre_unpatch)(obj);
-}
-
-static inline void klp_post_unpatch_callback(struct klp_object *obj)
-{
-	if (obj->callbacks.post_unpatch_enabled &&
-	    obj->callbacks.post_unpatch)
-		(*obj->callbacks.post_unpatch)(obj);
-
-	obj->callbacks.post_unpatch_enabled = false;
-}
-
 #endif /* _LIVEPATCH_CORE_H */

index f3dce9fe9897..c5e9dcf3e453 100644
@@ -155,15 +155,6 @@ static void klp_complete_transition(void)
 		klp_states_post_unpatch(klp_transition_patch);
 	}
 
-	klp_for_each_object(klp_transition_patch, obj) {
-		if (!klp_is_object_loaded(obj))
-			continue;
-		if (klp_target_state == KLP_TRANSITION_PATCHED)
-			klp_post_patch_callback(obj);
-		else if (klp_target_state == KLP_TRANSITION_UNPATCHED)
-			klp_post_unpatch_callback(obj);
-	}
-
 	pr_notice("'%s': %s complete\n", klp_transition_patch->mod->name,
 		  klp_target_state == KLP_TRANSITION_PATCHED ? "patching" : "unpatching");
 

2.47.1