Linux kernel and device drivers for NXP i.MX platforms
 help / color / mirror / Atom feed
* [PATCH] serial: mxs-auart: fix 64-bit cast in probe
@ 2026-05-28 20:30 Rosen Penev
  2026-05-28 20:49 ` sashiko-bot
  2026-05-29  7:37 ` Jiri Slaby
  0 siblings, 2 replies; 4+ messages in thread
From: Rosen Penev @ 2026-05-28 20:30 UTC (permalink / raw)
  To: linux-serial
  Cc: Greg Kroah-Hartman, Jiri Slaby, Frank Li, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam,
	open list:TTY LAYER AND SERIAL DRIVERS,
	open list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

of_device_get_match_data() returns a pointer. Casting it directly to
enum truncates on 64-bit platforms. Cast to unsigned long instead.

Fixes compilation with W=1.

Assisted-by: Opencode:Big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/tty/serial/mxs-auart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index 697318dbb146..de97c0f74e7d 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -1598,7 +1598,7 @@ static int mxs_auart_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}

-	s->devtype = (enum mxs_auart_type)of_device_get_match_data(&pdev->dev);
+	s->devtype = (unsigned long)of_device_get_match_data(&pdev->dev);

 	ret = mxs_get_clks(s, pdev);
 	if (ret)
--
2.54.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] serial: mxs-auart: fix 64-bit cast in probe
  2026-05-28 20:30 [PATCH] serial: mxs-auart: fix 64-bit cast in probe Rosen Penev
@ 2026-05-28 20:49 ` sashiko-bot
  2026-05-29  7:37 ` Jiri Slaby
  1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-05-28 20:49 UTC (permalink / raw)
  To: Rosen Penev; +Cc: imx, Frank.Li

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

Pre-existing issues:
- [High] Platform ID Logic / Out-of-Bounds Array Access and Dangling Pointer
- [High] Hardware Access Logic / Missing API Call causing External Abort
- [High] Use-After-Unmap / Race Condition between devm_request_irq and iounmap
--

commit 1f0a577246ef846dca74fbf44e87059664a89db4
Author: Rosen Penev <rosenp@gmail.com>

serial: mxs-auart: fix 64-bit cast in probe

of_device_get_match_data() returns a pointer. Casting it directly to
enum truncates on 64-bit platforms. Cast to unsigned long instead.

> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> index 693b491f1e755..3e14c2768c48e 100644
> --- a/drivers/tty/serial/mxs-auart.c
> +++ b/drivers/tty/serial/mxs-auart.c
> @@ -1598,7 +1598,7 @@ static int mxs_auart_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> -	s->devtype = (enum mxs_auart_type)of_device_get_match_data(&pdev->dev);
> +	s->devtype = (unsigned long)of_device_get_match_data(&pdev->dev);
>  
>  	ret = mxs_get_clks(s, pdev);

[Severity: High]
This isn't a bug introduced by this patch, but does this function properly
enable the clock before accessing hardware registers?

For non-ASM9260 variants, the driver fetches the clock via devm_clk_get()
in mxs_get_clks() but does not call clk_prepare_enable() for it. Later in
probe, it reads from the registers:

    version = mxs_read(s, REG_VERSION);

Accessing registers of a peripheral whose clock is disabled can trigger a
synchronous external abort. Is it safe to rely entirely on the bootloader
to leave the clock enabled during probe?

[Severity: High]
This is a pre-existing issue, but does the error path properly clear the
port from the global array?

During probe, the port is registered using the DT alias ID:

    auart_port[s->port.line] = s;

However, the error path uses pdev->id:

    out_free_qpio_irq:
        mxs_auart_free_gpio_irq(s);
        auart_port[pdev->id] = NULL;

For devices probed via the device tree, the platform bus implicitly sets
pdev->id to -1 (PLATFORM_DEVID_NONE). Could this cause an out-of-bounds
write to auart_port[-1], while leaving the actual slot with a dangling
pointer to the freed devm-allocated port?

[Severity: High]
This is a pre-existing issue, but is there a use-after-unmap race condition
in the error path?

The driver manually unmaps I/O memory but uses devm_request_irq() for the
interrupt:

    out_iounmap:
        iounmap(s->port.membase);

