[PATCH v4 4/4] kdb: Switch to use safer dbg_io_ops over console APIs

From: Sumit Garg
Date: Fri May 29 2020 - 07:27:46 EST


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 <daniel.thompson@xxxxxxxxxx>
Signed-off-by: Sumit Garg <sumit.garg@xxxxxxxxxx>
---
drivers/tty/serial/kgdb_nmi.c | 2 +-
drivers/tty/serial/kgdboc.c | 4 ++--
drivers/usb/early/ehci-dbgp.c | 3 ++-
include/linux/kgdb.h | 5 ++---
kernel/debug/kdb/kdb_io.c | 4 +++-
5 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/kgdb_nmi.c b/drivers/tty/serial/kgdb_nmi.c
index 5022447..6004c0c 100644
--- 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 c9f94fa..c7ffa87 100644
--- a/drivers/tty/serial/kgdboc.c
+++ b/drivers/tty/serial/kgdboc.c
@@ -153,7 +153,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;
@@ -173,7 +173,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;
}
}
diff --git a/drivers/usb/early/ehci-dbgp.c b/drivers/usb/early/ehci-dbgp.c
index ea0d531..8c32d1a 100644
--- 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 b072aeb..bc0face3 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -273,8 +273,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.
*/
struct kgdb_io {
const char *name;
@@ -284,7 +283,7 @@ struct kgdb_io {
int (*init) (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 9e5a40d..5e00bc8 100644
--- a/kernel/debug/kdb/kdb_io.c
+++ b/kernel/debug/kdb/kdb_io.c
@@ -560,12 +560,14 @@ static void kdb_msg_write(char *msg, int msg_len)
if (msg_len == 0)
return;

- if (dbg_io_ops && !dbg_io_ops->is_console)
+ if (dbg_io_ops)
kdb_io_write(msg, 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
--
2.7.4