tty: Serialize saved termios access with device registration

PATCH In review Chengfeng Ye 3 participants 4 messages Sep 26 – Sep 30

Subsystems (2)

Stable branch TTY layer and serial drivers

lore

Share & tools

lore

Chengfeng Ye

author

2026-09-26 18:42 UTC #0 lore raw

PATCH 1/1 tty: Serialize saved termios access with device registration

tty_register_device_attr() frees saved termios data when reusing a minor number without serializing with tty_init_termios() or tty_save_termios(). The tty_mutex held by the open and close paths does not protect against this registration-side free.

For example, an n_gsm mux reconfiguration can register a device while its previous tty is being released. tty_save_termios() loads the saved pointer, registration clears the array entry and frees the object, and tty_save_termios() then writes through its stale pointer. The saved-termios copy in tty_init_termios() is vulnerable to the same lifetime race.

KASAN reported:

  BUG: KASAN: slab-use-after-free in tty_save_termios+0x39a/0x3d0
  Write of size 44 at addr ffff8881012e8180 by task poc/114

  Call Trace:
   tty_save_termios+0x39a/0x3d0
   release_tty+0xb3/0x7a0
   tty_release_struct+0xa0/0xd0
   tty_release+0xc1b/0x11b0
   __fput+0x2f8/0x9e0
   fput_close_sync+0xe2/0x190
   __x64_sys_close+0x78/0xd0

  Allocated by task 110:
   tty_save_termios+0x2c1/0x3d0
   release_tty+0xb3/0x7a0
   tty_release_struct+0xa0/0xd0
   tty_release+0xc1b/0x11b0

  Freed by task 113:
   kfree+0x131/0x3c0
   tty_register_device_attr+0x498/0x8b0
   gsm_activate_mux+0xfb/0x210
   gsmld_ioctl+0x92f/0x14d0
   tty_ioctl+0x915/0x1240
   __x64_sys_ioctl+0x134/0x1c0

Serialize the saved-termios copies and reset with a private mutex. Taking tty_mutex during registration would invert existing driver lock ordering:
UART registration holds port->mutex, while tty_find_polling_driver() holds tty_mutex when calling uart_poll_init(), which takes port->mutex. Keep the new lock confined to the saved-termios operations, with no driver callbacks inside the critical sections.

Fixes 93857edd9829 ("tty: reset termios state on device registration")

Cc [email protected]

Assisted-by GPT-6-Astra

Signed-off-by Chengfeng Ye <[email protected]>

---
 drivers/tty/tty_io.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

drivers/tty/tty_io.c +11 -4

index 48569035da56..26bdc4560a4c 100644
@@ -143,6 +143,7 @@ LIST_HEAD(tty_drivers);			/* linked list of tty drivers */
 
 /* Mutex to protect creating and releasing a tty */
 DEFINE_MUTEX(tty_mutex);
+static DEFINE_MUTEX(tty_termios_mutex);
 
 static ssize_t tty_read(struct kiocb *, struct iov_iter *);
 static ssize_t tty_write(struct kiocb *, struct iov_iter *);

