[PATCH] serial: mps2-uart: Remove the ports_idr entry when probe fails

[PATCH] serial: mps2-uart: Remove the ports_idr entry when probe fails

[Changyul Lee]

mps2_of_get_port() adds mps_port to ports_idr, but mps2_serial_probe()
does not remove the entry when mps2_init_port() or uart_add_one_port()
fails. mps_port is allocated with devm_kzalloc(), so devres frees it
when probe fails, while ports_idr still points to it. When another port
later registers the console, mps2_uart_console_setup() looks up the
console index in ports_idr and uses the freed mps_port:

  BUG: KASAN: slab-use-after-free in uart_set_options+0x650/0x7a4
  Read of size 8 at addr ffff0000018c41a0 by task swapper/0/1
  Call trace:
   uart_set_options
   mps2_uart_console_setup
   try_enable_preferred_console
   register_console
   serial_core_register_port
   serial_ctrl_register_port
   uart_add_one_port
   mps2_serial_probe
  ...
  Freed by task 1:
   kfree
   dr_node_free
   devres_release_all
   device_unbind_cleanup
   really_probe

The stale entry also keeps the ID in use. A re-probe of the port (e.g.
after -EPROBE_DEFER) then gets a different ID than its alias.

Remove the entry on both error paths.

Fixes: 9f25e07b882b ("serial: mps2-uart: move to dynamic port allocation")
Signed-off-by: Changyul Lee <lcy8047@gmail.com>
---

Notes:
    Found with a static analysis tool we are developing.
    
    Reproduced in QEMU (arm64 virt, CONFIG_COMPILE_TEST, KASAN) with two
    arm,mps2-uart nodes added to the device tree: serial0 without a
    "clocks" property, so devm_clk_get() fails after the ID is allocated,
    and serial1 with a fixed clock, booted with console=ttyMPS0. Without
    this patch KASAN reports the use-after-free above. With it, there is no
    KASAN report and the boot goes on until it looks for a root filesystem
    (the test has none).
    Not tested on MPS2 hardware.

 drivers/tty/serial/mps2-uart.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/mps2-uart.c b/drivers/tty/serial/mps2-uart.c
index 2a4c09f3a834..c39aa534b822 100644
--- a/drivers/tty/serial/mps2-uart.c
+++ b/drivers/tty/serial/mps2-uart.c
@@ -590,15 +590,19 @@ static int mps2_serial_probe(struct platform_device *pdev)
 
 	ret = mps2_init_port(pdev, mps_port);
 	if (ret)
-		return ret;
+		goto err_remove_id;
 
 	ret = uart_add_one_port(&mps2_uart_driver, &mps_port->port);
 	if (ret)
-		return ret;
+		goto err_remove_id;
 
 	platform_set_drvdata(pdev, mps_port);
 
 	return 0;
+
+err_remove_id:
+	idr_remove(&ports_idr, mps_port->port.line);
+	return ret;
 }
 
 #ifdef CONFIG_OF

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.43.0

Re: [PATCH] serial: mps2-uart: Remove the ports_idr entry when probe fails

[Vladimir Murzin]

On 11/6/26 01:06, Changyul Lee wrote:
> mps2_of_get_port() adds mps_port to ports_idr, but mps2_serial_probe()
> does not remove the entry when mps2_init_port() or uart_add_one_port()
> fails. mps_port is allocated with devm_kzalloc(), so devres frees it
> when probe fails, while ports_idr still points to it. When another port
> later registers the console, mps2_uart_console_setup() looks up the
> console index in ports_idr and uses the freed mps_port:
> 
>   BUG: KASAN: slab-use-after-free in uart_set_options+0x650/0x7a4
>   Read of size 8 at addr ffff0000018c41a0 by task swapper/0/1
>   Call trace:
>    uart_set_options
>    mps2_uart_console_setup
>    try_enable_preferred_console
>    register_console
>    serial_core_register_port
>    serial_ctrl_register_port
>    uart_add_one_port
>    mps2_serial_probe
>   ...
>   Freed by task 1:
>    kfree
>    dr_node_free
>    devres_release_all
>    device_unbind_cleanup
>    really_probe
> 
> The stale entry also keeps the ID in use. A re-probe of the port (e.g.
> after -EPROBE_DEFER) then gets a different ID than its alias.
> 
> Remove the entry on both error paths.
> 
> Fixes: 9f25e07b882b ("serial: mps2-uart: move to dynamic port allocation")
> Signed-off-by: Changyul Lee <lcy8047@gmail.com>
> ---
> 
> Notes:
>     Found with a static analysis tool we are developing.
>     
>     Reproduced in QEMU (arm64 virt, CONFIG_COMPILE_TEST, KASAN) with two
>     arm,mps2-uart nodes added to the device tree: serial0 without a
>     "clocks" property, so devm_clk_get() fails after the ID is allocated,
>     and serial1 with a fixed clock, booted with console=ttyMPS0. Without
>     this patch KASAN reports the use-after-free above. With it, there is no
>     KASAN report and the boot goes on until it looks for a root filesystem
>     (the test has none).
>     Not tested on MPS2 hardware.

Before getting into the review, I'd like to clarify a few things:

- This driver is only used on a NOMMU platform, where any code
  can write to any memory location, so the security impact of
  this issue is likely minimal

- The only platform that uses this driver is deprecated and
  scheduled for removal [1], with the driver to be removed
  afterwards

@Arnd, could you please advise on how to proceed? Should we
review the patch, leave things as they are, or take a different
approach?

[1] https://lore.kernel.org/linux-arm-kernel/20260908152808.3928630-12-arnd@kernel.org/

> 
>  drivers/tty/serial/mps2-uart.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/mps2-uart.c b/drivers/tty/serial/mps2-uart.c
> index 2a4c09f3a834..c39aa534b822 100644
> --- a/drivers/tty/serial/mps2-uart.c
> +++ b/drivers/tty/serial/mps2-uart.c
> @@ -590,15 +590,19 @@ static int mps2_serial_probe(struct platform_device *pdev)
>  
>  	ret = mps2_init_port(pdev, mps_port);
>  	if (ret)
> -		return ret;
> +		goto err_remove_id;
>  
>  	ret = uart_add_one_port(&mps2_uart_driver, &mps_port->port);
>  	if (ret)
> -		return ret;
> +		goto err_remove_id;
>  
>  	platform_set_drvdata(pdev, mps_port);
>  
>  	return 0;
> +
> +err_remove_id:
> +	idr_remove(&ports_idr, mps_port->port.line);
> +	return ret;
>  }
>  
>  #ifdef CONFIG_OF
> 
> base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
> -- 2.43.0
> 

Re: [PATCH] serial: mps2-uart: Remove the ports_idr entry when probe fails

[Arnd Bergmann]

On Wed, Nov 11, 2026, at 15:17, Vladimir Murzin wrote:
> On 11/6/26 01:06, Changyul Lee wrote:
>
> Before getting into the review, I'd like to clarify a few things:
>
> - This driver is only used on a NOMMU platform, where any code
>   can write to any memory location, so the security impact of
>   this issue is likely minimal
>
> - The only platform that uses this driver is deprecated and
>   scheduled for removal [1], with the driver to be removed
>   afterwards
>
> @Arnd, could you please advise on how to proceed? Should we
> review the patch, leave things as they are, or take a different
> approach?
>
> [1] 
> https://lore.kernel.org/linux-arm-kernel/20260908152808.3928630-12-arnd@kernel.org/

I don't think it's super important, either applying as-is
or ignoring seems fine, just don't spend too much time
on a review.

For some of the platforms I'm planning to remove it would
be nice to backport fixesto the LTS kernels in case there
are some users after all.

Chanyul, it looks like you've done a thorough job testing this
patch, and you probably have similar work for other drivers
coming. I would suggest you try to prioritize the drivers that
are likely to be in use somewhere.

I have a branch for removing a lot of drivers including this
one, maybe check your other patches to see which ones are
obsolete.

      Arnd

[1] https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git/log/?h=board-remove-7.4