Commit 5946d1f5 authored by Sumit Garg's avatar Sumit Garg Committed by Daniel Thompson
Browse files

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: default avatarDaniel Thompson <daniel.thompson@linaro.org>
Signed-off-by: default avatarSumit Garg <sumit.garg@linaro.org>
Link: https://lore.kernel.org/r/1591264879-25920-5-git-send-email-sumit.garg@linaro.org


Reviewed-by: default avatarDouglas Anderson <dianders@chromium.org>
Reviewed-by: default avatarPetr Mladek <pmladek@suse.com>
Acked-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: default avatarDaniel Thompson <daniel.thompson@linaro.org>
parent 2a78b85b

drivers/tty/serial/kgdb_nmi.c

+1 −1
Original line number Diff line number Diff line
@@ -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;

}

drivers/tty/serial/kgdboc.c

+16 −16
Original line number Diff line number Diff line
@@ -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. */

drivers/usb/early/ehci-dbgp.c

+2 −1
Original line number Diff line number Diff line
@@ -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;

}

include/linux/kgdb.h

+2 −3
Original line number Diff line number Diff line
@@ -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;

kernel/debug/kdb/kdb_io.c

+3 −1
Original line number Diff line number Diff line
@@ -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

CRA Git | Maintained and supported by SUSTech CRA and CCSE