All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/14] uprobes: longjmp / special-mapping fixes
@ 2015-07-21 13:39 Oleg Nesterov
  2015-07-21 13:40 ` [PATCH v3 01/14] uprobes: Introduce get_uprobe() Oleg Nesterov
                   ` (13 more replies)
  0 siblings, 14 replies; 29+ messages in thread
From: Oleg Nesterov @ 2015-07-21 13:39 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Andy Lutomirski, Pratyush Anand, Srikar Dronamraju, linux-kernel

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(-)


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v3 01/14] uprobes: Introduce get_uprobe()
  2015-07-21 13:39 [PATCH v3 00/14] uprobes: longjmp / special-mapping fixes Oleg Nesterov
@ 2015-07-21 13:40 ` Oleg Nesterov
  2015-07-31 13:57   ` [tip:perf/core] " tip-bot for Oleg Nesterov
  2015-07-21 13:40 ` [PATCH v3 02/14] uprobes: Introduce free_ret_instance() Oleg Nesterov
                   ` (12 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Oleg Nesterov @ 2015-07-21 13:40 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Andy Lutomirski, Pratyush Anand, Srikar Dronamraju, linux-kernel

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 <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Anton Arapov <arapov@gmail.com>
Tested-by: Pratyush Anand <panand@redhat.com>
---
 kernel/events/uprobes.c |   39 ++++++++++++++++++++-------------------
 1 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index cb346f2..a9847b4 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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;
-- 
1.5.5.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 02/14] uprobes: Introduce free_ret_instance()
  2015-07-21 13:39 [PATCH v3 00/14] uprobes: longjmp / special-mapping fixes Oleg Nesterov
  2015-07-21 13:40 ` [PATCH v3 01/14] uprobes: Introduce get_uprobe() Oleg Nesterov
@ 2015-07-21 13:40 ` Oleg Nesterov
  2015-07-31 13:58   ` [tip:perf/core] " tip-bot for Oleg Nesterov
  2015-07-21 13:40 ` [PATCH v3 03/14] uprobes: Send SIGILL if handle_trampoline() fails Oleg Nesterov
                   ` (11 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Oleg Nesterov @ 2015-07-21 13:40 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Andy Lutomirski, Pratyush Anand, Srikar Dronamraju, linux-kernel

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 <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Anton Arapov <arapov@gmail.com>
Tested-by: Pratyush Anand <panand@redhat.com>
---
 kernel/events/uprobes.c |   27 +++++++++++++--------------
 1 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index a9847b4..d8c702f 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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)
-- 
1.5.5.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 03/14] uprobes: Send SIGILL if handle_trampoline() fails
  2015-07-21 13:39 [PATCH v3 00/14] uprobes: longjmp / special-mapping fixes Oleg Nesterov
  2015-07-21 13:40 ` [PATCH v3 01/14] uprobes: Introduce get_uprobe() Oleg Nesterov
  2015-07-21 13:40 ` [PATCH v3 02/14] uprobes: Introduce free_ret_instance() Oleg Nesterov
@ 2015-07-21 13:40 ` Oleg Nesterov
  2015-07-31 13:58   ` [tip:perf/core] " tip-bot for Oleg Nesterov
  2015-07-21 13:40 ` [PATCH v3 04/14] uprobes: Change prepare_uretprobe() to use uprobe_warn() Oleg Nesterov
                   ` (10 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Oleg Nesterov @ 2015-07-21 13:40 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Andy Lutomirski, Pratyush Anand, Srikar Dronamraju, linux-kernel

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 <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Anton Arapov <arapov@gmail.com>
Tested-by: Pratyush Anand <panand@redhat.com>
---
 kernel/events/uprobes.c |   21 ++++++++++-----------
 1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index d8c702f..eabdc21 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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) {
-- 
1.5.5.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 04/14] uprobes: Change prepare_uretprobe() to use uprobe_warn()
  2015-07-21 13:39 [PATCH v3 00/14] uprobes: longjmp / special-mapping fixes Oleg Nesterov
                   ` (2 preceding siblings ...)
  2015-07-21 13:40 ` [PATCH v3 03/14] uprobes: Send SIGILL if handle_trampoline() fails Oleg Nesterov
@ 2015-07-21 13:40 ` Oleg Nesterov
  2015-07-31 13:58   ` [tip:perf/core] " tip-bot for Oleg Nesterov
  2015-07-21 13:40 ` [PATCH v3 05/14] uprobes: Change handle_trampoline() to find the next chain beforehand Oleg Nesterov
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Oleg Nesterov @ 2015-07-21 13:40 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Andy Lutomirski, Pratyush Anand, Srikar Dronamraju, linux-kernel

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 <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Anton Arapov <arapov@gmail.com>
Tested-by: Pratyush Anand <panand@redhat.com>
---
 kernel/events/uprobes.c |   10 +++-------
 1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index eabdc21..4c941fe 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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);
 }
