From 37a756af580f7066b18f60b2693c41604e3268b9 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 19 Mar 2014 10:01:29 -0500 Subject: [PATCH 1/2] kmod/core: protect kpatch_[un]register with mutex Use a mutex in the register/unregister functions to protect changes to kpatch_num_registered, kpatch_func_hash and calls to the ftrace functions by other register/unregister invocations. --- kmod/core/core.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/kmod/core/core.c b/kmod/core/core.c index db82ebb..225c227 100644 --- a/kmod/core/core.c +++ b/kmod/core/core.c @@ -48,6 +48,8 @@ #define KPATCH_HASH_BITS 8 DEFINE_HASHTABLE(kpatch_func_hash, KPATCH_HASH_BITS); +DEFINE_SEMAPHORE(kpatch_mutex); + static int kpatch_num_registered; struct kpatch_backtrace_args { @@ -209,6 +211,8 @@ int kpatch_register(struct module *mod, struct kpatch_func *funcs, .num_funcs = num_funcs, }; + down(&kpatch_mutex); + for (i = 0; i < num_funcs; i++) { struct kpatch_func *func = &funcs[i]; @@ -225,7 +229,7 @@ int kpatch_register(struct module *mod, struct kpatch_func *funcs, } /* Register the ftrace trampoline if it hasn't been done already. */ - if (!kpatch_num_registered++) { /* TODO atomic */ + if (!kpatch_num_registered++) { ret = register_ftrace_function(&kpatch_ftrace_ops); if (ret) { printk("kpatch: can't register ftrace function \n"); @@ -252,6 +256,7 @@ int kpatch_register(struct module *mod, struct kpatch_func *funcs, pr_notice("loaded patch module \"%s\"\n", mod->name); out: + up(&kpatch_mutex); return ret; } EXPORT_SYMBOL(kpatch_register); @@ -265,6 +270,8 @@ int kpatch_unregister(struct module *mod, struct kpatch_func *funcs, .num_funcs = num_funcs, }; + down(&kpatch_mutex); + ret = stop_machine(kpatch_remove_patch, &args, NULL); if (ret) goto out; @@ -293,6 +300,7 @@ int kpatch_unregister(struct module *mod, struct kpatch_func *funcs, pr_notice("unloaded patch module \"%s\"\n", mod->name); out: + up(&kpatch_mutex); return ret; } EXPORT_SYMBOL(kpatch_unregister); From 29227a0fbda8e9108b26c1cf0c93a849caf67577 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Mon, 17 Mar 2014 10:36:11 -0500 Subject: [PATCH 2/2] kmod/core: improve performance for cumulative patching When multiple patch modules patch the same function, there's no need to patch all the intermediate functions. Just hook them all into the original function and use the ftrace handler to find the newest one. Also use a mutex in the register/unregister functions to protect changes to kpatch_num_registered, kpatch_func_hash and calls to the ftrace functions by other register/unregister invocations. --- kmod/core/core.c | 55 ++++++++++++++++++++++++++++++++++++------------ 1 file changed, 42 insertions(+), 13 deletions(-) diff --git a/kmod/core/core.c b/kmod/core/core.c index 225c227..db5d162 100644 --- a/kmod/core/core.c +++ b/kmod/core/core.c @@ -141,19 +141,8 @@ static int kpatch_apply_patch(void *data) goto out; for (i = 0; i < num_funcs; i++) { - struct kpatch_func *f; struct kpatch_func *func = &funcs[i]; - /* do any needed incremental patching */ - /* TODO: performance */ - hash_for_each_possible(kpatch_func_hash, f, node, - func->old_addr) { - if (f->old_addr == func->old_addr) { - func->old_addr = f->new_addr; - ref_module(func->mod, f->mod); - } - } - /* update the global list and go live */ hash_add(kpatch_func_hash, &func->node, func->old_addr); } @@ -187,6 +176,14 @@ void kpatch_ftrace_handler(unsigned long ip, unsigned long parent_ip, { struct kpatch_func *f; + /* + * This is where the magic happens. Update regs->ip to tell ftrace to + * return to the new function. + * + * If there are multiple patch modules that have registered to patch + * the same function, the last one to register wins, as it'll be first + * in the hash bucket. + */ preempt_disable_notrace(); hash_for_each_possible(kpatch_func_hash, f, node, ip) { if (f->old_addr == ip) { @@ -214,10 +211,26 @@ int kpatch_register(struct module *mod, struct kpatch_func *funcs, down(&kpatch_mutex); for (i = 0; i < num_funcs; i++) { - struct kpatch_func *func = &funcs[i]; + struct kpatch_func *f, *func = &funcs[i]; + bool found = false; func->mod = mod; + /* + * If any other modules have also patched this function, it + * already has an ftrace handler. + */ + hash_for_each_possible(kpatch_func_hash, f, node, + func->old_addr) { + if (f->old_addr == func->old_addr) { + found = true; + break; + } + } + if (found) + continue; + + /* Add an ftrace handler for this function. */ ret = ftrace_set_filter_ip(&kpatch_ftrace_ops, func->old_addr, 0, 0); if (ret) { @@ -285,8 +298,24 @@ int kpatch_unregister(struct module *mod, struct kpatch_func *funcs, } for (i = 0; i < num_funcs; i++) { - struct kpatch_func *func = &funcs[i]; + struct kpatch_func *f, *func = &funcs[i]; + bool found = false; + /* + * If any other modules have also patched this function, don't + * remove its ftrace handler. + */ + hash_for_each_possible(kpatch_func_hash, f, node, + func->old_addr) { + if (f->old_addr == func->old_addr) { + found = true; + break; + } + } + if (found) + continue; + + /* Remove the ftrace handler for this function. */ ret = ftrace_set_filter_ip(&kpatch_ftrace_ops, func->old_addr, 1, 0); if (ret) {