linux-kernel.vger.kernel.org archive mirror
 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>, "H. Peter Anvin" <hpa@zytor.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Roland McGrath <roland@hack.frob.com>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 2/7] uprobes: x86: Implement x86 specific arch_uprobe_*_step
Date: Mon, 3 Sep 2012 17:26:02 +0200	[thread overview]
Message-ID: <20120903152602.GA9061@redhat.com> (raw)
In-Reply-To: <20120903152525.GA9028@redhat.com>

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

The arch specific implementation behaves like user_enable_single_step()
except that it does not disable single stepping if it was already
enabled by ptrace. This allows the debugger to single step over an
uprobe. The state of block stepping is not restored. It makes only sense
together with TF and if that was enabled then the debugger is notified.

Note: this is still not correct. For example, TIF_SINGLESTEP check
is not right, the application itsel can set X86_EFLAGS_TF. And otoh
we leak TIF_SINGLESTEP (set by enable) if the probed insn is "popf".
See the next patches, we need the changes in arch/x86/kernel/step.c
first.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 arch/x86/include/asm/uprobes.h |    2 ++
 arch/x86/kernel/uprobes.c      |   33 +++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/uprobes.h b/arch/x86/include/asm/uprobes.h
index f3971bb..cee5862 100644
--- a/arch/x86/include/asm/uprobes.h
+++ b/arch/x86/include/asm/uprobes.h
@@ -46,6 +46,8 @@ struct arch_uprobe_task {
 #ifdef CONFIG_X86_64
 	unsigned long			saved_scratch_register;
 #endif
+#define UPROBE_CLEAR_TF			(1 << 0)
+	unsigned int			restore_flags;
 };
 
 extern int  arch_uprobe_analyze_insn(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long addr);
diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index 36fd420..309a0e0 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -41,6 +41,9 @@
 /* Adjust the return address of a call insn */
 #define UPROBE_FIX_CALL	0x2
 
+/* Instruction will modify TF, don't change it */
+#define UPROBE_FIX_SETF	0x4
+
 #define UPROBE_FIX_RIP_AX	0x8000
 #define UPROBE_FIX_RIP_CX	0x4000
 
@@ -239,6 +242,10 @@ static void prepare_fixups(struct arch_uprobe *auprobe, struct insn *insn)
 	insn_get_opcode(insn);	/* should be a nop */
 
 	switch (OPCODE1(insn)) {
+	case 0x9d:
+		/* popf */
+		auprobe->fixups |= UPROBE_FIX_SETF;
+		break;
 	case 0xc3:		/* ret/lret */
 	case 0xcb:
 	case 0xc2:
@@ -673,3 +680,29 @@ bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe, struct pt_regs *regs)
 	}
 	return false;
 }
+
+void arch_uprobe_enable_step(struct arch_uprobe *auprobe)
+{
+	struct uprobe_task	*utask		= current->utask;
+	struct arch_uprobe_task	*autask		= &utask->autask;
+
+	autask->restore_flags = 0;
+	if (!test_tsk_thread_flag(current, TIF_SINGLESTEP) &&
+			!(auprobe->fixups & UPROBE_FIX_SETF))
+		autask->restore_flags |= UPROBE_CLEAR_TF;
+	/*
+	 * The state of TIF_BLOCKSTEP is not saved. With the TF flag set we
+	 * would to examine the opcode and the flags to make it right. Without
+	 * TF block stepping makes no sense.
+	 */
+	user_enable_single_step(current);
+}
+
+void arch_uprobe_disable_step(struct arch_uprobe *auprobe)
+{
+	struct uprobe_task *utask		= current->utask;
+	struct arch_uprobe_task	*autask		= &utask->autask;
+
+	if (autask->restore_flags & UPROBE_CLEAR_TF)
+		user_disable_single_step(current);
+}
-- 
1.5.5.1


  parent reply	other threads:[~2012-09-03 15:25 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-03 15:25 [PATCH 0/7] uprobes: single-step fixes Oleg Nesterov
2012-09-03 15:25 ` [PATCH 1/7] uprobes: Introduce arch_uprobe_enable/disable_step() Oleg Nesterov
2012-09-07 14:57   ` Srikar Dronamraju
2012-09-03 15:26 ` Oleg Nesterov [this message]
2012-09-07 14:59   ` [PATCH 2/7] uprobes: x86: Implement x86 specific arch_uprobe_*_step Srikar Dronamraju
2012-09-03 15:26 ` [PATCH 3/7] ptrace: Introduce set_task_blockstep() helper Oleg Nesterov
2012-09-07 15:00   ` Srikar Dronamraju
2012-09-03 15:26 ` [PATCH 4/7] ptrace: Partly fix set_task_blockstep()->update_debugctlmsr() logic Oleg Nesterov
2012-09-07 15:14   ` Srikar Dronamraju
2012-09-10 16:57   ` Sebastian Andrzej Siewior
2012-09-10 17:45     ` Peter Zijlstra
2012-09-10 17:27   ` Oleg Nesterov
2012-09-03 15:26 ` [PATCH 5/7] uprobes: Do not (ab)use TIF_SINGLESTEP/user_*_single_step() for single-stepping Oleg Nesterov
2012-09-07 15:11   ` Srikar Dronamraju
2012-09-07 15:50     ` Oleg Nesterov
2012-09-08  7:49       ` Srikar Dronamraju
2012-09-03 15:26 ` [PATCH 6/7] uprobes: Xol should send SIGTRAP if X86_EFLAGS_TF was set Oleg Nesterov
2012-09-12 12:08   ` Srikar Dronamraju
2012-09-12 14:45     ` Oleg Nesterov
2012-09-03 15:26 ` [PATCH 7/7] uprobes: Make arch_uprobe_task->saved_trap_nr "unsigned int" Oleg Nesterov
2012-09-12 12:27   ` Srikar Dronamraju
2012-09-08 17:06 ` [PATCH 0/7] uprobes: single-step fixes Oleg Nesterov
2012-09-12 12:33   ` Srikar Dronamraju
2012-09-08 17:06 ` [PATCH 8/7] uprobes: Fix arch_uprobe_disable_step() && UTASK_SSTEP_TRAPPED interaction Oleg Nesterov
2012-09-12 12:36   ` Srikar Dronamraju
2012-09-10 16:57 ` [PATCH 0/7] uprobes: single-step fixes Sebastian Andrzej Siewior

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=20120903152602.GA9061@redhat.com \
    --to=oleg@redhat.com \
    --cc=ananth@in.ibm.com \
    --cc=anton@redhat.com \
    --cc=bigeasy@linutronix.de \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=roland@hack.frob.com \
    --cc=srikar@linux.vnet.ibm.com \
    --cc=torvalds@linux-foundation.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).