kdb: Switch to use safer dbg_io_ops over console APIs

In kgdb context, calling console handlers aren't safe due to locks used
in those handlers which could in turn lead to a deadlock. Although, using
oops_in_progress increases the chance to bypass locks in most console
handlers but it might not be sufficient enough in case a console uses
more locks (VT/TTY is good example).

Currently when a driver provides both polling I/O and a console then kdb
will output using the console. We can increase robustness by using the
currently active polling I/O driver (which should be lockless) instead
of the corresponding console. For several common cases (e.g. an
embedded system with a single serial port that is used both for console
output and debugger I/O) this will result in no console handler being
used.

In order to achieve this we need to reverse the order of preference to
use dbg_io_ops (uses polling I/O mode) over console APIs. So we just
store "struct console" that represents debugger I/O in dbg_io_ops and
while emitting kdb messages, skip console that matches dbg_io_ops
console in order to avoid duplicate messages. After this change,
"is_console" param becomes redundant and hence removed.

Suggested-by: Daniel Thompson <[email protected]>
Signed-off-by: Sumit Garg <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Reviewed-by: Douglas Anderson <[email protected]>
Reviewed-by: Petr Mladek <[email protected]>
Acked-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Daniel Thompson <[email protected]>

diff --git a/drivers/tty/serial/kgdb_nmi.c b/drivers/tty/serial/kgdb_nmi.c
index 5022447afa23eb8ba52efd30e540c0136ccebfa8..6004c0c1d173e6b4f9e5ba2f3be5ebbc76ffdbaa 100644 (file)
--- a/drivers/tty/serial/kgdb_nmi.c
+++ b/drivers/tty/serial/kgdb_nmi.c
@@ -50,7 +50,7 @@ static int kgdb_nmi_console_setup(struct console *co, char *options)
         * I/O utilities that messages sent to the console will automatically
         * be displayed on the dbg_io.
         */
-       dbg_io_ops->is_console = true;
+       dbg_io_ops->cons = co;
 
        return 0;
 }

diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
index 41396982e9e081815c7db4177172657ad5db77b8..84ffede27f230b0ae6c7d23261d8feadcc76490f 100644 (file)
--- a/drivers/tty/serial/kgdboc.c
+++ b/drivers/tty/serial/kgdboc.c
@@ -45,7 +45,6 @@ static struct platform_device *kgdboc_pdev;
 
 #if IS_BUILTIN(CONFIG_KGDB_SERIAL_CONSOLE)
 static struct kgdb_io          kgdboc_earlycon_io_ops;
-static struct console          *earlycon;
 static int                      (*earlycon_orig_exit)(struct console *con);
 #endif /* IS_BUILTIN(CONFIG_KGDB_SERIAL_CONSOLE) */
 
@@ -145,7 +144,7 @@ static void kgdboc_unregister_kbd(void)
 #if IS_BUILTIN(CONFIG_KGDB_SERIAL_CONSOLE)
 static void cleanup_earlycon(void)
 {
-       if (earlycon)
+       if (kgdboc_earlycon_io_ops.cons)
                kgdb_unregister_io_module(&kgdboc_earlycon_io_ops);
 }
 #else /* !IS_BUILTIN(CONFIG_KGDB_SERIAL_CONSOLE) */
@@ -178,7 +177,7 @@ static int configure_kgdboc(void)
                goto noconfig;
        }
 
-       kgdboc_io_ops.is_console = 0;
+       kgdboc_io_ops.cons = NULL;
        kgdb_tty_driver = NULL;
 
        kgdboc_use_kms = 0;
@@ -198,7 +197,7 @@ static int configure_kgdboc(void)
                int idx;
                if (cons->device && cons->device(cons, &idx) == p &&
                    idx == tty_line) {
-                       kgdboc_io_ops.is_console = 1;
+                       kgdboc_io_ops.cons = cons;
                        break;
                }
        }
