[PATCH 1/3] serial: qcom-geni: Propagate errors from optional IRQ lookup

[PATCH 1/3] serial: qcom-geni: Propagate errors from optional IRQ lookup

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

platform_get_irq_optional() returns a positive IRQ number on success or
a negative error code on failure. For an optional IRQ, -ENXIO indicates
that no IRQ is available, while other errors should be propagated.

Propagate errors such as -EPROBE_DEFER and -EINVAL instead of continuing
probe without the IRQ.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 drivers/tty/serial/qcom_geni_serial.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 7ead87b4eb65..240a6d67d847 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -1883,8 +1883,13 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
 	uport->irq = irq;
 	uport->has_sysrq = IS_ENABLED(CONFIG_SERIAL_QCOM_GENI_CONSOLE);
 
-	if (!data->console)
+	if (!data->console) {
 		port->wakeup_irq = platform_get_irq_optional(pdev, 1);
+		if (port->wakeup_irq < 0 && port->wakeup_irq != -ENXIO) {
+			ret = port->wakeup_irq;
+			goto error;
+		}
+	}
 
 	if (of_property_read_bool(pdev->dev.of_node, "rx-tx-swap"))
 		port->rx_tx_swap = true;
-- 
2.43.0

[PATCH 2/3] tty: serial: imx: Propagate errors from optional IRQ lookup

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

platform_get_irq_optional() returns a positive IRQ number on success or
a negative error code on failure. For an optional IRQ, -ENXIO indicates
that no IRQ is available, while other errors should be propagated.

Propagate errors such as -EPROBE_DEFER and -EINVAL instead of continuing
probe without the IRQ.

Signed-off-by: bui duc phuc <phucduc.bui@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..4224454d360e 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -2463,7 +2463,11 @@ static int imx_uart_probe(struct platform_device *pdev)
 	if (rxirq < 0)
 		return rxirq;
 	txirq = platform_get_irq_optional(pdev, 1);
+	if (txirq < 0 && txirq != -ENXIO)
+		return txirq;
 	rtsirq = platform_get_irq_optional(pdev, 2);
+	if (rtsirq < 0 && rtsirq != -ENXIO)
+		return rtsirq;
 
 	sport->port.dev = &pdev->dev;
 	sport->port.mapbase = res->start;
-- 
2.43.0

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

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

platform_get_irq_optional() returns a positive IRQ number on success or
a negative error code on failure. For an optional IRQ, -ENXIO indicates
that no IRQ is available, while other errors should be propagated.

Propagate errors such as -EPROBE_DEFER and -EINVAL instead of continuing
probe without the IRQ.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 drivers/tty/serial/8250/8250_mtk.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
index e6a56cf54ae0..d0d2aecf9005 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;
 
 	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
-- 
2.43.0

Re: [PATCH 1/3] serial: qcom-geni: Propagate errors from optional IRQ lookup

[sashiko-bot]

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

Pre-existing issues:
- [High] The error cleanup path in `qcom_geni_serial_probe()` leaks the IDA allocated for the UART line index, which is severely exacerbated by the newly added IRQ probe deferral path.
--

--- Patch [1]: [PATCH 1/3] serial: qcom-geni: Propagate errors from optional IRQ lookup ---
commit 9ec5f0b3d9989a6071c2d4680336709af497a5bf
Author: bui duc phuc <phucduc.bui@gmail.com>

serial: qcom-geni: Propagate errors from optional IRQ lookup

This commit updates qcom_geni_serial_probe() to propagate errors, such as
-EPROBE_DEFER, from platform_get_irq_optional() for the wakeup IRQ.
Previously, errors other than -ENXIO were ignored and probe continued.

> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 7ead87b4eb65b..240a6d67d8475 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -1883,8 +1883,13 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>  	uport->irq = irq;
>  	uport->has_sysrq = IS_ENABLED(CONFIG_SERIAL_QCOM_GENI_CONSOLE);
>  
> -	if (!data->console)
> +	if (!data->console) {
>  		port->wakeup_irq = platform_get_irq_optional(pdev, 1);
> +		if (port->wakeup_irq < 0 && port->wakeup_irq != -ENXIO) {
> +			ret = port->wakeup_irq;
> +			goto error;

[Severity: High]
This is a pre-existing issue, but does this goto error path leak the port IDA?

In get_port_from_line(), an IDA is allocated for the UART line index with a
very restricted range:

drivers/tty/serial/qcom_geni_serial.c:get_port_from_line() {
    ...
		if (line < 0)
			line = ida_alloc_range(&port_ida, max_alias_num + 1,
					       nr_ports - 1, GFP_KERNEL);
		else
			line = ida_alloc_range(&port_ida, line,
					       nr_ports - 1, GFP_KERNEL);
    ...
}

However, when jumping to the error label in qcom_geni_serial_probe(), the
cleanup only detaches PM domains and misses calling ida_free():

drivers/tty/serial/qcom_geni_serial.c:qcom_geni_serial_probe() {
    ...
error:
	dev_pm_domain_detach_list(port->pd_list);
	return ret;
}

Since this patch adds a check that propagates -EPROBE_DEFER when the optional
wakeup IRQ is not yet ready, this error path will be frequently hit during
normal boot.

Because the max IDA limit is very small, just a few probe deferrals will
exhaust the available IDs, causing get_port_from_line() to return -ENXIO and
permanently preventing the UART driver from probing on subsequent attempts.

Should ida_free(&port_ida, uport->line) be called before jumping to the error
label, or added to the error label itself, to prevent exhausting the ID space?

> +		}
> +	}
>  
>  	if (of_property_read_bool(pdev->dev.of_node, "rx-tx-swap"))
>  		port->rx_tx_swap = true;

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

Re: [PATCH 2/3] tty: serial: imx: Propagate errors from optional IRQ lookup

[sashiko-bot]

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

Pre-existing issues:
- [High] The global `imx_uart_ports` array retains a dangling pointer to a freed `sport` object if `uart_add_one_port()` fails during probe, or after device unbind.
--

--- Patch [2]: [PATCH 2/3] tty: serial: imx: Propagate errors from optional IRQ lookup ---
commit 1b33dc8f639bc337c3928860fbfd0de822e373c4
Author: bui duc phuc <phucduc.bui@gmail.com>

tty: serial: imx: Propagate errors from optional IRQ lookup

This commit updates imx_uart_probe() to properly propagate error codes
from platform_get_irq_optional(). It ensures that errors like probe
deferrals are returned rather than ignored.

> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 251a50c8aa387..4224454d360ef 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -2463,7 +2463,11 @@ static int imx_uart_probe(struct platform_device *pdev)
>  	if (rxirq < 0)
>  		return rxirq;
>  	txirq = platform_get_irq_optional(pdev, 1);
> +	if (txirq < 0 && txirq != -ENXIO)
> +		return txirq;
>  	rtsirq = platform_get_irq_optional(pdev, 2);
> +	if (rtsirq < 0 && rtsirq != -ENXIO)
> +		return rtsirq;
>  
>  	sport->port.dev = &pdev->dev;
>  	sport->port.mapbase = res->start;

[Severity: High]
This is a pre-existing issue, but looking further down in imx_uart_probe(),
does the error path for uart_add_one_port() leave a dangling pointer in the
global array?

If uart_add_one_port() fails, we see this sequence:

drivers/tty/serial/imx.c:imx_uart_probe() {
    ...
    imx_uart_ports[sport->port.line] = sport;
    ...
    ret = uart_add_one_port(&imx_uart_uart_driver, &sport->port);

err_clk:
    clk_disable_unprepare(sport->clk_ipg);
    return ret;
}

When this returns an error, devres will free the memory for sport, but
the global imx_uart_ports array still retains the pointer. Can a subsequent
console operation dereference this freed memory?

Additionally, the same issue appears to exist in imx_uart_remove() when
the device is unbound:

drivers/tty/serial/imx.c:imx_uart_remove() {
    struct imx_port *sport = platform_get_drvdata(pdev);

    uart_remove_one_port(&imx_uart_uart_driver, &sport->port);
}

