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 <[email protected]> Signed-off-by: Yafang Shao <[email protected]> --- 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
