[PATCH v3 4/4] kdb: Switch kdb_msg_write() to use safer polling I/O

From: Sumit Garg <sumit.garg@linaro.org>
To: daniel.thompson@linaro.org
Cc: kgdb-bugreport@lists.sourceforge.net, jason.wessel@windriver.com,
	dianders@chromium.org, pmladek@suse.com,
	sergey.senozhatsky@gmail.com, linux-kernel@vger.kernel.org,
	Sumit Garg <sumit.garg@linaro.org>
Subject: [PATCH v3 4/4] kdb: Switch kdb_msg_write() to use safer polling I/O
Date: Wed, 27 May 2020 11:55:59 +0530	[thread overview]
Message-ID: <1590560759-21453-5-git-send-email-sumit.garg@linaro.org> (raw)
In-Reply-To: <1590560759-21453-1-git-send-email-sumit.garg@linaro.org>

In kgdb NMI context, calling console handlers isn't safe due to locks
used in those handlers which could 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.

Suggested-by: Daniel Thompson <daniel.thompson@linaro.org>
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 drivers/tty/serial/kgdboc.c | 17 ++++++++---------
 include/linux/kgdb.h        |  2 ++
 kernel/debug/kdb/kdb_io.c   | 46 +++++++++++++++++++++++++++++++--------------
 3 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
index c9f94fa..6199fe1 100644
--- a/drivers/tty/serial/kgdboc.c
+++ b/drivers/tty/serial/kgdboc.c
@@ -35,7 +35,6 @@ static struct kparam_string kps = {
 };
 
 static int kgdboc_use_kms;  /* 1 if we use kernel mode switching */
-static struct tty_driver	*kgdb_tty_driver;
 static int			kgdb_tty_line;
 
 #ifdef CONFIG_KDB_KEYBOARD
@@ -154,7 +153,7 @@ static int configure_kgdboc(void)
 	}
 
 	kgdboc_io_ops.is_console = 0;
-	kgdb_tty_driver = NULL;
+	kgdboc_io_ops.tty_drv = NULL;
 
 	kgdboc_use_kms = 0;
 	if (strncmp(cptr, "kms,", 4) == 0) {
@@ -178,7 +177,7 @@ static int configure_kgdboc(void)
 		}
 	}
 
-	kgdb_tty_driver = p;
+	kgdboc_io_ops.tty_drv = p;
 	kgdb_tty_line = tty_line;
 
 do_register:
@@ -216,18 +215,18 @@ static int __init init_kgdboc(void)
 
 static int kgdboc_get_char(void)
 {
-	if (!kgdb_tty_driver)
+	if (!kgdboc_io_ops.tty_drv)
 		return -1;
-	return kgdb_tty_driver->ops->poll_get_char(kgdb_tty_driver,
-						kgdb_tty_line);
+	return kgdboc_io_ops.tty_drv->ops->poll_get_char(kgdboc_io_ops.tty_drv,
+							 kgdb_tty_line);
 }
 
 static void kgdboc_put_char(u8 chr)
 {
-	if (!kgdb_tty_driver)
+	if (!kgdboc_io_ops.tty_drv)
 		return;
-	kgdb_tty_driver->ops->poll_put_char(kgdb_tty_driver,
-					kgdb_tty_line, chr);
+	kgdboc_io_ops.tty_drv->ops->poll_put_char(kgdboc_io_ops.tty_drv,
+						  kgdb_tty_line, chr);
 }
 
 static int param_set_kgdboc_var(const char *kmessage,
diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index b072aeb..05d165d 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -275,6 +275,7 @@ struct kgdb_arch {
  * 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
+ * @tty_drv: Pointer to polling tty driver.
  */
 struct kgdb_io {
 	const char		*name;
@@ -285,6 +286,7 @@ struct kgdb_io {
 	void			(*pre_exception) (void);
 	void			(*post_exception) (void);
 	int			is_console;
+	struct tty_driver	*tty_drv;
 };
 
 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 f848482..c2efa52 100644
--- a/kernel/debug/kdb/kdb_io.c
+++ b/kernel/debug/kdb/kdb_io.c
@@ -24,6 +24,7 @@
 #include <linux/kgdb.h>
 #include <linux/kdb.h>
 #include <linux/kallsyms.h>
+#include <linux/tty_driver.h>
 #include "kdb_private.h"
 
 #define CMD_BUFLEN 256
@@ -542,13 +543,18 @@ static int kdb_search_string(char *searched, char *searchfor)
 	return 0;
 }
 
-static void kdb_io_write(char *cp, int len, void (*io_put_char)(u8 ch))
+static void kdb_io_write(char *cp, int len, void (*io_put_char)(u8),
+			 struct tty_driver *p, int line,
+			 void (*poll_put_char)(struct tty_driver *, int, char))
 {
 	if (len <= 0)
 		return;
 
 	while (len--) {
-		io_put_char(*cp);
+		if (io_put_char)
+			io_put_char(*cp);
+		if (poll_put_char)
+			poll_put_char(p, line, *cp);
 		cp++;
 	}
 }
@@ -561,22 +567,34 @@ static void kdb_msg_write(char *msg, int msg_len)
 		return;
 
 	if (dbg_io_ops && !dbg_io_ops->is_console)
-		kdb_io_write(msg, msg_len, dbg_io_ops->write_char);
+		kdb_io_write(msg, msg_len, dbg_io_ops->write_char,
+			     NULL, 0, NULL);
 
 	for_each_console(c) {
+		int line;
+		struct tty_driver *p;
+
 		if (!(c->flags & CON_ENABLED))
 			continue;
-		/*
-		 * While rounding up CPUs via NMIs, its possible that
-		 * a rounded up CPU maybe holding a console port lock
-		 * leading to kgdb master CPU stuck in a deadlock during
-		 * invocation of console write operations. So in order
-		 * to avoid such a deadlock, enable oops_in_progress
-		 * prior to invocation of console handlers.
-		 */
-		++oops_in_progress;
-		c->write(c, msg, msg_len);
-		--oops_in_progress;
+
+		p = c->device ? c->device(c, &line) : NULL;
+		if (p && dbg_io_ops && p == dbg_io_ops->tty_drv && p->ops &&
+		    p->ops->poll_put_char) {
+			kdb_io_write(msg, msg_len, NULL, p, line,
+				     p->ops->poll_put_char);
+		} else {
+			/*
+			 * While rounding up CPUs via NMIs, its possible that
+			 * a rounded up CPU maybe holding a console port lock
+			 * leading to kgdb master CPU stuck in a deadlock during
+			 * invocation of console write operations. So in order
+			 * to avoid such a deadlock, enable oops_in_progress
+			 * prior to invocation of console handlers.
+			 */
+			++oops_in_progress;
+			c->write(c, msg, msg_len);
+			--oops_in_progress;
+		}
 		touch_nmi_watchdog();
 	}
 }
-- 
2.7.4

