[PATCH 0/4] serial: fix console lifetime bugs on failed bind and removal

[PATCH 0/4] serial: fix console lifetime bugs on failed bind and removal

[Karl Mehltretter]

This series fixes serial/tty lifetime bugs found while injecting
allocation failures into the UART bind path. The failures were
reproduced on QEMU's raspi1ap (amba-pl011) and mcimx6ul-evk (imx)
machines.

The first three patches fix shared serial/tty error paths: move
fallible port allocations before console registration, clear state
after failed uart_driver registration, and track whether a live cdev
is registered. The final patch fixes the lifetime of the i.MX driver's
private console-port table on probe failure and removal.

The i.MX probe-failure fix spans patches 1 and 4. Serial core must not
return an error while leaving the console registered, and the i.MX
driver must clear its table entry before the failed probe frees the
port.

With all four patches applied, fail-nth sweeps of the console UART bind
path completed on both machines without a KASAN report or oops. Every
failed bind left the console unregistered, and the i.MX unbind/rebind
reproducer also ran cleanly.

This follows up on the discussion here:
https://lore.kernel.org/r/alyjOGyTd19Obemi@MacBook-Pro-von-Karl

Karl Mehltretter (4):
  serial: core: do fallible allocations before the console can be
    registered
  serial: core: clear freed pointers on uart_register_driver() failure
  tty: don't oops in tty_unregister_device() when no cdev is registered
  serial: imx: clear imx_uart_ports[] entry on probe failure and removal

 drivers/tty/serial/imx.c         |  4 ++++
 drivers/tty/serial/serial_core.c | 33 ++++++++++++++++++-------------
 drivers/tty/tty_io.c             |  6 ++++--
 3 files changed, 27 insertions(+), 16 deletions(-)

-- 
2.53.0

[PATCH 1/4] serial: core: do fallible allocations before the console can be registered

[Karl Mehltretter]

serial_core_add_one_port() allocates uport->tty_groups after
uart_configure_port() has already registered the port's console. If
that allocation fails, the function returns -ENOMEM with the console
still registered, and the driver's probe error path then tears down
the port state the console callbacks depend on.

Reproduced with fault injection on qemu's raspi1ap board. Failing the
tty_groups allocation during a PL011 sysfs bind makes
uart_add_one_port() return -ENOMEM. pl011_register_port() then clears
amba_ports[0], but ttyAMA0 remains registered as a console. The nbcon
printer thread dereferences the NULL entry and oopses:

  Unhandled fault: page domain fault (0x01b) at 0x00000178
  CPU: 0 UID: 0 PID: 43 Comm: pr/ttyAMA0 Not tainted 7.2.0-rc3+ #1
  PC is at pl011_console_write_thread+0x2c/0x168

This is not PL011-specific: the failing allocation is in serial core,
after uart_configure_port() has registered the console, so any console
UART driver is exposed. On i.MX the retained console references a
devm-allocated port that the failed probe frees, causing a
use-after-free. Reproduced on qemu's mcimx6ul-evk using the same
fail-nth harness under KASAN:

  BUG: KASAN: slab-use-after-free in imx_uart_console_write_thread+0x50/0x278
  Read of size 4 at addr c5246048 by task pr/ttymxc0/63
   imx_uart_console_write_thread from nbcon_emit_next_record+0x360/0x50c
   nbcon_emit_next_record from nbcon_emit_one+0x140/0x184
  Allocated by task 1:
   devm_kmalloc from imx_uart_probe+0x90/0xa5c
  Freed by task 1:
   devres_release_all from device_unbind_cleanup+0x38/0xdc
   device_unbind_cleanup from really_probe+0x2b4/0x388

The pre-existing kasprintf() failure path has a related problem: it
returns with state->uart_port already pointing at a port whose probe
is about to unwind and free it.

Reorder the function so the uport->name and uport->tty_groups
allocations both happen before the port is linked into the driver
state table and before uart_configure_port() registers the console:

 1. Allocate uport->name.
 2. Allocate the tty_groups array with room for three entries
    unconditionally (serial core group, optional driver group, NULL
    terminator). The optional group cannot be examined at this point:
    config_port() may only supply uport->attr_group during
    uart_configure_port(), e.g. 8250 sets it after autodetection.
 3. Only then link the port into the driver state table and run
    uart_configure_port().
 4. Fill in the optional attr_group slot afterwards.

A fail-nth sweep over the whole bind path on both boards left the
console unregistered after every failed bind and did not reproduce the
i.MX use-after-free.

