[Kgdb-bugreport] [PATCH] attempt fix up breakpoint on reenter to KGDB

[Jason W. <jas...@wi...>]

If there are no objections, I will commit the following patch.

--

This patch fixes some corner cases where KGDB will silently hang or
kill the system, if a user accidentally tries to source step into a
spin_unlock() call or source step in on a macro containing
smp_processor_id().  The use of raw_smp_processor_id is desired in
kernel/kgdb.c to fix this particular problem.

To fix issues with accidental source step in on spin_unlock(), the
idea is to check for the existence of a break point on the second
entry to kgdb and try to remove it.  Next an attempt will be made to
continue normal operations.  A third entry will generate a panic(), so
as to stop infinite loops.

Testing has shown that kgdb is much more robust with these changes
and "random accidental run control".

Signed-off-by: Jason Wessel <jas...@wi...>

---
 arch/i386/kernel/kgdb.c |    8 ++++++++
 kernel/kgdb.c           |   47 
++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 48 insertions(+), 7 deletions(-)

Index: linux-2.6.21.1/kernel/kgdb.c
===================================================================
--- linux-2.6.21.1.orig/kernel/kgdb.c
+++ linux-2.6.21.1/kernel/kgdb.c
@@ -70,6 +70,8 @@ int kgdb_connected;
 int kgdb_may_fault;
 /* All the KGDB handlers are installed */
 int kgdb_from_module_registered = 0;
+/* Guard for recursive entry */
+static int exception_level = 0;
 
 /* We provide a kgdb_io_ops structure that may be overriden. */
 struct kgdb_io __attribute__ ((weak)) kgdb_io_ops;
@@ -166,6 +168,12 @@ int __attribute__ ((weak))
     return 0;
 }
 
+unsigned long __attribute__ ((weak))
+    kgdb_arch_pc(int exception, struct pt_regs *regs)
+{
+    return instruction_pointer(regs);
+}
+
 static int hex(char ch)
 {
     if ((ch >= 'a') && (ch <= 'f'))
@@ -580,11 +588,11 @@ static void kgdb_wait(struct pt_regs *re
     int processor;
 
     local_irq_save(flags);
-    processor = smp_processor_id();
+    processor = raw_smp_processor_id();
     kgdb_info[processor].debuggerinfo = regs;
     kgdb_info[processor].task = current;
     atomic_set(&procindebug[processor], 1);
-    atomic_set(&kgdb_sync_softlockup[smp_processor_id()], 1);
+    atomic_set(&kgdb_sync_softlockup[raw_smp_processor_id()], 1);
 
     /* Wait till master processor goes completely into the debugger.
      * FIXME: this looks racy */
@@ -784,7 +792,7 @@ static inline int shadow_pid(int realpid
     if (realpid) {
         return realpid;
     }
-    return pid_max + smp_processor_id();
+    return pid_max + raw_smp_processor_id();
 }
 
 static char gdbmsgbuf[BUFMAX + 1];
@@ -848,12 +856,36 @@ int kgdb_handle_exception(int ex_vector,
     long kgdb_usethreadid = 0;
     int error = 0, all_cpus_synced = 0;
     struct pt_regs *shadowregs;
-    int processor = smp_processor_id();
+    int processor = raw_smp_processor_id();
     void *local_debuggerinfo;
 
     /* Panic on recursive debugger calls. */
-    if (atomic_read(&debugger_active) == smp_processor_id() + 1)
+    if (atomic_read(&debugger_active) == raw_smp_processor_id() + 1) {
+        exception_level++;
+        addr = kgdb_arch_pc(ex_vector, linux_regs);
+        kgdb_deactivate_sw_breakpoints();
+        if (kgdb_remove_sw_break(addr) == 0) {
+            /* If the break point removed ok at the place exception
+             * occurred, try to recover and print a warning to the end
+             * user because the user planted a breakpoint in a place
+             * that KGDB needs in order to function.
+             */
+            exception_level = 0;
+            kgdb_skipexception(ex_vector, linux_regs);
+            kgdb_activate_sw_breakpoints();
+            printk(KERN_CRIT "KGDB: re-enter exception: breakpoint 
removed\n");
+            WARN_ON(1);
+            return 0;
+        }
+        remove_all_break();
+        kgdb_skipexception(ex_vector, linux_regs);
+        if (exception_level > 1)
+            panic("Recursive entry to debugger");
+
+        printk(KERN_CRIT "KGDB: re-enter exception: ALL breakpoints 
removed\n");
+        panic("Recursive entry to debugger");
         return 0;
+    }
 
  acquirelock:
 
@@ -864,7 +896,7 @@ int kgdb_handle_exception(int ex_vector,
     local_irq_save(flags);
 
     /* Hold debugger_active */
-    procid = smp_processor_id();
+    procid = raw_smp_processor_id();
 
     while (cmpxchg(&atomic_read(&debugger_active), 0, (procid + 1)) != 0) {
         int i = 25;    /* an arbitrary number */
@@ -877,7 +909,7 @@ int kgdb_handle_exception(int ex_vector,
             udelay(1);
     }
 
-    atomic_set(&kgdb_sync_softlockup[smp_processor_id()], 1);
+    atomic_set(&kgdb_sync_softlockup[raw_smp_processor_id()], 1);
 
     /*
      * Don't enter if the last instance of the exception handler wanted to
@@ -945,6 +977,7 @@ int kgdb_handle_exception(int ex_vector,
     kgdb_deactivate_sw_breakpoints();
     debugger_step = 0;
     kgdb_contthread = NULL;
+    exception_level = 0;
 
     if (kgdb_connected) {
         /* If we're still unable to roundup all of the CPUs,
Index: linux-2.6.21.1/arch/i386/kernel/kgdb.c
===================================================================
--- linux-2.6.21.1.orig/arch/i386/kernel/kgdb.c
+++ linux-2.6.21.1/arch/i386/kernel/kgdb.c
@@ -302,6 +302,14 @@ int kgdb_skipexception(int exception, st
     return 0;
 }
 
+unsigned long kgdb_arch_pc(int exception, struct pt_regs *regs)
+{
+    if (exception == 3) {
+        return instruction_pointer(regs) - 1;
+    }
+    return instruction_pointer(regs);
+}
+
 struct kgdb_arch arch_kgdb_ops = {
     .gdb_bpt_instr = {0xcc},
     .flags = KGDB_HW_BREAKPOINT,