[PATCH v4 5/5] livepatch: Clean up klp_init_object_loaded() when fails
From: Petr Mladek <pmladek@suse.com>
Date: 2026-09-08 12:04:41
Also in:
lkml
Subsystem:
live patching, the rest · Maintainers:
Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek, Linus Torvalds
When loading a module, klp_module_coming() updates all enabled patches. If an error occurs, klp_cleanup_module_patches_limited() cleans up fully processed patches, but skips the patch that failed midway. The current code is a bit messy. The changes done by klp_init_object_loaded() should get cleared by klp_free_object_loaded(). But this function also clears obj->mod which is set by klp_module_coming(). And relocations are cleared separately. Fix the situations by updating klp_free_object_loaded(). It should revert all and only changes made by klp_init_object_loaded(). This requires some shuffling: + Clear obj->mod explicitly in klp_cleanup_module_patches_limited() and do not rely on klp_free_object_loaded(). + Clear relocations in klp_free_object_loaded(). Remove the explicit call from klp_cleanup_module_patches_limited(). This requires adding the @patch parameter. Next, klp_init_object_loaded() has to clear its own changes on failure. It just returns an error when relocations failed because they clear their own mess. It could call klp_free_object_loaded() in other situations because all relocations were done and other values are just cleared. Finally, in klp_module_coming(), avoid code duplication by goto targets. There is no need to clear relocations for other objects in the failing patch because klp_module_coming() operates strictly on the specific module being loaded. Reported-by: sashiko-bot@kernel.org Closes: https://lore.kernel.org/r/20260823062313.1321B1F000E9@smtp.kernel.org (local) Closes: https://lore.kernel.org/r/20260830175608.4BABB1F000E9@smtp.kernel.org (local) Acked-by: Song Liu <song@kernel.org> Signed-off-by: Petr Mladek <pmladek@suse.com> --- kernel/livepatch/core.c | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-)
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 714f97fdd271..87b2331486d4 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c@@ -738,18 +738,20 @@ static void __klp_free_funcs(struct klp_object *obj, bool nops_only) } /* Clean up when a patched object is unloaded */ -static void klp_free_object_loaded(struct klp_object *obj) +static void klp_free_object_loaded(struct klp_patch *patch, + struct klp_object *obj) { struct klp_func *func; - obj->mod = NULL; - klp_for_each_func(obj, func) { func->old_func = NULL; if (func->nop) func->new_func = NULL; } + + if (klp_is_module(obj)) + klp_clear_object_relocs(patch, obj); } static void __klp_free_objects(struct klp_patch *patch, bool nops_only)
@@ -896,7 +898,7 @@ static int klp_init_object_loaded(struct klp_patch *patch, func->old_sympos, (unsigned long *)&func->old_func); if (ret) - return ret; + goto err; /* * Aliased symbols share one address, so they would resolve to
@@ -909,7 +911,8 @@ static int klp_init_object_loaded(struct klp_patch *patch, if (prev_func->old_func == func->old_func) { pr_err("'%s' and '%s' resolve to the same address, aliased symbols are not supported\n", prev_func->old_name, func->old_name); - return -EINVAL; + ret = -EINVAL; + goto err; } }
@@ -918,7 +921,8 @@ static int klp_init_object_loaded(struct klp_patch *patch, if (!ret) { pr_err("kallsyms size lookup failed for '%s'\n", func->old_name); - return -ENOENT; + ret = -ENOENT; + goto err; } if (func->nop)
@@ -929,11 +933,17 @@ static int klp_init_object_loaded(struct klp_patch *patch, if (!ret) { pr_err("kallsyms size lookup failed for '%s' replacement\n", func->old_name); - return -ENOENT; + ret = -ENOENT; + goto err; } } return 0; + +err: + klp_free_object_loaded(patch, obj); + + return ret; } static int klp_init_object(struct klp_patch *patch, struct klp_object *obj)
@@ -1287,8 +1297,8 @@ static void klp_cleanup_module_patches_limited(struct module *mod, klp_unpatch_object(obj); klp_post_unpatch_callback(obj); - klp_clear_object_relocs(patch, obj); - klp_free_object_loaded(obj); + klp_free_object_loaded(patch, obj); + obj->mod = NULL; break; } }
@@ -1337,7 +1347,7 @@ int klp_module_coming(struct module *mod) if (ret) { pr_warn("pre-patch callback failed for object '%s'\n", obj->name); - goto err; + goto err_free_object; } ret = klp_patch_object(obj);
@@ -1345,8 +1355,7 @@ int klp_module_coming(struct module *mod) 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; + goto err_unpatch_callback; } if (patch != klp_transition_patch)
@@ -1360,6 +1369,10 @@ int klp_module_coming(struct module *mod) return 0; +err_unpatch_callback: + klp_post_unpatch_callback(obj); +err_free_object: + klp_free_object_loaded(patch, obj); err: /* * If a patch is unsuccessfully applied, return
--
2.55.0