From 681a6e80b98d25e604a43cdf661bef8802fedb5e Mon Sep 17 00:00:00 2001 From: Seth Jennings Date: Mon, 12 May 2014 14:36:10 -0500 Subject: [PATCH] refactor core <-> patch interface Make kpatch_funs truly internal by: Defining it in core.c Adding a struct kpatch_internal, declared in kpatch.h and defined in core.c, that contains per patch module internal data. Adding an "internal" field to struct kpatch_modules. Allocating internal and funcs data in core.c, not in the patch module, since the patch module has no knowledge of kpatch_func anymore. Adding a "patch" field to kpatch_func that points directly to the kpatch_patch provided by the module (rather than a field-by-field copy) Signed-off-by: Seth Jennings --- kmod/core/core.c | 100 ++++++++++++++++++++--------- kmod/core/kpatch.h | 32 ++++----- kmod/patch/kpatch-patch-hook.c | 21 +----- kmod/patch/kpatch-patch.h | 34 ---------- kpatch-build/add-patches-section.c | 2 +- 5 files changed, 86 insertions(+), 103 deletions(-) delete mode 100644 kmod/patch/kpatch-patch.h diff --git a/kmod/core/core.c b/kmod/core/core.c index a90b280..679bd13 100644 --- a/kmod/core/core.c +++ b/kmod/core/core.c @@ -105,6 +105,28 @@ enum { }; static atomic_t kpatch_state; +enum kpatch_op { + KPATCH_OP_NONE, + KPATCH_OP_PATCH, + KPATCH_OP_UNPATCH, +}; + +struct kpatch_func { + struct kpatch_patch *patch; + struct hlist_node node; + struct kpatch_module *kpmod; + enum kpatch_op op; +}; + +/* + * This structure is allocated on a per registered module basis + * and is stored in the struct kpatch_module "internal" field. + * Any data associated with each registered module used internally + * by this core module can be added here. + */ +struct kpatch_internal { + struct kpatch_func *funcs; +}; static inline void kpatch_state_idle(void) { @@ -134,7 +156,7 @@ static struct kpatch_func *kpatch_get_func(unsigned long ip) /* Here, we have to use rcu safe hlist because of NMI concurrency */ hash_for_each_possible_rcu(kpatch_func_hash, f, node, ip) - if (f->old_addr == ip) + if (f->patch->old_addr == ip) return f; return NULL; } @@ -143,7 +165,7 @@ static struct kpatch_func *kpatch_get_prev_func(struct kpatch_func *f, unsigned long ip) { hlist_for_each_entry_continue_rcu(f, node) - if (f->old_addr == ip) + if (f->patch->old_addr == ip) return f; return NULL; } @@ -173,20 +195,20 @@ static void kpatch_backtrace_address_verify(void *data, unsigned long address, return; /* check kpmod funcs */ - for (i = 0; i < kpmod->num_funcs; i++) { + for (i = 0; i < kpmod->patches_nr; i++) { unsigned long func_addr, func_size; struct kpatch_func *active_func; - func = &kpmod->funcs[i]; - active_func = kpatch_get_func(func->old_addr); + func = &kpmod->internal->funcs[i]; + active_func = kpatch_get_func(func->patch->old_addr); if (!active_func) { /* patching an unpatched func */ - func_addr = func->old_addr; - func_size = func->old_size; + func_addr = func->patch->old_addr; + func_size = func->patch->old_size; } else { /* repatching or unpatching */ - func_addr = active_func->new_addr; - func_size = active_func->new_size; + func_addr = active_func->patch->new_addr; + func_size = active_func->patch->new_size; } args->ret = kpatch_compare_addresses(address, func_addr, @@ -199,8 +221,8 @@ static void kpatch_backtrace_address_verify(void *data, unsigned long address, hash_for_each_rcu(kpatch_func_hash, i, func, node) { if (func->op == KPATCH_OP_UNPATCH) { args->ret = kpatch_compare_addresses(address, - func->new_addr, - func->new_size); + func->patch->new_addr, + func->patch->new_size); if (args->ret) return; } @@ -251,8 +273,8 @@ out: static int kpatch_apply_patch(void *data) { struct kpatch_module *kpmod = data; - struct kpatch_func *funcs = kpmod->funcs; - int num_funcs = kpmod->num_funcs; + struct kpatch_func *funcs = kpmod->internal->funcs; + int num_funcs = kpmod->patches_nr; int i, ret; ret = kpatch_verify_activeness_safety(kpmod); @@ -264,7 +286,7 @@ static int kpatch_apply_patch(void *data) /* tentatively add the new funcs to the global func hash */ for (i = 0; i < num_funcs; i++) hash_add_rcu(kpatch_func_hash, &funcs[i].node, - funcs[i].old_addr); + funcs[i].patch->old_addr); /* memory barrier between func hash add and state change */ smp_wmb(); @@ -291,8 +313,8 @@ static int kpatch_apply_patch(void *data) static int kpatch_remove_patch(void *data) { struct kpatch_module *kpmod = data; - struct kpatch_func *funcs = kpmod->funcs; - int num_funcs = kpmod->num_funcs; + struct kpatch_func *funcs = kpmod->internal->funcs; + int num_funcs = kpmod->patches_nr; int ret, i; ret = kpatch_verify_activeness_safety(kpmod); @@ -362,7 +384,7 @@ kpatch_ftrace_handler(unsigned long ip, unsigned long parent_ip, } done: if (func) - regs->ip = func->new_addr; + regs->ip = func->patch->new_addr; preempt_enable_notrace(); } @@ -385,30 +407,40 @@ static void kpatch_remove_funcs_from_filter(struct kpatch_func *funcs, * If any other modules have also patched this function, don't * remove its ftrace handler. */ - if (kpatch_get_func(func->old_addr)) + if (kpatch_get_func(func->patch->old_addr)) continue; /* Remove the ftrace handler for this function. */ - ret = ftrace_set_filter_ip(&kpatch_ftrace_ops, func->old_addr, - 1, 0); + ret = ftrace_set_filter_ip(&kpatch_ftrace_ops, + func->patch->old_addr, 1, 0); WARN(ret, "can't remove ftrace filter at address 0x%lx (rc=%d)", - func->old_addr, ret); + func->patch->old_addr, ret); } } int kpatch_register(struct kpatch_module *kpmod, bool replace) { int ret, i; - struct kpatch_func *funcs = kpmod->funcs; - struct kpatch_func *func; - int num_funcs = kpmod->num_funcs; + struct kpatch_func *funcs, *func; + int num_funcs = kpmod->patches_nr; - if (!kpmod->mod || !funcs || !num_funcs) + if (!kpmod->mod || !kpmod->patches || !num_funcs) return -EINVAL; kpmod->enabled = false; + kpmod->internal = kmalloc(sizeof(*kpmod->internal), GFP_KERNEL); + if (!kpmod->internal) + return -ENOMEM; + + funcs = kmalloc(sizeof(*funcs) * num_funcs, GFP_KERNEL); + if (!funcs) { + kfree(kpmod->internal); + return -ENOMEM; + } + kpmod->internal->funcs = funcs; + down(&kpatch_mutex); if (!try_module_get(kpmod->mod)) { @@ -421,20 +453,21 @@ int kpatch_register(struct kpatch_module *kpmod, bool replace) func->op = KPATCH_OP_PATCH; func->kpmod = kpmod; + func->patch = &kpmod->patches[i]; /* * If any other modules have also patched this function, it * already has an ftrace handler. */ - if (kpatch_get_func(func->old_addr)) + if (kpatch_get_func(func->patch->old_addr)) continue; /* Add an ftrace handler for this function. */ - ret = ftrace_set_filter_ip(&kpatch_ftrace_ops, func->old_addr, - 0, 0); + ret = ftrace_set_filter_ip(&kpatch_ftrace_ops, + func->patch->old_addr, 0, 0); if (ret) { pr_err("can't set ftrace filter at address 0x%lx\n", - func->old_addr); + func->patch->old_addr); num_funcs = i; goto err_rollback; } @@ -534,14 +567,16 @@ err_rollback: module_put(kpmod->mod); err_up: up(&kpatch_mutex); + kfree(kpmod->internal->funcs); + kfree(kpmod->internal); return ret; } EXPORT_SYMBOL(kpatch_register); int kpatch_unregister(struct kpatch_module *kpmod) { - struct kpatch_func *funcs = kpmod->funcs; - int num_funcs = kpmod->num_funcs; + struct kpatch_func *funcs = kpmod->internal->funcs; + int num_funcs = kpmod->patches_nr; int i, ret; if (!kpmod->enabled) @@ -588,6 +623,9 @@ int kpatch_unregister(struct kpatch_module *kpmod) kpatch_remove_funcs_from_filter(funcs, num_funcs); + kfree(kpmod->internal->funcs); + kfree(kpmod->internal); + pr_notice("unloaded patch module \"%s\"\n", kpmod->mod->name); kpmod->enabled = false; diff --git a/kmod/core/kpatch.h b/kmod/core/kpatch.h index 121e383..ecb1485 100644 --- a/kmod/core/kpatch.h +++ b/kmod/core/kpatch.h @@ -23,34 +23,26 @@ #ifndef _KPATCH_H_ #define _KPATCH_H_ -#include -#include - -enum kpatch_op { - KPATCH_OP_NONE, - KPATCH_OP_PATCH, - KPATCH_OP_UNPATCH, -}; - -struct kpatch_func { - /* public */ +struct kpatch_patch { unsigned long new_addr; unsigned long new_size; unsigned long old_addr; unsigned long old_size; - - /* private */ - struct hlist_node node; - struct kpatch_module *kpmod; - enum kpatch_op op; }; +#ifdef __KERNEL__ + +#include +#include + +struct kpatch_internal; + struct kpatch_module { struct module *mod; - struct kpatch_func *funcs; - int num_funcs; - + struct kpatch_patch *patches; + int patches_nr; bool enabled; + struct kpatch_internal *internal; /* used internally by core module */ }; extern struct kobject *kpatch_patches_kobj; @@ -58,4 +50,6 @@ extern struct kobject *kpatch_patches_kobj; extern int kpatch_register(struct kpatch_module *kpmod, bool replace); extern int kpatch_unregister(struct kpatch_module *kpmod); +#endif /* __KERNEL__ */ + #endif /* _KPATCH_H_ */ diff --git a/kmod/patch/kpatch-patch-hook.c b/kmod/patch/kpatch-patch-hook.c index 3c085b3..895aeac 100644 --- a/kmod/patch/kpatch-patch-hook.c +++ b/kmod/patch/kpatch-patch-hook.c @@ -23,7 +23,6 @@ #include #include #include "kpatch.h" -#include "kpatch-patch.h" static bool replace; module_param(replace, bool, S_IRUGO); @@ -73,24 +72,12 @@ static struct kobj_attribute patch_enabled_attr = static int __init patch_init(void) { struct kpatch_patch *patches; - int i, ret; - - patches = (struct kpatch_patch *)&__kpatch_patches; + int ret; kpmod.mod = THIS_MODULE; - kpmod.num_funcs = (&__kpatch_patches_end - &__kpatch_patches) / + kpmod.patches = (struct kpatch_patch *)&__kpatch_patches; + kpmod.patches_nr = (&__kpatch_patches_end - &__kpatch_patches) / sizeof(*patches); - kpmod.funcs = kmalloc(kpmod.num_funcs * sizeof(struct kpatch_func), - GFP_KERNEL); - if (!kpmod.funcs) - return -ENOMEM; - - for (i = 0; i < kpmod.num_funcs; i++) { - kpmod.funcs[i].old_addr = patches[i].old_addr; - kpmod.funcs[i].old_size = patches[i].old_size; - kpmod.funcs[i].new_addr = patches[i].new_addr; - kpmod.funcs[i].new_size = patches[i].new_size; - } patch_kobj = kobject_create_and_add(THIS_MODULE->name, kpatch_patches_kobj); @@ -114,7 +101,6 @@ err_sysfs: err_put: kobject_put(patch_kobj); err_free: - kfree(kpmod.funcs); return ret; } @@ -123,7 +109,6 @@ static void __exit patch_exit(void) WARN_ON(kpmod.enabled); sysfs_remove_file(patch_kobj, &patch_enabled_attr.attr); kobject_put(patch_kobj); - kfree(kpmod.funcs); } module_init(patch_init); diff --git a/kmod/patch/kpatch-patch.h b/kmod/patch/kpatch-patch.h deleted file mode 100644 index 1b43846..0000000 --- a/kmod/patch/kpatch-patch.h +++ /dev/null @@ -1,34 +0,0 @@ -/* - * kpatch-patch.h - * - * Copyright (C) 2014 Josh Poimboeuf - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License - * as published by the Free Software Foundation; either version 2 - * of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA, - * 02110-1301, USA. - * - * Contains definitions needed for creating patch modules - */ - -#ifndef _KPATCH_PATCH_H_ -#define _KPATCH_PATCH_H_ - -struct kpatch_patch { - unsigned long new_addr; - unsigned long new_size; - unsigned long old_addr; - unsigned long old_size; -}; - -#endif /* _KPATCH_PATCH_H_ */ diff --git a/kpatch-build/add-patches-section.c b/kpatch-build/add-patches-section.c index 1e373eb..b102494 100644 --- a/kpatch-build/add-patches-section.c +++ b/kpatch-build/add-patches-section.c @@ -43,7 +43,7 @@ #include #include -#include "kpatch-patch.h" +#include "kpatch.h" #define ERROR(format, ...) \ error(1, 0, "%s: %d: " format, __FUNCTION__, __LINE__, ##__VA_ARGS__)