kmod/core: get rid of kpatch_internal

The kpatch_internal struct is a good idea, in that it documents which
parts of kpatch_module shouldn't be used by the patch module.  But it
creates extra code and will require more extra code if we want to keep a
list of kpmods, which is needed to create a module notifier for module
patching of future loaded modules.

Embedding the private data directly in the public struct allows the code
to be simpler: no extra kmallocs/kfrees, no need to store pointers
between the public and private structs.  I think the simpler code is
worth the tradeoff (exposing implementation detail).  Kernel code
usually doesn't bother with hiding a internal struct data from other
kernel code anyway.  For example, see ftrace_ops or struct kprobe.

The private fields are documented with a "private" comment.
This commit is contained in:
Josh Poimboeuf 2014-06-10 12:02:34 -05:00
parent b58e77ae9c
commit 063e9a62f5
2 changed files with 13 additions and 29 deletions

View File

@ -127,16 +127,6 @@ struct kpatch_func {
struct module *mod;
};
/*
* 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)
{
int state = atomic_read(&kpatch_state);
@ -208,7 +198,7 @@ static void kpatch_backtrace_address_verify(void *data, unsigned long address,
char *func_name;
struct kpatch_func *active_func;
func = &kpmod->internal->funcs[i];
func = &kpmod->funcs[i];
active_func = kpatch_get_func(func->old_addr);
if (!active_func) {
/* patching an unpatched func */
@ -285,7 +275,7 @@ out:
static int kpatch_apply_patch(void *data)
{
struct kpatch_module *kpmod = data;
struct kpatch_func *funcs = kpmod->internal->funcs;
struct kpatch_func *funcs = kpmod->funcs;
int num_funcs = kpmod->patches_nr;
int i, ret;
@ -325,7 +315,7 @@ static int kpatch_apply_patch(void *data)
static int kpatch_remove_patch(void *data)
{
struct kpatch_module *kpmod = data;
struct kpatch_func *funcs = kpmod->internal->funcs;
struct kpatch_func *funcs = kpmod->funcs;
int num_funcs = kpmod->patches_nr;
int ret, i;
@ -650,16 +640,11 @@ int kpatch_register(struct kpatch_module *kpmod, bool replace)
kpmod->enabled = false;
kpmod->internal = kmalloc(sizeof(*kpmod->internal), GFP_KERNEL);
if (!kpmod->internal)
funcs = kzalloc(sizeof(*funcs) * num_funcs, GFP_KERNEL);
if (!funcs)
return -ENOMEM;
funcs = kzalloc(sizeof(*funcs) * num_funcs, GFP_KERNEL);
if (!funcs) {
kfree(kpmod->internal);
return -ENOMEM;
}
kpmod->internal->funcs = funcs;
kpmod->funcs = funcs;
down(&kpatch_mutex);
@ -775,15 +760,14 @@ err_rollback:
module_put(kpmod->mod);
err_up:
up(&kpatch_mutex);
kfree(kpmod->internal->funcs);
kfree(kpmod->internal);
kfree(kpmod->funcs);
return ret;
}
EXPORT_SYMBOL(kpatch_register);
int kpatch_unregister(struct kpatch_module *kpmod)
{
struct kpatch_func *funcs = kpmod->internal->funcs;
struct kpatch_func *funcs = kpmod->funcs;
int num_funcs = kpmod->patches_nr;
int i, ret;
@ -831,8 +815,7 @@ int kpatch_unregister(struct kpatch_module *kpmod)
kpatch_put_modules(funcs, num_funcs);
kfree(kpmod->internal->funcs);
kfree(kpmod->internal);
kfree(kpmod->funcs);
pr_notice("unloaded patch module '%s'\n", kpmod->mod->name);

View File

@ -46,16 +46,17 @@ struct kpatch_dynrela {
#include <linux/types.h>
#include <linux/module.h>
struct kpatch_internal;
struct kpatch_module {
/* public */
struct module *mod;
struct kpatch_patch *patches;
struct kpatch_dynrela *dynrelas;
int patches_nr;
int dynrelas_nr;
bool enabled;
struct kpatch_internal *internal; /* used internally by core module */
/* private */
struct kpatch_func *funcs;
};
extern struct kobject *kpatch_patches_kobj;