On Wed 2019-06-19 13:12:12, Miroslav Benes wrote:
> On Mon, 17 Jun 2019, Peter Zijlstra wrote:
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -3638,9 +3638,10 @@ static int prepare_coming_module(struct module *mod)
> >     if (err)
> >             return err;
> >  
> > -   blocking_notifier_call_chain(&module_notify_list,
> > -                                MODULE_STATE_COMING, mod);
> > -   return 0;
> > +   ret = blocking_notifier_call_chain(&module_notify_list,
> > +                                      MODULE_STATE_COMING, mod);
> > +   ret = notifier_to_errno(ret);
> > +   return ret;
> >  }
> >  
> >  static int unknown_module_param_cb(char *param, char *val, const char 
> > *modname,
> > @@ -3780,7 +3781,7 @@ static int load_module(struct load_info *info, const 
> > char __user *uargs,
> >  
> >     err = prepare_coming_module(mod);
> >     if (err)
> > -           goto bug_cleanup;
> > +           goto coming_cleanup;
> 
> Not good. klp_module_going() is not prepared to be called without 
> klp_module_coming() succeeding. "Funny" things might happen.

We have discussed it with Miroslav. The best solution seems to
be to allow to call klp_module_going() even when
klp_module_comming() failed. Something like:

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index c4ce08f43bd6..42d72b62edb2 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -1188,6 +1188,10 @@ void klp_module_going(struct module *mod)
                return;
 
        mutex_lock(&klp_mutex);
+
+       /* Nope when klp_module_comming failed */
+       if (!mod->klp_alive)
+               goto out;
        /*
         * Each module has to know that klp_module_going()
         * has been called. We never know what module will
@@ -1196,7 +1200,7 @@ void klp_module_going(struct module *mod)
        mod->klp_alive = false;
 
        klp_cleanup_module_patches_limited(mod, NULL);
-
+out:
        mutex_unlock(&klp_mutex);
 }

Peter's patch actually allows to convert the klp hooks into
proper module notifiers. But we could do it later once this
patch is upstream or queued.

Best Regards,
Petr

Reply via email to