Linux Serial subsystem development
 help / color / mirror / Atom feed
* [PATCH v2] serial: core: fix NULL pointer dereference in serial_core_unregister_port()
@ 2026-08-26  8:46 Ruslan Valiyev
  2026-08-26  9:03 ` sashiko-bot
  2026-08-28  7:10 ` Tony Lindgren
  0 siblings, 2 replies; 3+ messages in thread
From: Ruslan Valiyev @ 2026-08-26  8:46 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: Andy Shevchenko, Tony Lindgren, Hugo Villeneuve, John Ogness,
	Lukas Wunner, Gerhard Engleder, linux-serial, linux-kernel,
	syzkaller-bugs, Ruslan Valiyev, syzbot+9f57c1b2792029198fcf,
	stable

port->port_dev is NULL when no port device is installed: it is cleared
on teardown, and never set if registration failed before
serial_core_port_device_add().  serial_core_unregister_port() passes it
straight to serial_core_get_ctrl_dev(), which dereferences it:

  KASAN: null-ptr-deref in range [0x0000000000000040-0x0000000000000047]
  RIP: serial_core_unregister_port
  Call Trace:
   serial8250_unregister_port
   serial8250_remove
   unbind_store

Return early when there is no port device, and read port->port_dev
under port_mutex.

Also clear port->port_dev on the serial_core_register_port() error
path, where the port device has already been removed.

Fixes: 84a9582fd203 ("serial: core: Start managing serial controllers to enable runtime PM")
Reported-by: syzbot+9f57c1b2792029198fcf@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=9f57c1b2792029198fcf
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: Ruslan Valiyev <linuxoid@gmail.com>
---
Reproduced on 8d3ae59288f1 with syzbot's config under QEMU/KVM x86_64:
6/6 runs oops on stock, 0/6 patched.  checkpatch clean, no new W=1
warnings.

The reproducer still does not run to completion on a patched kernel.  It
goes on to hit two pre-existing problems in the tty layer that this patch
does not touch: tty_cdev_add() leaves driver->cdevs[index] pointing at a
freed cdev when cdev_add() fails, and tty_unregister_device() deletes that
entry unconditionally when it is NULL.  Mentioning it so the remaining
crashes are not mistaken for this fix failing.

v1: https://lore.kernel.org/all/20260826073237.1377668-1-linuxoid@gmail.com/
v2: trimmed the commit message and backtrace per Andy Shevchenko's
    review, added the Assisted-by tags.
 drivers/tty/serial/serial_core.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index a530ad372b434..5bf71d7bbd223 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -3327,6 +3327,7 @@ int serial_core_register_port(struct uart_driver *drv, struct uart_port *port)
 
 err_unregister_port_dev:
 	serial_base_port_device_remove(port->port_dev);
+	port->port_dev = NULL;
 
 err_unregister_ctrl_dev:
 	serial_base_ctrl_device_remove(new_ctrl_dev);
