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@xxxxxxx>
> ---
> 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
--
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
