mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] serial: core: shut down initialized port on removal
@ 2026-09-24 19:22 Igor Putko
  2026-09-25  5:34 ` Greg Kroah-Hartman
  2026-09-26 13:05 ` IgorpetinDev
  0 siblings, 2 replies; 4+ messages in thread
From: Igor Putko @ 2026-09-24 19:22 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: Rob Herring, Vignesh R, linux-serial, linux-kernel, Igor Putko,
	syzbot+843bf2f48f4d12e6682e

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).

Reported-by: syzbot+843bf2f48f4d12e6682e@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=843bf2f48f4d12e6682e
Fixes: 761ed4a94582 ("tty: serial_core: convert uart_close to use tty_port_close")
Fixes: f7048b15900f ("tty: serial_core: Add name field to uart_port struct")
Signed-off-by: Igor Putko <igorpetindev@gmail.com>
---
 drivers/tty/serial/serial_core.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 95774b0f1484..030735da6b8c 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] serial: core: shut down initialized port on removal
  2026-09-24 19:22 [PATCH] serial: core: shut down initialized port on removal Igor Putko
@ 2026-09-25  5:34 ` Greg Kroah-Hartman
  2026-09-26 13:05 ` IgorpetinDev
  1 sibling, 0 replies; 4+ messages in thread
From: Greg Kroah-Hartman @ 2026-09-25  5:34 UTC (permalink / raw)
  To: Igor Putko
  Cc: Jiri Slaby, Rob Herring, Vignesh R, linux-serial, linux-kernel,
	syzbot+843bf2f48f4d12e6682e

On Thu, Sep 24, 2026 at 10:22:24PM +0300, Igor Putko wrote:
> 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:

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] serial: core: shut down initialized port on removal
  2026-09-24 19:22 [PATCH] serial: core: shut down initialized port on removal Igor Putko
  2026-09-25  5:34 ` Greg Kroah-Hartman
@ 2026-09-26 13:05 ` IgorpetinDev
  2026-10-01  8:58   ` Greg KH
  1 sibling, 1 reply; 4+ messages in thread
From: IgorpetinDev @ 2026-09-26 13:05 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: linux-kernel, syzbot+843bf2f48f4d12e6682e, linux-serial,
	Rob Herring, Vignesh R

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

On Thu, Sep 24, 2026 at 10:22 PM Igor Putko <igorpetindev@gmail.com> wrote:
>
> 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).
>
> Reported-by: syzbot+843bf2f48f4d12e6682e@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=843bf2f48f4d12e6682e
> Fixes: 761ed4a94582 ("tty: serial_core: convert uart_close to use tty_port_close")
> Fixes: f7048b15900f ("tty: serial_core: Add name field to uart_port struct")
> Signed-off-by: Igor Putko <igorpetindev@gmail.com>
> ---
>  drivers/tty/serial/serial_core.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 95774b0f1484..030735da6b8c 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -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
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] serial: core: shut down initialized port on removal
  2026-09-26 13:05 ` IgorpetinDev
@ 2026-10-01  8:58   ` Greg KH
  0 siblings, 0 replies; 4+ messages in thread
From: Greg KH @ 2026-10-01  8:58 UTC (permalink / raw)
  To: IgorpetinDev
  Cc: Jiri Slaby, linux-kernel, syzbot+843bf2f48f4d12e6682e,
	linux-serial, Rob Herring, Vignesh R

On Sat, Sep 26, 2026 at 04:05:31PM +0300, IgorpetinDev wrote:
> 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.

Please do so, thanks.

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-01  8:58 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-24 19:22 [PATCH] serial: core: shut down initialized port on removal Igor Putko
2026-09-25  5:34 ` Greg Kroah-Hartman
2026-09-26 13:05 ` IgorpetinDev
2026-10-01  8:58   ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®