uprobes: longjmp / special-mapping fixes

PATCH New Oleg Nesterov 2 participants 29 messages Jul 21, 2015 – Jul 31, 2015

Subsystems (3)

x86 perf events uprobes

lore

Share & tools

lore

Oleg Nesterov

author maintainer

2015-07-21 13:41 UTC #0 lore raw

v3 cover uprobes: longjmp / special-mapping fixes

Ingo,

This is the changes I asked you to pull. I added v3 tag to avoid the confusion, but the only change is that I added the acks I got.

Currently ret-probes can't work (the application will likely crash) if the probed function does not return, and this is even documented in handle_trampoline(). This tries to make the first step to fix the problem, assuming that the probed functions use the same stack.

Also, xol_add_vma() doesn't use install_special_mapping() correctly, and we can name the xol vma which currently looks like anon mapping.

Oleg Nesterov (14):
uprobes: Introduce get_uprobe()
uprobes: Introduce free_ret_instance()
uprobes: Send SIGILL if handle_trampoline() fails
uprobes: Change prepare_uretprobe() to use uprobe_warn()
uprobes: Change handle_trampoline() to find the next chain beforehand
uprobes: Export struct return_instance, introduce arch_uretprobe_is_alive() uprobes/x86: Reimplement arch_uretprobe_is_alive()
uprobes: Change handle_trampoline() to flush the frames invalidated by longjmp()
uprobes: Change prepare_uretprobe() to (try to) flush the dead frames
uprobes: Add the "enum rp_check ctx" arg to arch_uretprobe_is_alive() uprobes/x86: Make arch_uretprobe_is_alive(RP_CHECK_CALL) more clever
uprobes: fix the usage of install_special_mapping()
uprobes: use vm_special_mapping to name the xol vma
uprobes: fix the waitqueue_active() check in xol_free_insn_slot()

 arch/x86/kernel/uprobes.c |    9 ++
 include/linux/uprobes.h   |   17 ++++
 kernel/events/uprobes.c   |  228 ++++++++++++++++++++++++++-------------------
 3 files changed, 156 insertions(+), 98 deletions(-)

Oleg Nesterov

author maintainer

2015-07-21 13:41 UTC #1 lore raw

PATCH v3 1/14 uprobes: Introduce get_uprobe()

Cosmetic. Add the new trivial helper, get_uprobe(). It matches put_uprobe() we already have and we can simplify a couple of its users.

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Acked-by Anton Arapov <[email protected]>

Tested-by Pratyush Anand <[email protected]>

---
 kernel/events/uprobes.c |   39 ++++++++++++++++++++-------------------
 1 files changed, 20 insertions(+), 19 deletions(-)

kernel/events/uprobes.c +20 ~~-19~~

index cb346f2..a9847b4 100644
@@ -366,6 +366,18 @@ set_orig_insn(struct arch_uprobe *auprobe, struct mm_struct *mm, unsigned long v
 	return uprobe_write_opcode(mm, vaddr, *(uprobe_opcode_t *)&auprobe->insn);
 }
 
+static struct uprobe *get_uprobe(struct uprobe *uprobe)
+{
+	atomic_inc(&uprobe->ref);
+	return uprobe;
+}
+
+static void put_uprobe(struct uprobe *uprobe)
+{
+	if (atomic_dec_and_test(&uprobe->ref))
+		kfree(uprobe);
+}
+
 static int match_uprobe(struct uprobe *l, struct uprobe *r)
 {
 	if (l->inode < r->inode)

@@ -393,10 +405,8 @@ static struct uprobe *__find_uprobe(struct inode *inode, loff_t offset)
 	while (n) {
 		uprobe = rb_entry(n, struct uprobe, rb_node);
 		match = match_uprobe(&u, uprobe);
-		if (!match) {
-			atomic_inc(&uprobe->ref);
-			return uprobe;
-		}
+		if (!match)
+			return get_uprobe(uprobe);
 
 		if (match < 0)
 			n = n->rb_left;

@@ -432,10 +442,8 @@ static struct uprobe *__insert_uprobe(struct uprobe *uprobe)
 		parent = *p;
 		u = rb_entry(parent, struct uprobe, rb_node);
 		match = match_uprobe(uprobe, u);
-		if (!match) {
-			atomic_inc(&u->ref);
-			return u;
-		}
+		if (!match)
+			return get_uprobe(u);
 
 		if (match < 0)
 			p = &parent->rb_left;

@@ -472,12 +480,6 @@ static struct uprobe *insert_uprobe(struct uprobe *uprobe)
 	return u;
 }
 
-static void put_uprobe(struct uprobe *uprobe)
-{
-	if (atomic_dec_and_test(&uprobe->ref))
-		kfree(uprobe);
-}
-
 static struct uprobe *alloc_uprobe(struct inode *inode, loff_t offset)
 {
 	struct uprobe *uprobe, *cur_uprobe;

@@ -1039,14 +1041,14 @@ static void build_probe_list(struct inode *inode,
 			if (u->inode != inode || u->offset < min)
 				break;
 			list_add(&u->pending_list, head);
-			atomic_inc(&u->ref);
+			get_uprobe(u);
 		}
 		for (t = n; (t = rb_next(t)); ) {
 			u = rb_entry(t, struct uprobe, rb_node);
 			if (u->inode != inode || u->offset > max)
 				break;
 			list_add(&u->pending_list, head);
-			atomic_inc(&u->ref);
+			get_uprobe(u);
 		}
 	}
 	spin_unlock(&uprobes_treelock);

@@ -1437,7 +1439,7 @@ static int dup_utask(struct task_struct *t, struct uprobe_task *o_utask)
 			return -ENOMEM;
 
 		*n = *o;
-		atomic_inc(&n->uprobe->ref);
+		get_uprobe(n->uprobe);
 		n->next = NULL;
 
 		*p = n;

@@ -1565,8 +1567,7 @@ static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
 		orig_ret_vaddr = utask->return_instances->orig_ret_vaddr;
 	}
 
-	atomic_inc(&uprobe->ref);
-	ri->uprobe = uprobe;
+	ri->uprobe = get_uprobe(uprobe);
 	ri->func = instruction_pointer(regs);
 	ri->orig_ret_vaddr = orig_ret_vaddr;
 	ri->chained = chained;

Signature · 1 line

1.5.5.1

tip-bot for Oleg Nesterov

merge bot

2015-07-31 13:58 UTC #2 lore raw

[tip:perf/core] uprobes: Introduce get_uprobe()

uprobes: Introduce get_uprobe()

Commit-ID:  f231722a2b27ee99cbcd0c6bcf4c866612b78137
Gitweb:     http://git.kernel.org/tip/f231722a2b27ee99cbcd0c6bcf4c866612b78137
Author:     Oleg Nesterov <[email protected]>
AuthorDate: Tue, 21 Jul 2015 15:40:03 +0200
Committer:  Ingo Molnar <[email protected]>
CommitDate: Fri, 31 Jul 2015 10:38:03 +0200

uprobes: Introduce get_uprobe()

Cosmetic. Add the new trivial helper, get_uprobe(). It matches put_uprobe() we already have and we can simplify a couple of its users.

Tested-by Pratyush Anand <[email protected]>

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Acked-by Anton Arapov <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 kernel/events/uprobes.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

kernel/events/uprobes.c +20 ~~-19~~

index cb346f2..a9847b4 100644
@@ -366,6 +366,18 @@ set_orig_insn(struct arch_uprobe *auprobe, struct mm_struct *mm, unsigned long v
 	return uprobe_write_opcode(mm, vaddr, *(uprobe_opcode_t *)&auprobe->insn);
 }
 
+static struct uprobe *get_uprobe(struct uprobe *uprobe)
+{
+	atomic_inc(&uprobe->ref);
+	return uprobe;
+}
+
+static void put_uprobe(struct uprobe *uprobe)
+{
+	if (atomic_dec_and_test(&uprobe->ref))
+		kfree(uprobe);
+}
+
 static int match_uprobe(struct uprobe *l, struct uprobe *r)
 {
 	if (l->inode < r->inode)

@@ -393,10 +405,8 @@ static struct uprobe *__find_uprobe(struct inode *inode, loff_t offset)
 	while (n) {
 		uprobe = rb_entry(n, struct uprobe, rb_node);
 		match = match_uprobe(&u, uprobe);
-		if (!match) {
-			atomic_inc(&uprobe->ref);
-			return uprobe;
-		}
+		if (!match)
+			return get_uprobe(uprobe);
 
 		if (match < 0)
 			n = n->rb_left;

@@ -432,10 +442,8 @@ static struct uprobe *__insert_uprobe(struct uprobe *uprobe)
 		parent = *p;
 		u = rb_entry(parent, struct uprobe, rb_node);
 		match = match_uprobe(uprobe, u);
-		if (!match) {
-			atomic_inc(&u->ref);
-			return u;
-		}
+		if (!match)
+			return get_uprobe(u);
 
 		if (match < 0)
 			p = &parent->rb_left;

@@ -472,12 +480,6 @@ static struct uprobe *insert_uprobe(struct uprobe *uprobe)
 	return u;
 }
 
-static void put_uprobe(struct uprobe *uprobe)
-{
-	if (atomic_dec_and_test(&uprobe->ref))
-		kfree(uprobe);
-}
-
 static struct uprobe *alloc_uprobe(struct inode *inode, loff_t offset)
 {
 	struct uprobe *uprobe, *cur_uprobe;

@@ -1039,14 +1041,14 @@ static void build_probe_list(struct inode *inode,
 			if (u->inode != inode || u->offset < min)
 				break;
 			list_add(&u->pending_list, head);
-			atomic_inc(&u->ref);
+			get_uprobe(u);
 		}
 		for (t = n; (t = rb_next(t)); ) {
 			u = rb_entry(t, struct uprobe, rb_node);
 			if (u->inode != inode || u->offset > max)
 				break;
 			list_add(&u->pending_list, head);
-			atomic_inc(&u->ref);
+			get_uprobe(u);
 		}
 	}
 	spin_unlock(&uprobes_treelock);

@@ -1437,7 +1439,7 @@ static int dup_utask(struct task_struct *t, struct uprobe_task *o_utask)
 			return -ENOMEM;
 
 		*n = *o;
-		atomic_inc(&n->uprobe->ref);
+		get_uprobe(n->uprobe);
 		n->next = NULL;
 
 		*p = n;

@@ -1565,8 +1567,7 @@ static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
 		orig_ret_vaddr = utask->return_instances->orig_ret_vaddr;
 	}
 
-	atomic_inc(&uprobe->ref);
-	ri->uprobe = uprobe;
+	ri->uprobe = get_uprobe(uprobe);
 	ri->func = instruction_pointer(regs);
 	ri->orig_ret_vaddr = orig_ret_vaddr;
 	ri->chained = chained;

Oleg Nesterov

author maintainer

2015-07-21 13:41 UTC #3 lore raw

PATCH v3 2/14 uprobes: Introduce free_ret_instance()

In reply to Oleg Nesterov

We can simplify uprobe_free_utask() and handle_uretprobe_chain() if we add a simple helper which does put_uprobe/kfree and returns the ->next return_instance.

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Acked-by Anton Arapov <[email protected]>

Tested-by Pratyush Anand <[email protected]>

---
 kernel/events/uprobes.c |   27 +++++++++++++--------------
 1 files changed, 13 insertions(+), 14 deletions(-)

kernel/events/uprobes.c +13 ~~-14~~

index a9847b4..d8c702f 100644
@@ -1378,6 +1378,14 @@ unsigned long uprobe_get_trap_addr(struct pt_regs *regs)
 	return instruction_pointer(regs);
 }
 
+static struct return_instance *free_ret_instance(struct return_instance *ri)
+{
+	struct return_instance *next = ri->next;
+	put_uprobe(ri->uprobe);
+	kfree(ri);
+	return next;
+}
+
 /*
  * Called with no locks held.
  * Called in context of a exiting or a exec-ing thread.

@@ -1385,7 +1393,7 @@ unsigned long uprobe_get_trap_addr(struct pt_regs *regs)
 void uprobe_free_utask(struct task_struct *t)
 {
 	struct uprobe_task *utask = t->utask;
-	struct return_instance *ri, *tmp;
+	struct return_instance *ri;
 
 	if (!utask)
 		return;

@@ -1394,13 +1402,8 @@ void uprobe_free_utask(struct task_struct *t)
 		put_uprobe(utask->active_uprobe);
 
 	ri = utask->return_instances;
-	while (ri) {
-		tmp = ri;
-		ri = ri->next;
-
-		put_uprobe(tmp->uprobe);
-		kfree(tmp);
-	}
+	while (ri)
+		ri = free_ret_instance(ri);
 
 	xol_free_insn_slot(t);
 	kfree(utask);

@@ -1770,7 +1773,7 @@ handle_uretprobe_chain(struct return_instance *ri, struct pt_regs *regs)
 static bool handle_trampoline(struct pt_regs *regs)
 {
 	struct uprobe_task *utask;
-	struct return_instance *ri, *tmp;
+	struct return_instance *ri;
 	bool chained;
 
 	utask = current->utask;

@@ -1792,11 +1795,7 @@ static bool handle_trampoline(struct pt_regs *regs)
 		handle_uretprobe_chain(ri, regs);
 
 		chained = ri->chained;
-		put_uprobe(ri->uprobe);
-
-		tmp = ri;
-		ri = ri->next;
-		kfree(tmp);
+		ri = free_ret_instance(ri);
 		utask->depth--;
 
 		if (!chained)

Signature · 1 line

1.5.5.1

tip-bot for Oleg Nesterov

merge bot

2015-07-31 13:58 UTC #4 lore raw

[tip:perf/core] uprobes: Introduce free_ret_instance()

uprobes: Introduce free_ret_instance()

Commit-ID:  2bb5e840e873f8778a41801141771f54f547fa65
Gitweb:     http://git.kernel.org/tip/2bb5e840e873f8778a41801141771f54f547fa65
Author:     Oleg Nesterov <[email protected]>
AuthorDate: Tue, 21 Jul 2015 15:40:06 +0200
Committer:  Ingo Molnar <[email protected]>
CommitDate: Fri, 31 Jul 2015 10:38:03 +0200

uprobes: Introduce free_ret_instance()

We can simplify uprobe_free_utask() and handle_uretprobe_chain() if we add a simple helper which does put_uprobe/kfree and returns the ->next return_instance.

Tested-by Pratyush Anand <[email protected]>

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Acked-by Anton Arapov <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 kernel/events/uprobes.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

kernel/events/uprobes.c +13 ~~-14~~

index a9847b4..d8c702f 100644
@@ -1378,6 +1378,14 @@ unsigned long uprobe_get_trap_addr(struct pt_regs *regs)
 	return instruction_pointer(regs);
 }
 
+static struct return_instance *free_ret_instance(struct return_instance *ri)
+{
+	struct return_instance *next = ri->next;
+	put_uprobe(ri->uprobe);
+	kfree(ri);
+	return next;
+}
+
 /*
  * Called with no locks held.
  * Called in context of a exiting or a exec-ing thread.

@@ -1385,7 +1393,7 @@ unsigned long uprobe_get_trap_addr(struct pt_regs *regs)
 void uprobe_free_utask(struct task_struct *t)
 {
 	struct uprobe_task *utask = t->utask;
-	struct return_instance *ri, *tmp;
+	struct return_instance *ri;
 
 	if (!utask)
 		return;

@@ -1394,13 +1402,8 @@ void uprobe_free_utask(struct task_struct *t)
 		put_uprobe(utask->active_uprobe);
 
 	ri = utask->return_instances;
-	while (ri) {
-		tmp = ri;
-		ri = ri->next;
-
-		put_uprobe(tmp->uprobe);
-		kfree(tmp);
-	}
+	while (ri)
+		ri = free_ret_instance(ri);
 
 	xol_free_insn_slot(t);
 	kfree(utask);

@@ -1770,7 +1773,7 @@ handle_uretprobe_chain(struct return_instance *ri, struct pt_regs *regs)
 static bool handle_trampoline(struct pt_regs *regs)
 {
 	struct uprobe_task *utask;
-	struct return_instance *ri, *tmp;
+	struct return_instance *ri;
 	bool chained;
 
 	utask = current->utask;

@@ -1792,11 +1795,7 @@ static bool handle_trampoline(struct pt_regs *regs)
 		handle_uretprobe_chain(ri, regs);
 
 		chained = ri->chained;
-		put_uprobe(ri->uprobe);
-
-		tmp = ri;
-		ri = ri->next;
-		kfree(tmp);
+		ri = free_ret_instance(ri);
 		utask->depth--;
 
 		if (!chained)

Oleg Nesterov

author maintainer

2015-07-21 13:46 UTC #5 lore raw

PATCH v3 3/14 uprobes: Send SIGILL if handle_trampoline() fails

In reply to Oleg Nesterov

1. It doesn't make sense to continue if handle_trampoline() fails, change handle_swbp() to always return after this call.

2. Turn pr_warn() into uprobe_warn(), and change handle_trampoline() to send SIGILL on failure. It is pointless to return to user mode with the corrupted instruction_pointer() which we can't restore.

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Acked-by Anton Arapov <[email protected]>

Tested-by Pratyush Anand <[email protected]>

---
 kernel/events/uprobes.c |   21 ++++++++++-----------
 1 files changed, 10 insertions(+), 11 deletions(-)

kernel/events/uprobes.c +10 ~~-11~~

index d8c702f..eabdc21 100644
@@ -1770,7 +1770,7 @@ handle_uretprobe_chain(struct return_instance *ri, struct pt_regs *regs)
 	up_read(&uprobe->register_rwsem);
 }
 
-static bool handle_trampoline(struct pt_regs *regs)
+static void handle_trampoline(struct pt_regs *regs)
 {
 	struct uprobe_task *utask;
 	struct return_instance *ri;

@@ -1778,11 +1778,11 @@ static bool handle_trampoline(struct pt_regs *regs)
 
 	utask = current->utask;
 	if (!utask)
-		return false;
+		goto sigill;
 
 	ri = utask->return_instances;
 	if (!ri)
-		return false;
+		goto sigill;
 
 	/*
 	 * TODO: we should throw out return_instance's invalidated by

@@ -1804,8 +1804,12 @@ static bool handle_trampoline(struct pt_regs *regs)
 	}
 
 	utask->return_instances = ri;
+	return;
+
+ sigill:
+	uprobe_warn(current, "handle uretprobe, sending SIGILL.");
+	force_sig_info(SIGILL, SEND_SIG_FORCED, current);
 
-	return true;
 }
 
 bool __weak arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs)

@@ -1824,13 +1828,8 @@ static void handle_swbp(struct pt_regs *regs)
 	int uninitialized_var(is_swbp);
 
 	bp_vaddr = uprobe_get_swbp_addr(regs);
-	if (bp_vaddr == get_trampoline_vaddr()) {
-		if (handle_trampoline(regs))
-			return;
-
-		pr_warn("uprobe: unable to handle uretprobe pid/tgid=%d/%d\n",
-						current->pid, current->tgid);
-	}
+	if (bp_vaddr == get_trampoline_vaddr())
+		return handle_trampoline(regs);
 
 	uprobe = find_active_uprobe(bp_vaddr, &is_swbp);
 	if (!uprobe) {

Signature · 1 line

1.5.5.1

tip-bot for Oleg Nesterov

merge bot

2015-07-31 13:59 UTC #6 lore raw

[tip:perf/core] uprobes: Send SIGILL if handle_trampoline() fails

uprobes: Send SIGILL if handle_trampoline() fails

Commit-ID:  0b5256c7f173258b19d98364adb57f707dda22f3
Gitweb:     http://git.kernel.org/tip/0b5256c7f173258b19d98364adb57f707dda22f3
Author:     Oleg Nesterov <[email protected]>
AuthorDate: Tue, 21 Jul 2015 15:40:08 +0200
Committer:  Ingo Molnar <[email protected]>
CommitDate: Fri, 31 Jul 2015 10:38:03 +0200

uprobes: Send SIGILL if handle_trampoline() fails

1. It doesn't make sense to continue if handle_trampoline() fails, change handle_swbp() to always return after this call.

Tested-by Pratyush Anand <[email protected]>

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Acked-by Anton Arapov <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 kernel/events/uprobes.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

kernel/events/uprobes.c +10 ~~-11~~

index d8c702f..eabdc21 100644
@@ -1770,7 +1770,7 @@ handle_uretprobe_chain(struct return_instance *ri, struct pt_regs *regs)
 	up_read(&uprobe->register_rwsem);
 }
 
-static bool handle_trampoline(struct pt_regs *regs)
+static void handle_trampoline(struct pt_regs *regs)
 {
 	struct uprobe_task *utask;
 	struct return_instance *ri;

@@ -1778,11 +1778,11 @@ static bool handle_trampoline(struct pt_regs *regs)
 
 	utask = current->utask;
 	if (!utask)
-		return false;
+		goto sigill;
 
 	ri = utask->return_instances;
 	if (!ri)
-		return false;
+		goto sigill;
 
 	/*
 	 * TODO: we should throw out return_instance's invalidated by

@@ -1804,8 +1804,12 @@ static bool handle_trampoline(struct pt_regs *regs)
 	}
 
 	utask->return_instances = ri;
+	return;
+
+ sigill:
+	uprobe_warn(current, "handle uretprobe, sending SIGILL.");
+	force_sig_info(SIGILL, SEND_SIG_FORCED, current);
 
-	return true;
 }
 
 bool __weak arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs)

@@ -1824,13 +1828,8 @@ static void handle_swbp(struct pt_regs *regs)
 	int uninitialized_var(is_swbp);
 
 	bp_vaddr = uprobe_get_swbp_addr(regs);
-	if (bp_vaddr == get_trampoline_vaddr()) {
-		if (handle_trampoline(regs))
-			return;
-
-		pr_warn("uprobe: unable to handle uretprobe pid/tgid=%d/%d\n",
-						current->pid, current->tgid);
-	}
+	if (bp_vaddr == get_trampoline_vaddr())
+		return handle_trampoline(regs);
 
 	uprobe = find_active_uprobe(bp_vaddr, &is_swbp);
 	if (!uprobe) {

Oleg Nesterov

author maintainer

2015-07-21 13:42 UTC #7 lore raw

PATCH v3 4/14 uprobes: Change prepare_uretprobe() to use uprobe_warn()

In reply to Oleg Nesterov

Turn the last pr_warn() in uprobes.c into uprobe_warn().

While at it:

- s/kzalloc/kmalloc, we initialize every member of ri

- remove the pointless comment above the obvious code

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Acked-by Anton Arapov <[email protected]>

Tested-by Pratyush Anand <[email protected]>

---
 kernel/events/uprobes.c |   10 +++-------
 1 files changed, 3 insertions(+), 7 deletions(-)

kernel/events/uprobes.c +3 -7

index eabdc21..4c941fe 100644
@@ -1541,9 +1541,9 @@ static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
 		return;
 	}
 
-	ri = kzalloc(sizeof(struct return_instance), GFP_KERNEL);
+	ri = kmalloc(sizeof(struct return_instance), GFP_KERNEL);
 	if (!ri)
-		goto fail;
+		return;
 
 	trampoline_vaddr = get_trampoline_vaddr();
 	orig_ret_vaddr = arch_uretprobe_hijack_return_addr(trampoline_vaddr, regs);

@@ -1561,8 +1561,7 @@ static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
 			 * This situation is not possible. Likely we have an
 			 * attack from user-space.
 			 */
-			pr_warn("uprobe: unable to set uretprobe pid/tgid=%d/%d\n",
-						current->pid, current->tgid);
+			uprobe_warn(current, "handle tail call");
 			goto fail;
 		}

@@ -1576,13 +1575,10 @@ static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
 	ri->chained = chained;
 
 	utask->depth++;
-
-	/* add instance to the stack */
 	ri->next = utask->return_instances;
 	utask->return_instances = ri;
 
 	return;
-
  fail:
 	kfree(ri);
 }

Signature · 1 line

1.5.5.1

tip-bot for Oleg Nesterov

merge bot

2015-07-31 13:59 UTC #8 lore raw

[tip:perf/core] uprobes: Change prepare_uretprobe() to use uprobe_warn()

uprobes: Change prepare_uretprobe() to use uprobe_warn()

Commit-ID:  6c58d0e4cc26ea8882928e64c0de9afed4fc37cb
Gitweb:     http://git.kernel.org/tip/6c58d0e4cc26ea8882928e64c0de9afed4fc37cb
Author:     Oleg Nesterov <[email protected]>
AuthorDate: Tue, 21 Jul 2015 15:40:10 +0200
Committer:  Ingo Molnar <[email protected]>
CommitDate: Fri, 31 Jul 2015 10:38:04 +0200

uprobes: Change prepare_uretprobe() to use uprobe_warn()

Turn the last pr_warn() in uprobes.c into uprobe_warn().

While at it:

- s/kzalloc/kmalloc, we initialize every member of 'ri'

- remove the pointless comment above the obvious code

Tested-by Pratyush Anand <[email protected]>

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Acked-by Anton Arapov <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 kernel/events/uprobes.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

kernel/events/uprobes.c +3 -7

index eabdc21..4c941fe 100644
@@ -1541,9 +1541,9 @@ static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
 		return;
 	}
 
-	ri = kzalloc(sizeof(struct return_instance), GFP_KERNEL);
+	ri = kmalloc(sizeof(struct return_instance), GFP_KERNEL);
 	if (!ri)
-		goto fail;
+		return;
 
 	trampoline_vaddr = get_trampoline_vaddr();
 	orig_ret_vaddr = arch_uretprobe_hijack_return_addr(trampoline_vaddr, regs);

@@ -1561,8 +1561,7 @@ static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
 			 * This situation is not possible. Likely we have an
 			 * attack from user-space.
 			 */
-			pr_warn("uprobe: unable to set uretprobe pid/tgid=%d/%d\n",
-						current->pid, current->tgid);
+			uprobe_warn(current, "handle tail call");
 			goto fail;
 		}

@@ -1576,13 +1575,10 @@ static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
 	ri->chained = chained;
 
 	utask->depth++;
-
-	/* add instance to the stack */
 	ri->next = utask->return_instances;
 	utask->return_instances = ri;
 
 	return;
-
  fail:
 	kfree(ri);
 }

Oleg Nesterov

author maintainer

2015-07-21 13:45 UTC #9 lore raw

PATCH v3 5/14 uprobes: Change handle_trampoline() to find the next chain beforehand

In reply to Oleg Nesterov

No functional changes, preparation.

Add the new helper, find_next_ret_chain(), which finds the first !chained entry and returns its ->next. Yes, it is suboptimal. We probably want to turn ->chained into ->start_of_this_chain pointer and avoid another loop. But this needs the boring changes in dup_utask(), so lets do this later.

Change the main loop in handle_trampoline() to unwind the stack until ri is equal to the pointer returned by this new helper.

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Acked-by Anton Arapov <[email protected]>

Tested-by Pratyush Anand <[email protected]>

---
 kernel/events/uprobes.c |   27 ++++++++++++++++-----------
 1 files changed, 16 insertions(+), 11 deletions(-)

kernel/events/uprobes.c +16 ~~-11~~

index 4c941fe..98e4d97 100644
@@ -1766,11 +1766,22 @@ handle_uretprobe_chain(struct return_instance *ri, struct pt_regs *regs)
 	up_read(&uprobe->register_rwsem);
 }
 
+static struct return_instance *find_next_ret_chain(struct return_instance *ri)
+{
+	bool chained;
+
+	do {
+		chained = ri->chained;
+		ri = ri->next;	/* can't be NULL if chained */
+	} while (chained);
+
+	return ri;
+}
+
 static void handle_trampoline(struct pt_regs *regs)
 {
 	struct uprobe_task *utask;
-	struct return_instance *ri;
-	bool chained;
+	struct return_instance *ri, *next;
 
 	utask = current->utask;
 	if (!utask)

@@ -1780,24 +1791,18 @@ static void handle_trampoline(struct pt_regs *regs)
 	if (!ri)
 		goto sigill;
 
+	next = find_next_ret_chain(ri);
 	/*
 	 * TODO: we should throw out return_instance's invalidated by
 	 * longjmp(), currently we assume that the probed function always
 	 * returns.
 	 */
 	instruction_pointer_set(regs, ri->orig_ret_vaddr);
-
-	for (;;) {
+	do {
 		handle_uretprobe_chain(ri, regs);
-
-		chained = ri->chained;
 		ri = free_ret_instance(ri);
 		utask->depth--;
-
-		if (!chained)
-			break;
-		BUG_ON(!ri);
-	}
+	} while (ri != next);
 
 	utask->return_instances = ri;
 	return;

Signature · 1 line

1.5.5.1

tip-bot for Oleg Nesterov

merge bot

2015-07-31 13:59 UTC #10 lore raw

[tip:perf/core] uprobes: Change handle_trampoline() to find the next chain beforehand

uprobes: Change handle_trampoline() to find the next chain beforehand

Commit-ID:  a83cfeb92132c279b20bbc8ed3cef833b0fe417e
Gitweb:     http://git.kernel.org/tip/a83cfeb92132c279b20bbc8ed3cef833b0fe417e
Author:     Oleg Nesterov <[email protected]>
AuthorDate: Tue, 21 Jul 2015 15:40:13 +0200
Committer:  Ingo Molnar <[email protected]>
CommitDate: Fri, 31 Jul 2015 10:38:04 +0200

uprobes: Change handle_trampoline() to find the next chain beforehand

No functional changes, preparation.

Change the main loop in handle_trampoline() to unwind the stack until ri is equal to the pointer returned by this new helper.

Tested-by Pratyush Anand <[email protected]>

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Acked-by Anton Arapov <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 kernel/events/uprobes.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

kernel/events/uprobes.c +16 ~~-11~~

index 4c941fe..98e4d97 100644
@@ -1766,11 +1766,22 @@ handle_uretprobe_chain(struct return_instance *ri, struct pt_regs *regs)
 	up_read(&uprobe->register_rwsem);
 }
 
+static struct return_instance *find_next_ret_chain(struct return_instance *ri)
+{
+	bool chained;
+
+	do {
+		chained = ri->chained;
+		ri = ri->next;	/* can't be NULL if chained */
+	} while (chained);
+
+	return ri;
+}
+
 static void handle_trampoline(struct pt_regs *regs)
 {
 	struct uprobe_task *utask;
-	struct return_instance *ri;
-	bool chained;
+	struct return_instance *ri, *next;
 
 	utask = current->utask;
 	if (!utask)

@@ -1780,24 +1791,18 @@ static void handle_trampoline(struct pt_regs *regs)
 	if (!ri)
 		goto sigill;
 
+	next = find_next_ret_chain(ri);
 	/*
 	 * TODO: we should throw out return_instance's invalidated by
 	 * longjmp(), currently we assume that the probed function always
 	 * returns.
 	 */
 	instruction_pointer_set(regs, ri->orig_ret_vaddr);
-
-	for (;;) {
+	do {
 		handle_uretprobe_chain(ri, regs);
-
-		chained = ri->chained;
 		ri = free_ret_instance(ri);
 		utask->depth--;
-
-		if (!chained)
-			break;
-		BUG_ON(!ri);
-	}
+	} while (ri != next);
 
 	utask->return_instances = ri;
 	return;

Oleg Nesterov

author maintainer

2015-07-21 13:42 UTC #11 lore raw

PATCH v3 6/14 uprobes: Export struct return_instance, introduce arch_uretprobe_is_alive()

In reply to Oleg Nesterov

Add the new "weak" helper, arch_uretprobe_is_alive(), used by the next patches. It should return true if this return_instance is still valid. The arch agnostic version just always returns true.

The patch exports "struct return_instance" for the architectures which want to override this hook. We can also cleanup prepare_uretprobe() if we pass the new return_instance to arch_uretprobe_hijack_return_addr().

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Acked-by Anton Arapov <[email protected]>

Tested-by Pratyush Anand <[email protected]>

---
 include/linux/uprobes.h |   10 ++++++++++
 kernel/events/uprobes.c |   14 +++++---------
 2 files changed, 15 insertions(+), 9 deletions(-)

include/linux/uprobes.h +10 -0

index 60beb5d..50d2764 100644
@@ -92,6 +92,15 @@ struct uprobe_task {
 	unsigned int			depth;
 };
 
+struct return_instance {
+	struct uprobe		*uprobe;
+	unsigned long		func;
+	unsigned long		orig_ret_vaddr; /* original return address */
+	bool			chained;	/* true, if instance is nested */
+
+	struct return_instance	*next;		/* keep as stack */
+};
+
 struct xol_area;
 
 struct uprobes_state {

@@ -128,6 +137,7 @@ extern bool arch_uprobe_xol_was_trapped(struct task_struct *tsk);
 extern int  arch_uprobe_exception_notify(struct notifier_block *self, unsigned long val, void *data);
 extern void arch_uprobe_abort_xol(struct arch_uprobe *aup, struct pt_regs *regs);
 extern unsigned long arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr, struct pt_regs *regs);
+extern bool arch_uretprobe_is_alive(struct return_instance *ret, struct pt_regs *regs);
 extern bool arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs);
 extern void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
 					 void *src, unsigned long len);

kernel/events/uprobes.c +5 -9

index 98e4d97..1c71b62 100644
@@ -86,15 +86,6 @@ struct uprobe {
 	struct arch_uprobe	arch;
 };
 
-struct return_instance {
-	struct uprobe		*uprobe;
-	unsigned long		func;
-	unsigned long		orig_ret_vaddr; /* original return address */
-	bool			chained;	/* true, if instance is nested */
-
-	struct return_instance	*next;		/* keep as stack */
-};
-
 /*
  * Execute out of line area: anonymous executable mapping installed
  * by the probed task to execute the copy of the original instruction

@@ -1818,6 +1809,11 @@ bool __weak arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs)
 	return false;
 }
 
+bool __weak arch_uretprobe_is_alive(struct return_instance *ret, struct pt_regs *regs)
+{
+	return true;
+}
+
 /*
  * Run handler and ask thread to singlestep.
  * Ensure all non-fatal signals cannot interrupt thread while it singlesteps.

Signature · 1 line

1.5.5.1

tip-bot for Oleg Nesterov

merge bot

2015-07-31 14:00 UTC #12 lore raw

[tip:perf/core] uprobes: Export 'struct return_instance', introduce arch_uretprobe_is_alive()

uprobes: Export 'struct return_instance', introduce arch_uretprobe_is_alive()

Commit-ID:  97da89767d398c1dfa1f34e5f312eb8ebb382f7f
Gitweb:     http://git.kernel.org/tip/97da89767d398c1dfa1f34e5f312eb8ebb382f7f
Author:     Oleg Nesterov <[email protected]>
AuthorDate: Tue, 21 Jul 2015 15:40:16 +0200
Committer:  Ingo Molnar <[email protected]>
CommitDate: Fri, 31 Jul 2015 10:38:04 +0200

uprobes: Export 'struct return_instance', introduce arch_uretprobe_is_alive()

Add the new "weak" helper, arch_uretprobe_is_alive(), used by the next patches. It should return true if this return_instance is still valid. The arch agnostic version just always returns true.

Tested-by Pratyush Anand <[email protected]>

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Acked-by Anton Arapov <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 include/linux/uprobes.h | 10 ++++++++++
 kernel/events/uprobes.c | 14 +++++---------
 2 files changed, 15 insertions(+), 9 deletions(-)

include/linux/uprobes.h +10 -0

index 60beb5d..50d2764 100644
@@ -92,6 +92,15 @@ struct uprobe_task {
 	unsigned int			depth;
 };
 
+struct return_instance {
+	struct uprobe		*uprobe;
+	unsigned long		func;
+	unsigned long		orig_ret_vaddr; /* original return address */
+	bool			chained;	/* true, if instance is nested */
+
+	struct return_instance	*next;		/* keep as stack */
+};
+
 struct xol_area;
 
 struct uprobes_state {

@@ -128,6 +137,7 @@ extern bool arch_uprobe_xol_was_trapped(struct task_struct *tsk);
 extern int  arch_uprobe_exception_notify(struct notifier_block *self, unsigned long val, void *data);
 extern void arch_uprobe_abort_xol(struct arch_uprobe *aup, struct pt_regs *regs);
 extern unsigned long arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr, struct pt_regs *regs);
+extern bool arch_uretprobe_is_alive(struct return_instance *ret, struct pt_regs *regs);
 extern bool arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs);
 extern void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
 					 void *src, unsigned long len);

kernel/events/uprobes.c +5 -9

index 98e4d97..1c71b62 100644
@@ -86,15 +86,6 @@ struct uprobe {
 	struct arch_uprobe	arch;
 };
 
-struct return_instance {
-	struct uprobe		*uprobe;
-	unsigned long		func;
-	unsigned long		orig_ret_vaddr; /* original return address */
-	bool			chained;	/* true, if instance is nested */
-
-	struct return_instance	*next;		/* keep as stack */
-};
-
 /*
  * Execute out of line area: anonymous executable mapping installed
  * by the probed task to execute the copy of the original instruction

@@ -1818,6 +1809,11 @@ bool __weak arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs)
 	return false;
 }
 
+bool __weak arch_uretprobe_is_alive(struct return_instance *ret, struct pt_regs *regs)
+{
+	return true;
+}
+
 /*
  * Run handler and ask thread to singlestep.
  * Ensure all non-fatal signals cannot interrupt thread while it singlesteps.

Oleg Nesterov

author maintainer

2015-07-21 13:43 UTC #13 lore raw

PATCH v3 7/14 uprobes/x86: Reimplement arch_uretprobe_is_alive()

In reply to Oleg Nesterov

Add the x86-specific version of arch_uretprobe_is_alive() helper. It returns true if the stack frame mangled by prepare_uretprobe() is still on stack. So if it returns false, we know that the probed function has already returned.

We add the new return_instance->stack member and change the generic code to initialize it in prepare_uretprobe, but it should be equally useful for other architectures.

TODO: this assumes that the probed application can't use multiple stacks (say sigaltstack). We will try to improve this logic later.

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Acked-by Anton Arapov <[email protected]>

Tested-by Pratyush Anand <[email protected]>

---
 arch/x86/kernel/uprobes.c |    5 +++++
 include/linux/uprobes.h   |    1 +
 kernel/events/uprobes.c   |    1 +
 3 files changed, 7 insertions(+), 0 deletions(-)

arch/x86/kernel/uprobes.c +5 -0

index 6647624..58e9b84 100644
@@ -985,3 +985,8 @@ arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr, struct pt_regs
 
 	return -1;
 }
+
+bool arch_uretprobe_is_alive(struct return_instance *ret, struct pt_regs *regs)
+{
+	return regs->sp <= ret->stack;
+}

include/linux/uprobes.h +1 -0

index 50d2764..7ab6d2c 100644
@@ -95,6 +95,7 @@ struct uprobe_task {
 struct return_instance {
 	struct uprobe		*uprobe;
 	unsigned long		func;
+	unsigned long		stack;		/* stack pointer */
 	unsigned long		orig_ret_vaddr; /* original return address */
 	bool			chained;	/* true, if instance is nested */

kernel/events/uprobes.c +1 -0

index 1c71b62..c5f316e 100644
@@ -1562,6 +1562,7 @@ static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
 
 	ri->uprobe = get_uprobe(uprobe);
 	ri->func = instruction_pointer(regs);
+	ri->stack = user_stack_pointer(regs);
 	ri->orig_ret_vaddr = orig_ret_vaddr;
 	ri->chained = chained;

Signature · 1 line

1.5.5.1

tip-bot for Oleg Nesterov

merge bot

2015-07-31 14:00 UTC #14 lore raw

[tip:perf/core] uprobes/x86: Reimplement arch_uretprobe_is_alive( )

uprobes/x86: Reimplement arch_uretprobe_is_alive()

Commit-ID:  7b868e4802a86d867aad1be0471b5767d9c20e10
Gitweb:     http://git.kernel.org/tip/7b868e4802a86d867aad1be0471b5767d9c20e10
Author:     Oleg Nesterov <[email protected]>
AuthorDate: Tue, 21 Jul 2015 15:40:18 +0200
Committer:  Ingo Molnar <[email protected]>
CommitDate: Fri, 31 Jul 2015 10:38:05 +0200

uprobes/x86: Reimplement arch_uretprobe_is_alive()

Add the x86 specific version of arch_uretprobe_is_alive() helper. It returns true if the stack frame mangled by prepare_uretprobe() is still on stack. So if it returns false, we know that the probed function has already returned.

We add the new return_instance->stack member and change the generic code to initialize it in prepare_uretprobe, but it should be equally useful for other architectures.

TODO: this assumes that the probed application can't use
multiple stacks (say sigaltstack). We will try to improve this logic later.

Tested-by Pratyush Anand <[email protected]>

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Acked-by Anton Arapov <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/x86/kernel/uprobes.c | 5 +++++
 include/linux/uprobes.h   | 1 +
 kernel/events/uprobes.c   | 1 +
 3 files changed, 7 insertions(+)

arch/x86/kernel/uprobes.c +5 -0

index 6647624..58e9b84 100644
@@ -985,3 +985,8 @@ arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr, struct pt_regs
 
 	return -1;
 }
+
+bool arch_uretprobe_is_alive(struct return_instance *ret, struct pt_regs *regs)
+{
+	return regs->sp <= ret->stack;
+}

include/linux/uprobes.h +1 -0

index 50d2764..7ab6d2c 100644
@@ -95,6 +95,7 @@ struct uprobe_task {
 struct return_instance {
 	struct uprobe		*uprobe;
 	unsigned long		func;
+	unsigned long		stack;		/* stack pointer */
 	unsigned long		orig_ret_vaddr; /* original return address */
 	bool			chained;	/* true, if instance is nested */

kernel/events/uprobes.c +1 -0

index 1c71b62..c5f316e 100644
@@ -1562,6 +1562,7 @@ static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
 
 	ri->uprobe = get_uprobe(uprobe);
 	ri->func = instruction_pointer(regs);
+	ri->stack = user_stack_pointer(regs);
 	ri->orig_ret_vaddr = orig_ret_vaddr;
 	ri->chained = chained;

Oleg Nesterov

author maintainer

2015-07-21 13:42 UTC #15 lore raw

PATCH v3 8/14 uprobes: Change handle_trampoline() to flush the frames invalidated by longjmp()

In reply to Oleg Nesterov

Test-case:

	#include <stdio.h>
	#include <setjmp.h>

	jmp_buf jmp;

	void func_2(void)
	{
		longjmp(jmp, 1);
	}

	void func_1(void)
	{
		if (setjmp(jmp))
			return;
		func_2();
		printf("ERR!! I am running on the caller's stack\n");
	}

	int main(void)
	{
		func_1();
		return 0;
	}

fails if you probe func_1() and func_2() because handle_trampoline() assumes that the probed function should must return and hit the bp installed be prepare_uretprobe(). But in this case func_2() does not return, so when func_1() returns the kernel uses the no longer valid return_instance of func_2().

Change handle_trampoline() to unwind ->return_instances until we know that the next chain is alive or NULL, this ensures that the current chain is the last we need to report and free.

Alternatively, every return_instance could use unique trampoline_vaddr, in this case we could use it as a key. And this could solve the problem with sigaltstack() automatically.

But this approach needs more changes, and it puts the "hard" limit on MAX_URETPROBE_DEPTH. Plus it can not solve another problem partially fixed by the next patch.

Note: this change has no effect on !x86, the arch-agnostic version of arch_uretprobe_is_alive() just returns "true".

TODO: as documented by the previous change, arch_uretprobe_is_alive() can be fooled by sigaltstack/etc.

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Acked-by Anton Arapov <[email protected]>

Tested-by Pratyush Anand <[email protected]>

---
 kernel/events/uprobes.c |   29 ++++++++++++++++++-----------
 1 files changed, 18 insertions(+), 11 deletions(-)

kernel/events/uprobes.c +18 ~~-11~~

index c5f316e..93d939c 100644
@@ -1774,6 +1774,7 @@ static void handle_trampoline(struct pt_regs *regs)
 {
 	struct uprobe_task *utask;
 	struct return_instance *ri, *next;
+	bool valid;
 
 	utask = current->utask;
 	if (!utask)

@@ -1783,18 +1784,24 @@ static void handle_trampoline(struct pt_regs *regs)
 	if (!ri)
 		goto sigill;
 
-	next = find_next_ret_chain(ri);
-	/*
-	 * TODO: we should throw out return_instance's invalidated by
-	 * longjmp(), currently we assume that the probed function always
-	 * returns.
-	 */
-	instruction_pointer_set(regs, ri->orig_ret_vaddr);
 	do {
-		handle_uretprobe_chain(ri, regs);
-		ri = free_ret_instance(ri);
-		utask->depth--;
-	} while (ri != next);
+		/*
+		 * We should throw out the frames invalidated by longjmp().
+		 * If this chain is valid, then the next one should be alive
+		 * or NULL; the latter case means that nobody but ri->func
+		 * could hit this trampoline on return. TODO: sigaltstack().
+		 */
+		next = find_next_ret_chain(ri);
+		valid = !next || arch_uretprobe_is_alive(next, regs);
+
+		instruction_pointer_set(regs, ri->orig_ret_vaddr);
+		do {
+			if (valid)
+				handle_uretprobe_chain(ri, regs);
+			ri = free_ret_instance(ri);
+			utask->depth--;
+		} while (ri != next);
+	} while (!valid);
 
 	utask->return_instances = ri;
 	return;

Signature · 1 line

1.5.5.1

tip-bot for Oleg Nesterov

merge bot

2015-07-31 14:00 UTC #16 lore raw

[tip:perf/core] uprobes: Change handle_trampoline() to flush the frames invalidated by longjmp()

uprobes: Change handle_trampoline() to flush the frames invalidated by longjmp()

Commit-ID:  5eeb50de42fd3251845d03c556db012267c72b3f
Gitweb:     http://git.kernel.org/tip/5eeb50de42fd3251845d03c556db012267c72b3f
Author:     Oleg Nesterov <[email protected]>
AuthorDate: Tue, 21 Jul 2015 15:40:21 +0200
Committer:  Ingo Molnar <[email protected]>
CommitDate: Fri, 31 Jul 2015 10:38:05 +0200

uprobes: Change handle_trampoline() to flush the frames invalidated by longjmp()

Test-case:

	#include <stdio.h>
	#include <setjmp.h>

	jmp_buf jmp;

	void func_2(void)
	{
		longjmp(jmp, 1);
	}

	void func_1(void)
	{
		if (setjmp(jmp))
			return;
		func_2();
		printf("ERR!! I am running on the caller's stack\n");
	}

	int main(void)
	{
		func_1();
		return 0;
	}

Change handle_trampoline() to unwind ->return_instances until we know that the next chain is alive or NULL, this ensures that the current chain is the last we need to report and free.

Alternatively, every return_instance could use unique trampoline_vaddr, in this case we could use it as a key. And this could solve the problem with sigaltstack() automatically.

But this approach needs more changes, and it puts the "hard" limit on MAX_URETPROBE_DEPTH. Plus it can not solve another problem partially fixed by the next patch.

Note: this change has no effect on !x86, the arch-agnostic version of arch_uretprobe_is_alive() just returns "true".

TODO: as documented by the previous change, arch_uretprobe_is_alive()
can be fooled by sigaltstack/etc.

Tested-by Pratyush Anand <[email protected]>

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Acked-by Anton Arapov <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 kernel/events/uprobes.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

kernel/events/uprobes.c +18 ~~-11~~

index c5f316e..93d939c8 100644
@@ -1774,6 +1774,7 @@ static void handle_trampoline(struct pt_regs *regs)
 {
 	struct uprobe_task *utask;
 	struct return_instance *ri, *next;
+	bool valid;
 
 	utask = current->utask;
 	if (!utask)

@@ -1783,18 +1784,24 @@ static void handle_trampoline(struct pt_regs *regs)
 	if (!ri)
 		goto sigill;
 
-	next = find_next_ret_chain(ri);
-	/*
-	 * TODO: we should throw out return_instance's invalidated by
-	 * longjmp(), currently we assume that the probed function always
-	 * returns.
-	 */
-	instruction_pointer_set(regs, ri->orig_ret_vaddr);
 	do {
-		handle_uretprobe_chain(ri, regs);
-		ri = free_ret_instance(ri);
-		utask->depth--;
-	} while (ri != next);
+		/*
+		 * We should throw out the frames invalidated by longjmp().
+		 * If this chain is valid, then the next one should be alive
+		 * or NULL; the latter case means that nobody but ri->func
+		 * could hit this trampoline on return. TODO: sigaltstack().
+		 */
+		next = find_next_ret_chain(ri);
+		valid = !next || arch_uretprobe_is_alive(next, regs);
+
+		instruction_pointer_set(regs, ri->orig_ret_vaddr);
+		do {
+			if (valid)
+				handle_uretprobe_chain(ri, regs);
+			ri = free_ret_instance(ri);
+			utask->depth--;
+		} while (ri != next);
+	} while (!valid);
 
 	utask->return_instances = ri;
 	return;

Oleg Nesterov

author maintainer

2015-07-21 13:42 UTC #17 lore raw

PATCH v3 9/14 uprobes: Change prepare_uretprobe() to (try to) flush the dead frames

In reply to Oleg Nesterov

Change prepare_uretprobe() to flush the !arch_uretprobe_is_alive() return_instance's. This is not needed correctness-wise, but can help to avoid the failure caused by MAX_URETPROBE_DEPTH.

Note: in this case arch_uretprobe_is_alive() can be false positive, the stack can grow after longjmp(). Unfortunately, the kernel can't 100% solve this problem, but see the next patch.

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Acked-by Anton Arapov <[email protected]>

Tested-by Pratyush Anand <[email protected]>

---
 kernel/events/uprobes.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

kernel/events/uprobes.c +13 -0

index 93d939c..7e61c8c 100644
@@ -1511,6 +1511,16 @@ static unsigned long get_trampoline_vaddr(void)
 	return trampoline_vaddr;
 }
 
+static void cleanup_return_instances(struct uprobe_task *utask, struct pt_regs *regs)
+{
+	struct return_instance *ri = utask->return_instances;
+	while (ri && !arch_uretprobe_is_alive(ri, regs)) {
+		ri = free_ret_instance(ri);
+		utask->depth--;
+	}
+	utask->return_instances = ri;
+}
+
 static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
 {
 	struct return_instance *ri;

@@ -1541,6 +1551,9 @@ static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
 	if (orig_ret_vaddr == -1)
 		goto fail;
 
+	/* drop the entries invalidated by longjmp() */
+	cleanup_return_instances(utask, regs);
+
 	/*
 	 * We don't want to keep trampoline address in stack, rather keep the
 	 * original return address of first caller thru all the consequent

Signature · 1 line

1.5.5.1

tip-bot for Oleg Nesterov

merge bot

2015-07-31 14:01 UTC #18 lore raw

[tip:perf/core] uprobes: Change prepare_uretprobe() to (try to) flush the dead frames

uprobes: Change prepare_uretprobe() to (try to) flush the dead frames

Commit-ID:  a5b7e1a89b820f2b9b23634ca4c59b555e8d9a0d
Gitweb:     http://git.kernel.org/tip/a5b7e1a89b820f2b9b23634ca4c59b555e8d9a0d
Author:     Oleg Nesterov <[email protected]>
AuthorDate: Tue, 21 Jul 2015 15:40:23 +0200
Committer:  Ingo Molnar <[email protected]>
CommitDate: Fri, 31 Jul 2015 10:38:05 +0200

uprobes: Change prepare_uretprobe() to (try to) flush the dead frames

Change prepare_uretprobe() to flush the !arch_uretprobe_is_alive() return_instance's. This is not needed correctness-wise, but can help to avoid the failure caused by MAX_URETPROBE_DEPTH.

Note: in this case arch_uretprobe_is_alive() can be false positive, the stack can grow after longjmp(). Unfortunately, the kernel can't 100% solve this problem, but see the next patch.

Tested-by Pratyush Anand <[email protected]>

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Acked-by Anton Arapov <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 kernel/events/uprobes.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

kernel/events/uprobes.c +13 -0

index 93d939c8..7e61c8c 100644
@@ -1511,6 +1511,16 @@ static unsigned long get_trampoline_vaddr(void)
 	return trampoline_vaddr;
 }
 
+static void cleanup_return_instances(struct uprobe_task *utask, struct pt_regs *regs)
+{
+	struct return_instance *ri = utask->return_instances;
+	while (ri && !arch_uretprobe_is_alive(ri, regs)) {
+		ri = free_ret_instance(ri);
+		utask->depth--;
+	}
+	utask->return_instances = ri;
+}
+
 static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
 {
 	struct return_instance *ri;

@@ -1541,6 +1551,9 @@ static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
 	if (orig_ret_vaddr == -1)
 		goto fail;
 
+	/* drop the entries invalidated by longjmp() */
+	cleanup_return_instances(utask, regs);
+
 	/*
 	 * We don't want to keep trampoline address in stack, rather keep the
 	 * original return address of first caller thru all the consequent

Oleg Nesterov

author maintainer

2015-07-21 13:42 UTC #19 lore raw

PATCH v3 10/14 uprobes: Add the "enum rp_check ctx" arg to arch_uretprobe_is_alive()

In reply to Oleg Nesterov

arch/x86 doesn't care (so far), but as Pratyush Anand pointed out other architectures might want why arch_uretprobe_is_alive() was called and use different checks depending on the context. Add the new argument to distinguish 2 callers.

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Acked-by Anton Arapov <[email protected]>

Tested-by Pratyush Anand <[email protected]>

---
 arch/x86/kernel/uprobes.c |    3 ++-
 include/linux/uprobes.h   |    7 ++++++-
 kernel/events/uprobes.c   |    9 ++++++---
 3 files changed, 14 insertions(+), 5 deletions(-)

arch/x86/kernel/uprobes.c +2 -1

index 58e9b84..acf8b90 100644
@@ -986,7 +986,8 @@ arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr, struct pt_regs
 	return -1;
 }
 
-bool arch_uretprobe_is_alive(struct return_instance *ret, struct pt_regs *regs)
+bool arch_uretprobe_is_alive(struct return_instance *ret, enum rp_check ctx,
+				struct pt_regs *regs)
 {
 	return regs->sp <= ret->stack;
 }

include/linux/uprobes.h +6 -1

index 7ab6d2c..c0a5402 100644
@@ -102,6 +102,11 @@ struct return_instance {
 	struct return_instance	*next;		/* keep as stack */
 };
 
+enum rp_check {
+	RP_CHECK_CALL,
+	RP_CHECK_RET,
+};
+
 struct xol_area;
 
 struct uprobes_state {

@@ -138,7 +143,7 @@ extern bool arch_uprobe_xol_was_trapped(struct task_struct *tsk);
 extern int  arch_uprobe_exception_notify(struct notifier_block *self, unsigned long val, void *data);
 extern void arch_uprobe_abort_xol(struct arch_uprobe *aup, struct pt_regs *regs);
 extern unsigned long arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr, struct pt_regs *regs);
-extern bool arch_uretprobe_is_alive(struct return_instance *ret, struct pt_regs *regs);
+extern bool arch_uretprobe_is_alive(struct return_instance *ret, enum rp_check ctx, struct pt_regs *regs);
 extern bool arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs);
 extern void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
 					 void *src, unsigned long len);