-- 
1.5.5.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 05/14] uprobes: Change handle_trampoline() to find the next chain beforehand
  2015-07-21 13:39 [PATCH v3 00/14] uprobes: longjmp / special-mapping fixes Oleg Nesterov
                   ` (3 preceding siblings ...)
  2015-07-21 13:40 ` [PATCH v3 04/14] uprobes: Change prepare_uretprobe() to use uprobe_warn() Oleg Nesterov
@ 2015-07-21 13:40 ` Oleg Nesterov
  2015-07-31 13:59   ` [tip:perf/core] " tip-bot for Oleg Nesterov
  2015-07-21 13:40 ` [PATCH v3 06/14] uprobes: Export struct return_instance, introduce arch_uretprobe_is_alive() Oleg Nesterov
                   ` (8 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Oleg Nesterov @ 2015-07-21 13:40 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Andy Lutomirski, Pratyush Anand, Srikar Dronamraju, linux-kernel

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 <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Anton Arapov <arapov@gmail.com>
Tested-by: Pratyush Anand <panand@redhat.com>
---
 kernel/events/uprobes.c |   27 ++++++++++++++++-----------
 1 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 4c941fe..98e4d97 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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;
-- 
1.5.5.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 06/14] uprobes: Export struct return_instance, introduce arch_uretprobe_is_alive()
  2015-07-21 13:39 [PATCH v3 00/14] uprobes: longjmp / special-mapping fixes Oleg Nesterov
                   ` (4 preceding siblings ...)
  2015-07-21 13:40 ` [PATCH v3 05/14] uprobes: Change handle_trampoline() to find the next chain beforehand Oleg Nesterov
@ 2015-07-21 13:40 ` Oleg Nesterov
  2015-07-31 13:59   ` [tip:perf/core] uprobes: Export 'struct return_instance', " tip-bot for Oleg Nesterov
  2015-07-21 13:40 ` [PATCH v3 07/14] uprobes/x86: Reimplement arch_uretprobe_is_alive() Oleg Nesterov
                   ` (7 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Oleg Nesterov @ 2015-07-21 13:40 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Andy Lutomirski, Pratyush Anand, Srikar Dronamraju, linux-kernel

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 <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Anton Arapov <arapov@gmail.com>
Tested-by: Pratyush Anand <panand@redhat.com>
---
 include/linux/uprobes.h |   10 ++++++++++
 kernel/events/uprobes.c |   14 +++++---------
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index 60beb5d..50d2764 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -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);
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 98e4d97..1c71b62 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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.
-- 
1.5.5.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 07/14] uprobes/x86: Reimplement arch_uretprobe_is_alive()
  2015-07-21 13:39 [PATCH v3 00/14] uprobes: longjmp / special-mapping fixes Oleg Nesterov
                   ` (5 preceding siblings ...)
  2015-07-21 13:40 ` [PATCH v3 06/14] uprobes: Export struct return_instance, introduce arch_uretprobe_is_alive() Oleg Nesterov
@ 2015-07-21 13:40 ` Oleg Nesterov
  2015-07-31 13:59   ` [tip:perf/core] uprobes/x86: Reimplement arch_uretprobe_is_alive( ) tip-bot for Oleg Nesterov
  2015-07-21 13:40 ` [PATCH v3 08/14] uprobes: Change handle_trampoline() to flush the frames invalidated by longjmp() Oleg Nesterov
                   ` (6 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Oleg Nesterov @ 2015-07-21 13:40 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Andy Lutomirski, Pratyush Anand, Srikar Dronamraju, linux-kernel

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 <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Anton Arapov <arapov@gmail.com>
Tested-by: Pratyush Anand <panand@redhat.com>
---
 arch/x86/kernel/uprobes.c |    5 +++++
 include/linux/uprobes.h   |    1 +
 kernel/events/uprobes.c   |    1 +
 3 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index 6647624..58e9b84 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -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;
+}
diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index 50d2764..7ab6d2c 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -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 */
 
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 1c71b62..c5f316e 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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;
 
