[PATCH v2 1/2] livepatch/module: Apply patch when loaded module is unformed

[Petr Mladek]

Existing live patches are applied to loaded modules using a notify handler.
There are two problems with this approach.

First, errors from module notifiers are ignored and could not stop the module
from being loaded. But we will need to refuse the module when there are
semantics dependencies between functions and there are some problems
to apply the patch to the module. Otherwise, the system might become
into an inconsistent state.

Second, the module notifiers are called when the module is in
STATE_MODULE_COMING. It means that it is visible by find_module()
and can be detected by klp_find_object_module() when a new patch is
registered.

Now, the timing is important. If the new patch is registered after the module
notifier has been called, it has to initialize the module object for the new
patch. Note that, in this case, the new patch has to see the module as loaded
even when it is still in the COMING state.

But when the new patch is registered before the module notifier, it _should_
not initialize the module object, see below for detailed explanation.

This patch solves both problems by calling klp_module_init() directly in
load_module(). We could handle the error there. Also it is called in
MODULE_STATE_UNFORMED and therefore before the module is visible via
find_module().

The implementation creates three functions for module init and three
functions for going modules. We need to revert already initialized
patches when something fails and thus need to be able to call
the code for going modules without leaving klp_mutex.

Detailed explanation of the last problem:

Why should not we initialize the module object for a new patch when
the related module coming notifier has not been called yet?

Note that the notifier could _not_ _simply_ ignore already initialized module
objects. The notifier initializes the module object for all existing patches.
If the new patch is registered and enabled before, it would crate wrong
order of patches in fops->func_stack.

For example, let's have three patches (P1, P2, P3) for the functions a()
and b() where a() is from vmcore and b() is from a module M. Something
like:

	a()	b()
P1	a1()	b1()
P2	a2()	b2()
P3	a3()	b3(3)

If you load the module M after all patches are registered and enabled.
The ftrace ops for function a() and b() has listed the functions in this
order

	ops_a->func_stack -> list(a3,a2,a1)
	ops_b->func_stack -> list(b3,b2,b1)

, so the pointer to b3() is the first and will be used.

Then you might have the following scenario. Let's start with state
when patches P1 and P2 are registered and enabled but the module M
is not loaded. Then ftrace ops for b() does not exist. Then we
get into the following race:

CPU0					CPU1

load_module(M)

  complete_formation()

  mod->state = MODULE_STATE_COMING;
  mutex_unlock(&module_mutex);

					klp_register_patch(P3);
					klp_enable_patch(P3);

					# STATE 1

  klp_module_notify(M)
    klp_module_notify_coming(P1);
    klp_module_notify_coming(P2);
    klp_module_notify_coming(P3);

					# STATE 2

The ftrace ops for a() and b() then looks:

  STATE1:

	ops_a->func_stack -> list(a3,a2,a1);
	ops_b->func_stack -> list(b3);

  STATE2:
	ops_a->func_stack -> list(a3,a2,a1);
	ops_b->func_stack -> list(b2,b1,b3);

therefore, b2() is used for the module but a3() is used for vmcore
because they were the last added.

Signed-off-by: Petr Mladek <pmladek@xxxxxxx>
---
 include/linux/livepatch.h | 10 +++++
 kernel/livepatch/core.c   | 94 +++++++++++++++++++++++++++++++++++------------
 kernel/module.c           |  9 +++++
 3 files changed, 89 insertions(+), 24 deletions(-)

diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index ee6dbb39a809..78ac10546160 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -128,6 +128,16 @@ int klp_unregister_patch(struct klp_patch *);
 int klp_enable_patch(struct klp_patch *);
 int klp_disable_patch(struct klp_patch *);
 
+int klp_module_init(struct module *mod);
+
+#else /* CONFIG_LIVEPATCH */
+
+inline int klp_module_init(struct module *mod)
+{
+	return 0;
+}
+
 #endif /* CONFIG_LIVEPATCH */
 
+
 #endif /* _LINUX_LIVEPATCH_H_ */
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 7e0c83dc7215..198f7733604b 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -869,8 +869,8 @@ int klp_register_patch(struct klp_patch *patch)
 }
 EXPORT_SYMBOL_GPL(klp_register_patch);
 
