[RFC PATCH] livepatch/module: Do not patch modules that are not ready

[Petr Mladek]

There is a notifier that handles live patches for coming and going modules.
It takes klp_mutex lock to avoid races with coming and going patches.

Unfortunately, there are some possible races in the current implementation.
The problem is that we do not keep the klp_mutex lock all the time when
the module is being added or removed.

First, the module is visible even before ftrace is ready. If we enable a patch
in this time frame, adding ftrace ops will fail and the patch will get rejected
just because bad timing.

Second, if we are "lucky" and enable the patch for the coming module when the
ftrace is ready but before the module notifier has been called. The notifier
will try to enable the patch as well. It will detect that it is already patched,
return error, and the module will get rejected just because bad timing.
The more serious problem is that it will not call the notifier for
going module, so that the mess will stay there and we wont be able to load
the module later.

Third, similar problems are there for going module. If a patch is enabled after
the notifier finishes but before the module is removed from the list of modules,
the new patch will be applied to the module. The module might disappear at
anytime when the patch enabling is in progress, so there might be an access out
of memory. Or the whole patch might be applied and some mess will be left,
so it will not be possible to load/patch the module again.

This patch solves the problem by adding two flags into struct module. They are
switched when the notifier is called. Note that we try to solve a race with a
coming patch, therefore we do not know which modules will get patched and we
need to monitor all modules. This is why I added this to the struct module.

The flags are set and checked under the klp_mutex lock. The related operation
is finished under the same lock. Therefore they are properly serialized now.

Note that the patch solves only the situation when a new patch is registered or
enabled. There are no such problems when the patch is being removed. it does
not matter who disable the patch first, whether the normal disable_patch() or
the module notifier. There is nothing to do once the patch is disabled.

Signed-off-by: Petr Mladek <pmladek@xxxxxxx>
---
 include/linux/module.h  |  5 +++++
 kernel/livepatch/core.c | 20 +++++++++++++++++++-
 kernel/module.c         |  6 +++++-
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index b653d7c0a05a..7e50d87da510 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -344,6 +344,11 @@ struct module {
 	unsigned long *ftrace_callsites;
 #endif
 
+#ifdef CONFIG_LIVEPATCH
+	bool klp_patched;
+	bool klp_unpatched;
+#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 a664e485365f..dee4bbcb60e6 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -89,6 +89,8 @@ 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;
 
@@ -98,7 +100,14 @@ static void klp_find_object_module(struct klp_object *obj)
 	 * the klp_mutex, which is also taken by the module notifier.  This
 	 * prevents any module from unloading until we release the klp_mutex.
 	 */
-	obj->mod = find_module(obj->name);
+	mod = find_module(obj->name);
+	/* Do not mess work of the module notifier */
+	if ((mod->state == MODULE_STATE_COMING && !mod->klp_patched) ||
+	    (mod->state == MODULE_STATE_GOING && mod->klp_unpatched))
+		obj->mod = NULL;
+	else
+		obj->mod = mod;
+
 	mutex_unlock(&module_mutex);
 }
 
@@ -927,6 +936,15 @@ static int klp_module_notify(struct notifier_block *nb, unsigned long action,
 
 	mutex_lock(&klp_mutex);
 
+	/*
+	 * Each module has to know that the notifier has been called.
+	 * We never know what module will get patched by a new patch.
+	 */
+	if (action == MODULE_STATE_COMING)
+		mod->klp_patched = true;
+	else
+		mod->klp_unpatched = true;
+
 	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))
diff --git a/kernel/module.c b/kernel/module.c
index d856e96a3cce..8357f15b7ed0 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -852,7 +852,6 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
 
 	/* Store the name of the last unloaded module for diagnostic purposes */
 	strlcpy(last_unloaded_module, mod->name, sizeof(last_unloaded_module));
-
 	free_module(mod);
 	return 0;
 out:
@@ -3271,6 +3270,11 @@ static int load_module(struct load_info *info, const char __user *uargs,
 	}
 #endif
 
+#ifdef CONFIG_LIVEPATCH
+	mod->klp_patched = false;
+	mod->klp_unpatched = false;
+#endif
+
 	/* To avoid stressing percpu allocator, do this once we're unique. */
 	err = percpu_modalloc(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/