[RFC PATCH 5/6] livepatch: Remove obsolete per-object callbacks

[Yafang Shao <laoar.shao@gmail.com>]

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.

Originally-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 include/linux/livepatch.h                | 40 ---------------
 include/linux/livepatch_external.h       | 62 +++++++++++++++---------
 kernel/livepatch/core.c                  | 29 -----------
 kernel/livepatch/core.h                  | 33 -------------
 kernel/livepatch/transition.c            |  9 ----
 scripts/livepatch/init.c                 |  2 -
 tools/include/linux/livepatch_external.h | 62 +++++++++++++++---------
 tools/objtool/klp-diff.c                 | 16 +++---
 8 files changed, 84 insertions(+), 169 deletions(-)

diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index 340b04a0de83..221f176f1f51 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -95,7 +95,6 @@ struct klp_object {
 	/* external */
 	const char *name;
 	struct klp_func *funcs;
-	struct klp_callbacks callbacks;
 
 	/* internal */
 	struct kobject kobj;
@@ -106,45 +105,6 @@ struct klp_object {
 	bool patched;
 };
 
-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
- *			 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
- * @shadow_dtor:	 destructor for the related shadow variable
- * @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);
-	klp_shadow_dtor_t shadow_dtor;
-	bool pre_patch_succeeded;
-};
-
 /**
  * struct klp_state - state of the system modified by the livepatch
  * @id:		system state identifier (non-zero)
diff --git a/include/linux/livepatch_external.h b/include/linux/livepatch_external.h
index 138af19b0f5c..d9123d0c5dff 100644
--- a/include/linux/livepatch_external.h
+++ b/include/linux/livepatch_external.h
@@ -21,33 +21,48 @@
 #define KLP_PRE_UNPATCH_PREFIX		__stringify(__KLP_PRE_UNPATCH_PREFIX)
 #define KLP_POST_UNPATCH_PREFIX		__stringify(__KLP_POST_UNPATCH_PREFIX)
 
-struct klp_object;
-
-typedef int (*klp_pre_patch_t)(struct klp_object *obj);
-typedef void (*klp_post_patch_t)(struct klp_object *obj);
-typedef void (*klp_pre_unpatch_t)(struct klp_object *obj);
-typedef void (*klp_post_unpatch_t)(struct klp_object *obj);
+struct klp_state;
+struct klp_patch;
+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_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
+ * 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
+ * @shadow_dtor:	 destructor for the related shadow variable
+ * @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.
  *
- * 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.
+ * 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_callbacks {
-	klp_pre_patch_t		pre_patch;
-	klp_post_patch_t	post_patch;
-	klp_pre_unpatch_t	pre_unpatch;
-	klp_post_unpatch_t	post_unpatch;
-	bool post_unpatch_enabled;
+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);
+	klp_shadow_dtor_t shadow_dtor;
+	bool pre_patch_succeeded;
+};
+
+struct klp_state_ext {
+	unsigned long id;
+	unsigned int version;
+	struct klp_state_callbacks callbacks;
 };
 
 /*
@@ -69,7 +84,6 @@ struct klp_func_ext {
 struct klp_object_ext {
 	const char *name;
 	struct klp_func_ext *funcs;
-	struct klp_callbacks callbacks;
 	unsigned int nr_funcs;
 };
 
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 95c099a8f594..eae807916ca0 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -1009,8 +1009,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;
 
@@ -1021,10 +1019,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
@@ -1075,13 +1069,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",
@@ -1253,14 +1240,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;
@@ -1307,25 +1290,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;
 		}
 	}
diff --git a/kernel/livepatch/core.h b/kernel/livepatch/core.h
index 38209c7361b6..02b8364f6779 100644
--- a/kernel/livepatch/core.h
+++ b/kernel/livepatch/core.h
@@ -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 */
diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
index 1a2b11be7b5a..f844283b5423 100644
--- a/kernel/livepatch/transition.c
+++ b/kernel/livepatch/transition.c
@@ -145,15 +145,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");
 
diff --git a/scripts/livepatch/init.c b/scripts/livepatch/init.c
index 659db21a5b53..04e8d20bab2a 100644
--- a/scripts/livepatch/init.c
+++ b/scripts/livepatch/init.c
@@ -63,8 +63,6 @@ static int __init livepatch_mod_init(void)
 
 		obj->name = obj_ext->name;
 		obj->funcs = funcs;
-
-		memcpy(&obj->callbacks, &obj_ext->callbacks, sizeof(struct klp_callbacks));
 	}
 
 	patch->mod = THIS_MODULE;
