Re: [PATCHv3 1/6] serial: mxs-auart: fix cast type for of_device_get_match_data

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Out-of-bounds array access on probe error and driver remove paths.
- [High] NULL pointer dereference and uninitialized spinlock access due to early IRQ request.
--

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

serial: mxs-auart: fix cast type for of_device_get_match_data

of_device_get_match_data returns const void*. Cast to unsigned long to
avoid implicit integer truncation warnings. All the data parameters are
correct anyway.

> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> --- 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)

[Severity: High]
This is a pre-existing issue, but I noticed a potential out-of-bounds array
access on the probe error and driver remove paths in mxs_auart_probe().

For Device Tree-probed devices, pdev->id is typically set to -1. The logical
port number is correctly retrieved via of_alias_get_id() and stored in
s->port.line, and the driver instance is registered in the array using:

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

However, on the probe error path, the driver uses pdev->id to clear the array:

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

Would this write NULL to auart_port[-1], silently corrupting adjacent memory
located before the auart_port array?

[Severity: High]
This is also a pre-existing issue, but does this function register the IRQ
handler too early?

In mxs_auart_probe(), devm_request_irq() is called before the hardware is
fully reset and before uart_add_one_port() registers the port:

    ret = devm_request_irq(&pdev->dev, irq, mxs_auart_irq_handle, 0,
                           dev_name(&pdev->dev), s);

If the bootloader leaves the UART hardware with interrupts enabled and
pending, could the interrupt handler fire immediately upon registration?

If it fires early, the handler attempts to lock s->port.lock, which is
uninitialized at this stage, and then calls mxs_auart_rx_chars() which pushes
to the buffer:

    tty_flip_buffer_push(&s->port.state->port);

Since s->port.state is only allocated later during uart_add_one_port(),
would this result in a NULL pointer dereference?

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