From e1890e627a9bef94d9ad6153b01460ba51f820e3 Mon Sep 17 00:00:00 2001
From: Josh Poimboeuf <jpoimboe@redhat.com>
Date: Wed, 9 Jul 2014 20:26:30 -0500
Subject: [PATCH] prevent rmmod of forced modules

I found a bad bug:

- Module A is loaded, and registers function foo() with
  KPATCH_FORCE_UNSAFE.
- Module A is unloaded.  The new version of foo() is on the backtrace of
  a task, but the core module ignores it because of the force flag, so
  the unloading succeeds.
- The task returns to the new version of foo() which no longer exists.
- BOOM.

The only way I can think of to prevent this scenario is to prevent
forced modules from being unloaded (but still allow them to be
disabled).

An annoying side effect of this approach is that forced modules stay
loaded and in memory forever.  And that after "kpatch unload" of a
forced module, you can't ever load it again because the previous
instance of it is still loaded (but permanently disabled).

This is ugly but I can't really think of a better way to handle it.  If
necessary we could create a workqueue and periodically check to see if
we can safely call module_put() so that the module could be eventually
removed.
---
 kmod/core/core.c | 26 ++++++++++++++++++++++----
 kpatch/kpatch    |  6 +++++-
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/kmod/core/core.c b/kmod/core/core.c
index c0da875..232f5d6 100644
--- a/kmod/core/core.c
+++ b/kmod/core/core.c
@@ -794,7 +794,7 @@ out:
 
 int kpatch_register(struct kpatch_module *kpmod, bool replace)
 {
-	int ret, i;
+	int ret, i, force = 0;
 	struct kpatch_object *object;
 	struct kpatch_func *func;
 
@@ -856,6 +856,8 @@ int kpatch_register(struct kpatch_module *kpmod, bool replace)
 		hash_for_each_rcu(kpatch_func_hash, i, func, node) {
 			if (func->op != KPATCH_OP_UNPATCH)
 				continue;
+			if (func->force)
+				force = 1;
 			hash_del_rcu(&func->node);
 			WARN_ON(kpatch_ftrace_remove_func(func->old_addr));
 		}
@@ -867,7 +869,14 @@ int kpatch_register(struct kpatch_module *kpmod, bool replace)
 			kpmod2->enabled = false;
 			pr_notice("unloaded patch module '%s'\n",
 				  kpmod2->mod->name);
-			module_put(kpmod2->mod);
+
+			/*
+			 * Don't allow modules with forced functions to be
+			 * removed because they might still be in use.
+			 */
+			if (!force)
+				module_put(kpmod2->mod);
+
 			list_del(&kpmod2->list);
 		}
 	}
@@ -926,7 +935,7 @@ int kpatch_unregister(struct kpatch_module *kpmod)
 {
 	struct kpatch_object *object;
 	struct kpatch_func *func;
-	int ret;
+	int ret, force = 0;
 
 	if (!kpmod->enabled)
 		return -EINVAL;
@@ -935,6 +944,8 @@ int kpatch_unregister(struct kpatch_module *kpmod)
 
 	do_for_each_linked_func(kpmod, func) {
 		func->op = KPATCH_OP_UNPATCH;
+		if (func->force)
+			force = 1;
 	} while_for_each_linked_func();
 
 	/* memory barrier between func hash and state write */
@@ -974,7 +985,14 @@ int kpatch_unregister(struct kpatch_module *kpmod)
 	pr_notice("unloaded patch module '%s'\n", kpmod->mod->name);
 
 	kpmod->enabled = false;
-	module_put(kpmod->mod);
+
+	/*
+	 * Don't allow modules with forced functions to be removed because they
+	 * might still be in use.
+	 */
+	if (!force)
+		module_put(kpmod->mod);
+
 	list_del(&kpmod->list);
 
 out:
diff --git a/kpatch/kpatch b/kpatch/kpatch
index f196c2e..e7d7c99 100755
--- a/kpatch/kpatch
+++ b/kpatch/kpatch
@@ -115,7 +115,11 @@ unload_module () {
 		echo 0 > $ENABLED || die "can't disable $PATCH"
 	fi
 	echo "unloading patch module: $PATCH"
-	rmmod $PATCH
+	rmmod $PATCH 2> /dev/null
+
+	# ignore any error here because rmmod can fail if the module used
+	# KPATCH_FORCE_UNSAFE.
+	return
 }
 
 unload_disabled_modules() {