uprobes: Kill UPROBE_SKIP_SSTEP and can_skip_sstep()

UPROBE_COPY_INSN, UPROBE_SKIP_SSTEP, and uprobe->flags must die. This
patch kills UPROBE_SKIP_SSTEP. I never understood why it was added;
not only it doesn't help, it harms.

It can only help to avoid arch_uprobe_skip_sstep() if it was already
called before and failed. But this is ugly, if we want to know whether
we can emulate this instruction or not we should do this analysis in
arch_uprobe_analyze_insn(), not when we hit this probe for the first
time.

And in fact this logic is simply wrong. arch_uprobe_skip_sstep() can
fail or not depending on the task/register state, if this insn can be
emulated but, say, put_user() fails we need to xol it this time, but
this doesn't mean we shouldn't try to emulate it when this or another
thread hits this bp next time.

And this is the actual reason for this change. We need to emulate the
"call" insn, but push(return-address) can obviously fail.

Per-arch notes:

	x86: __skip_sstep() can only emulate "rep;nop". With this
	     change it will be called every time and most probably
	     for no reason.

	     This will be fixed by the next changes. We need to
	     change this suboptimal code anyway.

	arm: Should not be affected. It has its own "bool simulate"
	     flag checked in arch_uprobe_skip_sstep().

	ppc: Looks like, it can emulate almost everything. Does it
	     actually need to record the fact that emulate_step()
	     failed? Hopefully not. But if yes, it can add the ppc-
	     specific flag into arch_uprobe.

TODO: rename arch_uprobe_skip_sstep() to arch_uprobe_emulate_insn(),

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Reviewed-by: David A. Long <dave.long@linaro.org>
Reviewed-by: Jim Keniston <jkenisto@us.ibm.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 04709b66369d83d1dc883ab5eea92505d5eed410..ea2a7c0728bbe3c2374ab5574395bb45ccd254d4 100644 (file)
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -60,8 +60,6 @@ static struct percpu_rw_semaphore dup_mmap_sem;
 
 /* Have a copy of original instruction */
 #define UPROBE_COPY_INSN       0
-/* Can skip singlestep */
-#define UPROBE_SKIP_SSTEP      1
 
 struct uprobe {
        struct rb_node          rb_node;        /* node in the rb tree */
@@ -491,12 +489,9 @@ static struct uprobe *alloc_uprobe(struct inode *inode, loff_t offset)
        uprobe->offset = offset;
        init_rwsem(&uprobe->register_rwsem);
        init_rwsem(&uprobe->consumer_rwsem);
-       /* For now assume that the instruction need not be single-stepped */
-       __set_bit(UPROBE_SKIP_SSTEP, &uprobe->flags);
 
        /* add to uprobes_tree, sorted on inode:offset */
        cur_uprobe = insert_uprobe(uprobe);
-
        /* a uprobe exists for this inode:offset combination */
        if (cur_uprobe) {
                kfree(uprobe);
@@ -1628,20 +1623,6 @@ bool uprobe_deny_signal(void)
        return true;
 }
 
-/*
- * Avoid singlestepping the original instruction if the original instruction
- * is a NOP or can be emulated.
- */
-static bool can_skip_sstep(struct uprobe *uprobe, struct pt_regs *regs)
-{
-       if (test_bit(UPROBE_SKIP_SSTEP, &uprobe->flags)) {
-               if (arch_uprobe_skip_sstep(&uprobe->arch, regs))
-                       return true;
-               clear_bit(UPROBE_SKIP_SSTEP, &uprobe->flags);
-       }
-       return false;
-}
-
 static void mmf_recalc_uprobes(struct mm_struct *mm)
 {
        struct vm_area_struct *vma;
@@ -1868,13 +1849,13 @@ static void handle_swbp(struct pt_regs *regs)
 
        handler_chain(uprobe, regs);
 
-       if (can_skip_sstep(uprobe, regs))
+       if (arch_uprobe_skip_sstep(&uprobe->arch, regs))
                goto out;
 
        if (!pre_ssout(uprobe, regs, bp_vaddr))
                return;
 
-       /* can_skip_sstep() succeeded, or restart if can't singlestep */
+       /* arch_uprobe_skip_sstep() succeeded, or restart if can't singlestep */
 out:
        put_uprobe(uprobe);
 }