[RFC] livepatch: unpatch all klp_objects if klp_module_coming fails

Joe Lawrence
2017-09-13T21:37:31Z

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