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: [email protected] Closes: https://lore.kernel.org/r/[email protected] Closes: https://lore.kernel.org/r/[email protected] Acked-by: Song Liu <[email protected]> Signed-off-by: Petr Mladek <[email protected]> --- 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
