[RFC] livepatch: unpatch all klp_objects if klp_module_coming fails

[Joe Lawrence <joe.lawrence@redhat.com>]

Hi Miroslav,

I worked out the code that I posted earlier today and I think this could
address the multiple-patch module_coming() issue you pointed out.

Note that this was tacked onto the end of the "[PATCH v5 0/3] livepatch
callbacks" patchset, so it includes unpatching callbacks.  I can easily
strip those out (and remove the additional debugging pr_'s) and make
this a stand-alone patch that would apply before the callback patchset.

See the test case below.

-- Joe

Test X
------

Multiple livepatches targeting the same klp_objects may be loaded at
the same time.  If a target module loads and any of the livepatch's
pre-patch callbacks fail, then the module is not allowed to load.
Furthermore, any livepatches that that did succeed will be reverted
(only the incoming module / klp_object) and their pre/post-unpatch
callbacks executed.

  - load livepatch
  - load livepatch2
  - load livepatch3
  - setup livepatch3 pre-patch return of -ENODEV
  - load target module (should fail)
  - disable livepatch3
  - disable livepatch2
  - disable livepatch
  - unload livepatch3
  - unload livepatch2
  - unload livepatch


Load three livepatches, each target a livepatch_callbacks_mod module and
vmlinux:

  % insmod samples/livepatch/livepatch-callbacks-demo.ko 
  [   26.032048] livepatch_callbacks_demo: module verification failed: signature and/or required key missing - tainting kernel
  [   26.033701] livepatch: enabling patch 'livepatch_callbacks_demo'
  [   26.034294] livepatch_callbacks_demo: pre_patch_callback: vmlinux
  [   26.034850] livepatch: 'livepatch_callbacks_demo': starting patching transition
  [   27.743212] livepatch_callbacks_demo: post_patch_callback: vmlinux
  [   27.744130] livepatch: 'livepatch_callbacks_demo': patching complete

  % insmod samples/livepatch/livepatch-callbacks-demo2.ko 
  [   29.120553] livepatch: enabling patch 'livepatch_callbacks_demo2'
  [   29.121077] livepatch_callbacks_demo2: pre_patch_callback: vmlinux
  [   29.121610] livepatch: 'livepatch_callbacks_demo2': starting patching transition
  [   30.751215] livepatch_callbacks_demo2: post_patch_callback: vmlinux
  [   30.751786] livepatch: 'livepatch_callbacks_demo2': patching complete

  % insmod samples/livepatch/livepatch-callbacks-demo3.ko 
  [   32.144285] livepatch: enabling patch 'livepatch_callbacks_demo3'
  [   32.144779] livepatch_callbacks_demo3: pre_patch_callback: vmlinux
  [   32.145360] livepatch: 'livepatch_callbacks_demo3': starting patching transition
  [   33.695211] livepatch_callbacks_demo3: post_patch_callback: vmlinux
  [   33.695739] livepatch: 'livepatch_callbacks_demo3': patching complete

Setup the third livepatch to fail its pre-patch callback when the target
module is loaded:

  % echo samples/livepatch/livepatch-callbacks-demo3.ko > /sys/module/livepatch_callbacks_demo3/parameters/pre_patch_ret

Load the target module:

  % insmod samples/livepatch/livepatch-callbacks-mod.ko 

The first livepatch pre-patch callback succeeds, the klp_object is
patched, and its post-patch callback is executed:

  [   38.210512] livepatch: applying patch 'livepatch_callbacks_demo' to loading module 'livepatch_callbacks_mod'
  [   38.211430] livepatch_callbacks_demo: pre_patch_callback: livepatch_callbacks_mod -> [MODULE_STATE_COMING] Full formed, running module_init
  [   38.212426] livepatch: JL: klp_patch_object(ffffffffc02a9128) patch=ffffffffc02a9000 obj->name: livepatch_callbacks_mod
  [   38.213243] livepatch_callbacks_demo: post_patch_callback: livepatch_callbacks_mod -> [MODULE_STATE_COMING] Full formed, running module_init

Likewise for the second livepatch:

  [   38.214578] livepatch: applying patch 'livepatch_callbacks_demo2' to loading module 'livepatch_callbacks_mod'
  [   38.215754] livepatch_callbacks_demo2: pre_patch_callback: livepatch_callbacks_mod -> [MODULE_STATE_COMING] Full formed, running module_init
  [   38.217066] livepatch: JL: klp_patch_object(ffffffffc02ae128) patch=ffffffffc02ae000 obj->name: livepatch_callbacks_mod
  [   38.218072] livepatch_callbacks_demo2: post_patch_callback: livepatch_callbacks_mod -> [MODULE_STATE_COMING] Full formed, running module_init

But the third livepatch fails its pre-patch callback:

  [   38.219290] livepatch: applying patch 'livepatch_callbacks_demo3' to loading module 'livepatch_callbacks_mod'
  [   38.220182] livepatch_callbacks_demo3: pre_patch_callback: livepatch_callbacks_mod -> [MODULE_STATE_COMING] Full formed, running module_init
  [   38.221256] livepatch: pre-patch callback failed for object 'livepatch_callbacks_mod'

We refuse to load the target module:

  [   38.221906] livepatch: patch 'livepatch_callbacks_demo3' failed for module 'livepatch_callbacks_mod', refusing to load module 'livepatch_callbacks_mod'

So we double back and unpatch (including pre-unpatch and post-unpatch
callbacks) the first livepatch, then the second:

  [   38.223080] livepatch_callbacks_demo: pre_unpatch_callback: livepatch_callbacks_mod -> [MODULE_STATE_COMING] Full formed, running module_init
  [   38.223966] livepatch: JL: klp_unpatch_object(ffffffffc02a9128) patch=ffffffffc02a9000 obj->name: livepatch_callbacks_mod
  [   38.224980] livepatch_callbacks_demo: post_unpatch_callback: livepatch_callbacks_mod -> [MODULE_STATE_COMING] Full formed, running module_init
  [   38.226174] livepatch_callbacks_demo2: pre_unpatch_callback: livepatch_callbacks_mod -> [MODULE_STATE_COMING] Full formed, running module_init
  [   38.227127] livepatch: JL: klp_unpatch_object(ffffffffc02ae128) patch=ffffffffc02ae000 obj->name: livepatch_callbacks_mod
  [   38.228231] livepatch_callbacks_demo2: post_unpatch_callback: livepatch_callbacks_mod -> [MODULE_STATE_COMING] Full formed, running module_init

Finally the module loader reports an error:

  [   38.242684] insmod: ERROR: could not insert module samples/livepatch/livepatch-callbacks-mod.ko: No such device

Clean it all up:

  % echo 0 > /sys/kernel/livepatch/livepatch_callbacks_demo3/enabled
  [   41.248198] livepatch_callbacks_demo3: pre_unpatch_callback: vmlinux
  [   41.248799] livepatch: 'livepatch_callbacks_demo3': starting unpatching transition
  [   42.719135] livepatch_callbacks_demo3: post_unpatch_callback: vmlinux
  [   42.719622] livepatch: 'livepatch_callbacks_demo3': unpatching complete
  
  % echo 0 > /sys/kernel/livepatch/livepatch_callbacks_demo2/enabled
  [   47.269103] livepatch_callbacks_demo2: pre_unpatch_callback: vmlinux
  [   47.269682] livepatch: 'livepatch_callbacks_demo2': starting unpatching transition
  [   48.735253] livepatch_callbacks_demo2: post_unpatch_callback: vmlinux
  [   48.735928] livepatch: 'livepatch_callbacks_demo2': unpatching complete

  % echo 0 > /sys/kernel/livepatch/livepatch_callbacks_demo/enabled
  [   53.289287] livepatch_callbacks_demo: pre_unpatch_callback: vmlinux
  [   53.289987] livepatch: 'livepatch_callbacks_demo': starting unpatching transition
  [   54.751146] livepatch_callbacks_demo: post_unpatch_callback: vmlinux
  [   54.751656] livepatch: 'livepatch_callbacks_demo': unpatching complete

  % rmmod samples/livepatch/livepatch-callbacks-demo3.ko
  % rmmod samples/livepatch/livepatch-callbacks-demo2.ko
  % rmmod samples/livepatch/livepatch-callbacks-demo.ko


-->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8--

>From b80b90cb54b498d2b1165d409ce4b0ca47610b36 Mon Sep 17 00:00:00 2001
From: Joe Lawrence <joe.lawrence@redhat.com>
Date: Wed, 13 Sep 2017 16:51:13 -0400
Subject: [RFC] livepatch: unpatch all klp_objects if klp_module_coming fails

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 fail to patch for whatever
reason, klp_module_coming()'s error path should double back and unpatch
any previous kernel object that was patched for a previous patch.

Reported-by: Miroslav Benes <mbenes@suse.cz>
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
 kernel/livepatch/core.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index aca62c4b8616..7f5192618cc8 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -889,6 +889,8 @@ int klp_module_coming(struct module *mod)
 				goto err;
 			}
 
+pr_err("JL: klp_patch_object(%p) patch=%p obj->name: %s\n", obj, patch, obj->name);
+
 			ret = klp_patch_object(obj);
 			if (ret) {
 				pr_warn("failed to apply patch '%s' to module '%s' (%d)\n",
@@ -919,7 +921,33 @@ 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);
+
+	/*
+	 * Run back through the patch list and unpatch any klp_object that
+	 * was patched before hitting an error above.
+	 */
+
+	list_for_each_entry(patch, &klp_patches, list) {
+
+		if (!patch->enabled || patch == klp_transition_patch)
+			continue;
+
+		klp_for_each_object(patch, obj) {
+
+			if (!obj->patched || !klp_is_module(obj) ||
+			    strcmp(obj->name, mod->name))
+				continue;
+
+			klp_pre_unpatch_callback(obj);
+pr_err("JL: klp_unpatch_object(%p) patch=%p obj->name: %s\n", obj, patch, obj->name);
+			klp_unpatch_object(obj);
+			klp_post_unpatch_callback(obj);
+			klp_free_object_loaded(obj);
+
+			break;
+		}
+	}
+
 	mutex_unlock(&klp_mutex);
 
 	return ret;
-- 
2.7.5