Fixes: 266dcff03eed ("Serial: allow port drivers to have a default attribute group")
Fixes: f7048b15900f ("tty: serial_core: Add name field to uart_port struct")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-fable-5
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
 drivers/tty/serial/serial_core.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index a530ad372b43..887b1dd80ad2 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -3056,7 +3056,6 @@ static int serial_core_add_one_port(struct uart_driver *drv, struct uart_port *u
 	struct uart_state *state;
 	struct tty_port *port;
 	struct device *tty_dev;
-	int num_groups;
 
 	if (uport->line >= drv->nr)
 		return -EINVAL;
@@ -3068,6 +3067,23 @@ static int serial_core_add_one_port(struct uart_driver *drv, struct uart_port *u
 	if (state->uart_port)
 		return -EINVAL;
 
+	uport->name = kasprintf(GFP_KERNEL, "%s%u", drv->dev_name,
+				drv->tty_driver->name_base + uport->line);
+	if (!uport->name)
+		return -ENOMEM;
+
+	/*
+	 * uart_configure_port() may set uport->attr_group and register the
+	 * console. Allocate room for both groups and a NULL terminator first.
+	 */
+	uport->tty_groups = kzalloc_objs(*uport->tty_groups, 3);
+	if (!uport->tty_groups) {
+		kfree(uport->name);
+		uport->name = NULL;
+		return -ENOMEM;
+	}
+	uport->tty_groups[0] = &tty_dev_attr_group;
+
 	/* Link the port to the driver state table and vice versa */
 	atomic_set(&state->refcount, 1);
 	init_waitqueue_head(&state->remove_wait);
@@ -3084,10 +3100,6 @@ static int serial_core_add_one_port(struct uart_driver *drv, struct uart_port *u
 	state->pm_state = UART_PM_STATE_UNDEFINED;
 	uart_port_set_cons(uport, drv->cons);
 	uport->minor = drv->tty_driver->minor_start + uport->line;
-	uport->name = kasprintf(GFP_KERNEL, "%s%u", drv->dev_name,
-				drv->tty_driver->name_base + uport->line);
-	if (!uport->name)
-		return -ENOMEM;
 
 	if (uport->cons && uport->dev)
 		of_console_check(uport->dev->of_node, uport->cons->name, uport->line);
@@ -3102,15 +3114,6 @@ static int serial_core_add_one_port(struct uart_driver *drv, struct uart_port *u
 
 	port->console = uart_console(uport);
 
-	num_groups = 2;
-	if (uport->attr_group)
-		num_groups++;
-
-	uport->tty_groups = kzalloc_objs(*uport->tty_groups, num_groups);
-	if (!uport->tty_groups)
-		return -ENOMEM;
-
-	uport->tty_groups[0] = &tty_dev_attr_group;
 	if (uport->attr_group)
 		uport->tty_groups[1] = uport->attr_group;
 
-- 
2.53.0

[PATCH 2/4] serial: core: clear freed pointers on uart_register_driver() failure

[Karl Mehltretter]

uart_register_driver() leaves freed pointers behind on failure. If
tty_alloc_driver() fails, it frees drv->state without clearing it. If
tty_register_driver() fails, it also drops the tty driver reference
without clearing drv->tty_driver.

Several drivers register the uart_driver lazily and use drv->state as
an "already registered" sentinel. After a failed registration, the
next probe sees the stale pointer, skips re-registration and calls
uart_add_one_port() with freed state.

The resulting unwind can call uart_unregister_driver() with a NULL or
dangling drv->tty_driver and oops in tty_unregister_driver():

  Unhandled fault: page domain fault (0x01b) at 0x00000018
  PC is at tty_unregister_driver+0x10/0x68
  LR is at uart_unregister_driver+0x1c/0x60

Reproduced with failslab fail-nth injection on qemu's raspi1ap board:
fail the tty_alloc_driver() allocation during a sysfs bind of the
PL011 port, then bind again in the same boot.

Clear drv->state after freeing the state array and clear
drv->tty_driver after dropping the tty driver reference, as
uart_unregister_driver() already does.

The tty_register_driver() failure case predates Git history. The
tty_alloc_driver() failure case was introduced by commit 9e845abfc8a8
("serial: fix NULL pointer dereference"), which made that error path
return cleanly instead of crashing in put_tty_driver(NULL).

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Fixes: 9e845abfc8a8 ("serial: fix NULL pointer dereference")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-fable-5
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
 drivers/tty/serial/serial_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 887b1dd80ad2..ba9145c5a38a 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2777,8 +2777,10 @@ int uart_register_driver(struct uart_driver *drv)
 	for (i = 0; i < drv->nr; i++)
 		tty_port_destroy(&drv->state[i].port);
 	tty_driver_kref_put(normal);
+	drv->tty_driver = NULL;
 out_kfree:
 	kfree(drv->state);
+	drv->state = NULL;
 out:
 	return retval;
 }
-- 
2.53.0

[PATCH 3/4] tty: don't oops in tty_unregister_device() when no cdev is registered

[Karl Mehltretter]

serial_core_add_one_port() keeps a uart_port when tty device registration
fails so setserial can still use it. It marks the port UPF_DEAD and
returns success. Removing the port later reaches tty_unregister_device(),
which unconditionally passes driver->cdevs[index] to cdev_del().

The slot does not always contain a live cdev. A serdev registration error
other than -ENODEV returns before tty_register_device_attr(), leaving the
slot NULL. If cdev_add() fails, tty_cdev_add() drops the cdev reference
but leaves the slot pointing at freed memory. The later cdev_del() is
therefore a NULL dereference or use-after-free.

The NULL path was reproduced with failslab during UART bind on qemu's
mcimx6ul-evk and raspi1ap boards:

  Unhandled fault: page domain fault (0x01b) at 0x00000038
  PC is at cdev_del+0x14/0x34

Clear the slot after cdev_add() fails and only call cdev_del() when it is
non-NULL. This makes a non-NULL slot mean that a live cdev is registered.

Fixes: c1a752ba2d6b ("tty: don't leak cdev in tty_cdev_add()")
Fixes: 8cde11b2baa1 ("tty/serdev: add serdev registration interface")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-fable-5
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
 drivers/tty/tty_io.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 6b283fd03ff8..4889076b975f 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -3167,8 +3167,10 @@ static int tty_cdev_add(struct tty_driver *driver, dev_t dev,
 	driver->cdevs[index]->ops = &tty_fops;
 	driver->cdevs[index]->owner = driver->owner;
 	err = cdev_add(driver->cdevs[index], dev, count);
-	if (err)
+	if (err) {
 		kobject_put(&driver->cdevs[index]->kobj);
+		driver->cdevs[index] = NULL;
+	}
 	return err;
 }
 
@@ -3305,7 +3307,7 @@ EXPORT_SYMBOL_GPL(tty_register_device_attr);
 void tty_unregister_device(struct tty_driver *driver, unsigned index)
 {
 	device_destroy(&tty_class, MKDEV(driver->major, driver->minor_start) + index);
-	if (!(driver->flags & TTY_DRIVER_DYNAMIC_ALLOC)) {
+	if (!(driver->flags & TTY_DRIVER_DYNAMIC_ALLOC) && driver->cdevs[index]) {
 		cdev_del(driver->cdevs[index]);
 		driver->cdevs[index] = NULL;
 	}
-- 
2.53.0

[PATCH 4/4] serial: imx: clear imx_uart_ports[] entry on probe failure and removal

[Karl Mehltretter]

imx_uart_probe() stores the port in imx_uart_ports[] before calling
uart_add_one_port() because console setup during that call uses the
table. The entry remains set if uart_add_one_port() fails.
imx_uart_remove() also leaves it set after uart_remove_one_port().

sport is devm-allocated, so a failed probe or unbind frees it while
imx_uart_ports[] still points to it. A later sibling probe can register
the shared console with the old preferred index and dereference the
stale entry in imx_uart_console_setup().

Reproduced on qemu's mcimx6ul-evk by unbinding a sibling UART, unbinding
the console UART and rebinding the sibling:

  BUG: KASAN: slab-use-after-free in imx_uart_console_setup+0xd0/0x3d8
  Read of size 4 at addr c49ad9d4 by task init/1
  Call trace:
   ...
   imx_uart_console_setup from try_enable_preferred_console+0x158/0x1f8
   try_enable_preferred_console from register_console+0x1cc/0x80c
   register_console from serial_core_register_port+0xe58/0xeb0
   ...
  Freed by task 1:
   ...
   devres_release_all+0x100/0x18c
   device_unbind_cleanup+0x38/0xdc
   device_release_driver_internal+0x230/0x288
   unbind_store+0x64/0xa8
   ...

The entry must remain visible while uart_add_one_port() and
uart_remove_one_port() run. Clear it when adding the port fails and
after removing the port. Save the line index before removal because the
uart_port is no longer valid afterward.

The probe-failure cleanup relies on the preceding serial-core change
"serial: core: do fallible allocations before the console can be
registered", which ensures that uart_add_one_port() cannot fail after
registering the console. For complete coverage, both changes should be
backported together.

Fixes: dbff4e9ea2e8 ("IMX UART: remove statically initialized tables")
Fixes: 9f322ad064f9 ("imx: serial: handle initialisation failure correctly")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-fable-5
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
 drivers/tty/serial/imx.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 251a50c8aa38..617c35772056 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -2637,6 +2637,8 @@ static int imx_uart_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, sport);
 
 	ret = uart_add_one_port(&imx_uart_uart_driver, &sport->port);
+	if (ret)
+		imx_uart_ports[sport->port.line] = NULL;
 
 err_clk:
 	clk_disable_unprepare(sport->clk_ipg);
@@ -2647,8 +2649,10 @@ static int imx_uart_probe(struct platform_device *pdev)
 static void imx_uart_remove(struct platform_device *pdev)
 {
 	struct imx_port *sport = platform_get_drvdata(pdev);
+	unsigned int line = sport->port.line;
 
 	uart_remove_one_port(&imx_uart_uart_driver, &sport->port);
+	imx_uart_ports[line] = NULL;
 }
 
 static void imx_uart_restore_context(struct imx_port *sport)
-- 
2.53.0

Re: [PATCH 4/4] serial: imx: clear imx_uart_ports[] entry on probe failure and removal

[sashiko-bot]

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

New issues:
- [High] Lockless updates to the global imx_uart_ports[] array leave a Time-Of-Check to Time-Of-Use (TOCTOU) race condition during concurrent sibling probes and removals, allowing a stale pointer to be used for console registration and causing a deferred NULL pointer dereference.

Pre-existing issues:
- [High] Missing hrtimer cancellation for RS485 tx timers during device removal and shutdown leads to a Use-After-Free of the sport structure.
--

--- Patch [4]: [PATCH 4/4] serial: imx: clear imx_uart_ports[] entry on probe failure and removal ---
commit 85b874e619e770afd678fef4f09d60cd44a0c477
Author: Karl Mehltretter <kmehltretter@gmail.com>

serial: imx: clear imx_uart_ports[] entry on probe failure and removal

This commit clears the imx_uart_ports[] entry on probe failure and driver
removal. It prevents a stale pointer from being used by a sibling probe's
console setup during a devres free.

> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 251a50c8aa387..617c35772056c 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -2637,6 +2637,8 @@ static int imx_uart_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, sport);
>  
>  	ret = uart_add_one_port(&imx_uart_uart_driver, &sport->port);
> +	if (ret)
> +		imx_uart_ports[sport->port.line] = NULL;
>  
>  err_clk:

[ ... ]

> @@ -2647,8 +2649,10 @@ static int imx_uart_probe(struct platform_device *pdev)
>  static void imx_uart_remove(struct platform_device *pdev)
>  {
>  	struct imx_port *sport = platform_get_drvdata(pdev);
> +	unsigned int line = sport->port.line;
>  
>  	uart_remove_one_port(&imx_uart_uart_driver, &sport->port);
> +	imx_uart_ports[line] = NULL;

[Severity: High]
Does clearing imx_uart_ports[] after uart_remove_one_port() create a race
condition with concurrent sibling probes?

When uart_remove_one_port() returns, all locks (including console_list_lock
and port->mutex) have been dropped.

If Thread A unbinds the UART, it unregisters the console inside
uart_remove_one_port(), drops the locks, but has not yet executed the newly
added line to clear imx_uart_ports[line].

Concurrently, Thread B probing a sibling UART can see the console is
unregistered, and successfully register it. During registration, Thread B's
imx_uart_console_setup() locklessly reads the still-valid 
imx_uart_ports[0].

When Thread A resumes, it clears imx_uart_ports[0] = NULL, and devres frees
the port. A subsequent printk calls imx_uart_console_write_atomic(), which
reads the NULL pointer from the array and dereferences it.

>  }

[Severity: High]
This is a pre-existing issue, but does device removal fail to cancel RS485
tx timers, leading to a use-after-free?

During device unbind, imx_uart_shutdown() loops waiting for 
tx_state != OFF.

If the trigger_start_tx or trigger_stop_tx hrtimer is delayed or executing
concurrently, the loop times out, forcefully sets tx_state to OFF, and
returns without explicitly cancelling the timer using hrtimer_cancel().

Because imx_uart_remove() finishes without cancelling these driver-specific
hrtimers, devres will free the sport structure memory. If the delayed timer
subsequently fires, it will dereference the freed sport memory.

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

Re: [PATCH 4/4] serial: imx: clear imx_uart_ports[] entry on probe failure and removal

[Karl Mehltretter]

On Sun, Jul 19, 2026 at 04:28:49PM +0100, sashiko-bot@kernel.org wrote:
> >  	uart_remove_one_port(&imx_uart_uart_driver, &sport->port);
> > +	imx_uart_ports[line] = NULL;
> 
> [Severity: High]
> Does clearing imx_uart_ports[] after uart_remove_one_port() create a race
> condition with concurrent sibling probes?
> 

Yes, this is valid, but the underlying issue is not new.

Before this change, a dangling pointer remained in the table and could
be used by a later sibling probe.

This change narrows it to a window where a NULL-pointer dereference is
possible.

I'll send a v2 with a driver-wide mutex around the table updates and
port add/remove.

Thanks,
Karl