[PATCH v2] serial: core: Shut down a running console port before removing it

[PATCH v2] serial: core: Shut down a running console port before removing it

[Phil Rosenthal via B4 Relay]

From: Phil Rosenthal <phil@phil.gs>

A console port is not shut down when its last user closes it:
tty_port_shutdown() returns early when tty_port::console is set, so the
port stays initialized.  For an 8250 port this leaves its interrupt
requested or, for a port without an interrupt, its poll timer armed.

serial_core_remove_one_port() relies on tty_port_tty_vhangup() to shut
the port down before releasing its resources, but the hangup reaches
uart_hangup() -> uart_shutdown() only when a tty is attached.  For a
console that was opened and closed, for example a serial console whose
getty has been stopped, nothing shuts the port down.  The console is
unregistered, ->release_port() releases the registers and the core frees
uport->name while the port is still running.  On tty-next with 8250
ports:

 - A card bound to 8250_pci with an INTx interrupt and unbound through
   sysfs keeps its interrupt requested after the driver is gone.  The
   irqaction name points to the freed uport->name, so reading
   /proc/interrupts is a slab-use-after-free in irq_seq_show().

 - A polled (irq = 0) MMIO port with UPF_IOREMAP keeps running
   serial8250_timeout() after serial8250_release_port() has cleared
   membase, which dereferences NULL in timer softirq context about
   10 ms after "console [ttyS5] disabled".

After unregistering the console and before releasing the resources, call
uart_shutdown().  It runs with port->mutex held, as for its other
callers, and with no tty, so it also drops DTR and RTS.  For a port that
is not initialized, because the hangup already shut it down or because it
is suspended, uart_shutdown() leaves the hardware alone and only releases
what the core still holds, such as the transmit buffer of a suspended
port.

Fixes: 761ed4a94582 ("tty: serial_core: convert uart_close to use tty_port_close")
Cc: stable@vger.kernel.org # 6.6.x
Assisted-by: LLM sparse
Signed-off-by: Phil Rosenthal <phil@phil.gs>
---
The bug goes back to v4.9 (761ed4a94582), but scoped_guard() for
mutexes was only added in v6.5, so this version doesn't build on the
6.1, 5.15 and 5.10 stable trees.  For those I'll send a backport that
uses mutex_lock() and mutex_unlock() instead and is otherwise the same.

I hit the bug on real hardware.  My serial console is a polled ASPEED
BMC VUART, then driven by an out-of-tree module.  After I stopped its
getty, unloading the driver oopsed in mem32_serial_in() from
serial8250_timeout().

History: before 761ed4a94582, uart_close() called uart_shutdown() for
every port, consoles included.  761ed4a94582 made console ports skip the
shutdown on close.  be2c92b8f164 reverted that part and 4dda864d7307
reinstated it, all within v4.9.  Keeping a console running after close is
intended, but the removal path was never updated for it.

Tested in a QEMU guest on tty-next 36844ea19656 with KASAN, using in-tree
8250_pci and a "serial8250" platform port.  Reproducers are available on
request.  Without this patch:

  8250_pci with INTx, reading /proc/interrupts after unbind:
    BUG: KASAN: slab-use-after-free in string+0x3b6/0x4c0
     irq_seq_show+0x34d/0x810
    Allocated by task 1244:
     kasprintf+0xac/0x100
     serial_core_register_port+0x55f/0x1720
    Freed by task 1276:
     kfree+0x1a0/0x4a0
     serial_core_unregister_port+0x44d/0xa80
     serial8250_unregister_port+0x10d/0x6c0
     pciserial_detach_ports+0x92/0x160

  Polled MMIO port (irq 0, UPF_IOREMAP) after removal:
    printk: console [ttyS5] disabled
    BUG: kernel NULL pointer dereference, address: 0000000000000008
    RIP: 0010:mem32_serial_in+0x74/0xa0
     serial8250_timeout+0x44/0x140
     call_timer_fn+0x32/0x260

With this patch both cases are clean: the IRQ is freed on unbind,
rebinding restores the console, and the polled port is shut down on
removal.  A port held open across removal and a console that was never
opened did not fail before and still work.  v2 was retested the same
way.  W=1, sparse and checkpatch --strict are clean.

