All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Ingo Molnar <mingo@elte.hu>,
	Peter Zijlstra <peterz@infradead.org>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>,
	Anton Arapov <anton@redhat.com>, Frank Eigler <fche@redhat.com>,
	Josh Stone <jistone@redhat.com>,
	"Suzuki K. Poulose" <suzuki@in.ibm.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 4/7] uprobes: Do not play with utask in xol_get_insn_slot()
Date: Mon, 31 Dec 2012 18:52:23 +0100	[thread overview]
Message-ID: <20121231175223.GA32101@redhat.com> (raw)
In-Reply-To: <20121231175150.GA32066@redhat.com>

pre_ssout()->xol_get_insn_slot() path is confusing and buggy. This patch
cleanups the code, the next one fixes the bug.

Change xol_get_insn_slot() to only allocate the slot and do nothing more,
move the initialization of utask->xol_vaddr/vaddr into pre_ssout().

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/events/uprobes.c |   37 +++++++++++++++++++++----------------
 1 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index b09b3ba..2ed6239 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1176,30 +1176,26 @@ static unsigned long xol_take_insn_slot(struct xol_area *area)
 }
 
 /*
- * xol_get_insn_slot - If was not allocated a slot, then
- * allocate a slot.
+ * xol_get_insn_slot - allocate a slot for xol.
  * Returns the allocated slot address or 0.
  */
-static unsigned long xol_get_insn_slot(struct uprobe *uprobe, unsigned long slot_addr)
+static unsigned long xol_get_insn_slot(struct uprobe *uprobe)
 {
 	struct xol_area *area;
 	unsigned long offset;
+	unsigned long xol_vaddr;
 	void *vaddr;
 
 	area = get_xol_area();
 	if (!area)
 		return 0;
 
-	current->utask->xol_vaddr = xol_take_insn_slot(area);
-	/*
-	 * Initialize the slot if xol_vaddr points to valid
-	 * instruction slot.
-	 */
-	if (unlikely(!current->utask->xol_vaddr))
+	xol_vaddr = xol_take_insn_slot(area);
+	if (unlikely(!xol_vaddr))
 		return 0;
 
-	current->utask->vaddr = slot_addr;
-	offset = current->utask->xol_vaddr & ~PAGE_MASK;
+	/* Initialize the slot */
+	offset = xol_vaddr & ~PAGE_MASK;
 	vaddr = kmap_atomic(area->page);
 	memcpy(vaddr + offset, uprobe->arch.insn, MAX_UINSN_BYTES);
 	kunmap_atomic(vaddr);
@@ -1209,7 +1205,7 @@ static unsigned long xol_get_insn_slot(struct uprobe *uprobe, unsigned long slot
 	 */
 	flush_dcache_page(area->page);
 
-	return current->utask->xol_vaddr;
+	return xol_vaddr;
 }
 
 /*
@@ -1306,12 +1302,21 @@ static struct uprobe_task *get_utask(void)
 
 /* Prepare to single-step probed instruction out of line. */
 static int
-pre_ssout(struct uprobe *uprobe, struct pt_regs *regs, unsigned long vaddr)
+pre_ssout(struct uprobe *uprobe, struct pt_regs *regs, unsigned long bp_vaddr)
 {
-	if (xol_get_insn_slot(uprobe, vaddr) && !arch_uprobe_pre_xol(&uprobe->arch, regs))
-		return 0;
+	struct uprobe_task *utask;
+	unsigned long xol_vaddr;
+
+	utask = current->utask;
+
+	xol_vaddr = xol_get_insn_slot(uprobe);
+	if (!xol_vaddr)
+		return -ENOMEM;
+
+	utask->xol_vaddr = xol_vaddr;
+	utask->vaddr = bp_vaddr;
 
-	return -EFAULT;
+	return arch_uprobe_pre_xol(&uprobe->arch, regs);
 }
 
 /*
-- 
1.5.5.1


  parent reply	other threads:[~2012-12-31 17:52 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-31 17:51 [PATCH 0/7] uprobes: alloc utask/xol_area cleanups and minor fix Oleg Nesterov
2012-12-31 17:52 ` [PATCH 1/7] uprobes: Move alloc_page() from xol_add_vma() to xol_alloc_area() Oleg Nesterov
2013-01-07  9:16   ` Anton Arapov
2013-01-07 16:11     ` Oleg Nesterov
2013-01-08 11:46   ` Srikar Dronamraju
2013-01-08 17:58     ` Oleg Nesterov
2013-01-09 17:44       ` Srikar Dronamraju
2012-12-31 17:52 ` [PATCH 2/7] uprobes: Fold xol_alloc_area() into get_xol_area() Oleg Nesterov
2013-01-08 11:55   ` Srikar Dronamraju
2013-01-09 10:16   ` Anton Arapov
2013-01-09 15:51     ` Oleg Nesterov
2012-12-31 17:52 ` [PATCH 3/7] uprobes: Turn add_utask() into get_utask() Oleg Nesterov
2013-01-08 11:57   ` Srikar Dronamraju
2012-12-31 17:52 ` Oleg Nesterov [this message]
2013-01-08 12:07   ` [PATCH 4/7] uprobes: Do not play with utask in xol_get_insn_slot() Srikar Dronamraju
2012-12-31 17:52 ` [PATCH 5/7] uprobes: Fix utask->xol_vaddr leak in pre_ssout() Oleg Nesterov
2013-01-08 12:13   ` Srikar Dronamraju
2013-01-08 17:44     ` Oleg Nesterov
2013-01-10 12:48       ` Srikar Dronamraju
2012-12-31 17:52 ` [PATCH 6/7] uprobes: Do not allocate current->utask unnecessary Oleg Nesterov
2013-01-08 12:20   ` Srikar Dronamraju
2013-01-08 18:13     ` Oleg Nesterov
2012-12-31 17:52 ` [PATCH 7/7] uprobes: Kill the bogus IS_ERR_VALUE(xol_vaddr) check Oleg Nesterov
2013-01-08 12:23   ` Srikar Dronamraju
2013-01-09 10:25 ` [PATCH 0/7] uprobes: alloc utask/xol_area cleanups and minor fix Anton Arapov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121231175223.GA32101@redhat.com \
    --to=oleg@redhat.com \
    --cc=ananth@in.ibm.com \
    --cc=anton@redhat.com \
    --cc=fche@redhat.com \
    --cc=jistone@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=srikar@linux.vnet.ibm.com \
    --cc=suzuki@in.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.