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

On Fri, Aug 7, 2026 at 4:18 PM <[email protected]> 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 <[email protected]>
>
> 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/[email protected]?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.