From 0937e3b025f70e33f018aa55ee8d32b8731730a7 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Mon, 9 Feb 2015 11:31:13 -0600 Subject: livepatch: simplify disable error path If registering the function with ftrace has previously succeeded, unregistering will almost never fail. Even if it does, it's not a fatal error. We can still carry on and disable the klp_func from being used by removing it from the klp_ops func stack. Signed-off-by: Josh Poimboeuf Reviewed-by: Miroslav Benes Reviewed-by: Petr Mladek Signed-off-by: Jiri Kosina --- kernel/livepatch/core.c | 67 +++++++++++++------------------------------------ 1 file changed, 17 insertions(+), 50 deletions(-) (limited to 'kernel') diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c index ff7f47d026ac..26df09d56f7c 100644 --- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c @@ -322,32 +322,20 @@ static void notrace klp_ftrace_handler(unsigned long ip, klp_arch_set_pc(regs, (unsigned long)func->new_func); } -static int klp_disable_func(struct klp_func *func) +static void klp_disable_func(struct klp_func *func) { struct klp_ops *ops; - int ret; - - if (WARN_ON(func->state != KLP_ENABLED)) - return -EINVAL; - if (WARN_ON(!func->old_addr)) - return -EINVAL; + WARN_ON(func->state != KLP_ENABLED); + WARN_ON(!func->old_addr); ops = klp_find_ops(func->old_addr); if (WARN_ON(!ops)) - return -EINVAL; + return; if (list_is_singular(&ops->func_stack)) { - ret = unregister_ftrace_function(&ops->fops); - if (ret) { - pr_err("failed to unregister ftrace handler for function '%s' (%d)\n", - func->old_name, ret); - return ret; - } - - ret = ftrace_set_filter_ip(&ops->fops, func->old_addr, 1, 0); - if (ret) - pr_warn("function unregister succeeded but failed to clear the filter\n"); + 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); list_del(&ops->node); @@ -357,8 +345,6 @@ static int klp_disable_func(struct klp_func *func) } func->state = KLP_DISABLED; - - return 0; } static int klp_enable_func(struct klp_func *func) @@ -419,23 +405,15 @@ err: return ret; } -static int klp_disable_object(struct klp_object *obj) +static void klp_disable_object(struct klp_object *obj) { struct klp_func *func; - int ret; - for (func = obj->funcs; func->old_name; func++) { - if (func->state != KLP_ENABLED) - continue; - - ret = klp_disable_func(func); - if (ret) - return ret; - } + for (func = obj->funcs; func->old_name; func++) + if (func->state == KLP_ENABLED) + klp_disable_func(func); obj->state = KLP_DISABLED; - - return 0; } static int klp_enable_object(struct klp_object *obj) @@ -451,22 +429,19 @@ static int klp_enable_object(struct klp_object *obj) for (func = obj->funcs; func->old_name; func++) { ret = klp_enable_func(func); - if (ret) - goto unregister; + if (ret) { + klp_disable_object(obj); + return ret; + } } obj->state = KLP_ENABLED; return 0; - -unregister: - WARN_ON(klp_disable_object(obj)); - return ret; } static int __klp_disable_patch(struct klp_patch *patch) { struct klp_object *obj; - int ret; /* enforce stacking: only the last enabled patch can be disabled */ if (!list_is_last(&patch->list, &klp_patches) && @@ -476,12 +451,8 @@ static int __klp_disable_patch(struct klp_patch *patch) pr_notice("disabling patch '%s'\n", patch->mod->name); for (obj = patch->objs; obj->funcs; obj++) { - if (obj->state != KLP_ENABLED) - continue; - - ret = klp_disable_object(obj); - if (ret) - return ret; + if (obj->state == KLP_ENABLED) + klp_disable_object(obj); } patch->state = KLP_DISABLED; @@ -931,7 +902,6 @@ static void klp_module_notify_going(struct klp_patch *patch, { struct module *pmod = patch->mod; struct module *mod = obj->mod; - int ret; if (patch->state == KLP_DISABLED) goto disabled; @@ -939,10 +909,7 @@ static void klp_module_notify_going(struct klp_patch *patch, pr_notice("reverting patch '%s' on unloading module '%s'\n", pmod->name, mod->name); - ret = klp_disable_object(obj); - if (ret) - pr_warn("failed to revert patch '%s' on module '%s' (%d)\n", - pmod->name, mod->name, ret); + klp_disable_object(obj); disabled: klp_free_object_loaded(obj); -- cgit v1.2.3-59-g8ed1b From 2e3ac940f2754d7dc616aba1643a668954fe892f Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Tue, 3 Mar 2015 17:02:21 -0600 Subject: livepatch: remove unnecessary call to klp_find_object_module() klp_find_object_module() is called from both the klp register and enable paths. Only the call from the register path is necessary because the module notifier will let us know if the patched module gets loaded or unloaded. Signed-off-by: Josh Poimboeuf Reviewed-by: Petr Mladek Signed-off-by: Jiri Kosina --- kernel/livepatch/core.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'kernel') diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c index 26df09d56f7c..d03d6134e824 100644 --- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c @@ -511,8 +511,6 @@ static int __klp_enable_patch(struct klp_patch *patch) pr_notice("enabling patch '%s'\n", patch->mod->name); for (obj = patch->objs; obj->funcs; obj++) { - klp_find_object_module(obj); - if (!klp_is_object_loaded(obj)) continue; -- cgit v1.2.3-59-g8ed1b