diff --git a/tools/include/linux/livepatch_external.h b/tools/include/linux/livepatch_external.h
index 138af19b0f5c..d9123d0c5dff 100644
--- a/tools/include/linux/livepatch_external.h
+++ b/tools/include/linux/livepatch_external.h
@@ -21,33 +21,48 @@
 #define KLP_PRE_UNPATCH_PREFIX		__stringify(__KLP_PRE_UNPATCH_PREFIX)
 #define KLP_POST_UNPATCH_PREFIX		__stringify(__KLP_POST_UNPATCH_PREFIX)
 
-struct klp_object;
-
-typedef int (*klp_pre_patch_t)(struct klp_object *obj);
-typedef void (*klp_post_patch_t)(struct klp_object *obj);
-typedef void (*klp_pre_unpatch_t)(struct klp_object *obj);
-typedef void (*klp_post_unpatch_t)(struct klp_object *obj);
+struct klp_state;
+struct klp_patch;
+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_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
+ * 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
+ * @shadow_dtor:	 destructor for the related shadow variable
+ * @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.
  *
- * 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.
+ * 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_callbacks {
-	klp_pre_patch_t		pre_patch;
-	klp_post_patch_t	post_patch;
-	klp_pre_unpatch_t	pre_unpatch;
-	klp_post_unpatch_t	post_unpatch;
-	bool post_unpatch_enabled;
+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);
+	klp_shadow_dtor_t shadow_dtor;
+	bool pre_patch_succeeded;
+};
+
+struct klp_state_ext {
+	unsigned long id;
+	unsigned int version;
+	struct klp_state_callbacks callbacks;
 };
 
 /*
@@ -69,7 +84,6 @@ struct klp_func_ext {
 struct klp_object_ext {
 	const char *name;
 	struct klp_func_ext *funcs;
-	struct klp_callbacks callbacks;
 	unsigned int nr_funcs;
 };
 
diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index c2c4e4968bc2..128fbe054417 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -1606,8 +1606,8 @@ static int create_klp_sections(struct elfs *e)
 		reloc = find_reloc_by_dest(e->out, sym->sec, sym->offset);
 
 		if (!elf_create_reloc(e->out, obj_sec,
-				      offsetof(struct klp_object_ext, callbacks) +
-				      offsetof(struct klp_callbacks, pre_patch),
+				      offsetof(struct klp_state_ext, callbacks) +
+				      offsetof(struct klp_state_callbacks, pre_patch),
 				      reloc->sym, reloc_addend(reloc), R_ABS64))
 			return -1;
 	}
@@ -1622,8 +1622,8 @@ static int create_klp_sections(struct elfs *e)
 		reloc = find_reloc_by_dest(e->out, sym->sec, sym->offset);
 
 		if (!elf_create_reloc(e->out, obj_sec,
-				      offsetof(struct klp_object_ext, callbacks) +
-				      offsetof(struct klp_callbacks, post_patch),
+				      offsetof(struct klp_state_ext, callbacks) +
+				      offsetof(struct klp_state_callbacks, post_patch),
 				      reloc->sym, reloc_addend(reloc), R_ABS64))
 			return -1;
 	}
@@ -1638,8 +1638,8 @@ static int create_klp_sections(struct elfs *e)
 		reloc = find_reloc_by_dest(e->out, sym->sec, sym->offset);
 
 		if (!elf_create_reloc(e->out, obj_sec,
-				      offsetof(struct klp_object_ext, callbacks) +
-				      offsetof(struct klp_callbacks, pre_unpatch),
+				      offsetof(struct klp_state_ext, callbacks) +
+				      offsetof(struct klp_state_callbacks, pre_unpatch),
 				      reloc->sym, reloc_addend(reloc), R_ABS64))
 			return -1;
 	}
@@ -1654,8 +1654,8 @@ static int create_klp_sections(struct elfs *e)
 		reloc = find_reloc_by_dest(e->out, sym->sec, sym->offset);
 
 		if (!elf_create_reloc(e->out, obj_sec,
-				      offsetof(struct klp_object_ext, callbacks) +
-				      offsetof(struct klp_callbacks, post_unpatch),
+				      offsetof(struct klp_state_ext, callbacks) +
+				      offsetof(struct klp_state_callbacks, post_unpatch),
 				      reloc->sym, reloc_addend(reloc), R_ABS64))
 			return -1;
 	}
-- 
2.47.3