Re: [PATCH] tty: serial: meson: fix UART driver lifetime

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Failure to map device memory during port configuration causes a null pointer dereference during UART startup.
--

commit f142190e115d4a0b9969d2dd8bd4d9461c01f15a
Author: Myeonghun Pak <mhun512@gmail.com>

    tty: serial: meson: fix UART driver lifetime

    Moving Amlogic UART registration into probe left the first driver
    registration behind if uart_add_one_port() failed. Track whether probe
    registered the driver and undo it when port addition fails. Serialize
    the shared lifecycle decisions against parallel probe and remove
    operations.

> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index a6cb2a535f9dc..4bd9d98268b43 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
[ ... ]
> @@ -781,9 +800,14 @@ static int meson_uart_probe(struct platform_device *pdev)
>  	}
>  
>  	ret = uart_add_one_port(uart_driver, port);

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a pre-existing issue
where failure to map device memory during port configuration causes a null
pointer dereference during UART startup.

If devm_request_mem_region() or devm_ioremap() fails during
meson_uart_request_port() (which is called via the .config_port callback
during uart_add_one_port), the failure is silently ignored:

drivers/tty/serial/meson_uart.c:meson_uart_config_port() {
	if (flags & UART_CONFIG_TYPE) {
		port->type = PORT_MESON;
		meson_uart_request_port(port);
	}
}

Because port->type is unconditionally set to PORT_MESON, the serial core
believes the port is successfully configured. When user space subsequently
opens the corresponding tty device, meson_uart_startup() is invoked and
executes:

drivers/tty/serial/meson_uart.c:meson_uart_startup() {
	uart_port_lock_irqsave(port, &flags);

	val = readl(port->membase + AML_UART_CONTROL);
	...
}

Since port->membase is NULL, this readl() causes an immediate kernel panic on
device open.

Could meson_uart_config_port() be updated to only set port->type if
meson_uart_request_port() succeeds, preventing the serial core from using an
unconfigured port?

> -	if (ret)
> +	if (ret) {
>  		meson_ports[pdev->id] = NULL;
> +		if (registered)
> +			uart_unregister_driver(uart_driver);
> +	}
>  
> +out_unlock:
> +	mutex_unlock(&meson_uart_mutex);
>  	return ret;
>  }

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

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic