serial: core: shut down initialized port on removal

When a serial port configured as a console is opened and subsequently closed, tty_port_shutdown() skips invoking port->ops->shutdown() because port->console is true. As a result, tty_port_initialized() remains true and the port's interrupt handler stays registered in the irq subsystem.

If the underlying device is later unbound or removed (e.g. via sysfs unbind), serial_core_remove_one_port() unregisters the console, frees uport->name with kfree(), and clears state->uart_port without ever shutting down the port or freeing its IRQ. Consequently, the irqaction remains linked in the genirq descriptor with action->name pointing to freed memory.

When another device later requests the same IRQ line, __setup_irq() encounters the stale action, detects a flags mismatch, and attempts to print old->name in pr_err(), triggering a KASAN use-after-free read:

  BUG: KASAN: slab-use-after-free in string_nocheck lib/vsprintf.c:648
  BUG: KASAN: slab-use-after-free in string+0x471/0x4d0 lib/vsprintf.c:730
  Read of size 1 at addr ffff88802643b8a0 by task syz.0.818/8415
  Call Trace:
   <TASK>
   string_nocheck lib/vsprintf.c:648
   string+0x471/0x4d0 lib/vsprintf.c:730
   vsnprintf+0x422/0x1300 lib/vsprintf.c:2949
   vprintk_store+0x3b3/0xbe0 kernel/printk/printk.c:2307
   vprintk_emit+0x139/0x6b0 kernel/printk/printk.c:2455
   _printk+0xcf/0x110 kernel/printk/printk.c:2504
   __setup_irq.cold+0x3be/0x3f1 kernel/irq/manage.c:1821
   request_threaded_irq+0x261/0x3e0 kernel/irq/manage.c:2184
   pcl812_attach+0x1b62/0x2300 drivers/comedi/drivers/pcl812.c:1174
   ...

Fix this by clearing port->console after unregistering the console and calling uart_shutdown(NULL, state) under port->mutex if the port remains initialized, ensuring the interrupt and hardware resources are freed before kfree(uport->name).

---
 drivers/tty/serial/serial_core.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

index 95774b0f1484..030735da6b8c 100644
@@ -3210,8 +3210,15 @@ static void serial_core_remove_one_port(struct uart_driver *drv,
 	/*
 	 * If the port is used as a console, unregister it
 	 */
-	if (uart_console(uport))
+	if (uart_console(uport)) {
 		unregister_console(uport->cons);
+		port->console = false;
+	}
+
+	guard(mutex)(&port->mutex);
+
+	if (tty_port_initialized(port))
+		uart_shutdown(NULL, state);
 
 	/*
 	 * Free the port IO and memory resources, if any.

@@ -3227,7 +3234,6 @@ static void serial_core_remove_one_port(struct uart_driver *drv,
 	uport->type = PORT_UNKNOWN;
 	uport->port_dev = NULL;
 
-	guard(mutex)(&port->mutex);
 	WARN_ON(atomic_dec_return(&state->refcount) < 0);
 	wait_event(state->remove_wait, !atomic_read(&state->refcount));
 	state->uart_port = NULL;

2.47.3

How can a different device request the same irq line on a real device?

How was this all tested, with "fake" devices?

thanks,

greg k-h

Hi Greg,
Note: syzbot did not actually provide a reproducer for this crash - this patch is based on code analysis of the trace, not on hardware testing. The pcl812 trigger from syzkaller is synthetic.

On a console port, tty_port_shutdown() skips ops->shutdown() because port->console is true, so the IRQ is never released.
If the device is later unbound/removed, serial_core_remove_one_port() frees uport->name via kfree(), but the IRQ is never freed.
Result: a stale irqaction pointing at freed memory in action->name, even without anything else requesting that IRQ line.

sashiko-bot also pointed out a data race on port->console = false outside the mutex - separate issue, but relevant.

If the underlying problem looks real to you, I'd rather withdraw this patch, put together an actual reproducer (unbind a console port, check for the stale irqaction), fix the locking properly, and send a v2.

thanks, igor

Please do so, thanks.

greg k-h

When a serial port configured as a console is opened and subsequently closed, tty_port_shutdown() skips invoking port->ops->shutdown() because port->console is true. As a result, tty_port_initialized() remains true and the port's interrupt handler stays registered in the irq subsystem.

If the underlying device is later unbound or removed (e.g. via sysfs driver unbind or device hot-unplug), serial_core_remove_one_port() unregisters the console, frees uport->name via kfree(), and tears down the port without shutting it down or freeing its IRQ. Consequently, the irqaction descriptor remains registered in genirq with action->name pointing to freed memory.

This triggers a use-after-free read whenever genirq accesses action->name, such as when reading /proc/interrupts:

  BUG: KASAN: slab-use-after-free in string+0x476/0x570
  Call Trace:
   string+0x476/0x570
   vsnprintf+0x422/0x1300
   seq_printf+0x...
   show_interrupts+...

Fix this by acquiring port->mutex in serial_core_remove_one_port() after console unregistration, clearing port->console under the mutex to prevent data races with concurrent tty_port_shutdown(), and invoking uart_shutdown(NULL, state) if the port remains initialized. This ensures that port->ops->shutdown() is called and the IRQ is freed before kfree(uport->name).

---
v1 -> v2:
- Clear port->console under port->mutex using scoped_guard() to fix the data race pointed out by sashiko-bot.
- Verified without synthetic devices by reproducing via sysfs driver unbind (00:04 serial pnp device) and inspecting /proc/interrupts.
- Do not hold port->mutex across release_port() to preserve existing locking semantics.

Reproducer steps (tested on QEMU with console=ttyS0):
1. Open and close the console port:
exec 3</dev/ttyS0 && exec 3<&-
2. Unbind the serial device:
echo "00:04" > /sys/bus/pnp/drivers/serial/unbind
3. Read interrupts:
cat /proc/interrupts
-> triggers KASAN use-after-free in show_interrupts() without this fix.

 drivers/tty/serial/serial_core.c | 7 +++++++
 1 file changed, 7 insertions(+)

index 95774b0f1..f6253cd30 100644
@@ -3213,6 +3213,13 @@ static void serial_core_remove_one_port(struct uart_driver *drv,
 	if (uart_console(uport))
 		unregister_console(uport->cons);
 
+	scoped_guard(mutex, &port->mutex) {
+		port->console = false;
+
+		if (tty_port_initialized(port))
+			uart_shutdown(NULL, state);
+	}
+
 	/*
 	 * Free the port IO and memory resources, if any.
 	 */

2.53.0

I sent a patch for the same bug [1] without having seen this one. I ran my reproducers on tty-next 36844ea19656 plus this commit (810d467c11f0 in tty-testing), in a QEMU guest with KASAN. The 8250_pci case with INTx is fixed: the IRQ is freed on unbind, reading /proc/interrupts is clean and rebinding restores the console. It also fixes a second symptom, on a polled port (irq 0, UPF_IOREMAP), where serial8250_timeout() kept running after removal and dereferenced the cleared membase. A port held open across removal and a console that was never opened still work.

The patch uses scoped_guard(), which exists for mutexes from v6.5, so it won't build on the 6.1, 5.15 and 5.10 stable trees. If you don't plan to send backports for those, I can.

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

Thanks,
Phil