mirror of https://github.com/dynup/kpatch
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.
This commit is contained in:
parent
6f38498d95
commit
e1890e627a
|
@ -794,7 +794,7 @@ out:
|
||||||
|
|
||||||
int kpatch_register(struct kpatch_module *kpmod, bool replace)
|
int kpatch_register(struct kpatch_module *kpmod, bool replace)
|
||||||
{
|
{
|
||||||
int ret, i;
|
int ret, i, force = 0;
|
||||||
struct kpatch_object *object;
|
struct kpatch_object *object;
|
||||||
struct kpatch_func *func;
|
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) {
|
hash_for_each_rcu(kpatch_func_hash, i, func, node) {
|
||||||
if (func->op != KPATCH_OP_UNPATCH)
|
if (func->op != KPATCH_OP_UNPATCH)
|
||||||
continue;
|
continue;
|
||||||
|
if (func->force)
|
||||||
|
force = 1;
|
||||||
hash_del_rcu(&func->node);
|
hash_del_rcu(&func->node);
|
||||||
WARN_ON(kpatch_ftrace_remove_func(func->old_addr));
|
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;
|
kpmod2->enabled = false;
|
||||||
pr_notice("unloaded patch module '%s'\n",
|
pr_notice("unloaded patch module '%s'\n",
|
||||||
kpmod2->mod->name);
|
kpmod2->mod->name);
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Don't allow modules with forced functions to be
|
||||||
|
* removed because they might still be in use.
|
||||||
|
*/
|
||||||
|
if (!force)
|
||||||
module_put(kpmod2->mod);
|
module_put(kpmod2->mod);
|
||||||
|
|
||||||
list_del(&kpmod2->list);
|
list_del(&kpmod2->list);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -926,7 +935,7 @@ int kpatch_unregister(struct kpatch_module *kpmod)
|
||||||
{
|
{
|
||||||
struct kpatch_object *object;
|
struct kpatch_object *object;
|
||||||
struct kpatch_func *func;
|
struct kpatch_func *func;
|
||||||
int ret;
|
int ret, force = 0;
|
||||||
|
|
||||||
if (!kpmod->enabled)
|
if (!kpmod->enabled)
|
||||||
return -EINVAL;
|
return -EINVAL;
|
||||||
|
@ -935,6 +944,8 @@ int kpatch_unregister(struct kpatch_module *kpmod)
|
||||||
|
|
||||||
do_for_each_linked_func(kpmod, func) {
|
do_for_each_linked_func(kpmod, func) {
|
||||||
func->op = KPATCH_OP_UNPATCH;
|
func->op = KPATCH_OP_UNPATCH;
|
||||||
|
if (func->force)
|
||||||
|
force = 1;
|
||||||
} while_for_each_linked_func();
|
} while_for_each_linked_func();
|
||||||
|
|
||||||
/* memory barrier between func hash and state write */
|
/* 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);
|
pr_notice("unloaded patch module '%s'\n", kpmod->mod->name);
|
||||||
|
|
||||||
kpmod->enabled = false;
|
kpmod->enabled = false;
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Don't allow modules with forced functions to be removed because they
|
||||||
|
* might still be in use.
|
||||||
|
*/
|
||||||
|
if (!force)
|
||||||
module_put(kpmod->mod);
|
module_put(kpmod->mod);
|
||||||
|
|
||||||
list_del(&kpmod->list);
|
list_del(&kpmod->list);
|
||||||
|
|
||||||
out:
|
out:
|
||||||
|
|
|
@ -115,7 +115,11 @@ unload_module () {
|
||||||
echo 0 > $ENABLED || die "can't disable $PATCH"
|
echo 0 > $ENABLED || die "can't disable $PATCH"
|
||||||
fi
|
fi
|
||||||
echo "unloading patch module: $PATCH"
|
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() {
|
unload_disabled_modules() {
|
||||||
|
|
Loading…
Reference in New Issue