uprobes: Fix UPROBE_SKIP_SSTEP checks in handle_swbp()

If handle_swbp()->add_utask() fails but UPROBE_SKIP_SSTEP is set,
cleanup_ret: path do not restart the insn, this is wrong. Remove
this check and add the additional label for can_skip_sstep() = T
case.

Note also that UPROBE_SKIP_SSTEP can be false positive, we simply
can not trust it unless arch_uprobe_skip_sstep() was already called.

Also, move another UPROBE_SKIP_SSTEP check before can_skip_sstep()
into this helper, this looks more clean and understandable.

Note: probably we should rename "skip" to "emulate" and I think
that "clear UPROBE_SKIP_SSTEP" should be moved to arch_can_skip.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 41f048c91425c1c8a0ddcd11c8ec4feb22612cf9..d2392968d4e65f5b38d4bf22a92e2fc914cb09c1 100644 (file)
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1389,10 +1389,11 @@ bool uprobe_deny_signal(void)
  */
 static bool can_skip_sstep(struct uprobe *uprobe, struct pt_regs *regs)
 {
-       if (arch_uprobe_skip_sstep(&uprobe->arch, regs))
-               return true;
-
-       uprobe->flags &= ~UPROBE_SKIP_SSTEP;
+       if (uprobe->flags & UPROBE_SKIP_SSTEP) {
+               if (arch_uprobe_skip_sstep(&uprobe->arch, regs))
+                       return true;
+               uprobe->flags &= ~UPROBE_SKIP_SSTEP;
+       }
        return false;
 }
 
@@ -1494,12 +1495,12 @@ static void handle_swbp(struct pt_regs *regs)
                utask = add_utask();
                /* Cannot allocate; re-execute the instruction. */
                if (!utask)
-                       goto cleanup_ret;
+                       goto restart;
        }
 
        handler_chain(uprobe, regs);
-       if (uprobe->flags & UPROBE_SKIP_SSTEP && can_skip_sstep(uprobe, regs))
-               goto cleanup_ret;
+       if (can_skip_sstep(uprobe, regs))
+               goto out;
 
        if (!pre_ssout(uprobe, regs, bp_vaddr)) {
                arch_uprobe_enable_step(&uprobe->arch);
@@ -1508,15 +1509,13 @@ static void handle_swbp(struct pt_regs *regs)
                return;
        }
 
-cleanup_ret:
-       if (!(uprobe->flags & UPROBE_SKIP_SSTEP))
-
-               /*
-                * cannot singlestep; cannot skip instruction;
-                * re-execute the instruction.
-                */
-               instruction_pointer_set(regs, bp_vaddr);
-
+restart:
+       /*
+        * cannot singlestep; cannot skip instruction;
+        * re-execute the instruction.
+        */
+       instruction_pointer_set(regs, bp_vaddr);
+out:
        put_uprobe(uprobe);
 }