@@ -3341,12 +3342,24 @@ int serial_core_register_port(struct uart_driver *drv, struct uart_port *port)
 void serial_core_unregister_port(struct uart_driver *drv, struct uart_port *port)
 {
 	struct device *phys_dev = port->dev;
-	struct serial_port_device *port_dev = port->port_dev;
-	struct serial_ctrl_device *ctrl_dev = serial_core_get_ctrl_dev(port_dev);
+	struct serial_port_device *port_dev;
+	struct serial_ctrl_device *ctrl_dev;
 	int ctrl_id = port->ctrl_id;
 
 	guard(mutex)(&port_mutex);
 
+	/*
+	 * A NULL port device means there is no registered port device to
+	 * remove: serial_core_remove_one_port() clears port_dev on
+	 * teardown, and it is never set if registration failed before
+	 * serial_core_port_device_add().
+	 */
+	port_dev = port->port_dev;
+	if (!port_dev)
+		return;
+
+	ctrl_dev = serial_core_get_ctrl_dev(port_dev);
+
 	port->flags |= UPF_DEAD;
 
 	serial_core_remove_one_port(drv, port);

base-commit: 8d3ae59288f1e7d58d76558a6ee96d533bc5019f
-- 
2.43.0


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

* Re: [PATCH v2] serial: core: fix NULL pointer dereference in serial_core_unregister_port()
  2026-08-26  8:46 [PATCH v2] serial: core: fix NULL pointer dereference in serial_core_unregister_port() Ruslan Valiyev
@ 2026-08-26  9:03 ` sashiko-bot
  2026-08-28  7:10 ` Tony Lindgren
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-08-26  9:03 UTC (permalink / raw)
  To: Ruslan Valiyev; +Cc: linux-serial

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Missing cleanup on error paths in serial_core_add_one_port() leaves dangling pointers resulting in a Use-After-Free.
- [Critical] tty_cdev_add() leaves a dangling pointer on error, leading to a UAF or NULL-ptr dereference in tty_unregister_device().
--

commit a575b72dd88626a6d4f9468762be3ccbc1fb5e60
Author: Ruslan Valiyev <linuxoid@gmail.com>

serial: core: fix NULL pointer dereference in serial_core_unregister_port()

This commit fixes a NULL pointer dereference in serial_core_unregister_port()
that happens when port->port_dev is NULL. It checks for a NULL port_dev early
and clears port->port_dev on the registration error path. The patch does not
touch two other pre-existing problems in the tty layer hit by the reproducer.

Link: https://syzkaller.appspot.com/bug?extid=9f57c1b2792029198fcf
Link: https://lore.kernel.org/all/20260826073237.1377668-1-linuxoid@gmail.com/

> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index a530ad372b434..5bf71d7bbd223 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -3327,6 +3327,7 @@ int serial_core_register_port(struct uart_driver *drv, struct uart_port *port)
>  
>  err_unregister_port_dev:
>  	serial_base_port_device_remove(port->port_dev);
> +	port->port_dev = NULL;

[Severity: Critical]
This is a pre-existing issue, but does serial_core_add_one_port() leave a
dangling pointer on its error paths?

If memory allocation fails in serial_core_add_one_port() (e.g., when 
allocating uport->name or uport->tty_groups), it returns -ENOMEM without
clearing state->uart_port, which was set to uport earlier:

drivers/tty/serial/serial_core.c:serial_core_add_one_port() {
    ...
    state->uart_port = uport;
    ...
    uport->name = kasprintf(...);
    if (!uport->name)
        return -ENOMEM;
    ...
}

Could this lead to a Use-After-Free if a subsequent read of
/proc/tty/driver/serial invokes uart_proc_show() -> uart_line_info(),
retrieving the freed uport via uart_port_check()?

[ ... ]

> @@ -3341,12 +3342,24 @@ int serial_core_register_port(struct uart_driver *drv, struct uart_port *port)
>  void serial_core_unregister_port(struct uart_driver *drv, struct uart_port *port)
>  {
[ ... ]
>  	port->flags |= UPF_DEAD;
>  
>  	serial_core_remove_one_port(drv, port);

[Severity: Critical]
This is a pre-existing issue, but does tty_cdev_add() leave a dangling 
pointer that causes a crash here during teardown?

In tty_cdev_add(), if cdev_add() fails, the cdev is freed but the array
entry is not cleared:

drivers/tty/tty_io.c:tty_cdev_add() {
    ...
    if (err)
        kobject_put(&driver->cdevs[index]->kobj);
    return err;
}

When serial_core_remove_one_port() is called here during teardown, it 
unconditionally calls tty_unregister_device(). For drivers without dynamic 
allocation, this calls cdev_del(driver->cdevs[index]), which directly 
dereferences it:

fs/char_dev.c:cdev_del() {
    cdev_unmap(p->dev, p->count);
    ...
}

Could this cause a Use-After-Free or NULL pointer dereference when tearing 
down a device that previously failed registration?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826084654.1392851-1-linuxoid@gmail.com?part=1

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

* Re: [PATCH v2] serial: core: fix NULL pointer dereference in serial_core_unregister_port()
  2026-08-26  8:46 [PATCH v2] serial: core: fix NULL pointer dereference in serial_core_unregister_port() Ruslan Valiyev
  2026-08-26  9:03 ` sashiko-bot
@ 2026-08-28  7:10 ` Tony Lindgren
  1 sibling, 0 replies; 3+ messages in thread
From: Tony Lindgren @ 2026-08-28  7:10 UTC (permalink / raw)
  To: Ruslan Valiyev
  Cc: Greg Kroah-Hartman, Jiri Slaby, Andy Shevchenko, Tony Lindgren,
	Hugo Villeneuve, John Ogness, Lukas Wunner, Gerhard Engleder,
	linux-serial, linux-kernel, syzkaller-bugs,
	syzbot+9f57c1b2792029198fcf, stable

On Wed, Aug 26, 2026 at 10:46:54AM +0200, Ruslan Valiyev wrote:
> Return early when there is no port device, and read port->port_dev
> under port_mutex.
> 
> Also clear port->port_dev on the serial_core_register_port() error
> path, where the port device has already been removed.

These look like valid fixes to me:

Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>

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

end of thread, other threads:[~2026-08-28  7:10 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-26  8:46 [PATCH v2] serial: core: fix NULL pointer dereference in serial_core_unregister_port() Ruslan Valiyev
2026-08-26  9:03 ` sashiko-bot
2026-08-28  7:10 ` Tony Lindgren

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