Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752070AbdIMVhb (ORCPT ); Wed, 13 Sep 2017 17:37:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52728 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751443AbdIMVhJ (ORCPT ); Wed, 13 Sep 2017 17:37:09 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com D6F81C04B310 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=joe.lawrence@redhat.com From: Joe Lawrence To: live-patching@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Josh Poimboeuf , Jessica Yu , Jiri Kosina , Miroslav Benes , Petr Mladek , Chris J Arges Subject: [RFC] livepatch: unpatch all klp_objects if klp_module_coming fails Date: Wed, 13 Sep 2017 17:36:38 -0400 Message-Id: <1505338598-16041-1-git-send-email-joe.lawrence@redhat.com> In-Reply-To: <6ed9c3ac-ac8f-8678-a884-754cdd07b447@redhat.com> References: <6ed9c3ac-ac8f-8678-a884-754cdd07b447@redhat.com> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Wed, 13 Sep 2017 21:37:09 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9605 Lines: 209 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 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 Signed-off-by: Joe Lawrence --- 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