From: Masami Hiramatsu (Google) <[email protected]>
When an ftrace-based kprobe is registered without a post_handler, it is
armed using kprobe_ftrace_ops. If a second probe with a post_handler is
subsequently registered at the same address, register_aggr_kprobe() sets
ap->post_handler to aggr_post_handler, but leaves the probe armed using
kprobe_ftrace_ops.
When the probe is later disarmed or disabled, disarm_kprobe_ftrace()
checks whether p->post_handler is non-NULL and attempts to disarm from
kprobe_ipmodify_ops. This fails with -ENOENT, decrements
kprobe_ipmodify_enabled below zero, leaves the probe armed on
kprobe_ftrace_ops, and aborts unregistration, leading to a use-after-free
if the probe module is unloaded.
Switch the armed probe from kprobe_ftrace_ops to kprobe_ipmodify_ops in
register_aggr_kprobe() outside of text_mutex and cpus_read_lock to avoid
lockdep and deadlock issues when modifying ftrace code. Also restore
ftrace ops registration and counter if ftrace_set_filter_ip() fails in
__disarm_kprobe_ftrace().
Fixes: 5dd7caf0bdc5 ("kprobes: Skip clearing aggrprobe's post_handler in
kprobe-on-ftrace case")
Cc: [email protected]
Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
---
kernel/kprobes.c | 56 ++++++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 46 insertions(+), 10 deletions(-)
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 0b41d69540ae..135fec5d49cf 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1435,9 +1435,14 @@ static int __disarm_kprobe_ftrace(struct kprobe *p,
struct ftrace_ops *ops,
(*cnt)--;
ret = ftrace_set_filter_ip(ops, (unsigned long)p->addr, 1, 0);
- WARN_ONCE(ret < 0, "Failed to disarm kprobe-ftrace at %pS (error %d)\n",
- p->addr, ret);
- return ret;
+ if (WARN_ONCE(ret < 0, "Failed to disarm kprobe-ftrace at %pS (error
%d)\n",
+ p->addr, ret)) {
+ if (*cnt == 0)
+ register_ftrace_function(ops);
+ (*cnt)++;
+ return ret;
+ }
+ return 0;
}
static int disarm_kprobe_ftrace(struct kprobe *p)
@@ -1566,16 +1571,12 @@ void kprobe_busy_end(void)
}
/* Add the new probe to 'ap->list'. */
-static int add_new_kprobe(struct kprobe *ap, struct kprobe *p)
+static void add_new_kprobe(struct kprobe *ap, struct kprobe *p)
{
if (p->post_handler)
unoptimize_kprobe(ap, true); /* Fall back to normal kprobe */
list_add_rcu(&p->list, &ap->list);
- if (p->post_handler && !ap->post_handler)
- ap->post_handler = aggr_post_handler;
-
- return 0;
}
/*
@@ -1608,6 +1609,8 @@ static int register_aggr_kprobe(struct kprobe *orig_p,
struct kprobe *p)
{
int ret = 0;
struct kprobe *ap = orig_p;
+ bool reset_post_handler = false;
+ bool ftrace_switch_ops = false;
scoped_guard(cpus_read_lock) {
/* For preparing optimization, jump_label_text_reserved() is
called */
@@ -1654,18 +1657,51 @@ static int register_aggr_kprobe(struct kprobe *orig_p,
struct kprobe *p)
| KPROBE_FLAG_DISABLED;
}
+ if (p->post_handler && !ap->post_handler) {
+ reset_post_handler = true;
+ if (kprobe_ftrace(ap) && !kprobes_all_disarmed &&
+ !kprobe_disabled(ap))
+ ftrace_switch_ops = true;
+ else
+ ap->post_handler = aggr_post_handler;
+ }
+
/* Copy the insn slot of 'p' to 'ap'. */
copy_kprobe(ap, p);
- ret = add_new_kprobe(ap, p);
+ add_new_kprobe(ap, p);
+ }
+
+ if (ftrace_switch_ops) {
+ /*
+ * Must switch ftrace ops outside of text_mutex and
cpus_read_lock
+ * to prevent deadlocks when ftrace code modification acquires
text_mutex.
+ */
+ ret = disarm_kprobe_ftrace(ap);
+ if (ret) {
+ list_del_rcu(&p->list);
+ synchronize_rcu();
+ return ret;
+ }
+ ap->post_handler = aggr_post_handler;
+ ret = arm_kprobe_ftrace(ap);
+ if (ret) {
+ ap->post_handler = NULL;
+ arm_kprobe_ftrace(ap);
+ list_del_rcu(&p->list);
+ synchronize_rcu();
+ return ret;
+ }
}
- if (ret == 0 && kprobe_disabled(ap) && !kprobe_disabled(p)) {
+ if (kprobe_disabled(ap) && !kprobe_disabled(p)) {
ap->flags &= ~KPROBE_FLAG_DISABLED;
if (!kprobes_all_disarmed) {
/* Arm the breakpoint again. */
ret = arm_kprobe(ap);
if (ret) {
ap->flags |= KPROBE_FLAG_DISABLED;
+ if (reset_post_handler)
+ ap->post_handler = NULL;
list_del_rcu(&p->list);
synchronize_rcu();
}
