lkml.org 
[lkml]   [2012]   [Sep]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/7] uprobes: x86: Implement x86 specific arch_uprobe_*_step
* Oleg Nesterov <oleg@redhat.com> [2012-09-03 17:26:02]:

> 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

nit:
s/itsel/itself

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


Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.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
>



\
 
 \ /
  Last update: 2012-09-07 17:42    [W:0.126 / U:6.708 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site