m68k: nfcon: don't call console_is_registered in nfcon_device

Since 7c2af0f634f1 ("tty: tty_io: use console_list_lock for list synchronization") show_cons_active calls the device method under the console_list_lock, but console_is_registered tries to acquire console_list_lock as well, causing a deadlock. It should not be necessary to check console_is_registered here since the function should not be called in the fist place when the console is not registered.

Fixes 7c2af0f634f1 ("tty: tty_io: use console_list_lock for list synchronization")

Signed-off-by Andreas Schwab <[email protected]>

Quoted 6 lines · Andreas Schwab

Since 7c2af0f634f1 ("tty: tty_io: use console_list_lock for list synchronization") show_cons_active calls the device method under the console_list_lock, but console_is_registered tries to acquire console_list_lock as well, causing a deadlock. It should not be necessary to check console_is_registered here since the function should not be called in the fist place when the console is not registered.

Nice catch!

Quoted 2 lines

Fixes 7c2af0f634f1 ("tty: tty_io: use console_list_lock for list synchronization")

Signed-off-by Andreas Schwab <[email protected]>

Reviewed-by Geert Uytterhoeven <[email protected]>

Hi Andreas,

CC john, petr, serial

Thanks for your patch!

On Sat, 15 Aug 2026 at 09:33, Andreas Schwab <[email protected]> wrote:

Quoted 6 lines

Since 7c2af0f634f1 ("tty: tty_io: use console_list_lock for list synchronization") show_cons_active calls the device method under the console_list_lock, but console_is_registered tries to acquire console_list_lock as well, causing a deadlock. It should not be necessary to check console_is_registered here since the function should not be called in the fist place when the console is not registered.

Nice catch!

Quoted 2 lines

Fixes 7c2af0f634f1 ("tty: tty_io: use console_list_lock for list synchronization")

Signed-off-by Andreas Schwab <[email protected]>

Reviewed-by Geert Uytterhoeven <[email protected]>

i.e. will queue in the m68k tree for v7.3.