-- 
1.5.5.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 08/14] uprobes: Change handle_trampoline() to flush the frames invalidated by longjmp()
  2015-07-21 13:39 [PATCH v3 00/14] uprobes: longjmp / special-mapping fixes Oleg Nesterov
                   ` (6 preceding siblings ...)
  2015-07-21 13:40 ` [PATCH v3 07/14] uprobes/x86: Reimplement arch_uretprobe_is_alive() Oleg Nesterov
@ 2015-07-21 13:40 ` Oleg Nesterov
  2015-07-31 14:00   ` [tip:perf/core] " tip-bot for Oleg Nesterov
  2015-07-21 13:40 ` [PATCH v3 09/14] uprobes: Change prepare_uretprobe() to (try to) flush the dead frames Oleg Nesterov
                   ` (5 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Oleg Nesterov @ 2015-07-21 13:40 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Andy Lutomirski, Pratyush Anand, Srikar Dronamraju, linux-kernel

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 <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Anton Arapov <arapov@gmail.com>
Tested-by: Pratyush Anand <panand@redhat.com>
---
 kernel/events/uprobes.c |   29 ++++++++++++++++++-----------
 1 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index c5f316e..93d939c 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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;
-- 
1.5.5.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 09/14] uprobes: Change prepare_uretprobe() to (try to) flush the dead frames
  2015-07-21 13:39 [PATCH v3 00/14] uprobes: longjmp / special-mapping fixes Oleg Nesterov
                   ` (7 preceding siblings ...)
  2015-07-21 13:40 ` [PATCH v3 08/14] uprobes: Change handle_trampoline() to flush the frames invalidated by longjmp() Oleg Nesterov
@ 2015-07-21 13:40 ` Oleg Nesterov
  2015-07-31 14:00   ` [tip:perf/core] " tip-bot for Oleg Nesterov
  2015-07-21 13:40 ` [PATCH v3 10/14] uprobes: Add the "enum rp_check ctx" arg to arch_uretprobe_is_alive() Oleg Nesterov
                   ` (4 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Oleg Nesterov @ 2015-07-21 13:40 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Andy Lutomirski, Pratyush Anand, Srikar Dronamraju, linux-kernel

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 <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Anton Arapov <arapov@gmail.com>
Tested-by: Pratyush Anand <panand@redhat.com>
---
 kernel/events/uprobes.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 93d939c..7e61c8c 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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
-- 
1.5.5.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 10/14] uprobes: Add the "enum rp_check ctx" arg to arch_uretprobe_is_alive()
  2015-07-21 13:39 [PATCH v3 00/14] uprobes: longjmp / special-mapping fixes Oleg Nesterov
                   ` (8 preceding siblings ...)
  2015-07-21 13:40 ` [PATCH v3 09/14] uprobes: Change prepare_uretprobe() to (try to) flush the dead frames Oleg Nesterov
@ 2015-07-21 13:40 ` Oleg Nesterov
  2015-07-31 14:00   ` [tip:perf/core] " tip-bot for Oleg Nesterov
  2015-07-21 13:40 ` [PATCH v3 11/14] uprobes/x86: Make arch_uretprobe_is_alive(RP_CHECK_CALL) more clever Oleg Nesterov
                   ` (3 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Oleg Nesterov @ 2015-07-21 13:40 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Andy Lutomirski, Pratyush Anand, Srikar Dronamraju, linux-kernel

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 <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Anton Arapov <arapov@gmail.com>
Tested-by: Pratyush Anand <panand@redhat.com>
---
 arch/x86/kernel/uprobes.c |    3 ++-
 include/linux/uprobes.h   |    7 ++++++-
 kernel/events/uprobes.c   |    9 ++++++---
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index 58e9b84..acf8b90 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -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;
 }
diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index 7ab6d2c..c0a5402 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -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);
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 7e61c8c..df5661a 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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;
 }
-- 
1.5.5.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 11/14] uprobes/x86: Make arch_uretprobe_is_alive(RP_CHECK_CALL) more clever
  2015-07-21 13:39 [PATCH v3 00/14] uprobes: longjmp / special-mapping fixes Oleg Nesterov
                   ` (9 preceding siblings ...)
  2015-07-21 13:40 ` [PATCH v3 10/14] uprobes: Add the "enum rp_check ctx" arg to arch_uretprobe_is_alive() Oleg Nesterov
