lkml.org 
[lkml]   [2015]   [Mar]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 2/2] livepatch/module: Correctly handle going modules
    On Thu, Mar 05, 2015 at 04:45:14PM +0100, Petr Mladek wrote:
    > Existing live patches are removed from going modules using a notify handler.
    > There are two problems with the current implementation.
    >
    > First, new patch could still see the module in the GOING state even after
    > the notifier has been called. It will try to initialize the related
    > object structures but the module could disappear at any time. There will
    > stay mess in the structures. It might even cause an invalid memory access.
    >
    > Second, if we start supporting patches with semantic changes between function
    > calls, we would need to apply any new patch even for going modules. Note that
    > the code from the module could be called even in the GOING state until
    > mod->exit() finishes. See below for example.
    >
    > This patch solves the problem by adding boolean flag into struct module.
    > It is switched when the going module handler is called. It marks the point
    > when it is safe and we actually have to ignore the going module.
    >
    > Alternative solutions:
    >
    > We could add another lock to make the switch to GOING state and mod->exit()
    > call an atomic operation. But this a nogo. It might cause a dead lock when
    > some mod->exit() depends on mod->exit() from another module.
    >
    > We could wait until the GOING module is moved to the UNFORMED state.
    > But this might take ages when mod->exit() has to wait for something.
    >
    > We could refuse to load the patch when a module is going but this is
    > pretty ugly.
    >
    > Example of the race:
    >
    > CPU0 CPU1
    >
    > delete_module() #SYSCALL
    >
    > try_stop_module()
    > mod->state = MODULE_STATE_GOING;
    >
    > mutex_unlock(&module_mutex);
    >
    > klp_register_patch()
    > klp_enable_patch()
    >
    > #save place to switch universe
    >
    > b() # from module that is going
    > a() # from core (patched)
    >
    > mod->exit();
    >
    > Note that the function b() can be called until we call mod->exit().
    >
    > If we do not apply patch against b() because it is in MODULE_STATE_GOING.
    > It will call patched a() with modified semantic and things might get wrong.
    >
    > Signed-off-by: Petr Mladek <pmladek@suse.cz>
    > ---
    > include/linux/module.h | 4 ++++
    > kernel/livepatch/core.c | 30 ++++++++++++++++++++++++++----
    > 2 files changed, 30 insertions(+), 4 deletions(-)
    >
    > diff --git a/include/linux/module.h b/include/linux/module.h
    > index b653d7c0a05a..c12f93497b74 100644
    > --- a/include/linux/module.h
    > +++ b/include/linux/module.h
    > @@ -344,6 +344,10 @@ struct module {
    > unsigned long *ftrace_callsites;
    > #endif
    >
    > +#ifdef CONFIG_LIVEPATCH
    > + bool klp_gone;
    > +#endif
    > +
    > #ifdef CONFIG_MODULE_UNLOAD
    > /* What modules depend on me? */
    > struct list_head source_list;
    > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
    > index 198f7733604b..0b38357fad0f 100644
    > --- a/kernel/livepatch/core.c
    > +++ b/kernel/livepatch/core.c
    > @@ -89,16 +89,32 @@ static bool klp_is_object_loaded(struct klp_object *obj)
    > /* sets obj->mod if object is not vmlinux and module is found */
    > static void klp_find_object_module(struct klp_object *obj)
    > {
    > + struct module *mod;
    > +
    > if (!klp_is_module(obj))
    > return;
    >
    > mutex_lock(&module_mutex);
    > +
    > + /*
    > + * We do not want to block removal of patched modules and therefore
    > + * we do not take a reference here. Instead, the patches are removed
    > + * by the going module handler instead.
    > + */

    Redundant "instead".


    --
    Josh


    \
     
     \ /
      Last update: 2015-03-05 20:41    [W:3.827 / U:0.292 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site