[PATCH v2 2/2] livepatch/module: Correctly handle going modules

[Petr Mladek]

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.
+	 */
+	mod = find_module(obj->name);
+
 	/*
-	 * We don't need to take a reference on the module here because we have
-	 * the klp_mutex, which is also taken by the module notifier.  This
-	 * prevents any module from unloading until we release the klp_mutex.
+	 * We must not init the object when the module is going and the notifier
+	 * has already been called. But the patch might still be needed before.
+	 * Note that module functions can be called even in the GOING state
+	 * until mod->exit() finishes. This is especially important for patches
+	 * that modify semantic of the functions.
 	 */
-	obj->mod = find_module(obj->name);
+	if (mod && mod->state == MODULE_STATE_GOING && mod->klp_gone)
+		mod = NULL;
+
+	obj->mod = mod;
+
 	mutex_unlock(&module_mutex);
 }
 
@@ -929,7 +945,10 @@ int klp_module_init(struct module *mod)
 	int ret = 0;
 
 	mutex_lock(&klp_mutex);
+
+	mod->klp_gone = false;
 	ret = klp_module_coming(mod);
+
 	mutex_unlock(&klp_mutex);
 
 	return ret;
@@ -985,7 +1004,10 @@ static int klp_module_notify_going(struct notifier_block *nb,
 		return 0;
 
 	mutex_lock(&klp_mutex);
+
 	klp_module_going(mod);
+	mod->klp_gone = true;
+
 	mutex_lock(&klp_mutex);
 
 	return 0;
-- 
1.8.5.6

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/