@@ -1219,6 +1220,8 @@ void tty_init_termios(struct tty_struct *tty)
 	if (tty->driver->flags & TTY_DRIVER_RESET_TERMIOS)
 		tty->termios = tty->driver->init_termios;
 	else {
+		guard(mutex)(&tty_termios_mutex);
+
 		/* Check for lazy saved data */
 		tp = tty->driver->termios[idx];
 		if (tp != NULL) {

@@ -1441,6 +1444,8 @@ void tty_save_termios(struct tty_struct *tty)
 	if (tty->driver->flags & TTY_DRIVER_RESET_TERMIOS)
 		return;
 
+	guard(mutex)(&tty_termios_mutex);
+
 	/* Stash the termios data */
 	tp = tty->driver->termios[idx];
 	if (tp == NULL) {

@@ -3242,10 +3247,12 @@ struct device *tty_register_device_attr(struct tty_driver *driver,
 		 * Free any saved termios data so that the termios state is
 		 * reset when reusing a minor number.
 		 */
-		tp = driver->termios[index];
-		if (tp) {
-			driver->termios[index] = NULL;
-			kfree(tp);
+		scoped_guard(mutex, &tty_termios_mutex) {
+			tp = driver->termios[index];
+			if (tp) {
+				driver->termios[index] = NULL;
+				kfree(tp);
+			}
 		}
 
 		retval = tty_cdev_add(driver, devt, index, 1);

Signature · 1 line

2.43.0

[email protected]

AI review

2026-09-26 18:52 UTC #1 lore raw

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] tty: mixing goto-based error handling with scoped cleanup helpers --

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] tty: mixing goto-based error handling with scoped cleanup helpers --

commit 6bd1bebdb72fb484cd4c55333cfb5b66866fd06e
Author: Chengfeng Ye <[email protected]>

tty: Serialize saved termios access with device registration

Quoted 4 lines

drivers/tty/tty_io.c +0 -0
index 48569035da56e..26bdc4560a4c8 100644

[ ... ]

Quoted 7 lines

drivers/tty/tty_io.c +0 -3

@@ -3242,10 +3247,12 @@ struct device *tty_register_device_attr(struct tty_driver *driver,
 		 * Free any saved termios data so that the termios state is
 		 * reset when reusing a minor number.
 		 */
-		tp = driver->termios[index];
-		if (tp) {
-			driver->termios[index] = NULL;

drivers/tty/tty_io.c struct device *tty_register_device_attr(struct tty_driver *driver, · line 3258

Quoted 10 lines

drivers/tty/tty_io.c +6 -1

-			kfree(tp);
+		scoped_guard(mutex, &tty_termios_mutex) {
+			tp = driver->termios[index];
+			if (tp) {
+				driver->termios[index] = NULL;
+				kfree(tp);
+			}
 		}
 
 		retval = tty_cdev_add(driver, devt, index, 1);

[Severity: Low]
Does this violate the guidelines against mixing goto-based error handling with scope-based cleanup helpers?

tty_register_device_attr() already uses explicit goto labels for error cleanup:

err_del:

	device_del(dev);
err_put:
	put_device(dev);

Mixing these paradigms can create confusing ownership semantics and increases the risk of resource leaks or double-free bugs during future refactorings.

Could the function be completely converted to scope-based cleanup first, or could this patch use traditional locking (mutex_lock/mutex_unlock) instead?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Johan Hovold

2026-09-30 09:20 UTC #2 lore raw

In reply to Chengfeng Ye

Quoted 9 lines · Chengfeng Ye

tty_register_device_attr() frees saved termios data when reusing a minor number without serializing with tty_init_termios() or tty_save_termios(). The tty_mutex held by the open and close paths does not protect against this registration-side free.
For example, an n_gsm mux reconfiguration can register a device while its previous tty is being released. tty_save_termios() loads the saved pointer, registration clears the array entry and frees the object, and tty_save_termios() then writes through its stale pointer.

Thanks for reporting this. The intention here was that the TTY drivers would not be reusing a minor number while it is still in use, but that is clearly not adhered to everywhere.

Quoted 2 lines

The saved-termios
copy in tty_init_termios() is vulnerable to the same lifetime race.

How could this happen? TTY drivers should hang up their ports during deregistration so this shouldn't be an issue unless we have a driver bug (which we do in n_gsm apparently).

Quoted 28 lines

KASAN reported:

  BUG: KASAN: slab-use-after-free in tty_save_termios+0x39a/0x3d0
  Write of size 44 at addr ffff8881012e8180 by task poc/114

  Call Trace:
   tty_save_termios+0x39a/0x3d0
   release_tty+0xb3/0x7a0
   tty_release_struct+0xa0/0xd0
   tty_release+0xc1b/0x11b0
   __fput+0x2f8/0x9e0
   fput_close_sync+0xe2/0x190
   __x64_sys_close+0x78/0xd0

  Allocated by task 110:
   tty_save_termios+0x2c1/0x3d0
   release_tty+0xb3/0x7a0
   tty_release_struct+0xa0/0xd0
   tty_release+0xc1b/0x11b0

  Freed by task 113:
   kfree+0x131/0x3c0
   tty_register_device_attr+0x498/0x8b0
   gsm_activate_mux+0xfb/0x210
   gsmld_ioctl+0x92f/0x14d0
   tty_ioctl+0x915/0x1240
   __x64_sys_ioctl+0x134/0x1c0

Quoted 6 lines

Serialize the saved-termios copies and reset with a private mutex. Taking tty_mutex during registration would invert existing driver lock ordering:
UART registration holds port->mutex, while tty_find_polling_driver() holds tty_mutex when calling uart_poll_init(), which takes port->mutex. Keep the new lock confined to the saved-termios operations, with no driver callbacks inside the critical sections.

I'm a bit torn about this, but I think we should avoid adding another mutex in favouring of making the saved termios reset opt-in.

Clearing the saved termios state on device registration doesn't make much sense if a minor number may still be in use as an open TTY would prevent the termios from being reset.

I've just posted a patch for this here:

	https://lore.kernel.org/r/[email protected]

Johan

Chengfeng Ye

author

2026-09-30 16:27 UTC #3 lore raw

Hi Johan,

Quoted 16 lines · Johan Hovold

Quoted 9 lines · Chengfeng Ye
tty_register_device_attr() frees saved termios data when reusing a minor number without serializing with tty_init_termios() or tty_save_termios(). The tty_mutex held by the open and close paths does not protect against this registration-side free.
For example, an n_gsm mux reconfiguration can register a device while its previous tty is being released. tty_save_termios() loads the saved pointer, registration clears the array entry and frees the object, and tty_save_termios() then writes through its stale pointer.
Thanks for reporting this. The intention here was that the TTY drivers would not be reusing a minor number while it is still in use, but that is clearly not adhered to everywhere.

Quoted 6 lines

Quoted 2 lines
The saved-termios
copy in tty_init_termios() is vulnerable to the same lifetime race.
How could this happen? TTY drivers should hang up their ports during deregistration so this shouldn't be an issue unless we have a driver bug (which we do in n_gsm apparently).

Right, I did not reproduce or otherwise establish a valid interleaving involving tty_init_termios(), and the KASAN report only demonstrates the use-after-free in tty_save_termios(). That statement came from an LLM-generated draft, sorry for the confusion.

Quoted 42 lines

Quoted 28 lines
KASAN reported:
  BUG: KASAN: slab-use-after-free in tty_save_termios+0x39a/0x3d0
  Write of size 44 at addr ffff8881012e8180 by task poc/114

  Call Trace:
   tty_save_termios+0x39a/0x3d0
   release_tty+0xb3/0x7a0
   tty_release_struct+0xa0/0xd0
   tty_release+0xc1b/0x11b0
   __fput+0x2f8/0x9e0
   fput_close_sync+0xe2/0x190
   __x64_sys_close+0x78/0xd0

  Allocated by task 110:
   tty_save_termios+0x2c1/0x3d0
   release_tty+0xb3/0x7a0
   tty_release_struct+0xa0/0xd0
   tty_release+0xc1b/0x11b0

  Freed by task 113:
   kfree+0x131/0x3c0
   tty_register_device_attr+0x498/0x8b0
   gsm_activate_mux+0xfb/0x210
   gsmld_ioctl+0x92f/0x14d0
   tty_ioctl+0x915/0x1240
   __x64_sys_ioctl+0x134/0x1c0
Quoted 6 lines
Serialize the saved-termios copies and reset with a private mutex. Taking tty_mutex during registration would invert existing driver lock ordering:
UART registration holds port->mutex, while tty_find_polling_driver() holds tty_mutex when calling uart_poll_init(), which takes port->mutex. Keep the new lock confined to the saved-termios operations, with no driver callbacks inside the critical sections.
I'm a bit torn about this, but I think we should avoid adding another mutex in favouring of making the saved termios reset opt-in.
Clearing the saved termios state on device registration doesn't make much sense if a minor number may still be in use as an open TTY would prevent the termios from being reset.

Quoted 5 lines

I've just posted a patch for this here:
        https://lore.kernel.org/r/[email protected]
Johan

Thank you for preparing the fix. Making the saved-termios reset opt-in is a much cleaner way to address the demonstrated issue.

Best regards,
Chengfeng