Re: [PATCH 3/3] serial: 8250_mtk: Propagate errors from optional IRQ lookup

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

New issues:
- [Critical] The probe function exits with an error without unregistering the previously registered 8250 port, leading to a port leak and a use-after-free of devm-allocated memory.

Pre-existing issues:
- [High] Runtime PM is enabled after the UART port is registered, violating TTY subsystem guidelines and risking circular wait conditions during probe.
--

--- Patch [3]: [PATCH 3/3] serial: 8250_mtk: Propagate errors from optional IRQ lookup ---
commit b5b9e4c48c3e7b1094e5e1c9834f9f8130a17c26
Author: bui duc phuc <[email protected]>

serial: 8250_mtk: Propagate errors from optional IRQ lookup

This commit updates mtk8250_probe() to propagate error codes such as
-EPROBE_DEFER or -EINVAL from platform_get_irq_optional(), rather than
continuing without an IRQ.

> diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> index e6a56cf54ae0c..d0d2aecf90059 100644
> --- a/drivers/tty/serial/8250/8250_mtk.c
> +++ b/drivers/tty/serial/8250/8250_mtk.c

[ ... ]

> @@ -587,6 +587,8 @@ static int mtk8250_probe(struct platform_device *pdev)
>  		return data->line;
>  
>  	data->rx_wakeup_irq = platform_get_irq_optional(pdev, 1);
> +	if (data->rx_wakeup_irq < 0 && data->rx_wakeup_irq != -ENXIO)
> +		return data->rx_wakeup_irq;

[Severity: Critical]
Does this error path leak the registered serial port?

The port was successfully registered earlier in mtk8250_probe() via
serial8250_register_8250_port(), but there is no call to
serial8250_unregister_port() before returning the error here.

Could this result in a use-after-free if the tty device is later accessed?
The devm-allocated memory for the data structure will be automatically freed
when the probe fails, but the port remains active globally.

>  
>  	pm_runtime_set_active(&pdev->dev);
>  	pm_runtime_enable(&pdev->dev);

[Severity: High]
This is a pre-existing issue, but should runtime PM be enabled before
the serial port is registered?

The mtk8250_do_pm() callback invokes pm_runtime_get_sync(). If
serial8250_register_8250_port() synchronously invokes the pm callback
during registration, it will attempt to use uninitialized runtime PM
infrastructure, which can lead to circular wait conditions.

>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3