kernel/events/uprobes.c +6 -3

index 7e61c8c..df5661a 100644
@@ -1514,7 +1514,9 @@ static unsigned long get_trampoline_vaddr(void)
 static void cleanup_return_instances(struct uprobe_task *utask, struct pt_regs *regs)
 {
 	struct return_instance *ri = utask->return_instances;
-	while (ri && !arch_uretprobe_is_alive(ri, regs)) {
+	enum rp_check ctx = RP_CHECK_CALL;
+
+	while (ri && !arch_uretprobe_is_alive(ri, ctx, regs)) {
 		ri = free_ret_instance(ri);
 		utask->depth--;
 	}

@@ -1805,7 +1807,7 @@ static void handle_trampoline(struct pt_regs *regs)
 		 * could hit this trampoline on return. TODO: sigaltstack().
 		 */
 		next = find_next_ret_chain(ri);
-		valid = !next || arch_uretprobe_is_alive(next, regs);
+		valid = !next || arch_uretprobe_is_alive(next, RP_CHECK_RET, regs);
 
 		instruction_pointer_set(regs, ri->orig_ret_vaddr);
 		do {

@@ -1830,7 +1832,8 @@ bool __weak arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs)
 	return false;
 }
 
-bool __weak arch_uretprobe_is_alive(struct return_instance *ret, struct pt_regs *regs)
+bool __weak arch_uretprobe_is_alive(struct return_instance *ret, enum rp_check ctx,
+					struct pt_regs *regs)
 {
 	return true;
 }

Signature · 1 line

1.5.5.1

tip-bot for Oleg Nesterov

merge bot

2015-07-31 14:23 UTC #20 lore raw

[tip:perf/core] uprobes: Add the "enum rp_check ctx" arg to arch_uretprobe_is_alive()

uprobes: Add the "enum rp_check ctx" arg to arch_uretprobe_is_alive()

Commit-ID:  86dcb702e74b8ab7d3b2d36984ef00671cea73b9
Gitweb:     http://git.kernel.org/tip/86dcb702e74b8ab7d3b2d36984ef00671cea73b9
Author:     Oleg Nesterov <[email protected]>
AuthorDate: Tue, 21 Jul 2015 15:40:26 +0200
Committer:  Ingo Molnar <[email protected]>
CommitDate: Fri, 31 Jul 2015 10:38:06 +0200

uprobes: Add the "enum rp_check ctx" arg to arch_uretprobe_is_alive()

Tested-by Pratyush Anand <[email protected]>

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Acked-by Anton Arapov <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/x86/kernel/uprobes.c | 3 ++-
 include/linux/uprobes.h   | 7 ++++++-
 kernel/events/uprobes.c   | 9 ++++++---
 3 files changed, 14 insertions(+), 5 deletions(-)

arch/x86/kernel/uprobes.c +2 -1

index 58e9b84..acf8b90 100644
@@ -986,7 +986,8 @@ arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr, struct pt_regs
 	return -1;
 }
 
-bool arch_uretprobe_is_alive(struct return_instance *ret, struct pt_regs *regs)
+bool arch_uretprobe_is_alive(struct return_instance *ret, enum rp_check ctx,
+				struct pt_regs *regs)
 {
 	return regs->sp <= ret->stack;
 }

include/linux/uprobes.h +6 -1

index 7ab6d2c..c0a5402 100644
@@ -102,6 +102,11 @@ struct return_instance {
 	struct return_instance	*next;		/* keep as stack */
 };
 
+enum rp_check {
+	RP_CHECK_CALL,
+	RP_CHECK_RET,
+};
+
 struct xol_area;
 
 struct uprobes_state {

@@ -138,7 +143,7 @@ extern bool arch_uprobe_xol_was_trapped(struct task_struct *tsk);
 extern int  arch_uprobe_exception_notify(struct notifier_block *self, unsigned long val, void *data);
 extern void arch_uprobe_abort_xol(struct arch_uprobe *aup, struct pt_regs *regs);
 extern unsigned long arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr, struct pt_regs *regs);
-extern bool arch_uretprobe_is_alive(struct return_instance *ret, struct pt_regs *regs);
+extern bool arch_uretprobe_is_alive(struct return_instance *ret, enum rp_check ctx, struct pt_regs *regs);
 extern bool arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs);
 extern void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
 					 void *src, unsigned long len);

kernel/events/uprobes.c +6 -3

