[kpatch] [RFC PATCH 2/2] kmod/core: Support live patching on NMI handlers
Masami Hiramatsu
masami.hiramatsu.pt at hitachi.com
Tue Mar 25 12:25:18 UTC 2014
Support live patching on NMI handlers. This adds checks for
possible inconsistency of live patching on NMI handlers.
The inconsistency problem means that any concurrent execution
of old function and new function, which can lead unexpected results.
Current kpatch checks possible inconsistency problem with
stop_machine, which can cover only threads and normal interrupts.
However, beacuse NMI can not stop with it, stop_machine is not
enough for live patching on NMI handlers or sub-functions which are
invoked in the NMI context.
To check for possible inconsistency of live patching on those
functions, add an atomic flag to count patching target functions
invoked in NMI context while updating kpatch hash table. If the
flag is set by the target functions in NMI, we can not ensure
there is no concurrent execution on it.
This fixes the issue #65.
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt at hitachi.com>
---
kmod/core/core.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++------
kmod/core/kpatch.h | 1 +
2 files changed, 89 insertions(+), 11 deletions(-)
diff --git a/kmod/core/core.c b/kmod/core/core.c
index 066dc13..531a9e0 100644
--- a/kmod/core/core.c
+++ b/kmod/core/core.c
@@ -57,6 +57,13 @@ struct kpatch_backtrace_args {
int num_funcs, ret;
};
+static atomic_t inconsistent_flag;
+static enum {
+ KPATCH_OP_NONE,
+ KPATCH_OP_PATCH,
+ KPATCH_OP_UNPATCH,
+} kpatch_operation;
+
void kpatch_backtrace_address_verify(void *data, unsigned long address,
int reliable)
{
@@ -144,9 +151,18 @@ static int kpatch_apply_patch(void *data)
struct kpatch_func *func = &funcs[i];
/* update the global list and go live */
- hash_add(kpatch_func_hash, &func->node, func->old_addr);
+ hash_add_rcu(kpatch_func_hash, &func->node, func->old_addr);
}
-
+ /* Check if any inconsistent NMI has happened while updating */
+ if (atomic_cmpxchg(&inconsistent_flag, 0, 2) != 0) {
+ /* Failed, we have to rollback patching process */
+ for (i = 0; i < num_funcs; i++)
+ hlist_del_rcu(&funcs[i].node);
+ ret = -EBUSY;
+ } else
+ /* Succeeded, clear updating flags */
+ for (i = 0; i < num_funcs; i++)
+ funcs[i].updating = false;
out:
return ret;
}
@@ -163,8 +179,16 @@ static int kpatch_remove_patch(void *data)
if (ret)
goto out;
- for (i = 0; i < num_funcs; i++)
- hlist_del(&funcs[i].node);
+ /* Check if any inconsistent NMI has happened while updating */
+ if (atomic_cmpxchg(&inconsistent_flag, 0, 2) != 0) {
+ /* Failed, we must keep funcs on hash table */
+ for (i = 0; i < num_funcs; i++)
+ funcs[i].updating = false;
+ ret = -EBUSY;
+ } else
+ /* Succeeded, remove all updating funcs from hash table */
+ for (i = 0; i < num_funcs; i++)
+ hlist_del_rcu(&funcs[i].node);
out:
return ret;
@@ -174,16 +198,27 @@ static struct kpatch_func *get_kpatch_func(unsigned long ip)
{
struct kpatch_func *f;
- hash_for_each_possible(kpatch_func_hash, f, node, ip)
+ /* Here, we have to use rcu safe hlist because of NMI concurrency */
+ hash_for_each_possible_rcu(kpatch_func_hash, f, node, ip)
if (f->old_addr == ip)
return f;
return NULL;
}
+static struct kpatch_func *get_committed_func(struct kpatch_func *f,
+ unsigned long ip)
+{
+ hlist_for_each_entry_continue_rcu(f, node) {
+ if (f->old_addr == ip && !f->updating)
+ break;
+ }
+ return f;
+}
+
void notrace kpatch_ftrace_handler(unsigned long ip, unsigned long parent_ip,
struct ftrace_ops *op, struct pt_regs *regs)
{
- struct kpatch_func *f;
+ struct kpatch_func *func;
/*
* This is where the magic happens. Update regs->ip to tell ftrace to
@@ -194,12 +229,39 @@ void notrace kpatch_ftrace_handler(unsigned long ip, unsigned long parent_ip,
* in the hash bucket.
*/
preempt_disable_notrace();
- hash_for_each_possible(kpatch_func_hash, f, node, ip) {
- if (f->old_addr == ip) {
- regs->ip = f->new_addr;
- break;
+retry:
+ func = get_kpatch_func(ip);
+ if (unlikely(kpatch_operation != KPATCH_OP_NONE)) {
+ /*
+ * Checking for NMI inconsistency
+ * If this can set the inconsistent_flag here, it is the NMI
+ * which occures in updating process. In that case, we should
+ * rollback the process.
+ */
+ if ((!func || func->updating) && in_nmi()) {
+ if (atomic_cmpxchg(&inconsistent_flag, 0, 1) != 2) {
+ /* Inconsistency happens here, Newly added
+ * funcs have to be ignored.
+ */
+ if (kpatch_operation == KPATCH_OP_PATCH)
+ func = get_committed_func(func, ip);
+ } else {
+ /*
+ * Here, the updating process has been finished
+ * successfully. Unpatched funcs have to be
+ * ignored.
+ */
+ if (kpatch_operation == KPATCH_OP_UNPATCH)
+ func = get_committed_func(func, ip);
+ /* Very rare case but possible */
+ else if (!func)
+ goto retry;
+ }
}
}
+ if (func)
+ regs->ip = func->new_addr;
+
preempt_enable_notrace();
}
@@ -252,6 +314,7 @@ int kpatch_register(struct module *mod, struct kpatch_func *funcs,
struct kpatch_func *func = &funcs[i];
func->mod = mod;
+ func->updating = true;
/*
* If any other modules have also patched this function, it
@@ -281,6 +344,8 @@ int kpatch_register(struct module *mod, struct kpatch_func *funcs,
}
}
+ kpatch_operation = KPATCH_OP_PATCH;
+ atomic_set(&inconsistent_flag, 0);
/*
* Idle the CPUs, verify activeness safety, and atomically make the new
* functions visible to the trampoline.
@@ -293,6 +358,8 @@ int kpatch_register(struct module *mod, struct kpatch_func *funcs,
printk("kpatch: unregister failed (%d)\n",
ret2);
}
+ /* We must ensure all the handlers on global hash table exit. */
+ synchronize_rcu();
err:
/* Finally rollback the ftrace filter if getting any error */
@@ -300,6 +367,7 @@ err:
} else
pr_notice("loaded patch module \"%s\"\n", mod->name);
+ kpatch_operation = KPATCH_OP_NONE;
up(&kpatch_mutex);
return ret;
}
@@ -308,7 +376,7 @@ EXPORT_SYMBOL(kpatch_register);
int kpatch_unregister(struct module *mod, struct kpatch_func *funcs,
int num_funcs)
{
- int ret;
+ int i, ret;
struct kpatch_stop_machine_args args = {
.funcs = funcs,
.num_funcs = num_funcs,
@@ -316,6 +384,12 @@ int kpatch_unregister(struct module *mod, struct kpatch_func *funcs,
down(&kpatch_mutex);
+ /* Start unpatching operation */
+ kpatch_operation = KPATCH_OP_UNPATCH;
+ atomic_set(&inconsistent_flag, 0);
+ for (i = 0; i < num_funcs; i++)
+ funcs[i].updating = true;
+
ret = stop_machine(kpatch_remove_patch, &args, NULL);
if (ret)
goto out;
@@ -327,12 +401,15 @@ int kpatch_unregister(struct module *mod, struct kpatch_func *funcs,
goto out;
}
}
+ /* We must ensure all the handlers on global hash table exit. */
+ synchronize_rcu();
ret = kpatch_remove_funcs_from_filter(funcs, num_funcs);
if (ret == 0)
pr_notice("unloaded patch module \"%s\"\n", mod->name);
out:
+ kpatch_operation = KPATCH_OP_NONE;
up(&kpatch_mutex);
return ret;
}
diff --git a/kmod/core/kpatch.h b/kmod/core/kpatch.h
index f649302..d9893bd 100644
--- a/kmod/core/kpatch.h
+++ b/kmod/core/kpatch.h
@@ -33,6 +33,7 @@ struct kpatch_func {
unsigned long old_size;
struct module *mod;
struct hlist_node node;
+ bool updating;
};
extern int kpatch_register(struct module *mod, struct kpatch_func *funcs,
More information about the kpatch
mailing list