On Fri, 15 May 2015, Minfei Huang wrote: > From: Minfei Huang <[email protected]> > > The previous patches can be applied, once the corresponding module is > loaded. In general, the patch will do relocation (if necessary) and > obtain/verify function address before we start to enable patch. > > There are three different situations in which the coming module notifier > can fail: > > 1) relocations are not applied for some reason. In this case kallsyms > for module symbol is not called at all. The patch is not applied to the > module. If the user disable and enable patch again, there is possible > bug in klp_enable_func. If the user specified func->old_addr for some > function in the module (and he shouldn't do that, but nevertheless) our > warning would not catch it, ftrace will reject to register the handler > because of wrong address or will register the handler for wrong address. > > 2) relocations are applied successfully, but kallsyms lookup fails. In > this case func->old_addr can be correct for all previous lookups, 0 for > current failed one, and "unspecified" for the rest. If we undergo the > same scenario as in 1, the behaviour differs for three cases, but the > patch is not enabled anyway. > > 3) the object is initialized, but klp_enable_object fails in the > notifier due to possible ftrace error. Since it is improbable that > ftrace would heal itself in the future, we would get those errors > everytime the patch is enabled. > > In order to fix above situations, we can make obj->mod to NULL, if the > coming modified notifier fails. > > Signed-off-by: Minfei Huang <[email protected]>
Reviewed-by: Miroslav Benes <[email protected]> Regards, Miroslav > --- > v4: > - remove the label "out" in function klp_init_object_loaded > v3: > - modify the code style > v2: > - add the error message to make it more friendly > - modify the commit log, base on the [email protected] suggesting > v1: > - modify the commit log, describe the issue more details > --- > kernel/livepatch/core.c | 29 ++++++++++++++++++----------- > 1 file changed, 18 insertions(+), 11 deletions(-) > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > index 284e269..4a87765 100644 > --- a/kernel/livepatch/core.c > +++ b/kernel/livepatch/core.c > @@ -883,7 +883,7 @@ int klp_register_patch(struct klp_patch *patch) > } > EXPORT_SYMBOL_GPL(klp_register_patch); > > -static void klp_module_notify_coming(struct klp_patch *patch, > +static int klp_module_notify_coming(struct klp_patch *patch, > struct klp_object *obj) > { > struct module *pmod = patch->mod; > @@ -891,22 +891,23 @@ static void klp_module_notify_coming(struct klp_patch > *patch, > int ret; > > ret = klp_init_object_loaded(patch, obj); > - if (ret) > - goto err; > + if (ret) { > + pr_warn("failed to initialize patch '%s' for module '%s' > (%d)\n", > + pmod->name, mod->name, ret); > + return ret; > + } > > if (patch->state == KLP_DISABLED) > - return; > + return 0; > > pr_notice("applying patch '%s' to loading module '%s'\n", > pmod->name, mod->name); > > ret = klp_enable_object(obj); > - if (!ret) > - return; > - > -err: > - pr_warn("failed to apply patch '%s' to module '%s' (%d)\n", > - pmod->name, mod->name, ret); > + if (ret) > + pr_warn("failed to apply patch '%s' to module '%s' (%d)\n", > + pmod->name, mod->name, ret); > + return ret; > } > > static void klp_module_notify_going(struct klp_patch *patch, > @@ -930,6 +931,7 @@ disabled: > static int klp_module_notify(struct notifier_block *nb, unsigned long action, > void *data) > { > + int ret; > struct module *mod = data; > struct klp_patch *patch; > struct klp_object *obj; > @@ -955,7 +957,12 @@ static int klp_module_notify(struct notifier_block *nb, > unsigned long action, > > if (action == MODULE_STATE_COMING) { > obj->mod = mod; > - klp_module_notify_coming(patch, obj); > + ret = klp_module_notify_coming(patch, obj); > + if (ret) { > + obj->mod = NULL; > + pr_warn("patch '%s' is in an > inconsistent state!\n", > + patch->mod->name); > + } > } else /* MODULE_STATE_GOING */ > klp_module_notify_going(patch, obj); > > -- > 2.2.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to [email protected] > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