Also tested on real hardware: the ASPEED BMC VUART that hit the bug
(polled MMIO, irq 0) as the console, now bound to 8250_pci with my
pending ASPEED support patch [1], on a 7.0.14 distribution kernel with
both patches.  After stopping the getty, opening and closing the port,
and unbinding the PCI device, there is no oops.  Rebinding restores the
console, the getty and in-band IPMI.  That run used v1; v2 behaves the
same on that path.

Not tested: serial drivers other than 8250, and removing a port while it
is suspended (the case v2 adds).

[1] https://lore.kernel.org/r/20261008-aspeed-vuart-v1-1-d2be17c99cd7@phil.gs

AI assistance: an LLM coding assistant (Claude, model claude-opus-5-5, in
Claude Code) analysed the crash, wrote the reproducers and the patch, and
built and ran the test kernels.  Two other LLMs reviewed the result
(GPT-5.6 Sol via Codex, and Claude Fable 5.1 via Claude Code).  I reviewed
it as well.
---
Changes in v2:
- Call uart_shutdown() without checking tty_port_initialized() first,
  so that a console port removed while suspended also gets its transmit
  buffer freed and its suspended flag cleared (found by Sashiko's
  review of v1).  uart_shutdown() already skips the hardware shutdown
  for a port that is not initialized.  Comment and changelog updated.
- Use scoped_guard() for port->mutex, as elsewhere in the function, and
  drop the comment (Greg).