arch/m68k/emu/nfcon.c static void nfcon_write(struct console *con, const char *str, · line 52

Quoted 7 lines

arch/m68k/emu/nfcon.c +0 -1

@@ -49,7 +49,7 @@ static void nfcon_write(struct console *con, const char *str,
 static struct tty_driver *nfcon_device(struct console *con, int *index)
 {
        *index = 0;
-       return console_is_registered(con) ? nfcon_tty_driver : NULL;

Interestingly, that call to console_is_registered() was added in commit de61a1a3a0830710 ("tty: nfcon: use console_is_registered()"), i.e. in the same series that added the extra locking...

arch/m68k/emu/nfcon.c static void nfcon_write(struct console *con, const char *str, · line 52

Quoted 1 line

arch/m68k/emu/nfcon.c +1 -0

+       return nfcon_tty_driver;

Quoted 11 lines

arch/m68k/emu/nfcon.c static void nfcon_write(struct console *con, const char *str, · line 52

Quoted 7 lines

arch/m68k/emu/nfcon.c +0 -1

@@ -49,7 +49,7 @@ static void nfcon_write(struct console *con, const char *str,
 static struct tty_driver *nfcon_device(struct console *con, int *index)
 {
        *index = 0;
-       return console_is_registered(con) ? nfcon_tty_driver : NULL;

Interestingly, that call to console_is_registered() was added in commit de61a1a3a0830710 ("tty: nfcon: use console_is_registered()"), i.e. in the same series that added the extra locking...

Sigh, we clearly did not check the callers of this function properly at that time :/

Quoted 1 line

arch/m68k/emu/nfcon.c static void nfcon_write(struct console *con, const char *str, · line 52

Quoted 1 line

arch/m68k/emu/nfcon.c +1 -0

+       return nfcon_tty_driver;

Honestly, I do not feel comfortable with removing the check completely.

The commit de61a1a3a083071 ("tty: nfcon: use console_is_registered()" used console_is_registered() instead of (con->flags & CON_ENABLED). And CON_ENABLED is historically cleared when the console is suspended, see console_suspend().

Is is OK to return a valid struct tty_driver for a non-registered or suspended console for all con->device() callers?

I have found only 4 callers:

  $> git grep "\->device("
  drivers/tty/serial/kgdboc.c:            if (cons->device && cons->device(cons, &idx) == p &&
  drivers/tty/tty_io.c:           struct tty_driver *drv = cs[i]->device(cs[i], &index);
  fs/proc/consoles.c:             driver = con->device(con, &index);
  kernel/printk/printk.c:         driver = c->device(c, index);

These are:

+ configure_kgdboc() in drivers/tty/serial/kgdboc.c
+ show_cons_active() in drivers/tty/tty_io.c
+ show_console_dev() in fs/proc/consoles.c
+ console_device() in kernel/printk/printk.c

, where

+ configure_kgdboc() is used during the system initialization when the console should not be suspended

+ show_cons_active() and show_console_dev() are questionable. Maybe, we should not advertise the device to the userspace when it is suspended.

+ console_device() is used in tty_kopen() -> tty_lookup_driver(). Also here we likely should not allow to open a suspended device.

Let's take a step back:

We should define the locking context for con->device() callback. It seems that it is called under console_lock() and console_srcu_read_lock() in:

+ configure_kgdboc()
+ console_device()

and under console_list_lock() and console_lock() in

+ show_console_dev()
+ show_cons_active()

I would put aside the console_lock(). We are trying to obsolete it. So it is down to

console_srcu_read_lock() vs console_list_lock()

Both of them look reasonable. So, I think about adding a variant of console_is_registered_srcu_read_locked() which would work with both locking. Something like:

arch/m68k/emu/nfcon.c +1 -1

index d41260672e24..d211b6c4da26 100644
@@ -49,7 +49,7 @@ static void nfcon_write(struct console *con, const char *str,
 static struct tty_driver *nfcon_device(struct console *con, int *index)
 {
 	*index = 0;
-	return console_is_registered(con) ? nfcon_tty_driver : NULL;
+	return console_is_registered_srcu_read_locked(con) ? nfcon_tty_driver : NULL;
 }
 
 static struct console nf_console = {

include/linux/console.h +19 -1

index d624200cfc17..b90527db4eca 100644
@@ -311,7 +311,7 @@ struct nbcon_write_context {
  * @name:		The name of the console driver
  * @write:		Legacy write callback to output messages (Optional)
  * @read:		Read callback for console input (Optional)
- * @device:		The underlying TTY device driver (Optional)
+ * @device:		The underlying TTY device driver (Optional) [1]
  * @unblank:		Callback to unblank the console (Optional)
  * @setup:		Callback for initializing the console (Optional)
  * @exit:		Callback for teardown of the console (Optional)

@@ -334,6 +334,9 @@ struct nbcon_write_context {
  * @kthread:		Printer kthread for this console
  * @rcuwait:		RCU-safe wait object for @kthread waking
  * @irq_work:		Defer @kthread waking to IRQ work context
+ *
+ * [1] The @device callback must be called under either console_list_lock()
+ *     or console_srcu_read_lock().
  */
 struct console {
 	char			name[16];

@@ -473,8 +476,13 @@ struct console {
 };
 
 #ifdef CONFIG_LOCKDEP
+extern bool console_list_lock_is_held(void);
 extern void lockdep_assert_console_list_lock_held(void);
 #else
+static inline bool console_list_lock_is_held(void)
+{
+	return 1;
+}
 static inline void lockdep_assert_console_list_lock_held(void)
 {
 }

@@ -555,6 +563,16 @@ static inline bool console_is_registered_locked(const struct console *con)
 	return !hlist_unhashed(&con->node);
 }
 
+/*
+ * Variant of console_is_registered() when either the console_srcu_read_lock
+ * or console_list_lock is held.
+*/
+static inline bool console_is_registered_srcu_read_locked(const struct console *con)
+{
+	lockdep_assert(console_srcu_read_lock_is_held() || console_list_lock_is_held());
+	return !hlist_unhashed(&con->node);
+}
+
 /*
  * console_is_registered - Check if the console is registered
  * @con:	struct console pointer of console to check

kernel/printk/printk.c +6 -0

index 2fe9a963c823..9b632b91f096 100644
@@ -109,6 +109,12 @@ static struct lockdep_map console_lock_dep_map = {
 	.name = "console_lock"
 };
 
+bool console_list_lock_is_held(void)
+{
+	return lockdep_is_held(&console_mutex);
+}
+EXPORT_SYMBOL(console_list_lock_is_held);
+
 void lockdep_assert_console_list_lock_held(void)
 {
 	lockdep_assert_held(&console_mutex);

Alternative solution would be to require the srcu locking. IMHO, it should be perfectly fine to use the srcu_read_lock in both show_console_dev() and show_cons_active().

What do you think, please?
John?

Quoted 20 lines · Petr Mladek

Quoted 11 lines

arch/m68k/emu/nfcon.c static void nfcon_write(struct console *con, const char *str, · line 52

Quoted 7 lines

arch/m68k/emu/nfcon.c +0 -1

@@ -49,7 +49,7 @@ static void nfcon_write(struct console *con, const char *str,
 static struct tty_driver *nfcon_device(struct console *con, int *index)
 {
        *index = 0;
-       return console_is_registered(con) ? nfcon_tty_driver : NULL;

Interestingly, that call to console_is_registered() was added in commit de61a1a3a0830710 ("tty: nfcon: use console_is_registered()"), i.e. in the same series that added the extra locking...

Sigh, we clearly did not check the callers of this function properly at that time :/

Quoted 1 line

arch/m68k/emu/nfcon.c static void nfcon_write(struct console *con, const char *str, · line 52

Quoted 1 line

arch/m68k/emu/nfcon.c +1 -0

+       return nfcon_tty_driver;

Honestly, I do not feel comfortable with removing the check completely.

Quoted 4 lines

The commit de61a1a3a083071 ("tty: nfcon: use console_is_registered()" used console_is_registered() instead of (con->flags & CON_ENABLED). And CON_ENABLED is historically cleared when the console is suspended, see console_suspend().

Since the NatFeat console driver never implemented suspend/resume callbacks, I assume it was intended to identify if the console was registered.

Quoted 27 lines

Is is OK to return a valid struct tty_driver for a non-registered or suspended console for all con->device() callers?

I have found only 4 callers:

  $> git grep "\->device("
  drivers/tty/serial/kgdboc.c:            if (cons->device && cons->device(cons, &idx) == p &&
  drivers/tty/tty_io.c:           struct tty_driver *drv = cs[i]->device(cs[i], &index);
  fs/proc/consoles.c:             driver = con->device(con, &index);
  kernel/printk/printk.c:         driver = c->device(c, index);

These are:

+ configure_kgdboc() in drivers/tty/serial/kgdboc.c
+ show_cons_active() in drivers/tty/tty_io.c
+ show_console_dev() in fs/proc/consoles.c
+ console_device() in kernel/printk/printk.c

, where

+ configure_kgdboc() is used during the system initialization when the console should not be suspended

+ show_cons_active() and show_console_dev() are questionable. Maybe, we should not advertise the device to the userspace when it is suspended.

Quoted 2 lines

+ console_device() is used in tty_kopen() -> tty_lookup_driver(). Also here we likely should not allow to open a suspended device.

All of these callbacks are done when the console is guaranteed to be registered (or not yet fully unregistered).

Quoted 113 lines

Let's take a step back:

We should define the locking context for con->device() callback. It seems that it is called under console_lock() and console_srcu_read_lock() in:

+ configure_kgdboc()
+ console_device()

and under console_list_lock() and console_lock() in

+ show_console_dev()
+ show_cons_active()

I would put aside the console_lock(). We are trying to obsolete it. So it is down to

console_srcu_read_lock() vs console_list_lock()

Both of them look reasonable. So, I think about adding a variant of console_is_registered_srcu_read_locked() which would work with both locking. Something like:

arch/m68k/emu/nfcon.c +1 -1

index d41260672e24..d211b6c4da26 100644
@@ -49,7 +49,7 @@ static void nfcon_write(struct console *con, const char *str,
 static struct tty_driver *nfcon_device(struct console *con, int *index)
 {
 	*index = 0;
-	return console_is_registered(con) ? nfcon_tty_driver : NULL;
+	return console_is_registered_srcu_read_locked(con) ? nfcon_tty_driver : NULL;
 }
 
 static struct console nf_console = {

include/linux/console.h +19 -1

index d624200cfc17..b90527db4eca 100644
@@ -311,7 +311,7 @@ struct nbcon_write_context {
  * @name:		The name of the console driver
  * @write:		Legacy write callback to output messages (Optional)
  * @read:		Read callback for console input (Optional)
- * @device:		The underlying TTY device driver (Optional)
+ * @device:		The underlying TTY device driver (Optional) [1]
  * @unblank:		Callback to unblank the console (Optional)
  * @setup:		Callback for initializing the console (Optional)
  * @exit:		Callback for teardown of the console (Optional)

@@ -334,6 +334,9 @@ struct nbcon_write_context {
  * @kthread:		Printer kthread for this console
  * @rcuwait:		RCU-safe wait object for @kthread waking
  * @irq_work:		Defer @kthread waking to IRQ work context
+ *
+ * [1] The @device callback must be called under either console_list_lock()
+ *     or console_srcu_read_lock().
  */
 struct console {
 	char			name[16];

@@ -473,8 +476,13 @@ struct console {
 };
 
 #ifdef CONFIG_LOCKDEP
+extern bool console_list_lock_is_held(void);
 extern void lockdep_assert_console_list_lock_held(void);
 #else
+static inline bool console_list_lock_is_held(void)
+{
+	return 1;
+}
 static inline void lockdep_assert_console_list_lock_held(void)
 {
 }

@@ -555,6 +563,16 @@ static inline bool console_is_registered_locked(const struct console *con)
 	return !hlist_unhashed(&con->node);
 }
 
+/*
+ * Variant of console_is_registered() when either the console_srcu_read_lock
+ * or console_list_lock is held.
+*/
+static inline bool console_is_registered_srcu_read_locked(const struct console *con)
+{
+	lockdep_assert(console_srcu_read_lock_is_held() || console_list_lock_is_held());
+	return !hlist_unhashed(&con->node);
+}
+
 /*
  * console_is_registered - Check if the console is registered
  * @con:	struct console pointer of console to check

kernel/printk/printk.c +6 -0

index 2fe9a963c823..9b632b91f096 100644
@@ -109,6 +109,12 @@ static struct lockdep_map console_lock_dep_map = {
 	.name = "console_lock"
 };
 
+bool console_list_lock_is_held(void)
+{
+	return lockdep_is_held(&console_mutex);
+}
+EXPORT_SYMBOL(console_list_lock_is_held);
+
 void lockdep_assert_console_list_lock_held(void)
 {
 	lockdep_assert_held(&console_mutex);

Alternative solution would be to require the srcu locking. IMHO, it should be perfectly fine to use the srcu_read_lock in both show_console_dev() and show_cons_active().

Quoted 2 lines

What do you think, please?
John?

I do not think any of that is necessary. I think it is enough to allow the ->device() callback to assume it is registered. I see no reason why the _callback_ should ever need to take the console_list_lock or the console_srcu_read_lock.