-static void klp_module_notify_coming(struct klp_patch *patch,
-				     struct klp_object *obj)
+static int klp_module_coming_update_patch(struct klp_patch *patch,
+					  struct klp_object *obj)
 {
 	struct module *pmod = patch->mod;
 	struct module *mod = obj->mod;
@@ -881,22 +881,62 @@ static void klp_module_notify_coming(struct klp_patch *patch,
 		goto err;
 
 	if (patch->state == KLP_DISABLED)
-		return;
+		return 0;
 
 	pr_notice("applying patch '%s' to loading module '%s'\n",
 		  pmod->name, mod->name);
 
 	ret = klp_enable_object(obj);
 	if (!ret)
-		return;
+		return 0;
 
 err:
 	pr_warn("failed to apply patch '%s' to module '%s' (%d)\n",
 		pmod->name, mod->name, ret);
+	return ret;
 }
 
-static void klp_module_notify_going(struct klp_patch *patch,
-				    struct klp_object *obj)
+static void klp_module_going(struct module *mod);
+
+int klp_module_coming(struct module *mod)
+{
+	struct klp_patch *patch;
+	struct klp_object *obj;
+	int ret = 0;
+
+	list_for_each_entry(patch, &klp_patches, list) {
+		for (obj = patch->objs; obj->funcs; obj++) {
+			if (!klp_is_module(obj) || strcmp(obj->name, mod->name))
+				continue;
+
+			obj->mod = mod;
+			ret = klp_module_coming_update_patch(patch, obj);
+			if (ret)
+				goto err;
+		}
+	}
+
+	return 0;
+
+err:
+	klp_module_going(mod);
+	return ret;
+}
+
+
+int klp_module_init(struct module *mod)
+{
+	int ret = 0;
+
+	mutex_lock(&klp_mutex);
+	ret = klp_module_coming(mod);
+	mutex_unlock(&klp_mutex);
+
+	return ret;
+}
+
+static void klp_module_going_update_patch(struct klp_patch *patch,
+					  struct klp_object *obj)
 {
 	struct module *pmod = patch->mod;
 	struct module *mod = obj->mod;
@@ -913,40 +953,46 @@ disabled:
 	klp_free_object_loaded(obj);
 }
 
-static int klp_module_notify(struct notifier_block *nb, unsigned long action,
-			     void *data)
+static void klp_module_going(struct module *mod)
 {
-	struct module *mod = data;
 	struct klp_patch *patch;
 	struct klp_object *obj;
 
-	if (action != MODULE_STATE_COMING && action != MODULE_STATE_GOING)
-		return 0;
-
-	mutex_lock(&klp_mutex);
-
 	list_for_each_entry(patch, &klp_patches, list) {
 		for (obj = patch->objs; obj->funcs; obj++) {
-			if (!klp_is_module(obj) || strcmp(obj->name, mod->name))
+			/*
+			 * Handle only loaded (initialized) modules.
+			 * This is needed when used in an error path.
+			 */
+			if (!klp_is_object_loaded(obj) ||
+			    strcmp(obj->name, mod->name))
 				continue;
 
-			if (action == MODULE_STATE_COMING) {
-				obj->mod = mod;
-				klp_module_notify_coming(patch, obj);
-			} else /* MODULE_STATE_GOING */
-				klp_module_notify_going(patch, obj);
-
-			break;
+			klp_module_going_update_patch(patch, obj);
 		}
 	}
 
-	mutex_unlock(&klp_mutex);
+	return;
+}
+
+static int klp_module_notify_going(struct notifier_block *nb,
+				   unsigned long action,
+				   void *data)
+{
+	struct module *mod = data;
+
+	if (action != MODULE_STATE_GOING)
+		return 0;
+
+	mutex_lock(&klp_mutex);
+	klp_module_going(mod);
+	mutex_lock(&klp_mutex);
 
 	return 0;
 }
 
 static struct notifier_block klp_module_nb = {
-	.notifier_call = klp_module_notify,
+	.notifier_call = klp_module_notify_going,
 	.priority = INT_MIN+1, /* called late but before ftrace notifier */
 };
 
diff --git a/kernel/module.c b/kernel/module.c
index d856e96a3cce..f744a639460d 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -53,6 +53,7 @@
 #include <asm/sections.h>
 #include <linux/tracepoint.h>
 #include <linux/ftrace.h>
+#include <linux/livepatch.h>
 #include <linux/async.h>
 #include <linux/percpu.h>
 #include <linux/kmemleak.h>
@@ -3321,6 +3322,14 @@ static int load_module(struct load_info *info, const char __user *uargs,
 	/* Ftrace init must be called in the MODULE_STATE_UNFORMED state */
 	ftrace_module_init(mod);
 
+	/*
+	 * LivePatch init must be called in the MODULE_STATE_UNFORMED state
+	 * and it might reject the module to avoid a system inconsistency.
+	 */
+	err = klp_module_init(mod);
+	if (err)
+		goto ddebug_cleanup;
+
 	/* Finally it's fully formed, ready to start executing. */
 	err = complete_formation(mod, info);
 	if (err)
-- 
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/