- Link to v1: https://lore.kernel.org/r/20261008-serial-console-remove-v1-1-288b9c315af7@phil.gs
---
 drivers/tty/serial/serial_core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 319a4b427f3a3727d12cb58252bd8959f9be4cd6..d96d84a985cd972f3974da28cf3daa7fc05eefcf 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -3239,6 +3239,9 @@ static void serial_core_remove_one_port(struct uart_driver *drv,
 	if (uart_console(uport))
 		unregister_console(uport->cons);
 
+	scoped_guard(mutex, &port->mutex)
+		uart_shutdown(NULL, state);
+
 	/*
 	 * Free the port IO and memory resources, if any.
 	 */

---
base-commit: 36844ea19656fb41278799ef3d5cd52120d89149
change-id: 20261008-serial-console-remove-d9ef4da86e0c

Best regards,
-- 
Phil Rosenthal <phil@phil.gs>

Re: [PATCH v2] serial: core: Shut down a running console port before removing it

[sashiko-bot]

> From: Phil Rosenthal <phil@phil.gs>
> 
> A console port is not shut down when its last user closes it:
> tty_port_shutdown() returns early when tty_port::console is set, so the
> port stays initialized.  For an 8250 port this leaves its interrupt
> requested or, for a port without an interrupt, its poll timer armed.
> 
> serial_core_remove_one_port() relies on tty_port_tty_vhangup() to shut
> the port down before releasing its resources, but the hangup reaches
> uart_hangup() -> uart_shutdown() only when a tty is attached.  For a
> console that was opened and closed, for example a serial console whose
> getty has been stopped, nothing shuts the port down.  The console is
> unregistered, ->release_port() releases the registers and the core frees
> uport->name while the port is still running.  On tty-next with 8250
> ports:
> [ ... ]
> 
> Fixes: 761ed4a94582 ("tty: serial_core: convert uart_close to use tty_port_close")
> Cc: stable@vger.kernel.org # 6.6.x
> Assisted-by: LLM sparse
> Signed-off-by: Phil Rosenthal <phil@phil.gs>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013-serial-console-remove-v2-1-2eca1a92c10d@phil.gs?part=1

Re: [PATCH v2] serial: core: Shut down a running console port before removing it

[Greg Kroah-Hartman]

On Tue, Oct 13, 2026 at 10:07:16AM -0400, Phil Rosenthal via B4 Relay wrote:
> From: Phil Rosenthal <phil@phil.gs>
> 
> A console port is not shut down when its last user closes it:
> tty_port_shutdown() returns early when tty_port::console is set, so the
> port stays initialized.  For an 8250 port this leaves its interrupt
> requested or, for a port without an interrupt, its poll timer armed.
> 
> serial_core_remove_one_port() relies on tty_port_tty_vhangup() to shut
> the port down before releasing its resources, but the hangup reaches
> uart_hangup() -> uart_shutdown() only when a tty is attached.  For a
> console that was opened and closed, for example a serial console whose
> getty has been stopped, nothing shuts the port down.  The console is
> unregistered, ->release_port() releases the registers and the core frees
> uport->name while the port is still running.  On tty-next with 8250
> ports:
> 
>  - A card bound to 8250_pci with an INTx interrupt and unbound through
>    sysfs keeps its interrupt requested after the driver is gone.  The
>    irqaction name points to the freed uport->name, so reading
>    /proc/interrupts is a slab-use-after-free in irq_seq_show().
> 
>  - A polled (irq = 0) MMIO port with UPF_IOREMAP keeps running
>    serial8250_timeout() after serial8250_release_port() has cleared
>    membase, which dereferences NULL in timer softirq context about
>    10 ms after "console [ttyS5] disabled".
> 
> After unregistering the console and before releasing the resources, call
> uart_shutdown().  It runs with port->mutex held, as for its other
> callers, and with no tty, so it also drops DTR and RTS.  For a port that
> is not initialized, because the hangup already shut it down or because it
> is suspended, uart_shutdown() leaves the hardware alone and only releases
> what the core still holds, such as the transmit buffer of a suspended
> port.
> 
> Fixes: 761ed4a94582 ("tty: serial_core: convert uart_close to use tty_port_close")
> Cc: stable@vger.kernel.org # 6.6.x
> Assisted-by: LLM sparse
> Signed-off-by: Phil Rosenthal <phil@phil.gs>
> ---
> The bug goes back to v4.9 (761ed4a94582), but scoped_guard() for
> mutexes was only added in v6.5, so this version doesn't build on the
> 6.1, 5.15 and 5.10 stable trees.  For those I'll send a backport that
> uses mutex_lock() and mutex_unlock() instead and is otherwise the same.
> 
> I hit the bug on real hardware.  My serial console is a polled ASPEED
> BMC VUART, then driven by an out-of-tree module.  After I stopped its
> getty, unloading the driver oopsed in mem32_serial_in() from
> serial8250_timeout().
> 
> History: before 761ed4a94582, uart_close() called uart_shutdown() for
> every port, consoles included.  761ed4a94582 made console ports skip the
> shutdown on close.  be2c92b8f164 reverted that part and 4dda864d7307
> reinstated it, all within v4.9.  Keeping a console running after close is
> intended, but the removal path was never updated for it.
> 
> Tested in a QEMU guest on tty-next 36844ea19656 with KASAN, using in-tree
> 8250_pci and a "serial8250" platform port.  Reproducers are available on
> request.  Without this patch:
> 
>   8250_pci with INTx, reading /proc/interrupts after unbind:
>     BUG: KASAN: slab-use-after-free in string+0x3b6/0x4c0
>      irq_seq_show+0x34d/0x810
>     Allocated by task 1244:
>      kasprintf+0xac/0x100
>      serial_core_register_port+0x55f/0x1720
>     Freed by task 1276:
>      kfree+0x1a0/0x4a0
>      serial_core_unregister_port+0x44d/0xa80
>      serial8250_unregister_port+0x10d/0x6c0
>      pciserial_detach_ports+0x92/0x160
> 
>   Polled MMIO port (irq 0, UPF_IOREMAP) after removal:
>     printk: console [ttyS5] disabled
>     BUG: kernel NULL pointer dereference, address: 0000000000000008
>     RIP: 0010:mem32_serial_in+0x74/0xa0
>      serial8250_timeout+0x44/0x140
>      call_timer_fn+0x32/0x260
> 
> With this patch both cases are clean: the IRQ is freed on unbind,
> rebinding restores the console, and the polled port is shut down on
> removal.  A port held open across removal and a console that was never
> opened did not fail before and still work.  v2 was retested the same
> way.  W=1, sparse and checkpatch --strict are clean.
> 
> Also tested on real hardware: the ASPEED BMC VUART that hit the bug
> (polled MMIO, irq 0) as the console, now bound to 8250_pci with my
> pending ASPEED support patch [1], on a 7.0.14 distribution kernel with
> both patches.  After stopping the getty, opening and closing the port,
> and unbinding the PCI device, there is no oops.  Rebinding restores the
> console, the getty and in-band IPMI.  That run used v1; v2 behaves the
> same on that path.
> 
> Not tested: serial drivers other than 8250, and removing a port while it
> is suspended (the case v2 adds).
> 
> [1] https://lore.kernel.org/r/20261008-aspeed-vuart-v1-1-d2be17c99cd7@phil.gs
> 
> AI assistance: an LLM coding assistant (Claude, model claude-opus-5-5, in
> Claude Code) analysed the crash, wrote the reproducers and the patch, and
> built and ran the test kernels.  Two other LLMs reviewed the result
> (GPT-5.6 Sol via Codex, and Claude Fable 5.1 via Claude Code).  I reviewed
> it as well.
> ---
> Changes in v2:
> - Call uart_shutdown() without checking tty_port_initialized() first,
>   so that a console port removed while suspended also gets its transmit
>   buffer freed and its suspended flag cleared (found by Sashiko's
>   review of v1).  uart_shutdown() already skips the hardware shutdown
>   for a port that is not initialized.  Comment and changelog updated.
> - Use scoped_guard() for port->mutex, as elsewhere in the function, and
>   drop the comment (Greg).
> - Link to v1: https://lore.kernel.org/r/20261008-serial-console-remove-v1-1-288b9c315af7@phil.gs
> ---
>  drivers/tty/serial/serial_core.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 319a4b427f3a3727d12cb58252bd8959f9be4cd6..d96d84a985cd972f3974da28cf3daa7fc05eefcf 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -3239,6 +3239,9 @@ static void serial_core_remove_one_port(struct uart_driver *drv,
>  	if (uart_console(uport))
>  		unregister_console(uport->cons);
>  
> +	scoped_guard(mutex, &port->mutex)
> +		uart_shutdown(NULL, state);
> +
>  	/*
>  	 * Free the port IO and memory resources, if any.
>  	 */
> 
> ---
> base-commit: 36844ea19656fb41278799ef3d5cd52120d89149
> change-id: 20261008-serial-console-remove-d9ef4da86e0c

Is this the same as:
	https://patch.msgid.link/20261003163337.45577-1-igorpetindev@gmail.com
?

Why does that commit set a field and this change does not?  Which is
correct?

thanks,

greg k-h

Re: [PATCH v2] serial: core: Shut down a running console port before removing it

[Phil Rosenthal]

On Tue, Oct 13, 2026 at 06:12:52PM +0200, Greg Kroah-Hartman wrote:
> Is this the same as:
> 	https://patch.msgid.link/20261003163337.45577-1-igorpetindev@gmail.com
> ?

Yes, it's the same bug and the same fix. I hadn't seen Igor's patch when I
sent mine, sorry for the noise. His is in tty-testing already, so
please drop mine.

> Why does that commit set a field and this change does not?  Which is
> correct?

Both work. Clearing port->console isn't needed for the fix, and it does
no harm. uart_shutdown() doesn't look at that flag. It is only read in
tty_port_shutdown_locked() and tty_port_close(), on the last close of a
tty, and by this point any open tty has been hung up, so
tty_port_close_start() returns before either is reached.
serial_core_add_one_port() sets the flag again when the line is reused.

The other difference is that Igor's patch calls uart_shutdown() only if
tty_port_initialized(), as my v1 did. Sashiko's review of my v1 pointed
out that a console port removed while suspended is not initialized but
still has its suspended flag set and its transmit buffer allocated, and
uart_shutdown() would clear both. That needs the device to go away
between system suspend and resume, and I haven't found a way to test
it. If you want it covered I can send a patch on top of Igor's commit
that drops the tty_port_initialized() check.

I ran my reproducers on tty-next 36844ea19656 plus 810d467c11f0, in a
QEMU guest with KASAN. Both cases that fail without it are fixed: the
8250_pci port with INTx (IRQ freed on unbind, reading /proc/interrupts
is clean, rebinding restores the console), and the polled MMIO port,
where serial8250_timeout() no longer runs after removal. A port held
open across removal and a console that was never opened still work.
I'll send a Tested-by on Igor's thread.

Thanks,
Phil