On Fri, Jan 29, 2016 at 03:04:51PM -0500, Jessica Yu wrote: > +++ Josh Poimboeuf [29/01/16 11:30 -0600]: > >On Fri, Jan 29, 2016 at 05:30:46PM +0100, Miroslav Benes wrote: > >>Otherwise than that it looks good. I agree there are advantages to split > >>the notifiers. For example we can replace the coming one with the function > >>call somewhere in load_module() to improve error handling if the patching > >>fails while loading a module. This would be handy with a consistency model > >>in the future. > > > >Yeah, we'll need something like that eventually. Though we'll need to > >make sure that ftrace_module_enable() is still called beforehand, after > >setting MODULE_STATE_COMING state, due to the race described in 5156dca. > > > >Something like: > > > >[note: klp_module_notify_coming() is replaced with klp_module_enable()] > > > >diff --git a/kernel/module.c b/kernel/module.c > >index 8358f46..aeabd81 100644 > >--- a/kernel/module.c > >+++ b/kernel/module.c > >@@ -3371,6 +3371,13 @@ static int complete_formation(struct module *mod, struct load_info *info) > > mod->state = MODULE_STATE_COMING; > > mutex_unlock(&module_mutex); > > > >+ ftrace_module_enable(mod); > >+ err = klp_module_enable(mod); > >+ if (err) { > >+ ftrace_release_mod(mod); > >+ return err; > >+ } > > If we go this route, should we should print a big warning ("Livepatch > couldn't patch loading module X") instead of aborting the module load > completely? I think aborting the module load is better. Otherwise the patch would be applied in an inconsistent state. Which might not be all that bad now, since we don't have a consistency model anyway; but it could be disastrous once we get one, if somebody is relying on that consistency. -- Josh -- To unsubscribe from this list: send the line "unsubscribe live-patching" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html