index 7e61c8c..df5661a 100644
@@ -1514,7 +1514,9 @@ static unsigned long get_trampoline_vaddr(void)
 static void cleanup_return_instances(struct uprobe_task *utask, struct pt_regs *regs)
 {
 	struct return_instance *ri = utask->return_instances;
-	while (ri && !arch_uretprobe_is_alive(ri, regs)) {
+	enum rp_check ctx = RP_CHECK_CALL;
+
+	while (ri && !arch_uretprobe_is_alive(ri, ctx, regs)) {
 		ri = free_ret_instance(ri);
 		utask->depth--;
 	}

@@ -1805,7 +1807,7 @@ static void handle_trampoline(struct pt_regs *regs)
 		 * could hit this trampoline on return. TODO: sigaltstack().
 		 */
 		next = find_next_ret_chain(ri);
-		valid = !next || arch_uretprobe_is_alive(next, regs);
+		valid = !next || arch_uretprobe_is_alive(next, RP_CHECK_RET, regs);
 
 		instruction_pointer_set(regs, ri->orig_ret_vaddr);
 		do {

@@ -1830,7 +1832,8 @@ bool __weak arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs)
 	return false;
 }
 
-bool __weak arch_uretprobe_is_alive(struct return_instance *ret, struct pt_regs *regs)
+bool __weak arch_uretprobe_is_alive(struct return_instance *ret, enum rp_check ctx,
+					struct pt_regs *regs)
 {
 	return true;
 }

Oleg Nesterov

author maintainer

2015-07-21 13:42 UTC #21 lore raw

PATCH v3 11/14 uprobes/x86: Make arch_uretprobe_is_alive(RP_CHECK_CALL) more clever

In reply to Oleg Nesterov

The previous change documents that cleanup_return_instances() can't always detect the dead frames, the stack can grow. But there is one special case which imho worth fixing: arch_uretprobe_is_alive() can return true when the stack didn't actually grow, but the next "call" insn uses the already invalidated frame.

Test-case:

	#include <stdio.h>
	#include <setjmp.h>

	jmp_buf jmp;
	int nr = 1024;

	void func_2(void)
	{
		if (--nr == 0)
			return;
		longjmp(jmp, 1);
	}

	void func_1(void)
	{
		setjmp(jmp);
		func_2();
	}

	int main(void)
	{
		func_1();
		return 0;
	}

If you ret-probe func_1() and func_2() prepare_uretprobe() hits the MAX_URETPROBE_DEPTH limit and "return" from func_2() is not reported.

When we know that the new call is not chained, we can do the more strict check. In this case "sp" points to the new ret-addr, so every frame which uses the same "sp" must be dead. The only complication is that arch_uretprobe_is_alive() needs to know was it chained or not, so we add the new RP_CHECK_CHAIN_CALL enum and change prepare_uretprobe() to pass RP_CHECK_CALL only if !chained.

Note: arch_uretprobe_is_alive() could also re-read *sp and check if this word is still trampoline_vaddr. This could obviously improve the logic, but I would like to avoid another copy_from_user() especially in the case when we can't avoid the false "alive == T" positives.

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Acked-by Anton Arapov <[email protected]>

Tested-by Pratyush Anand <[email protected]>

---
 arch/x86/kernel/uprobes.c |    5 ++++-
 include/linux/uprobes.h   |    1 +
 kernel/events/uprobes.c   |   14 +++++++-------
 3 files changed, 12 insertions(+), 8 deletions(-)

arch/x86/kernel/uprobes.c +4 -1

index acf8b90..bf4db6e 100644
@@ -989,5 +989,8 @@ arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr, struct pt_regs
 bool arch_uretprobe_is_alive(struct return_instance *ret, enum rp_check ctx,
 				struct pt_regs *regs)
 {
-	return regs->sp <= ret->stack;
+	if (ctx == RP_CHECK_CALL) /* sp was just decremented by "call" insn */
+		return regs->sp < ret->stack;
+	else
+		return regs->sp <= ret->stack;
 }

include/linux/uprobes.h +1 -0

index c0a5402..0bdc72f 100644
@@ -104,6 +104,7 @@ struct return_instance {
 
 enum rp_check {
 	RP_CHECK_CALL,
+	RP_CHECK_CHAIN_CALL,
 	RP_CHECK_RET,
 };

kernel/events/uprobes.c +7 -7

index df5661a..0f370ef 100644
@@ -1511,10 +1511,11 @@ static unsigned long get_trampoline_vaddr(void)
 	return trampoline_vaddr;
 }
 
-static void cleanup_return_instances(struct uprobe_task *utask, struct pt_regs *regs)
+static void cleanup_return_instances(struct uprobe_task *utask, bool chained,
+					struct pt_regs *regs)
 {
 	struct return_instance *ri = utask->return_instances;
-	enum rp_check ctx = RP_CHECK_CALL;
+	enum rp_check ctx = chained ? RP_CHECK_CHAIN_CALL : RP_CHECK_CALL;
 
 	while (ri && !arch_uretprobe_is_alive(ri, ctx, regs)) {
 		ri = free_ret_instance(ri);

@@ -1528,7 +1529,7 @@ static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
 	struct return_instance *ri;
 	struct uprobe_task *utask;
 	unsigned long orig_ret_vaddr, trampoline_vaddr;
-	bool chained = false;
+	bool chained;
 
 	if (!get_xol_area())
 		return;

@@ -1554,14 +1555,15 @@ static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
 		goto fail;
 
 	/* drop the entries invalidated by longjmp() */
-	cleanup_return_instances(utask, regs);
+	chained = (orig_ret_vaddr == trampoline_vaddr);
+	cleanup_return_instances(utask, chained, regs);
 
 	/*
 	 * We don't want to keep trampoline address in stack, rather keep the
 	 * original return address of first caller thru all the consequent
 	 * instances. This also makes breakpoint unwrapping easier.
 	 */
-	if (orig_ret_vaddr == trampoline_vaddr) {
+	if (chained) {
 		if (!utask->return_instances) {
 			/*
 			 * This situation is not possible. Likely we have an

@@ -1570,8 +1572,6 @@ static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
 			uprobe_warn(current, "handle tail call");
 			goto fail;
 		}
-
-		chained = true;
 		orig_ret_vaddr = utask->return_instances->orig_ret_vaddr;
 	}

Signature · 1 line

1.5.5.1

tip-bot for Oleg Nesterov

merge bot

2015-07-31 14:22 UTC #22 lore raw

[tip:perf/core] uprobes/x86: Make arch_uretprobe_is_alive( RP_CHECK_CALL) more clever

uprobes/x86: Make arch_uretprobe_is_alive(RP_CHECK_CALL) more clever

Commit-ID:  db087ef69a2b155ae001665bf0b3806abde7ee34
Gitweb:     http://git.kernel.org/tip/db087ef69a2b155ae001665bf0b3806abde7ee34
Author:     Oleg Nesterov <[email protected]>
AuthorDate: Tue, 21 Jul 2015 15:40:28 +0200
Committer:  Ingo Molnar <[email protected]>
CommitDate: Fri, 31 Jul 2015 10:38:06 +0200

uprobes/x86: Make arch_uretprobe_is_alive(RP_CHECK_CALL) more clever

The previous change documents that cleanup_return_instances() can't always detect the dead frames, the stack can grow. But there is one special case which imho worth fixing:
arch_uretprobe_is_alive() can return true when the stack didn't actually grow, but the next "call" insn uses the already invalidated frame.

Test-case:

	#include <stdio.h>
	#include <setjmp.h>

	jmp_buf jmp;
	int nr = 1024;

	void func_2(void)
	{
		if (--nr == 0)
			return;
		longjmp(jmp, 1);
	}

	void func_1(void)
	{
		setjmp(jmp);
		func_2();
	}

	int main(void)
	{
		func_1();
		return 0;
	}

If you ret-probe func_1() and func_2() prepare_uretprobe() hits the MAX_URETPROBE_DEPTH limit and "return" from func_2() is not reported.

Tested-by Pratyush Anand <[email protected]>

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Acked-by Anton Arapov <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/x86/kernel/uprobes.c |  5 ++++-
 include/linux/uprobes.h   |  1 +
 kernel/events/uprobes.c   | 14 +++++++-------
 3 files changed, 12 insertions(+), 8 deletions(-)

arch/x86/kernel/uprobes.c +4 -1

index acf8b90..bf4db6e 100644
@@ -989,5 +989,8 @@ arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr, struct pt_regs
 bool arch_uretprobe_is_alive(struct return_instance *ret, enum rp_check ctx,
 				struct pt_regs *regs)
 {
-	return regs->sp <= ret->stack;
+	if (ctx == RP_CHECK_CALL) /* sp was just decremented by "call" insn */
+		return regs->sp < ret->stack;
+	else
+		return regs->sp <= ret->stack;
 }

include/linux/uprobes.h +1 -0

index c0a5402..0bdc72f 100644
@@ -104,6 +104,7 @@ struct return_instance {
 
 enum rp_check {
 	RP_CHECK_CALL,
+	RP_CHECK_CHAIN_CALL,
 	RP_CHECK_RET,
 };

kernel/events/uprobes.c +7 -7

index df5661a..0f370ef 100644
@@ -1511,10 +1511,11 @@ static unsigned long get_trampoline_vaddr(void)
 	return trampoline_vaddr;
 }
 
-static void cleanup_return_instances(struct uprobe_task *utask, struct pt_regs *regs)
+static void cleanup_return_instances(struct uprobe_task *utask, bool chained,
+					struct pt_regs *regs)
 {
 	struct return_instance *ri = utask->return_instances;
-	enum rp_check ctx = RP_CHECK_CALL;
+	enum rp_check ctx = chained ? RP_CHECK_CHAIN_CALL : RP_CHECK_CALL;
 
 	while (ri && !arch_uretprobe_is_alive(ri, ctx, regs)) {
 		ri = free_ret_instance(ri);

@@ -1528,7 +1529,7 @@ static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
 	struct return_instance *ri;
 	struct uprobe_task *utask;
 	unsigned long orig_ret_vaddr, trampoline_vaddr;
-	bool chained = false;
+	bool chained;
 
 	if (!get_xol_area())
 		return;

@@ -1554,14 +1555,15 @@ static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
 		goto fail;
 
 	/* drop the entries invalidated by longjmp() */
-	cleanup_return_instances(utask, regs);
+	chained = (orig_ret_vaddr == trampoline_vaddr);
+	cleanup_return_instances(utask, chained, regs);
 
 	/*
 	 * We don't want to keep trampoline address in stack, rather keep the
 	 * original return address of first caller thru all the consequent
 	 * instances. This also makes breakpoint unwrapping easier.
 	 */
-	if (orig_ret_vaddr == trampoline_vaddr) {
+	if (chained) {
 		if (!utask->return_instances) {
 			/*
 			 * This situation is not possible. Likely we have an

@@ -1570,8 +1572,6 @@ static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
 			uprobe_warn(current, "handle tail call");
 			goto fail;
 		}
-
-		chained = true;
 		orig_ret_vaddr = utask->return_instances->orig_ret_vaddr;
 	}

Oleg Nesterov

author maintainer

2015-07-21 13:43 UTC #23 lore raw

PATCH v3 12/14 uprobes: fix the usage of install_special_mapping()

In reply to Oleg Nesterov

install_special_mapping(pages) expects that "pages" is the zero- terminated array while xol_add_vma() passes &area->page, this means that special_mapping_fault() can wrongly use the next member in xol_area (vaddr) as "struct page *".

Fortunately, this area is not expandable so pgoff != 0 isn't possible (modulo bugs in special_mapping_vmops), but still this does not look good.

Signed-off-by Oleg Nesterov <[email protected]>

---
 kernel/events/uprobes.c |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

kernel/events/uprobes.c +9 -8

index 0f370ef..4b8ac5f 100644
@@ -99,7 +99,7 @@ struct xol_area {
 	wait_queue_head_t 	wq;		/* if all slots are busy */
 	atomic_t 		slot_count;	/* number of in-use slots */
 	unsigned long 		*bitmap;	/* 0 = free slot */
-	struct page 		*page;
+	struct page 		*pages[2];
 
 	/*
 	 * We keep the vma's vm_start rather than a pointer to the vma

@@ -1142,7 +1142,7 @@ static int xol_add_vma(struct mm_struct *mm, struct xol_area *area)
 	}
 
 	ret = install_special_mapping(mm, area->vaddr, PAGE_SIZE,
-				VM_EXEC|VM_MAYEXEC|VM_DONTCOPY|VM_IO, &area->page);
+				VM_EXEC|VM_MAYEXEC|VM_DONTCOPY|VM_IO, area->pages);
 	if (ret)
 		goto fail;

@@ -1168,21 +1168,22 @@ static struct xol_area *__create_xol_area(unsigned long vaddr)
 	if (!area->bitmap)
 		goto free_area;
 
-	area->page = alloc_page(GFP_HIGHUSER);
-	if (!area->page)
+	area->pages[0] = alloc_page(GFP_HIGHUSER);
+	if (!area->pages[0])
 		goto free_bitmap;
+	area->pages[1] = NULL;
 
 	area->vaddr = vaddr;
 	init_waitqueue_head(&area->wq);
 	/* Reserve the 1st slot for get_trampoline_vaddr() */
 	set_bit(0, area->bitmap);
 	atomic_set(&area->slot_count, 1);
-	copy_to_page(area->page, 0, &insn, UPROBE_SWBP_INSN_SIZE);
+	copy_to_page(area->pages[0], 0, &insn, UPROBE_SWBP_INSN_SIZE);
 
 	if (!xol_add_vma(mm, area))
 		return area;
 
-	__free_page(area->page);
+	__free_page(area->pages[0]);
  free_bitmap:
 	kfree(area->bitmap);
  free_area:

@@ -1220,7 +1221,7 @@ void uprobe_clear_state(struct mm_struct *mm)
 	if (!area)
 		return;
 
-	put_page(area->page);
+	put_page(area->pages[0]);
 	kfree(area->bitmap);
 	kfree(area);
 }

@@ -1289,7 +1290,7 @@ static unsigned long xol_get_insn_slot(struct uprobe *uprobe)
 	if (unlikely(!xol_vaddr))
 		return 0;
 
-	arch_uprobe_copy_ixol(area->page, xol_vaddr,
+	arch_uprobe_copy_ixol(area->pages[0], xol_vaddr,
 			      &uprobe->arch.ixol, sizeof(uprobe->arch.ixol));
 
 	return xol_vaddr;

Signature · 1 line

1.5.5.1

tip-bot for Oleg Nesterov

merge bot

2015-07-31 14:02 UTC #24 lore raw

[tip:perf/core] uprobes: Fix the usage of install_special_mapping ()

uprobes: Fix the usage of install_special_mapping()

Commit-ID:  f58bea2fec63db72f8050ade709358257e9102ab
Gitweb:     http://git.kernel.org/tip/f58bea2fec63db72f8050ade709358257e9102ab
Author:     Oleg Nesterov <[email protected]>
AuthorDate: Tue, 21 Jul 2015 15:40:31 +0200
Committer:  Ingo Molnar <[email protected]>
CommitDate: Fri, 31 Jul 2015 10:38:06 +0200

uprobes: Fix the usage of install_special_mapping()

Fortunately, this area is not expandable so pgoff != 0 isn't possible (modulo bugs in special_mapping_vmops), but still this does not look good.

Signed-off-by Oleg Nesterov <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Pratyush Anand <[email protected]>

Cc Srikar Dronamraju <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 kernel/events/uprobes.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

kernel/events/uprobes.c +9 -8

index 0f370ef..4b8ac5f 100644
@@ -99,7 +99,7 @@ struct xol_area {
 	wait_queue_head_t 	wq;		/* if all slots are busy */
 	atomic_t 		slot_count;	/* number of in-use slots */
 	unsigned long 		*bitmap;	/* 0 = free slot */
-	struct page 		*page;
+	struct page 		*pages[2];
 
 	/*
 	 * We keep the vma's vm_start rather than a pointer to the vma

@@ -1142,7 +1142,7 @@ static int xol_add_vma(struct mm_struct *mm, struct xol_area *area)
 	}
 
 	ret = install_special_mapping(mm, area->vaddr, PAGE_SIZE,
-				VM_EXEC|VM_MAYEXEC|VM_DONTCOPY|VM_IO, &area->page);
+				VM_EXEC|VM_MAYEXEC|VM_DONTCOPY|VM_IO, area->pages);
 	if (ret)
 		goto fail;

@@ -1168,21 +1168,22 @@ static struct xol_area *__create_xol_area(unsigned long vaddr)
 	if (!area->bitmap)
 		goto free_area;
 
-	area->page = alloc_page(GFP_HIGHUSER);
-	if (!area->page)
+	area->pages[0] = alloc_page(GFP_HIGHUSER);
+	if (!area->pages[0])
 		goto free_bitmap;
+	area->pages[1] = NULL;
 
 	area->vaddr = vaddr;
 	init_waitqueue_head(&area->wq);
 	/* Reserve the 1st slot for get_trampoline_vaddr() */
 	set_bit(0, area->bitmap);
 	atomic_set(&area->slot_count, 1);
-	copy_to_page(area->page, 0, &insn, UPROBE_SWBP_INSN_SIZE);
+	copy_to_page(area->pages[0], 0, &insn, UPROBE_SWBP_INSN_SIZE);
 
 	if (!xol_add_vma(mm, area))
 		return area;
 
-	__free_page(area->page);
+	__free_page(area->pages[0]);
  free_bitmap:
 	kfree(area->bitmap);
  free_area:

@@ -1220,7 +1221,7 @@ void uprobe_clear_state(struct mm_struct *mm)
 	if (!area)
 		return;
 
-	put_page(area->page);
+	put_page(area->pages[0]);
 	kfree(area->bitmap);
 	kfree(area);
 }

@@ -1289,7 +1290,7 @@ static unsigned long xol_get_insn_slot(struct uprobe *uprobe)
 	if (unlikely(!xol_vaddr))
 		return 0;
 
-	arch_uprobe_copy_ixol(area->page, xol_vaddr,
+	arch_uprobe_copy_ixol(area->pages[0], xol_vaddr,
 			      &uprobe->arch.ixol, sizeof(uprobe->arch.ixol));
 
 	return xol_vaddr;

Oleg Nesterov

author maintainer

2015-07-21 13:42 UTC #25 lore raw

PATCH v3 13/14 uprobes: use vm_special_mapping to name the xol vma

In reply to Oleg Nesterov

Change xol_add_vma() to use _install_special_mapping(), this way we can name the vma installed by uprobes. Currently it looks like private anonymous mapping, this is confusing and complicates the debugging. With this change /proc/$pid/maps reports "[uprobes]".

As a side effect this will cause core dumps to include xol vma and I think this is good; this can help to debug the problem if the app crashed because it was probed.

Signed-off-by Oleg Nesterov <[email protected]>

---
 kernel/events/uprobes.c |   30 ++++++++++++++++++++----------
 1 files changed, 20 insertions(+), 10 deletions(-)

kernel/events/uprobes.c +20 ~~-10~~

index 4b8ac5f..2d5b7bd 100644
@@ -96,17 +96,18 @@ struct uprobe {
  * allocated.
  */
 struct xol_area {
-	wait_queue_head_t 	wq;		/* if all slots are busy */
-	atomic_t 		slot_count;	/* number of in-use slots */
-	unsigned long 		*bitmap;	/* 0 = free slot */
-	struct page 		*pages[2];
+	wait_queue_head_t 		wq;		/* if all slots are busy */
+	atomic_t 			slot_count;	/* number of in-use slots */
+	unsigned long 			*bitmap;	/* 0 = free slot */
 
+	struct vm_special_mapping	xol_mapping;
+	struct page 			*pages[2];
 	/*
 	 * We keep the vma's vm_start rather than a pointer to the vma
 	 * itself.  The probed process or a naughty kernel module could make
 	 * the vma go away, and we must handle that reasonably gracefully.
 	 */
-	unsigned long 		vaddr;		/* Page(s) of instruction slots */
+	unsigned long 			vaddr;		/* Page(s) of instruction slots */
 };
 
 /*

@@ -1125,11 +1126,14 @@ void uprobe_munmap(struct vm_area_struct *vma, unsigned long start, unsigned lon
 /* Slot allocation for XOL */
 static int xol_add_vma(struct mm_struct *mm, struct xol_area *area)
 {
-	int ret = -EALREADY;
+	struct vm_area_struct *vma;
+	int ret;
 
 	down_write(&mm->mmap_sem);
-	if (mm->uprobes_state.xol_area)
+	if (mm->uprobes_state.xol_area) {
+		ret = -EALREADY;
 		goto fail;
+	}
 
 	if (!area->vaddr) {
 		/* Try to map as high as possible, this is only a hint. */

@@ -1141,11 +1145,15 @@ static int xol_add_vma(struct mm_struct *mm, struct xol_area *area)
 		}
 	}
 
-	ret = install_special_mapping(mm, area->vaddr, PAGE_SIZE,
-				VM_EXEC|VM_MAYEXEC|VM_DONTCOPY|VM_IO, area->pages);
-	if (ret)
+	vma = _install_special_mapping(mm, area->vaddr, PAGE_SIZE,
+				VM_EXEC|VM_MAYEXEC|VM_DONTCOPY|VM_IO,
+				&area->xol_mapping);
+	if (IS_ERR(vma)) {
+		ret = PTR_ERR(vma);
 		goto fail;
+	}
 
+	ret = 0;
 	smp_wmb();	/* pairs with get_xol_area() */
 	mm->uprobes_state.xol_area = area;
  fail:

@@ -1168,6 +1176,8 @@ static struct xol_area *__create_xol_area(unsigned long vaddr)
 	if (!area->bitmap)
 		goto free_area;
 
+	area->xol_mapping.name = "[uprobes]";
+	area->xol_mapping.pages = area->pages;
 	area->pages[0] = alloc_page(GFP_HIGHUSER);
 	if (!area->pages[0])
 		goto free_bitmap;

Signature · 1 line

1.5.5.1

tip-bot for Oleg Nesterov

merge bot

2015-07-31 14:19 UTC #26 lore raw

[tip:perf/core] uprobes: Use vm_special_mapping to name the XOL vma

uprobes: Use vm_special_mapping to name the XOL vma

Commit-ID:  704bde3cc26a4cb34386c164107b59e09745a022
Gitweb:     http://git.kernel.org/tip/704bde3cc26a4cb34386c164107b59e09745a022
Author:     Oleg Nesterov <[email protected]>
AuthorDate: Tue, 21 Jul 2015 15:40:33 +0200
Committer:  Ingo Molnar <[email protected]>
CommitDate: Fri, 31 Jul 2015 10:38:06 +0200

uprobes: Use vm_special_mapping to name the XOL vma

As a side effect this will cause core dumps to include the XOL vma and I think this is good; this can help to debug the problem if the app crashed because it was probed.

Signed-off-by Oleg Nesterov <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Pratyush Anand <[email protected]>

Cc Srikar Dronamraju <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 kernel/events/uprobes.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

kernel/events/uprobes.c +20 ~~-10~~

index 4b8ac5f..2d5b7bd 100644
@@ -96,17 +96,18 @@ struct uprobe {
  * allocated.
  */
 struct xol_area {
-	wait_queue_head_t 	wq;		/* if all slots are busy */
-	atomic_t 		slot_count;	/* number of in-use slots */
-	unsigned long 		*bitmap;	/* 0 = free slot */
-	struct page 		*pages[2];
+	wait_queue_head_t 		wq;		/* if all slots are busy */
+	atomic_t 			slot_count;	/* number of in-use slots */
+	unsigned long 			*bitmap;	/* 0 = free slot */
 
+	struct vm_special_mapping	xol_mapping;
+	struct page 			*pages[2];
 	/*
 	 * We keep the vma's vm_start rather than a pointer to the vma
 	 * itself.  The probed process or a naughty kernel module could make
 	 * the vma go away, and we must handle that reasonably gracefully.
 	 */
-	unsigned long 		vaddr;		/* Page(s) of instruction slots */
+	unsigned long 			vaddr;		/* Page(s) of instruction slots */
 };
 
 /*

@@ -1125,11 +1126,14 @@ void uprobe_munmap(struct vm_area_struct *vma, unsigned long start, unsigned lon
 /* Slot allocation for XOL */
 static int xol_add_vma(struct mm_struct *mm, struct xol_area *area)
 {
-	int ret = -EALREADY;
+	struct vm_area_struct *vma;
+	int ret;
 
 	down_write(&mm->mmap_sem);
-	if (mm->uprobes_state.xol_area)
+	if (mm->uprobes_state.xol_area) {
+		ret = -EALREADY;
 		goto fail;
+	}
 
 	if (!area->vaddr) {
 		/* Try to map as high as possible, this is only a hint. */

@@ -1141,11 +1145,15 @@ static int xol_add_vma(struct mm_struct *mm, struct xol_area *area)
 		}
 	}
 
-	ret = install_special_mapping(mm, area->vaddr, PAGE_SIZE,
-				VM_EXEC|VM_MAYEXEC|VM_DONTCOPY|VM_IO, area->pages);
-	if (ret)
+	vma = _install_special_mapping(mm, area->vaddr, PAGE_SIZE,
+				VM_EXEC|VM_MAYEXEC|VM_DONTCOPY|VM_IO,
+				&area->xol_mapping);
+	if (IS_ERR(vma)) {
+		ret = PTR_ERR(vma);
 		goto fail;
+	}
 
+	ret = 0;
 	smp_wmb();	/* pairs with get_xol_area() */
 	mm->uprobes_state.xol_area = area;
  fail:

@@ -1168,6 +1176,8 @@ static struct xol_area *__create_xol_area(unsigned long vaddr)
 	if (!area->bitmap)
 		goto free_area;
 
+	area->xol_mapping.name = "[uprobes]";
+	area->xol_mapping.pages = area->pages;
 	area->pages[0] = alloc_page(GFP_HIGHUSER);
 	if (!area->pages[0])
 		goto free_bitmap;

Oleg Nesterov

author maintainer

2015-07-21 13:42 UTC #27 lore raw

PATCH v3 14/14 uprobes: fix the waitqueue_active() check in xol_free_insn_slot()

In reply to Oleg Nesterov

The xol_free_insn_slot()->waitqueue_active() check is buggy. We need mb() after we set the conditon for wait_event(), or xol_take_insn_slot() can miss the wakeup.

Signed-off-by Oleg Nesterov <[email protected]>

---
 kernel/events/uprobes.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

kernel/events/uprobes.c +1 -0

index 2d5b7bd..4e5e979 100644
@@ -1337,6 +1337,7 @@ static void xol_free_insn_slot(struct task_struct *tsk)
 
 		clear_bit(slot_nr, area->bitmap);
 		atomic_dec(&area->slot_count);
+		smp_mb__after_atomic(); /* pairs with prepare_to_wait() */
 		if (waitqueue_active(&area->wq))
 			wake_up(&area->wq);

Signature · 1 line

1.5.5.1

tip-bot for Oleg Nesterov

merge bot

2015-07-31 14:02 UTC #28 lore raw

[tip:perf/core] uprobes: Fix the waitqueue_active() check in xol_free_insn_slot()

uprobes: Fix the waitqueue_active() check in xol_free_insn_slot()

Commit-ID:  2a742cedcf13572999436676cbe36c3a9b733b0f
Gitweb:     http://git.kernel.org/tip/2a742cedcf13572999436676cbe36c3a9b733b0f
Author:     Oleg Nesterov <[email protected]>
AuthorDate: Tue, 21 Jul 2015 15:40:36 +0200
Committer:  Ingo Molnar <[email protected]>
CommitDate: Fri, 31 Jul 2015 10:38:07 +0200

uprobes: Fix the waitqueue_active() check in xol_free_insn_slot()

The xol_free_insn_slot()->waitqueue_active() check is buggy. We need mb() after we set the conditon for wait_event(), or xol_take_insn_slot() can miss the wakeup.

Signed-off-by Oleg Nesterov <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Pratyush Anand <[email protected]>

Cc Srikar Dronamraju <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 kernel/events/uprobes.c | 1 +
 1 file changed, 1 insertion(+)

kernel/events/uprobes.c +1 -0

index 2d5b7bd..4e5e979 100644
@@ -1337,6 +1337,7 @@ static void xol_free_insn_slot(struct task_struct *tsk)
 
 		clear_bit(slot_nr, area->bitmap);
 		atomic_dec(&area->slot_count);
+		smp_mb__after_atomic(); /* pairs with prepare_to_wait() */
 		if (waitqueue_active(&area->wq))
 			wake_up(&area->wq);