From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752957AbbEDLl1 (ORCPT ); Mon, 4 May 2015 07:41:27 -0400 Received: from ip4-83-240-67-251.cust.nbox.cz ([83.240.67.251]:50911 "EHLO ip4-83-240-18-248.cust.nbox.cz" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752548AbbEDLk2 (ORCPT ); Mon, 4 May 2015 07:40:28 -0400 From: Jiri Slaby To: live-patching@vger.kernel.org Cc: jpoimboe@redhat.com, sjenning@redhat.com, jkosina@suse.cz, vojtech@suse.cz, mingo@redhat.com, linux-kernel@vger.kernel.org, Jiri Slaby Subject: [RFC kgr on klp 6/9] livepatch: do not allow failure while really patching Date: Mon, 4 May 2015 13:40:22 +0200 Message-Id: <1430739625-4658-6-git-send-email-jslaby@suse.cz> X-Mailer: git-send-email 2.3.5 In-Reply-To: <1430739625-4658-1-git-send-email-jslaby@suse.cz> References: <1430739625-4658-1-git-send-email-jslaby@suse.cz> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org We separate the patching in two phases: * preparation: this one can fail, but in the presence of the kgraft-like patching can be handled easily. * patching: this cannot fail, so that kgraft-like patching need not handle failures in a very complicated way Signed-off-by: Jiri Slaby --- include/linux/livepatch.h | 11 ++++++- include/linux/sched.h | 5 +++ kernel/livepatch/core.c | 84 +++++++++++++++++++++++++++++++---------------- 3 files changed, 70 insertions(+), 30 deletions(-) diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h index 009f308ff756..add2b1bd1cce 100644 --- a/include/linux/livepatch.h +++ b/include/linux/livepatch.h @@ -53,9 +53,18 @@ struct klp_cmodel { struct pt_regs *regs); }; +/** + * enum klp_state -- state of patches, objects, functions + * @KLP_DISABLED: completely disabled + * @KLP_ENABLED: completely enabled (applied) + * @KLP_PREPARED: being applied + * + * @KLP_DISABLED & @KLP_ENABLED are part of the /sys ABI + */ enum klp_state { KLP_DISABLED, - KLP_ENABLED + KLP_ENABLED, + KLP_PREPARED, }; /** diff --git a/include/linux/sched.h b/include/linux/sched.h index 4c0555261cb1..6b2f4c01c516 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -3086,6 +3086,11 @@ static inline void mm_update_next_owner(struct mm_struct *mm) #endif /* CONFIG_MEMCG */ #if IS_ENABLED(CONFIG_LIVEPATCH) +/* + * This function is called only when the given thread is not running + * any patched function. Therefore the flag might be cleared without + * klp_kgr_state_lock. + */ static inline void klp_kgraft_mark_task_safe(struct task_struct *p) { clear_tsk_thread_flag(p, TIF_KGR_IN_PROGRESS); diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c index ab6a36688c93..87d94aadfebf 100644 --- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c @@ -41,11 +41,13 @@ * @node: node for the global klp_ops list * @func_stack: list head for the stack of klp_func's (active func is on top) * @fops: registered ftrace ops struct + * @old_addr: the address of the function which is beine patched */ struct klp_ops { struct list_head node; struct list_head func_stack; struct ftrace_ops fops; + unsigned long old_addr; }; /* @@ -65,12 +67,9 @@ static struct kobject *klp_root_kobj; static struct klp_ops *klp_find_ops(unsigned long old_addr) { struct klp_ops *ops; - struct klp_func *func; list_for_each_entry(ops, &klp_ops, node) { - func = list_first_entry(&ops->func_stack, struct klp_func, - stack_node); - if (func->old_addr == old_addr) + if (ops->old_addr == old_addr) return ops; } @@ -326,11 +325,8 @@ static void notrace klp_ftrace_handler(unsigned long ip, rcu_read_lock(); func = list_first_or_null_rcu(&ops->func_stack, struct klp_func, stack_node); - if (WARN_ON_ONCE(!func)) - goto unlock; - - func->stub(&ops->func_stack, func, regs); -unlock: + if (func) + func->stub(&ops->func_stack, func, regs); rcu_read_unlock(); } @@ -338,18 +334,20 @@ static void klp_disable_func(struct klp_func *func) { struct klp_ops *ops; - WARN_ON(func->state != KLP_ENABLED); + WARN_ON(func->state == KLP_DISABLED); WARN_ON(!func->old_addr); ops = klp_find_ops(func->old_addr); if (WARN_ON(!ops)) return; - if (list_is_singular(&ops->func_stack)) { + if (list_empty(&ops->func_stack) || + list_is_singular(&ops->func_stack)) { WARN_ON(unregister_ftrace_function(&ops->fops)); WARN_ON(ftrace_set_filter_ip(&ops->fops, func->old_addr, 1, 0)); - list_del_rcu(&func->stack_node); + if (!list_empty(&ops->func_stack)) + list_del_rcu(&func->stack_node); list_del(&ops->node); kfree(ops); } else { @@ -359,7 +357,7 @@ static void klp_disable_func(struct klp_func *func) func->state = KLP_DISABLED; } -static int klp_enable_func(struct klp_func *func) +static int klp_prepare_enable_func(struct klp_func *func) { struct klp_ops *ops; int ret; @@ -376,6 +374,7 @@ static int klp_enable_func(struct klp_func *func) if (!ops) return -ENOMEM; + ops->old_addr = func->old_addr; ops->fops.func = klp_ftrace_handler; ops->fops.flags = FTRACE_OPS_FL_SAVE_REGS | FTRACE_OPS_FL_DYNAMIC | @@ -384,7 +383,6 @@ static int klp_enable_func(struct klp_func *func) list_add(&ops->node, &klp_ops); INIT_LIST_HEAD(&ops->func_stack); - list_add_rcu(&func->stack_node, &ops->func_stack); ret = ftrace_set_filter_ip(&ops->fops, func->old_addr, 0, 0); if (ret) { @@ -400,35 +398,43 @@ static int klp_enable_func(struct klp_func *func) ftrace_set_filter_ip(&ops->fops, func->old_addr, 1, 0); goto err; } - - - } else { - list_add_rcu(&func->stack_node, &ops->func_stack); } - func->state = KLP_ENABLED; + func->state = KLP_PREPARED; return 0; err: - list_del_rcu(&func->stack_node); list_del(&ops->node); kfree(ops); return ret; } +static void klp_enable_func(struct klp_func *func) +{ + struct klp_ops *ops; + + ops = klp_find_ops(func->old_addr); + if (WARN_ON(!ops)) /* we have just added that, so? */ + return; + + list_add_rcu(&func->stack_node, &ops->func_stack); + + func->state = KLP_ENABLED; +} + static void klp_disable_object(struct klp_object *obj) { struct klp_func *func; klp_for_each_func(obj, func) - if (func->state == KLP_ENABLED) + if (func->state != KLP_DISABLED) klp_disable_func(func); obj->state = KLP_DISABLED; } -static int klp_enable_object(struct klp_object *obj) +static int klp_prepare_enable_object(struct klp_object *obj) { struct klp_func *func; int ret; @@ -440,17 +446,27 @@ static int klp_enable_object(struct klp_object *obj) return -EINVAL; klp_for_each_func(obj, func) { - ret = klp_enable_func(func); + ret = klp_prepare_enable_func(func); if (ret) { klp_disable_object(obj); return ret; } } - obj->state = KLP_ENABLED; + obj->state = KLP_PREPARED; return 0; } +static void klp_enable_object(struct klp_object *obj) +{ + struct klp_func *func; + + klp_for_each_func(obj, func) + klp_enable_func(func); + + obj->state = KLP_ENABLED; +} + static int __klp_disable_patch(struct klp_patch *patch) { struct klp_object *obj; @@ -463,7 +479,7 @@ static int __klp_disable_patch(struct klp_patch *patch) pr_notice("disabling patch '%s'\n", patch->mod->name); klp_for_each_object(patch, obj) { - if (obj->state == KLP_ENABLED) + if (obj->state == KLP_PREPARED || obj->state == KLP_ENABLED) klp_disable_object(obj); } @@ -526,11 +542,18 @@ static int __klp_enable_patch(struct klp_patch *patch) if (!klp_is_object_loaded(obj)) continue; - ret = klp_enable_object(obj); + ret = klp_prepare_enable_object(obj); if (ret) goto unregister; } + klp_for_each_object(patch, obj) { + if (!klp_is_object_loaded(obj)) + continue; + + klp_enable_object(obj); + } + patch->state = KLP_ENABLED; return 0; @@ -937,10 +960,13 @@ static void klp_module_notify_coming(struct klp_patch *patch, pr_notice("applying patch '%s' to loading module '%s'\n", pmod->name, mod->name); - ret = klp_enable_object(obj); - if (!ret) - return; + ret = klp_prepare_enable_object(obj); + if (ret) + goto err; + + klp_enable_object(obj); + return; err: pr_warn("failed to apply patch '%s' to module '%s' (%d)\n", pmod->name, mod->name, ret); -- 2.3.5