lkml.org 
[lkml]   [2016]   [Jan]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/2] livepatch: Implement separate coming and going module notifiers
    [ Added Rusty, as he's still maintainer of the module code ]

    On Fri, 29 Jan 2016 11:30:10 -0600
    Josh Poimboeuf <jpoimboe@redhat.com> wrote:

    > 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;
    > + }
    > +
    > blocking_notifier_call_chain(&module_notify_list,
    > MODULE_STATE_COMING, mod);
    > return 0;
    > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
    > index eca592f..c42cf37 100644
    > --- a/kernel/trace/ftrace.c
    > +++ b/kernel/trace/ftrace.c
    > @@ -5045,9 +5045,6 @@ static int ftrace_module_notify(struct notifier_block *self,
    > struct module *mod = data;
    >
    > switch (val) {
    > - case MODULE_STATE_COMING:
    > - ftrace_module_enable(mod);
    > - break;
    > case MODULE_STATE_GOING:
    > ftrace_release_mod(mod);
    > break;

    If we end up doing something like this, I would just say punt and have
    the ftrace code be hardcoded into the module code and remove the
    notifiers completely. ftrace (and live kernel patching for that matter)
    are rather special. They are not a filesystem or driver. They are core
    utilities and having them called directly from the module code may be
    prudent and better to understand and control.

    Note, you still need to have prototypes for ftrace_module_enable() and
    a stub when ftrace is not configured. Same goes for klp_module_enable().

    -- Steve

    \
     
     \ /
      Last update: 2016-01-29 19:01    [W:3.900 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site