@ 2015-07-21 13:40 ` Oleg Nesterov
  2015-07-31 14:01   ` [tip:perf/core] uprobes/x86: Make arch_uretprobe_is_alive( RP_CHECK_CALL) " tip-bot for Oleg Nesterov
  2015-07-21 13:40 ` [PATCH v3 12/14] uprobes: fix the usage of install_special_mapping() Oleg Nesterov
                   ` (2 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Oleg Nesterov @ 2015-07-21 13:40 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Andy Lutomirski, Pratyush Anand, Srikar Dronamraju, linux-kernel

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 <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Anton Arapov <arapov@gmail.com>
Tested-by: Pratyush Anand <panand@redhat.com>
---
 arch/x86/kernel/uprobes.c |    5 ++++-
 include/linux/uprobes.h   |    1 +
 kernel/events/uprobes.c   |   14 +++++++-------
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index acf8b90..bf4db6e 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -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;
 }
diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index c0a5402..0bdc72f 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -104,6 +104,7 @@ struct return_instance {
 
 enum rp_check {
 	RP_CHECK_CALL,
+	RP_CHECK_CHAIN_CALL,
 	RP_CHECK_RET,
 };
 
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index df5661a..0f370ef 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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;
 	}
 
-- 
1.5.5.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 12/14] uprobes: fix the usage of install_special_mapping()
  2015-07-21 13:39 [PATCH v3 00/14] uprobes: longjmp / special-mapping fixes Oleg Nesterov
                   ` (10 preceding siblings ...)
  2015-07-21 13:40 ` [PATCH v3 11/14] uprobes/x86: Make arch_uretprobe_is_alive(RP_CHECK_CALL) more clever Oleg Nesterov
@ 2015-07-21 13:40 ` Oleg Nesterov
  2015-07-31 14:01   ` [tip:perf/core] uprobes: Fix the usage of install_special_mapping () tip-bot for Oleg Nesterov
  2015-07-21 13:40 ` [PATCH v3 13/14] uprobes: use vm_special_mapping to name the xol vma Oleg Nesterov
  2015-07-21 13:40 ` [PATCH v3 14/14] uprobes: fix the waitqueue_active() check in xol_free_insn_slot() Oleg Nesterov
  13 siblings, 1 reply; 29+ messages in thread
From: Oleg Nesterov @ 2015-07-21 13:40 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Andy Lutomirski, Pratyush Anand, Srikar Dronamraju, linux-kernel

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 <oleg@redhat.com>
---
 kernel/events/uprobes.c |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 0f370ef..4b8ac5f 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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;
-- 
1.5.5.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 13/14] uprobes: use vm_special_mapping to name the xol vma
  2015-07-21 13:39 [PATCH v3 00/14] uprobes: longjmp / special-mapping fixes Oleg Nesterov
                   ` (11 preceding siblings ...)
  2015-07-21 13:40 ` [PATCH v3 12/14] uprobes: fix the usage of install_special_mapping() Oleg Nesterov
@ 2015-07-21 13:40 ` Oleg Nesterov
  2015-07-31 14:01   ` [tip:perf/core] uprobes: Use vm_special_mapping to name the XOL vma tip-bot for Oleg Nesterov
  2015-07-21 13:40 ` [PATCH v3 14/14] uprobes: fix the waitqueue_active() check in xol_free_insn_slot() Oleg Nesterov
  13 siblings, 1 reply; 29+ messages in thread
From: Oleg Nesterov @ 2015-07-21 13:40 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Andy Lutomirski, Pratyush Anand, Srikar Dronamraju, linux-kernel

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 <oleg@redhat.com>
---
 kernel/events/uprobes.c |   30 ++++++++++++++++++++----------
 1 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 4b8ac5f..2d5b7bd 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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;
-- 
1.5.5.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 14/14] uprobes: fix the waitqueue_active() check in xol_free_insn_slot()
  2015-07-21 13:39 [PATCH v3 00/14] uprobes: longjmp / special-mapping fixes Oleg Nesterov
                   ` (12 preceding siblings ...)
  2015-07-21 13:40 ` [PATCH v3 13/14] uprobes: use vm_special_mapping to name the xol vma Oleg Nesterov
@ 2015-07-21 13:40 ` Oleg Nesterov
  2015-07-31 14:02   ` [tip:perf/core] uprobes: Fix " tip-bot for Oleg Nesterov
  13 siblings, 1 reply; 29+ messages in thread
From: Oleg Nesterov @ 2015-07-21 13:40 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Andy Lutomirski, Pratyush Anand, Srikar Dronamraju, linux-kernel

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 <oleg@redhat.com>
---
 kernel/events/uprobes.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 2d5b7bd..4e5e979 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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);
 
-- 
1.5.5.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [tip:perf/core] uprobes: Introduce get_uprobe()
  2015-07-21 13:40 ` [PATCH v3 01/14] uprobes: Introduce get_uprobe() Oleg Nesterov