@@ -433,7 +432,8 @@ static int kgdboc_earlycon_get_char(void)
 {
        char c;
 
-       if (!earlycon->read(earlycon, &c, 1))
+       if (!kgdboc_earlycon_io_ops.cons->read(kgdboc_earlycon_io_ops.cons,
+                                              &c, 1))
                return NO_POLL_CHAR;
 
        return c;
@@ -441,7 +441,8 @@ static int kgdboc_earlycon_get_char(void)
 
 static void kgdboc_earlycon_put_char(u8 chr)
 {
-       earlycon->write(earlycon, &chr, 1);
+       kgdboc_earlycon_io_ops.cons->write(kgdboc_earlycon_io_ops.cons, &chr,
+                                          1);
 }
 
 static void kgdboc_earlycon_pre_exp_handler(void)
@@ -461,7 +462,7 @@ static void kgdboc_earlycon_pre_exp_handler(void)
         * boot if we detect this case.
         */
        for_each_console(con)
-               if (con == earlycon)
+               if (con == kgdboc_earlycon_io_ops.cons)
                        return;
 
        already_warned = true;
@@ -484,25 +485,25 @@ static int kgdboc_earlycon_deferred_exit(struct console *con)
 
 static void kgdboc_earlycon_deinit(void)
 {
-       if (!earlycon)
+       if (!kgdboc_earlycon_io_ops.cons)
                return;
 
-       if (earlycon->exit == kgdboc_earlycon_deferred_exit)
+       if (kgdboc_earlycon_io_ops.cons->exit == kgdboc_earlycon_deferred_exit)
                /*
                 * kgdboc_earlycon is exiting but original boot console exit
                 * was never called (AKA kgdboc_earlycon_deferred_exit()
                 * didn't ever run).  Undo our trap.
                 */
-               earlycon->exit = earlycon_orig_exit;
-       else if (earlycon->exit)
+               kgdboc_earlycon_io_ops.cons->exit = earlycon_orig_exit;
+       else if (kgdboc_earlycon_io_ops.cons->exit)
                /*
                 * We skipped calling the exit() routine so we could try to
                 * keep using the boot console even after it went away.  We're
                 * finally done so call the function now.
                 */
-               earlycon->exit(earlycon);
+               kgdboc_earlycon_io_ops.cons->exit(kgdboc_earlycon_io_ops.cons);
 
-       earlycon = NULL;
+       kgdboc_earlycon_io_ops.cons = NULL;
 }
 
 static struct kgdb_io kgdboc_earlycon_io_ops = {
@@ -511,7 +512,6 @@ static struct kgdb_io kgdboc_earlycon_io_ops = {
        .write_char             = kgdboc_earlycon_put_char,
        .pre_exception          = kgdboc_earlycon_pre_exp_handler,
        .deinit                 = kgdboc_earlycon_deinit,
-       .is_console             = true,
 };
 
 #define MAX_CONSOLE_NAME_LEN (sizeof((struct console *) 0)->name)
@@ -557,10 +557,10 @@ static int __init kgdboc_earlycon_init(char *opt)
                goto unlock;
        }
 
-       earlycon = con;
+       kgdboc_earlycon_io_ops.cons = con;
        pr_info("Going to register kgdb with earlycon '%s'\n", con->name);
        if (kgdb_register_io_module(&kgdboc_earlycon_io_ops) != 0) {
-               earlycon = NULL;
+               kgdboc_earlycon_io_ops.cons = NULL;
                pr_info("Failed to register kgdb with earlycon\n");
        } else {
                /* Trap exit so we can keep earlycon longer if needed. */

diff --git a/drivers/usb/early/ehci-dbgp.c b/drivers/usb/early/ehci-dbgp.c
index ea0d531c63e21670edd86a7a8dc560868e04b83f..775cf70cfb3ed3f82a1e821b0d1bf78886043bed 100644 (file)
--- a/drivers/usb/early/ehci-dbgp.c
+++ b/drivers/usb/early/ehci-dbgp.c
@@ -1058,7 +1058,8 @@ static int __init kgdbdbgp_parse_config(char *str)
                kgdbdbgp_wait_time = simple_strtoul(ptr, &ptr, 10);
        }
        kgdb_register_io_module(&kgdbdbgp_io_ops);
-       kgdbdbgp_io_ops.is_console = early_dbgp_console.index != -1;
+       if (early_dbgp_console.index != -1)
+               kgdbdbgp_io_ops.cons = &early_dbgp_console;
 
        return 0;
 }

diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index c62d76478adcbf64814644ccdfceecc442243e5a..529116b0cabe2879ff95ca027cd8848394795f00 100644 (file)
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -276,8 +276,7 @@ struct kgdb_arch {
  * the I/O driver.
  * @post_exception: Pointer to a function that will do any cleanup work
  * for the I/O driver.
- * @is_console: 1 if the end device is a console 0 if the I/O device is
- * not a console
+ * @cons: valid if the I/O device is a console; else NULL.
  */
 struct kgdb_io {
        const char              *name;
@@ -288,7 +287,7 @@ struct kgdb_io {
        void                    (*deinit) (void);
        void                    (*pre_exception) (void);
        void                    (*post_exception) (void);
-       int                     is_console;
+       struct console          *cons;
 };
 
 extern const struct kgdb_arch          arch_kgdb_ops;

diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
index 0e4f2eda96d8982929d925a910d5f3acdbec1d5b..683a799618ade75589ebe151f218e4ab561d298f 100644 (file)
--- a/kernel/debug/kdb/kdb_io.c
+++ b/kernel/debug/kdb/kdb_io.c
@@ -549,7 +549,7 @@ static void kdb_msg_write(const char *msg, int msg_len)
        if (msg_len == 0)
                return;
 
-       if (dbg_io_ops && !dbg_io_ops->is_console) {
+       if (dbg_io_ops) {
                const char *cp = msg;
                int len = msg_len;
 
@@ -562,6 +562,8 @@ static void kdb_msg_write(const char *msg, int msg_len)
        for_each_console(c) {
                if (!(c->flags & CON_ENABLED))
                        continue;
+               if (c == dbg_io_ops->cons)
+                       continue;
                /*
                 * Set oops_in_progress to encourage the console drivers to
                 * disregard their internal spin locks: in the current calling