[PATCH] livepatch: unpatch all klp_objects if klp_module_coming fails

From: Joe Lawrence
Date: Thu Sep 28 2017 - 15:15:19 EST


When an incoming module is considered for livepatching by
klp_module_coming(), it iterates over multiple patches and multiple
kernel objects in this order:

list_for_each_entry(patch, &klp_patches, list) {
klp_for_each_object(patch, obj) {

which means that if one of the kernel objects fails to patch,
klp_module_coming()'s error path needs to unpatch and cleanup any kernel
objects that were already patched by a previous patch.

Reported-by: Miroslav Benes <mbenes@xxxxxxx>
Suggested-by: Petr Mladek <pmladek@xxxxxxxx>
Signed-off-by: Joe Lawrence <joe.lawrence@xxxxxxxxxx>
---
v1:
- separated this patch from the klp_callbacks patchset
- adopted Petr's suggestion to reuse code from klp_module_going()

kernel/livepatch/core.c | 61 ++++++++++++++++++++++++++++++-------------------
1 file changed, 38 insertions(+), 23 deletions(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index b9628e43c78f..3d457e0bbe26 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -830,6 +830,42 @@ int klp_register_patch(struct klp_patch *patch)
}
EXPORT_SYMBOL_GPL(klp_register_patch);

+/*
+ * Revert patches (up to a given limit) to objects belonging to a given
+ * kernel module. After unpatching such objects, the function also
+ * frees them. When limit is NULL, all patches to the given module will
+ * be reverted.
+ */
+static void klp_cleanup_module_objects_limited(struct module *mod,
+ struct klp_patch *limit)
+{
+ struct klp_patch *patch;
+ struct klp_object *obj;
+
+ list_for_each_entry(patch, &klp_patches, list) {
+ if (patch == limit)
+ break;
+
+ klp_for_each_object(patch, obj) {
+ if (!klp_is_module(obj) || strcmp(obj->name, mod->name))
+ continue;
+
+ /*
+ * Only unpatch the module if the patch is enabled or
+ * is in transition.
+ */
+ if (patch->enabled || patch == klp_transition_patch) {
+ pr_notice("reverting patch '%s' on unloading module '%s'\n",
+ patch->mod->name, obj->mod->name);
+ klp_unpatch_object(obj);
+ }
+
+ klp_free_object_loaded(obj);
+ break;
+ }
+ }
+}
+
int klp_module_coming(struct module *mod)
{
int ret;
@@ -894,7 +930,7 @@ int klp_module_coming(struct module *mod)
pr_warn("patch '%s' failed for module '%s', refusing to load module '%s'\n",
patch->mod->name, obj->mod->name, obj->mod->name);
mod->klp_alive = false;
- klp_free_object_loaded(obj);
+ klp_cleanup_module_objects_limited(mod, patch);
mutex_unlock(&klp_mutex);

return ret;
@@ -902,9 +938,6 @@ int klp_module_coming(struct module *mod)

void klp_module_going(struct module *mod)
{
- struct klp_patch *patch;
- struct klp_object *obj;
-
if (WARN_ON(mod->state != MODULE_STATE_GOING &&
mod->state != MODULE_STATE_COMING))
return;
@@ -917,25 +950,7 @@ void klp_module_going(struct module *mod)
*/
mod->klp_alive = false;

- list_for_each_entry(patch, &klp_patches, list) {
- klp_for_each_object(patch, obj) {
- if (!klp_is_module(obj) || strcmp(obj->name, mod->name))
- continue;
-
- /*
- * Only unpatch the module if the patch is enabled or
- * is in transition.
- */
- if (patch->enabled || patch == klp_transition_patch) {
- pr_notice("reverting patch '%s' on unloading module '%s'\n",
- patch->mod->name, obj->mod->name);
- klp_unpatch_object(obj);
- }
-
- klp_free_object_loaded(obj);
- break;
- }
- }
+ klp_cleanup_module_objects_limited(mod, NULL);

mutex_unlock(&klp_mutex);
}
--
1.8.3.1