@ 2015-07-31 13:57   ` tip-bot for Oleg Nesterov
  0 siblings, 0 replies; 29+ messages in thread
From: tip-bot for Oleg Nesterov @ 2015-07-31 13:57 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: torvalds, hpa, srikar, arapov, linux-kernel, tglx, luto, mingo,
	panand, peterz, oleg

Commit-ID:  f231722a2b27ee99cbcd0c6bcf4c866612b78137
Gitweb:     http://git.kernel.org/tip/f231722a2b27ee99cbcd0c6bcf4c866612b78137
Author:     Oleg Nesterov <oleg@redhat.com>
AuthorDate: Tue, 21 Jul 2015 15:40:03 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
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 <panand@redhat.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Anton Arapov <arapov@gmail.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20150721134003.GA4736@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/uprobes.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index cb346f2..a9847b4 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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;

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [tip:perf/core] uprobes: Introduce free_ret_instance()
  2015-07-21 13:40 ` [PATCH v3 02/14] uprobes: Introduce free_ret_instance() Oleg Nesterov
@ 2015-07-31 13:58   ` tip-bot for Oleg Nesterov
  0 siblings, 0 replies; 29+ messages in thread
From: tip-bot for Oleg Nesterov @ 2015-07-31 13:58 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: luto, arapov, tglx, hpa, peterz, torvalds, oleg, srikar, mingo,
	linux-kernel, panand

Commit-ID:  2bb5e840e873f8778a41801141771f54f547fa65
Gitweb:     http://git.kernel.org/tip/2bb5e840e873f8778a41801141771f54f547fa65
Author:     Oleg Nesterov <oleg@redhat.com>
AuthorDate: Tue, 21 Jul 2015 15:40:06 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
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 <panand@redhat.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Anton Arapov <arapov@gmail.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20150721134006.GA4740@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/uprobes.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index a9847b4..d8c702f 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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)

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [tip:perf/core] uprobes: Send SIGILL if handle_trampoline() fails
  2015-07-21 13:40 ` [PATCH v3 03/14] uprobes: Send SIGILL if handle_trampoline() fails Oleg Nesterov
@ 2015-07-31 13:58   ` tip-bot for Oleg Nesterov
  0 siblings, 0 replies; 29+ messages in thread
From: tip-bot for Oleg Nesterov @ 2015-07-31 13:58 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mingo, linux-kernel, panand, tglx, oleg, peterz, hpa, arapov,
	luto, srikar, torvalds

Commit-ID:  0b5256c7f173258b19d98364adb57f707dda22f3
Gitweb:     http://git.kernel.org/tip/0b5256c7f173258b19d98364adb57f707dda22f3
Author:     Oleg Nesterov <oleg@redhat.com>
AuthorDate: Tue, 21 Jul 2015 15:40:08 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
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.

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.

Tested-by: Pratyush Anand <panand@redhat.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Anton Arapov <arapov@gmail.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20150721134008.GA4745@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/uprobes.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index d8c702f..eabdc21 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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) {

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [tip:perf/core] uprobes: Change prepare_uretprobe() to use uprobe_warn()
  2015-07-21 13:40 ` [PATCH v3 04/14] uprobes: Change prepare_uretprobe() to use uprobe_warn() Oleg Nesterov
@ 2015-07-31 13:58   ` tip-bot for Oleg Nesterov
  0 siblings, 0 replies; 29+ messages in thread
From: tip-bot for Oleg Nesterov @ 2015-07-31 13:58 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: srikar, panand, mingo, tglx, arapov, torvalds, luto,
	linux-kernel, oleg, peterz, hpa

Commit-ID:  6c58d0e4cc26ea8882928e64c0de9afed4fc37cb
Gitweb:     http://git.kernel.org/tip/6c58d0e4cc26ea8882928e64c0de9afed4fc37cb
Author:     Oleg Nesterov <oleg@redhat.com>
AuthorDate: Tue, 21 Jul 2015 15:40:10 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
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 <panand@redhat.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Anton Arapov <arapov@gmail.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20150721134010.GA4752@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/uprobes.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index eabdc21..4c941fe 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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);
 }

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [tip:perf/core] uprobes: Change handle_trampoline() to find the next chain beforehand
  2015-07-21 13:40 ` [PATCH v3 05/14] uprobes: Change handle_trampoline() to find the next chain beforehand Oleg Nesterov
