* [PATCH v2 0/4] serial: fix console lifetime bugs on failed bind and removal
@ 2026-07-19 22:10 Karl Mehltretter
2026-07-19 22:10 ` [PATCH v2 1/4] serial: core: do fallible allocations before the console can be registered Karl Mehltretter
2026-07-19 22:10 ` [PATCH v2 4/4] serial: imx: serialize imx_uart_ports[] lifetime Karl Mehltretter
0 siblings, 2 replies; 6+ messages in thread
From: Karl Mehltretter @ 2026-07-19 22:10 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Karl Mehltretter, Frank Li, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, linux-serial, linux-kernel, imx, linux-arm-kernel
This series fixes three serial/tty bind-error bugs and an i.MX console
port-table lifetime bug. The fixes were found with failslab fail-nth sweeps
and sysfs bind/unbind sequences under KASAN on QEMU's raspi1ap and
mcimx6ul-evk machines.
Patches 1-3 fix shared serial/tty core code. Patch 1 moves fallible serial
core allocations before console registration; patch 2 clears serial-core
pointers after uart_driver registration fails; and patch 3 makes a non-NULL
tty cdev slot mean that the cdev is live.
Patch 4 clears i.MX's devm-allocated port-table entry on probe failure and
removal. It keeps the entry valid through console unregistration and
serializes the table updates against sibling probes.
Backporting note: the probe-failure half of patch 4 relies on patch 1, which
ensures that uart_add_one_port() cannot fail after registering the console.
For complete coverage, patches 1 and 4 should be backported together.
Validation for patch 4 includes reproducing the sibling-probe race under
KASAN with and without the fix, a sequential unbind/rebind regression, and
the same test with CONFIG_PROVE_LOCKING=y. The fixed kernel keeps the
console unregistered and survives an explicit printk in each applicable run.
Changes in v2:
- Patch 4: serialize port registration and removal with imx_uart_ports[]
updates to close the reported sibling-probe race.
- Patches 1-3 are unchanged.
v1: https://lore.kernel.org/r/20260719160812.35407-1-kmehltretter@gmail.com
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: serialize imx_uart_ports[] lifetime
drivers/tty/serial/imx.c | 21 ++++++++++++++++++--
drivers/tty/serial/serial_core.c | 33 ++++++++++++++++++--------------
drivers/tty/tty_io.c | 6 ++++--
3 files changed, 42 insertions(+), 18 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 1/4] serial: core: do fallible allocations before the console can be registered 2026-07-19 22:10 [PATCH v2 0/4] serial: fix console lifetime bugs on failed bind and removal Karl Mehltretter @ 2026-07-19 22:10 ` Karl Mehltretter 2026-07-30 14:28 ` Greg Kroah-Hartman 2026-07-19 22:10 ` [PATCH v2 4/4] serial: imx: serialize imx_uart_ports[] lifetime Karl Mehltretter 1 sibling, 1 reply; 6+ messages in thread From: Karl Mehltretter @ 2026-07-19 22:10 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby Cc: Karl Mehltretter, Frank Li, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, linux-serial, linux-kernel, imx, linux-arm-kernel, stable 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/4] serial: core: do fallible allocations before the console can be registered 2026-07-19 22:10 ` [PATCH v2 1/4] serial: core: do fallible allocations before the console can be registered Karl Mehltretter @ 2026-07-30 14:28 ` Greg Kroah-Hartman 0 siblings, 0 replies; 6+ messages in thread From: Greg Kroah-Hartman @ 2026-07-30 14:28 UTC (permalink / raw) To: Karl Mehltretter Cc: Jiri Slaby, Frank Li, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, linux-serial, linux-kernel, imx, linux-arm-kernel, stable On Mon, Jul 20, 2026 at 12:10:11AM +0200, Karl Mehltretter wrote: > 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 As this can only be duplicated with fault-injection, it really doesn't need to go to any stable kernels, right? > 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; Why set this to NULL? thanks, greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 4/4] serial: imx: serialize imx_uart_ports[] lifetime 2026-07-19 22:10 [PATCH v2 0/4] serial: fix console lifetime bugs on failed bind and removal Karl Mehltretter 2026-07-19 22:10 ` [PATCH v2 1/4] serial: core: do fallible allocations before the console can be registered Karl Mehltretter @ 2026-07-19 22:10 ` Karl Mehltretter 2026-07-30 14:31 ` Greg Kroah-Hartman 2026-07-30 19:02 ` Frank Li 1 sibling, 2 replies; 6+ messages in thread From: Karl Mehltretter @ 2026-07-19 22:10 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby Cc: Karl Mehltretter, Frank Li, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, linux-serial, linux-kernel, imx, linux-arm-kernel, Sashiko, stable imx_uart_probe() publishes the port in imx_uart_ports[] before uart_add_one_port(), because console setup during that call uses the table. The entry is not cleared if adding the port fails or after imx_uart_remove() removes it. The port is devm-allocated, so a failed probe or unbind leaves the table pointing at freed memory. A later registration of the shared console can then dereference the stale entry. 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 The entry must remain valid until uart_remove_one_port() unregisters the console. Clearing it afterward without serialization still races a sibling probe: the sibling can register the shared console using the dying entry before it is cleared. The next console write then dereferences NULL. Use a driver-wide mutex to serialize table publication, port addition and rollback with port removal and table clearing. Console callbacks remain lockless because uart_add_one_port() may invoke setup while probe holds the mutex. The probe-failure path also relies on "serial: core: do fallible allocations before the console can be registered", which moves the uport->name and uport->tty_groups allocations before console registration. Both changes should be backported together. Fixes: dbff4e9ea2e8 ("IMX UART: remove statically initialized tables") Fixes: 9f322ad064f9 ("imx: serial: handle initialisation failure correctly") Reported-by: Sashiko <sashiko-bot@kernel.org> Link: https://lore.kernel.org/all/20260719162850.043B41F000E9@smtp.kernel.org Cc: stable@vger.kernel.org Assisted-by: Claude:claude-fable-5 Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com> --- drivers/tty/serial/imx.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index 251a50c8aa38..def874f9cd00 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -22,6 +22,7 @@ #include <linux/clk.h> #include <linux/delay.h> #include <linux/ktime.h> +#include <linux/mutex.h> #include <linux/pinctrl/consumer.h> #include <linux/rational.h> #include <linux/slab.h> @@ -2080,6 +2081,15 @@ static const struct uart_ops imx_uart_pops = { static struct imx_port *imx_uart_ports[UART_NR]; +/* + * Store the port in imx_uart_ports[] before uart_add_one_port() and clear + * it only after uart_remove_one_port() returns. Console callbacks in both + * calls use the table, so this mutex serializes these sequences between + * sibling ports. Callbacks must not take it because uart_add_one_port() + * may invoke setup while it is held. + */ +static DEFINE_MUTEX(imx_uart_ports_lock); + #if IS_ENABLED(CONFIG_SERIAL_IMX_CONSOLE) static void imx_uart_console_putchar(struct uart_port *port, unsigned char ch) { @@ -2632,11 +2642,14 @@ static int imx_uart_probe(struct platform_device *pdev) } } - imx_uart_ports[sport->port.line] = sport; - platform_set_drvdata(pdev, sport); + mutex_lock(&imx_uart_ports_lock); + imx_uart_ports[sport->port.line] = sport; ret = uart_add_one_port(&imx_uart_uart_driver, &sport->port); + if (ret) + imx_uart_ports[sport->port.line] = NULL; + mutex_unlock(&imx_uart_ports_lock); err_clk: clk_disable_unprepare(sport->clk_ipg); @@ -2647,8 +2660,12 @@ 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; + mutex_lock(&imx_uart_ports_lock); uart_remove_one_port(&imx_uart_uart_driver, &sport->port); + imx_uart_ports[line] = NULL; + mutex_unlock(&imx_uart_ports_lock); } static void imx_uart_restore_context(struct imx_port *sport) -- 2.53.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 4/4] serial: imx: serialize imx_uart_ports[] lifetime 2026-07-19 22:10 ` [PATCH v2 4/4] serial: imx: serialize imx_uart_ports[] lifetime Karl Mehltretter @ 2026-07-30 14:31 ` Greg Kroah-Hartman 2026-07-30 19:02 ` Frank Li 1 sibling, 0 replies; 6+ messages in thread From: Greg Kroah-Hartman @ 2026-07-30 14:31 UTC (permalink / raw) To: Karl Mehltretter Cc: Jiri Slaby, Frank Li, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, linux-serial, linux-kernel, imx, linux-arm-kernel, Sashiko, stable On Mon, Jul 20, 2026 at 12:10:14AM +0200, Karl Mehltretter wrote: > imx_uart_probe() publishes the port in imx_uart_ports[] before > uart_add_one_port(), because console setup during that call uses the > table. The entry is not cleared if adding the port fails or after > imx_uart_remove() removes it. > > The port is devm-allocated, so a failed probe or unbind leaves the table > pointing at freed memory. A later registration of the shared console can > then dereference the stale entry. 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 > > The entry must remain valid until uart_remove_one_port() unregisters the > console. Clearing it afterward without serialization still races a > sibling probe: the sibling can register the shared console using the > dying entry before it is cleared. The next console write then > dereferences NULL. > > Use a driver-wide mutex to serialize table publication, port addition > and rollback with port removal and table clearing. Console callbacks > remain lockless because uart_add_one_port() may invoke setup while > probe holds the mutex. > > The probe-failure path also relies on "serial: core: do fallible > allocations before the console can be registered", which moves the > uport->name and uport->tty_groups allocations before console > registration. Both changes should be backported together. > > Fixes: dbff4e9ea2e8 ("IMX UART: remove statically initialized tables") > Fixes: 9f322ad064f9 ("imx: serial: handle initialisation failure correctly") > Reported-by: Sashiko <sashiko-bot@kernel.org> > Link: https://lore.kernel.org/all/20260719162850.043B41F000E9@smtp.kernel.org > Cc: stable@vger.kernel.org > Assisted-by: Claude:claude-fable-5 > Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com> > --- > drivers/tty/serial/imx.c | 21 +++++++++++++++++++-- > 1 file changed, 19 insertions(+), 2 deletions(-) > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > index 251a50c8aa38..def874f9cd00 100644 > --- a/drivers/tty/serial/imx.c > +++ b/drivers/tty/serial/imx.c > @@ -22,6 +22,7 @@ > #include <linux/clk.h> > #include <linux/delay.h> > #include <linux/ktime.h> > +#include <linux/mutex.h> > #include <linux/pinctrl/consumer.h> > #include <linux/rational.h> > #include <linux/slab.h> > @@ -2080,6 +2081,15 @@ static const struct uart_ops imx_uart_pops = { > > static struct imx_port *imx_uart_ports[UART_NR]; > > +/* > + * Store the port in imx_uart_ports[] before uart_add_one_port() and clear > + * it only after uart_remove_one_port() returns. Console callbacks in both > + * calls use the table, so this mutex serializes these sequences between > + * sibling ports. Callbacks must not take it because uart_add_one_port() > + * may invoke setup while it is held. > + */ > +static DEFINE_MUTEX(imx_uart_ports_lock); Again, LLMs love to write comments/text, be sane please. This comment has nothing to do with the storing and such. > + > #if IS_ENABLED(CONFIG_SERIAL_IMX_CONSOLE) > static void imx_uart_console_putchar(struct uart_port *port, unsigned char ch) > { > @@ -2632,11 +2642,14 @@ static int imx_uart_probe(struct platform_device *pdev) > } > } > > - imx_uart_ports[sport->port.line] = sport; > - > platform_set_drvdata(pdev, sport); > > + mutex_lock(&imx_uart_ports_lock); guard()? > + imx_uart_ports[sport->port.line] = sport; > ret = uart_add_one_port(&imx_uart_uart_driver, &sport->port); > + if (ret) > + imx_uart_ports[sport->port.line] = NULL; > + mutex_unlock(&imx_uart_ports_lock); > > err_clk: > clk_disable_unprepare(sport->clk_ipg); > @@ -2647,8 +2660,12 @@ 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; Why are you reading this outside of the lock? > > + mutex_lock(&imx_uart_ports_lock); guard()? thanks, greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 4/4] serial: imx: serialize imx_uart_ports[] lifetime 2026-07-19 22:10 ` [PATCH v2 4/4] serial: imx: serialize imx_uart_ports[] lifetime Karl Mehltretter 2026-07-30 14:31 ` Greg Kroah-Hartman @ 2026-07-30 19:02 ` Frank Li 1 sibling, 0 replies; 6+ messages in thread From: Frank Li @ 2026-07-30 19:02 UTC (permalink / raw) To: Karl Mehltretter Cc: Greg Kroah-Hartman, Jiri Slaby, Frank Li, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, linux-serial, linux-kernel, imx, linux-arm-kernel, Sashiko, stable On Mon, Jul 20, 2026 at 12:10:14AM +0200, Karl Mehltretter wrote: > [You don't often get email from kmehltretter@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > imx_uart_probe() publishes the port in imx_uart_ports[] before > uart_add_one_port(), because console setup during that call uses the > table. The entry is not cleared if adding the port fails or after > imx_uart_remove() removes it. > > The port is devm-allocated, so a failed probe or unbind leaves the table > pointing at freed memory. A later registration of the shared console can > then dereference the stale entry. 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 > > The entry must remain valid until uart_remove_one_port() unregisters the > console. Clearing it afterward without serialization still races a > sibling probe: the sibling can register the shared console using the > dying entry before it is cleared. The next console write then > dereferences NULL. > > Use a driver-wide mutex to serialize table publication, port addition > and rollback with port removal and table clearing. Console callbacks > remain lockless because uart_add_one_port() may invoke setup while > probe holds the mutex. > > The probe-failure path also relies on "serial: core: do fallible > allocations before the console can be registered", which moves the > uport->name and uport->tty_groups allocations before console > registration. Both changes should be backported together. > > Fixes: dbff4e9ea2e8 ("IMX UART: remove statically initialized tables") > Fixes: 9f322ad064f9 ("imx: serial: handle initialisation failure correctly") > Reported-by: Sashiko <sashiko-bot@kernel.org> > Link: https://lore.kernel.org/all/20260719162850.043B41F000E9@smtp.kernel.org > Cc: stable@vger.kernel.org > Assisted-by: Claude:claude-fable-5 > Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com> > --- > drivers/tty/serial/imx.c | 21 +++++++++++++++++++-- > 1 file changed, 19 insertions(+), 2 deletions(-) > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > index 251a50c8aa38..def874f9cd00 100644 > --- a/drivers/tty/serial/imx.c > +++ b/drivers/tty/serial/imx.c > @@ -22,6 +22,7 @@ > #include <linux/clk.h> > #include <linux/delay.h> > #include <linux/ktime.h> > +#include <linux/mutex.h> > #include <linux/pinctrl/consumer.h> > #include <linux/rational.h> > #include <linux/slab.h> > @@ -2080,6 +2081,15 @@ static const struct uart_ops imx_uart_pops = { > > static struct imx_port *imx_uart_ports[UART_NR]; > > +/* > + * Store the port in imx_uart_ports[] before uart_add_one_port() and clear > + * it only after uart_remove_one_port() returns. Console callbacks in both > + * calls use the table, so this mutex serializes these sequences between > + * sibling ports. Callbacks must not take it because uart_add_one_port() > + * may invoke setup while it is held. > + */ > +static DEFINE_MUTEX(imx_uart_ports_lock); > + > #if IS_ENABLED(CONFIG_SERIAL_IMX_CONSOLE) > static void imx_uart_console_putchar(struct uart_port *port, unsigned char ch) > { > @@ -2632,11 +2642,14 @@ static int imx_uart_probe(struct platform_device *pdev) > } > } > > - imx_uart_ports[sport->port.line] = sport; > - > platform_set_drvdata(pdev, sport); > > + mutex_lock(&imx_uart_ports_lock); > + imx_uart_ports[sport->port.line] = sport; > ret = uart_add_one_port(&imx_uart_uart_driver, &sport->port); > + if (ret) > + imx_uart_ports[sport->port.line] = NULL; > + mutex_unlock(&imx_uart_ports_lock); ret = uart_add_one_port(&imx_uart_uart_driver, &sport->port); if (ret) goto err_clk; imx_uart_ports[sport->port.line] = sport; return 0 and at imx_uart_remove() imx_uart_remove() { imx_uart_ports[line] = NULL; uart_remove_one_port(&imx_uart_uart_driver, &sport->port); } Is above sequence to avoid use mutex? Frank > > err_clk: > clk_disable_unprepare(sport->clk_ipg); > @@ -2647,8 +2660,12 @@ 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; > > + mutex_lock(&imx_uart_ports_lock); > uart_remove_one_port(&imx_uart_uart_driver, &sport->port); > + imx_uart_ports[line] = NULL; > + mutex_unlock(&imx_uart_ports_lock); > } > > static void imx_uart_restore_context(struct imx_port *sport) > -- > 2.53.0 > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-30 19:02 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-19 22:10 [PATCH v2 0/4] serial: fix console lifetime bugs on failed bind and removal Karl Mehltretter 2026-07-19 22:10 ` [PATCH v2 1/4] serial: core: do fallible allocations before the console can be registered Karl Mehltretter 2026-07-30 14:28 ` Greg Kroah-Hartman 2026-07-19 22:10 ` [PATCH v2 4/4] serial: imx: serialize imx_uart_ports[] lifetime Karl Mehltretter 2026-07-30 14:31 ` Greg Kroah-Hartman 2026-07-30 19:02 ` Frank Li
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox