tty: serial: kgdboc: fix mutex locking order for configure_kgdboc()

Several mutexes are taken while setting up console serial ports. In
particular, the tty_port->mutex and @console_mutex are taken:

  serial_pnp_probe
    serial8250_register_8250_port
      uart_add_one_port (locks tty_port->mutex)
        uart_configure_port
          register_console (locks @console_mutex)

In order to synchronize kgdb's tty_find_polling_driver() with
register_console(), commit 6193bc9084 ("tty: serial: kgdboc:
synchronize tty_find_polling_driver() and register_console()") takes
the @console_mutex. However, this leads to the following call chain
(with locking):

  platform_probe
    kgdboc_probe
      configure_kgdboc (locks @console_mutex)
        tty_find_polling_driver
          uart_poll_init (locks tty_port->mutex)
            uart_set_options

This is clearly deadlock potential due to the reverse lock ordering.

Since uart_set_options() requires holding @console_mutex in order to
serialize early initialization of the serial-console lock, take the
@console_mutex in uart_poll_init() instead of configure_kgdboc().

Since configure_kgdboc() was using @console_mutex for safe traversal
of the console list, change it to use the SRCU iterator instead.

Add comments to uart_set_options() kerneldoc mentioning that it
requires holding @console_mutex (aka the console_list_lock).

Fixes: 6193bc9084 ("tty: serial: kgdboc: synchronize tty_find_polling_driver() and register_console()")
Signed-off-by: John Ogness <john.ogness@linutronix.de>
Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Reviewed-by: Petr Mladek <pmladek@suse.com>
[pmladek@suse.com: Export console_srcu_read_lock_is_held() to fix build kgdboc as a module.]
Signed-off-by: Petr Mladek <pmladek@suse.com>
Link: https://lore.kernel.org/r/20230112161213.1434854-1-john.ogness@linutronix.de
This commit is contained in:
John Ogness 2023-01-12 17:18:13 +01:06 committed by Petr Mladek
parent 5074ffbec6
commit 3ef5abd9b5
3 changed files with 11 additions and 15 deletions

View File

@ -171,6 +171,7 @@ static int configure_kgdboc(void)
int err = -ENODEV; int err = -ENODEV;
char *cptr = config; char *cptr = config;
struct console *cons; struct console *cons;
int cookie;
if (!strlen(config) || isspace(config[0])) { if (!strlen(config) || isspace(config[0])) {
err = 0; err = 0;
@ -189,20 +190,9 @@ static int configure_kgdboc(void)
if (kgdboc_register_kbd(&cptr)) if (kgdboc_register_kbd(&cptr))
goto do_register; goto do_register;
/*
* tty_find_polling_driver() can call uart_set_options()
* (via poll_init) to configure the uart. Take the console_list_lock
* in order to synchronize against register_console(), which can also
* configure the uart via uart_set_options(). This also allows safe
* traversal of the console list.
*/
console_list_lock();
p = tty_find_polling_driver(cptr, &tty_line); p = tty_find_polling_driver(cptr, &tty_line);
if (!p) { if (!p)
console_list_unlock();
goto noconfig; goto noconfig;
}
/* /*
* Take console_lock to serialize device() callback with * Take console_lock to serialize device() callback with
@ -211,7 +201,8 @@ static int configure_kgdboc(void)
*/ */
console_lock(); console_lock();
for_each_console(cons) { cookie = console_srcu_read_lock();
for_each_console_srcu(cons) {
int idx; int idx;
if (cons->device && cons->device(cons, &idx) == p && if (cons->device && cons->device(cons, &idx) == p &&
idx == tty_line) { idx == tty_line) {
@ -219,11 +210,10 @@ static int configure_kgdboc(void)
break; break;
} }
} }
console_srcu_read_unlock(cookie);
console_unlock(); console_unlock();
console_list_unlock();
kgdb_tty_driver = p; kgdb_tty_driver = p;
kgdb_tty_line = tty_line; kgdb_tty_line = tty_line;

View File

@ -2212,6 +2212,9 @@ EXPORT_SYMBOL_GPL(uart_parse_options);
* @parity: parity character - 'n' (none), 'o' (odd), 'e' (even) * @parity: parity character - 'n' (none), 'o' (odd), 'e' (even)
* @bits: number of data bits * @bits: number of data bits
* @flow: flow control character - 'r' (rts) * @flow: flow control character - 'r' (rts)
*
* Locking: Caller must hold console_list_lock in order to serialize
* early initialization of the serial-console lock.
*/ */
int int
uart_set_options(struct uart_port *port, struct console *co, uart_set_options(struct uart_port *port, struct console *co,
@ -2619,7 +2622,9 @@ static int uart_poll_init(struct tty_driver *driver, int line, char *options)
if (!ret && options) { if (!ret && options) {
uart_parse_options(options, &baud, &parity, &bits, &flow); uart_parse_options(options, &baud, &parity, &bits, &flow);
console_list_lock();
ret = uart_set_options(port, NULL, baud, parity, bits, flow); ret = uart_set_options(port, NULL, baud, parity, bits, flow);
console_list_unlock();
} }
out: out:
mutex_unlock(&tport->mutex); mutex_unlock(&tport->mutex);

View File

@ -123,6 +123,7 @@ bool console_srcu_read_lock_is_held(void)
{ {
return srcu_read_lock_held(&console_srcu); return srcu_read_lock_held(&console_srcu);
} }
EXPORT_SYMBOL(console_srcu_read_lock_is_held);
#endif #endif
enum devkmsg_log_bits { enum devkmsg_log_bits {