Should this removal function explicitly clear the pointer from
imx_uart_ports to avoid leaving a dangling pointer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807090945.41690-1-phucduc.bui@gmail.com?part=2

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

[sashiko-bot]

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 <phucduc.bui@gmail.com>

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/20260807090945.41690-1-phucduc.bui@gmail.com?part=3

Re: [PATCH 1/3] serial: qcom-geni: Propagate errors from optional IRQ lookup

[Bui Duc Phuc]

On Fri, Aug 7, 2026 at 4:17 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] The error cleanup path in `qcom_geni_serial_probe()` leaks the IDA allocated for the UART line index, which is severely exacerbated by the newly added IRQ probe deferral path.
> --
>
> --- Patch [1]: [PATCH 1/3] serial: qcom-geni: Propagate errors from optional IRQ lookup ---
> commit 9ec5f0b3d9989a6071c2d4680336709af497a5bf
> Author: bui duc phuc <phucduc.bui@gmail.com>
>
> serial: qcom-geni: Propagate errors from optional IRQ lookup
>
> This commit updates qcom_geni_serial_probe() to propagate errors, such as
> -EPROBE_DEFER, from platform_get_irq_optional() for the wakeup IRQ.
> Previously, errors other than -ENXIO were ignored and probe continued.
>
> > diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> > index 7ead87b4eb65b..240a6d67d8475 100644
> > --- a/drivers/tty/serial/qcom_geni_serial.c
> > +++ b/drivers/tty/serial/qcom_geni_serial.c
> > @@ -1883,8 +1883,13 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
> >       uport->irq = irq;
> >       uport->has_sysrq = IS_ENABLED(CONFIG_SERIAL_QCOM_GENI_CONSOLE);
> >
> > -     if (!data->console)
> > +     if (!data->console) {
> >               port->wakeup_irq = platform_get_irq_optional(pdev, 1);
> > +             if (port->wakeup_irq < 0 && port->wakeup_irq != -ENXIO) {
> > +                     ret = port->wakeup_irq;
> > +                     goto error;
>
> [Severity: High]
> This is a pre-existing issue, but does this goto error path leak the port IDA?
>
> In get_port_from_line(), an IDA is allocated for the UART line index with a
> very restricted range:
>
> drivers/tty/serial/qcom_geni_serial.c:get_port_from_line() {
>     ...
>                 if (line < 0)
>                         line = ida_alloc_range(&port_ida, max_alias_num + 1,
>                                                nr_ports - 1, GFP_KERNEL);
>                 else
>                         line = ida_alloc_range(&port_ida, line,
>                                                nr_ports - 1, GFP_KERNEL);
>     ...
> }
>
> However, when jumping to the error label in qcom_geni_serial_probe(), the
> cleanup only detaches PM domains and misses calling ida_free():
>
> drivers/tty/serial/qcom_geni_serial.c:qcom_geni_serial_probe() {
>     ...
> error:
>         dev_pm_domain_detach_list(port->pd_list);
>         return ret;
> }
>
> Since this patch adds a check that propagates -EPROBE_DEFER when the optional
> wakeup IRQ is not yet ready, this error path will be frequently hit during
> normal boot.
>
> Because the max IDA limit is very small, just a few probe deferrals will
> exhaust the available IDs, causing get_port_from_line() to return -ENXIO and
> permanently preventing the UART driver from probing on subsequent attempts.
>
> Should ida_free(&port_ida, uport->line) be called before jumping to the error
> label, or added to the error label itself, to prevent exhausting the ID space?
>
> > +             }
> > +     }
> >
> >       if (of_property_read_bool(pdev->dev.of_node, "rx-tx-swap"))
> >               port->rx_tx_swap = true;
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260807090945.41690-1-phucduc.bui@gmail.com?part=1


I noticed that get_port_from_line() is also called from other
functions, such as qcom_geni_console_setup()
and qcom_geni_serial_console_write().
Since this is a pre-existing issue, I think it would be better to
address it in a separate patch if we decide to fix it.
For the current patch, I would prefer to keep it as is.

Re: [PATCH 2/3] tty: serial: imx: Propagate errors from optional IRQ lookup

[Bui Duc Phuc]

On Fri, Aug 7, 2026 at 4:18 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] The global `imx_uart_ports` array retains a dangling pointer to a freed `sport` object if `uart_add_one_port()` fails during probe, or after device unbind.
> --
>
> --- Patch [2]: [PATCH 2/3] tty: serial: imx: Propagate errors from optional IRQ lookup ---
> commit 1b33dc8f639bc337c3928860fbfd0de822e373c4
> Author: bui duc phuc <phucduc.bui@gmail.com>
>
> tty: serial: imx: Propagate errors from optional IRQ lookup
>
> This commit updates imx_uart_probe() to properly propagate error codes
> from platform_get_irq_optional(). It ensures that errors like probe
> deferrals are returned rather than ignored.
>
> > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> > index 251a50c8aa387..4224454d360ef 100644
> > --- a/drivers/tty/serial/imx.c
> > +++ b/drivers/tty/serial/imx.c
> > @@ -2463,7 +2463,11 @@ static int imx_uart_probe(struct platform_device *pdev)
> >       if (rxirq < 0)
> >               return rxirq;
> >       txirq = platform_get_irq_optional(pdev, 1);
> > +     if (txirq < 0 && txirq != -ENXIO)
> > +             return txirq;
> >       rtsirq = platform_get_irq_optional(pdev, 2);
> > +     if (rtsirq < 0 && rtsirq != -ENXIO)
> > +             return rtsirq;
> >
> >       sport->port.dev = &pdev->dev;
> >       sport->port.mapbase = res->start;
>
> [Severity: High]
> This is a pre-existing issue, but looking further down in imx_uart_probe(),
> does the error path for uart_add_one_port() leave a dangling pointer in the
> global array?
>
> If uart_add_one_port() fails, we see this sequence:
>
> drivers/tty/serial/imx.c:imx_uart_probe() {
>     ...
>     imx_uart_ports[sport->port.line] = sport;
>     ...
>     ret = uart_add_one_port(&imx_uart_uart_driver, &sport->port);
>
> err_clk:
>     clk_disable_unprepare(sport->clk_ipg);
>     return ret;
> }
>
> When this returns an error, devres will free the memory for sport, but
> the global imx_uart_ports array still retains the pointer. Can a subsequent
> console operation dereference this freed memory?
>
> Additionally, the same issue appears to exist in imx_uart_remove() when
> the device is unbound:
>
> drivers/tty/serial/imx.c:imx_uart_remove() {
>     struct imx_port *sport = platform_get_drvdata(pdev);
>
>     uart_remove_one_port(&imx_uart_uart_driver, &sport->port);
> }
>
> Should this removal function explicitly clear the pointer from
> imx_uart_ports to avoid leaving a dangling pointer?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260807090945.41690-1-phucduc.bui@gmail.com?part=2

This is a pre-existing issue, so if it is confirmed to be a real problem,
I think it would be better to address it in a separate patch.
For the current patch, I'd prefer to keep the scope limited to propagating
errors from the optional IRQ lookup.

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

[Bui Duc Phuc]

On Fri, Aug 7, 2026 at 4:22 PM <sashiko-bot@kernel.org> wrote:
>
> 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 <phucduc.bui@gmail.com>
>
> 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);
>

I agree that this makes sense. I'll make the change and send v2.

> [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/20260807090945.41690-1-phucduc.bui@gmail.com?part=3

In mtk8250_probe(), uart.port.pm is set to mtk8250_do_pm:

uart.port.pm = mtk8250_do_pm;

In serial8250_register_8250_port(), this part:

if (uart->port.type != PORT_8250_CIR) {
        if (uart_console_registered(&uart->port))
                pm_runtime_get_sync(uart->port.dev);

calls pm_runtime_get_sync() directly on the device, and I don't see a
direct call to port.pm here.
it may be invoked in some other helper function called within
serial8250_register_8250_port() itself.

That said, this is a pre-existing issue, so I'd like to keep this
patch scoped as-is and not address it here.