@ 2015-07-31 13:59   ` tip-bot for Oleg Nesterov
  0 siblings, 0 replies; 29+ messages in thread
From: tip-bot for Oleg Nesterov @ 2015-07-31 13:59 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: arapov, torvalds, linux-kernel, srikar, oleg, peterz, mingo,
	tglx, hpa, luto, panand

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

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

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.

Tested-by: Pratyush Anand <panand@redhat.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Anton Arapov <arapov@gmail.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20150721134013.GA4755@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/uprobes.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 4c941fe..98e4d97 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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;

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [tip:perf/core] uprobes: Export 'struct return_instance', introduce arch_uretprobe_is_alive()
  2015-07-21 13:40 ` [PATCH v3 06/14] uprobes: Export struct return_instance, introduce arch_uretprobe_is_alive() Oleg Nesterov
@ 2015-07-31 13:59   ` tip-bot for Oleg Nesterov
  0 siblings, 0 replies; 29+ messages in thread
From: tip-bot for Oleg Nesterov @ 2015-07-31 13:59 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: peterz, mingo, torvalds, linux-kernel, panand, arapov, hpa, luto,
	srikar, oleg, tglx

Commit-ID:  97da89767d398c1dfa1f34e5f312eb8ebb382f7f
Gitweb:     http://git.kernel.org/tip/97da89767d398c1dfa1f34e5f312eb8ebb382f7f
Author:     Oleg Nesterov <oleg@redhat.com>
AuthorDate: Tue, 21 Jul 2015 15:40:16 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
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.

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().

Tested-by: Pratyush Anand <panand@redhat.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Anton Arapov <arapov@gmail.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20150721134016.GA4762@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 include/linux/uprobes.h | 10 ++++++++++
 kernel/events/uprobes.c | 14 +++++---------
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index 60beb5d..50d2764 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -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);
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 98e4d97..1c71b62 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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.

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [tip:perf/core] uprobes/x86: Reimplement arch_uretprobe_is_alive( )
  2015-07-21 13:40 ` [PATCH v3 07/14] uprobes/x86: Reimplement arch_uretprobe_is_alive() Oleg Nesterov
@ 2015-07-31 13:59   ` tip-bot for Oleg Nesterov
  0 siblings, 0 replies; 29+ messages in thread
From: tip-bot for Oleg Nesterov @ 2015-07-31 13:59 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: oleg, panand, mingo, arapov, srikar, hpa, peterz, tglx, torvalds,
	luto, linux-kernel

Commit-ID:  7b868e4802a86d867aad1be0471b5767d9c20e10
Gitweb:     http://git.kernel.org/tip/7b868e4802a86d867aad1be0471b5767d9c20e10
Author:     Oleg Nesterov <oleg@redhat.com>
AuthorDate: Tue, 21 Jul 2015 15:40:18 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
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 <panand@redhat.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Anton Arapov <arapov@gmail.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20150721134018.GA4766@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/uprobes.c | 5 +++++
 include/linux/uprobes.h   | 1 +
 kernel/events/uprobes.c   | 1 +
 3 files changed, 7 insertions(+)

diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index 6647624..58e9b84 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -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;
+}
diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index 50d2764..7ab6d2c 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -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 */
 
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 1c71b62..c5f316e 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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;
 

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [tip:perf/core] uprobes: Change handle_trampoline() to flush the frames invalidated by longjmp()
  2015-07-21 13:40 ` [PATCH v3 08/14] uprobes: Change handle_trampoline() to flush the frames invalidated by longjmp() Oleg Nesterov
@ 2015-07-31 14:00   ` tip-bot for Oleg Nesterov
  0 siblings, 0 replies; 29+ messages in thread
From: tip-bot for Oleg Nesterov @ 2015-07-31 14:00 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: srikar, luto, linux-kernel, panand, hpa, peterz, torvalds, tglx,
	oleg, arapov, mingo

Commit-ID:  5eeb50de42fd3251845d03c556db012267c72b3f
Gitweb:     http://git.kernel.org/tip/5eeb50de42fd3251845d03c556db012267c72b3f
Author:     Oleg Nesterov <oleg@redhat.com>
AuthorDate: Tue, 21 Jul 2015 15:40:21 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
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;
	}

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.

Tested-by: Pratyush Anand <panand@redhat.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Anton Arapov <arapov@gmail.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20150721134021.GA4773@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/uprobes.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index c5f316e..93d939c8 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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;

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [tip:perf/core] uprobes: Change prepare_uretprobe() to (try to) flush the dead frames
  2015-07-21 13:40 ` [PATCH v3 09/14] uprobes: Change prepare_uretprobe() to (try to) flush the dead frames Oleg Nesterov
@ 2015-07-31 14:00   ` tip-bot for Oleg Nesterov
  0 siblings, 0 replies; 29+ messages in thread
From: tip-bot for Oleg Nesterov @ 2015-07-31 14:00 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: luto, mingo, panand, peterz, hpa, srikar, oleg, arapov, torvalds,
	tglx, linux-kernel

Commit-ID:  a5b7e1a89b820f2b9b23634ca4c59b555e8d9a0d
Gitweb:     http://git.kernel.org/tip/a5b7e1a89b820f2b9b23634ca4c59b555e8d9a0d
Author:     Oleg Nesterov <oleg@redhat.com>
AuthorDate: Tue, 21 Jul 2015 15:40:23 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
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 <panand@redhat.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Anton Arapov <arapov@gmail.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20150721134023.GA4776@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/uprobes.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 93d939c8..7e61c8c 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [tip:perf/core] uprobes: Add the "enum rp_check ctx" arg to arch_uretprobe_is_alive()
  2015-07-21 13:40 ` [PATCH v3 10/14] uprobes: Add the "enum rp_check ctx" arg to arch_uretprobe_is_alive() Oleg Nesterov
@ 2015-07-31 14:00   ` tip-bot for Oleg Nesterov
  0 siblings, 0 replies; 29+ messages in thread
From: tip-bot for Oleg Nesterov @ 2015-07-31 14:00 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: peterz, mingo, srikar, luto, tglx, panand, hpa, oleg,
	linux-kernel, arapov, torvalds

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

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

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.

Tested-by: Pratyush Anand <panand@redhat.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Anton Arapov <arapov@gmail.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20150721134026.GA4779@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/uprobes.c | 3 ++-
 include/linux/uprobes.h   | 7 ++++++-
 kernel/events/uprobes.c   | 9 ++++++---
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index 58e9b84..acf8b90 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -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;
 }
diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index 7ab6d2c..c0a5402 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -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);
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 7e61c8c..df5661a 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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;
 }

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [tip:perf/core] uprobes/x86: Make arch_uretprobe_is_alive( RP_CHECK_CALL) more clever
  2015-07-21 13:40 ` [PATCH v3 11/14] uprobes/x86: Make arch_uretprobe_is_alive(RP_CHECK_CALL) more clever Oleg Nesterov
@ 2015-07-31 14:01   ` tip-bot for Oleg Nesterov
  0 siblings, 0 replies; 29+ messages in thread
From: tip-bot for Oleg Nesterov @ 2015-07-31 14:01 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mingo, peterz, hpa, panand, luto, arapov, torvalds, linux-kernel,
	oleg, tglx, srikar

Commit-ID:  db087ef69a2b155ae001665bf0b3806abde7ee34
Gitweb:     http://git.kernel.org/tip/db087ef69a2b155ae001665bf0b3806abde7ee34
Author:     Oleg Nesterov <oleg@redhat.com>
AuthorDate: Tue, 21 Jul 2015 15:40:28 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
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.

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.

Tested-by: Pratyush Anand <panand@redhat.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Anton Arapov <arapov@gmail.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20150721134028.GA4786@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/uprobes.c |  5 ++++-
 include/linux/uprobes.h   |  1 +
 kernel/events/uprobes.c   | 14 +++++++-------
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index acf8b90..bf4db6e 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -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;
 }
diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index c0a5402..0bdc72f 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -104,6 +104,7 @@ struct return_instance {
 
 enum rp_check {
 	RP_CHECK_CALL,
+	RP_CHECK_CHAIN_CALL,
 	RP_CHECK_RET,
 };
 
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index df5661a..0f370ef 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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;
 	}
 

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [tip:perf/core] uprobes: Fix the usage of install_special_mapping ()
  2015-07-21 13:40 ` [PATCH v3 12/14] uprobes: fix the usage of install_special_mapping() Oleg Nesterov
@ 2015-07-31 14:01   ` tip-bot for Oleg Nesterov
  0 siblings, 0 replies; 29+ messages in thread
From: tip-bot for Oleg Nesterov @ 2015-07-31 14:01 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, luto, tglx, mingo, srikar, torvalds, hpa, oleg,
	peterz, panand

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

uprobes: Fix the usage of install_special_mapping()

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 <oleg@redhat.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Pratyush Anand <panand@redhat.com>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20150721134031.GA4789@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/uprobes.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 0f370ef..4b8ac5f 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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;

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [tip:perf/core] uprobes: Use vm_special_mapping to name the XOL vma
  2015-07-21 13:40 ` [PATCH v3 13/14] uprobes: use vm_special_mapping to name the xol vma Oleg Nesterov
@ 2015-07-31 14:01   ` tip-bot for Oleg Nesterov
  0 siblings, 0 replies; 29+ messages in thread
From: tip-bot for Oleg Nesterov @ 2015-07-31 14:01 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: torvalds, peterz, tglx, linux-kernel, luto, oleg, hpa, srikar,
	mingo, panand

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

uprobes: Use vm_special_mapping to name the XOL vma

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 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 <oleg@redhat.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Pratyush Anand <panand@redhat.com>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20150721134033.GA4796@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/uprobes.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 4b8ac5f..2d5b7bd 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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;

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [tip:perf/core] uprobes: Fix the waitqueue_active() check in xol_free_insn_slot()
  2015-07-21 13:40 ` [PATCH v3 14/14] uprobes: fix the waitqueue_active() check in xol_free_insn_slot() Oleg Nesterov
@ 2015-07-31 14:02   ` tip-bot for Oleg Nesterov
  0 siblings, 0 replies; 29+ messages in thread
From: tip-bot for Oleg Nesterov @ 2015-07-31 14:02 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: oleg, srikar, panand, torvalds, tglx, luto, linux-kernel, mingo,
	peterz, hpa

Commit-ID:  2a742cedcf13572999436676cbe36c3a9b733b0f
Gitweb:     http://git.kernel.org/tip/2a742cedcf13572999436676cbe36c3a9b733b0f
Author:     Oleg Nesterov <oleg@redhat.com>
AuthorDate: Tue, 21 Jul 2015 15:40:36 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
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 <oleg@redhat.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Pratyush Anand <panand@redhat.com>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20150721134036.GA4799@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/uprobes.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 2d5b7bd..4e5e979 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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);
 

^ permalink raw reply related	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2015-07-31 14:23 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-21 13:39 [PATCH v3 00/14] uprobes: longjmp / special-mapping fixes Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 01/14] uprobes: Introduce get_uprobe() Oleg Nesterov
2015-07-31 13:57   ` [tip:perf/core] " tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 02/14] uprobes: Introduce free_ret_instance() Oleg Nesterov
2015-07-31 13:58   ` [tip:perf/core] " tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 03/14] uprobes: Send SIGILL if handle_trampoline() fails Oleg Nesterov
2015-07-31 13:58   ` [tip:perf/core] " tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 04/14] uprobes: Change prepare_uretprobe() to use uprobe_warn() Oleg Nesterov
2015-07-31 13:58   ` [tip:perf/core] " tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 05/14] uprobes: Change handle_trampoline() to find the next chain beforehand Oleg Nesterov
2015-07-31 13:59   ` [tip:perf/core] " tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 06/14] uprobes: Export struct return_instance, introduce arch_uretprobe_is_alive() Oleg Nesterov
2015-07-31 13:59   ` [tip:perf/core] uprobes: Export 'struct return_instance', " tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 07/14] uprobes/x86: Reimplement arch_uretprobe_is_alive() Oleg Nesterov
2015-07-31 13:59   ` [tip:perf/core] uprobes/x86: Reimplement arch_uretprobe_is_alive( ) tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 08/14] uprobes: Change handle_trampoline() to flush the frames invalidated by longjmp() Oleg Nesterov
2015-07-31 14:00   ` [tip:perf/core] " tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 09/14] uprobes: Change prepare_uretprobe() to (try to) flush the dead frames Oleg Nesterov
2015-07-31 14:00   ` [tip:perf/core] " tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 10/14] uprobes: Add the "enum rp_check ctx" arg to arch_uretprobe_is_alive() Oleg Nesterov
2015-07-31 14:00   ` [tip:perf/core] " tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 11/14] uprobes/x86: Make arch_uretprobe_is_alive(RP_CHECK_CALL) more clever Oleg Nesterov
2015-07-31 14:01   ` [tip:perf/core] uprobes/x86: Make arch_uretprobe_is_alive( RP_CHECK_CALL) " tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 12/14] uprobes: fix the usage of install_special_mapping() Oleg Nesterov
2015-07-31 14:01   ` [tip:perf/core] uprobes: Fix the usage of install_special_mapping () tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 13/14] uprobes: use vm_special_mapping to name the xol vma Oleg Nesterov
2015-07-31 14:01   ` [tip:perf/core] uprobes: Use vm_special_mapping to name the XOL vma tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 14/14] uprobes: fix the waitqueue_active() check in xol_free_insn_slot() Oleg Nesterov
2015-07-31 14:02   ` [tip:perf/core] uprobes: Fix " tip-bot for Oleg Nesterov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.