arm64: kprobes: Return DBG_HOOK_ERROR if kprobes can not handle a BRK

Return DBG_HOOK_ERROR if kprobes can not handle a BRK because it
fails to find a kprobe corresponding to the address.

Since arm64 kprobes uses stop_machine based text patching for removing
BRK, it ensures all running kprobe_break_handler() is done at that point.
And after removing the BRK, it removes the kprobe from its hash list.
Thus, if the kprobe_break_handler() fails to find kprobe from hash list,
there is a bug.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Link: https://lore.kernel.org/r/166994753273.439920.6629626290560350760.stgit@devnote3
Signed-off-by: Will Deacon <will@kernel.org>

diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index d2ae37f8977410b83a5d4eb3e179f779e7a04645..f35d059a9a366fa6bddefc14dd886981c95de6cd 100644 (file)
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -298,7 +298,8 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr)
        return 0;
 }
 
-static void __kprobes kprobe_handler(struct pt_regs *regs)
+static int __kprobes
+kprobe_breakpoint_handler(struct pt_regs *regs, unsigned long esr)
 {
        struct kprobe *p, *cur_kprobe;
        struct kprobe_ctlblk *kcb;
@@ -308,39 +309,44 @@ static void __kprobes kprobe_handler(struct pt_regs *regs)
        cur_kprobe = kprobe_running();
 
        p = get_kprobe((kprobe_opcode_t *) addr);
+       if (WARN_ON_ONCE(!p)) {
+               /*
+                * Something went wrong. This BRK used an immediate reserved
+                * for kprobes, but we couldn't find any corresponding probe.
+                */
+               return DBG_HOOK_ERROR;
+       }
 
-       if (p) {
-               if (cur_kprobe) {
-                       if (reenter_kprobe(p, regs, kcb))
-                               return;
-               } else {
-                       /* Probe hit */
-                       set_current_kprobe(p);
-                       kcb->kprobe_status = KPROBE_HIT_ACTIVE;
-
-                       /*
-                        * If we have no pre-handler or it returned 0, we
-                        * continue with normal processing.  If we have a
-                        * pre-handler and it returned non-zero, it will
-                        * modify the execution path and no need to single
-                        * stepping. Let's just reset current kprobe and exit.
-                        */
-                       if (!p->pre_handler || !p->pre_handler(p, regs)) {
-                               setup_singlestep(p, regs, kcb, 0);
-                       } else
-                               reset_current_kprobe();
-               }
+       if (cur_kprobe) {
+               /* Hit a kprobe inside another kprobe */
+               if (!reenter_kprobe(p, regs, kcb))
+                       return DBG_HOOK_ERROR;
+       } else {
+               /* Probe hit */
+               set_current_kprobe(p);
+               kcb->kprobe_status = KPROBE_HIT_ACTIVE;
+
+               /*
+                * If we have no pre-handler or it returned 0, we
+                * continue with normal processing.  If we have a
+                * pre-handler and it returned non-zero, it will
+                * modify the execution path and not need to single-step
+                * Let's just reset current kprobe and exit.
+                */
+               if (!p->pre_handler || !p->pre_handler(p, regs))
+                       setup_singlestep(p, regs, kcb, 0);
+               else
+                       reset_current_kprobe();
        }
-       /*
-        * The breakpoint instruction was removed right
-        * after we hit it.  Another cpu has removed
-        * either a probepoint or a debugger breakpoint
-        * at this address.  In either case, no further
-        * handling of this interrupt is appropriate.
-        * Return back to original instruction, and continue.
-        */
+
+       return DBG_HOOK_HANDLED;
 }
 
+static struct break_hook kprobes_break_hook = {
+       .imm = KPROBES_BRK_IMM,
+       .fn = kprobe_breakpoint_handler,
+};
+
 static int __kprobes
 kprobe_breakpoint_ss_handler(struct pt_regs *regs, unsigned long esr)
 {
@@ -365,18 +371,6 @@ static struct break_hook kprobes_break_ss_hook = {
        .fn = kprobe_breakpoint_ss_handler,
 };
 
-static int __kprobes
-kprobe_breakpoint_handler(struct pt_regs *regs, unsigned long esr)
-{
-       kprobe_handler(regs);
-       return DBG_HOOK_HANDLED;
-}
-
-static struct break_hook kprobes_break_hook = {
-       .imm = KPROBES_BRK_IMM,
-       .fn = kprobe_breakpoint_handler,
-};
-
 /*
  * Provide a blacklist of symbols identifying ranges which cannot be kprobed.
  * This blacklist is exposed to userspace via debugfs (kprobes/blacklist).