Because devm resources are freed only after probe returns, the explicit
iounmap happens while the interrupt handler is still registered. If an
interrupt fires during this window, mxs_auart_irq_handle() will attempt
to read from the unmapped s->port.membase, potentially causing a page
fault. Could the interrupt handling be synchronized with the unmapping?

>  	if (ret)

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] serial: mxs-auart: fix 64-bit cast in probe
  2026-05-28 20:30 [PATCH] serial: mxs-auart: fix 64-bit cast in probe Rosen Penev
  2026-05-28 20:49 ` sashiko-bot
@ 2026-05-29  7:37 ` Jiri Slaby
  2026-05-29  7:40   ` Rosen Penev
  1 sibling, 1 reply; 4+ messages in thread
From: Jiri Slaby @ 2026-05-29  7:37 UTC (permalink / raw)
  To: Rosen Penev, linux-serial
  Cc: Greg Kroah-Hartman, Frank Li, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam,
	open list:TTY LAYER AND SERIAL DRIVERS,
	open list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

On 28. 05. 26, 22:30, Rosen Penev wrote:
> of_device_get_match_data() returns a pointer. Casting it directly to
> enum truncates on 64-bit platforms. Cast to unsigned long instead.

This is a misleading commit log. It still truncates during the assignment.

> Fixes compilation with W=1.

Fixes a warning, not compilation, right?

> Assisted-by: Opencode:Big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>   drivers/tty/serial/mxs-auart.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> index 697318dbb146..de97c0f74e7d 100644
> --- a/drivers/tty/serial/mxs-auart.c
> +++ b/drivers/tty/serial/mxs-auart.c
> @@ -1598,7 +1598,7 @@ static int mxs_auart_probe(struct platform_device *pdev)
>   		return -EINVAL;
>   	}
> 
> -	s->devtype = (enum mxs_auart_type)of_device_get_match_data(&pdev->dev);
> +	s->devtype = (unsigned long)of_device_get_match_data(&pdev->dev);
> 
>   	ret = mxs_get_clks(s, pdev);
>   	if (ret)
> --
> 2.54.0
> 


-- 
js
suse labs

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] serial: mxs-auart: fix 64-bit cast in probe
  2026-05-29  7:37 ` Jiri Slaby
@ 2026-05-29  7:40   ` Rosen Penev
  0 siblings, 0 replies; 4+ messages in thread
From: Rosen Penev @ 2026-05-29  7:40 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: linux-serial, Greg Kroah-Hartman, Frank Li, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam,
	open list:TTY LAYER AND SERIAL DRIVERS,
	open list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

On Fri, May 29, 2026 at 12:37 AM Jiri Slaby <jirislaby@kernel.org> wrote:
>
> On 28. 05. 26, 22:30, Rosen Penev wrote:
> > of_device_get_match_data() returns a pointer. Casting it directly to
> > enum truncates on 64-bit platforms. Cast to unsigned long instead.
>
> This is a misleading commit log. It still truncates during the assignment.
>
> > Fixes compilation with W=1.
>
> Fixes a warning, not compilation, right?
compilation. I believe -Werror is passed.
>
> > Assisted-by: Opencode:Big-pickle
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> >   drivers/tty/serial/mxs-auart.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> > index 697318dbb146..de97c0f74e7d 100644
> > --- a/drivers/tty/serial/mxs-auart.c
> > +++ b/drivers/tty/serial/mxs-auart.c
> > @@ -1598,7 +1598,7 @@ static int mxs_auart_probe(struct platform_device *pdev)
> >               return -EINVAL;
> >       }
> >
> > -     s->devtype = (enum mxs_auart_type)of_device_get_match_data(&pdev->dev);
> > +     s->devtype = (unsigned long)of_device_get_match_data(&pdev->dev);
> >
> >       ret = mxs_get_clks(s, pdev);
> >       if (ret)
> > --
> > 2.54.0
> >
>
>
> --
> js
> suse labs

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-05-29  7:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-28 20:30 [PATCH] serial: mxs-auart: fix 64-bit cast in probe Rosen Penev
2026-05-28 20:49 ` sashiko-bot
2026-05-29  7:37 ` Jiri Slaby
2026-05-29  7:40   ` Rosen Penev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox