Re: [PATCH] serial: mxs-auart: fix 64-bit cast in probe

[sashiko-bot@kernel.org]

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