Re: [PATCH 2/5] mailbox: imx: Move the RX part of the mailbox into the threaded handler

[sashiko-bot@kernel.org]

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

New issues:
- [High] Unconditional unmasking of the RX interrupt in the threaded IRQ handler races with `imx_mu_shutdown()`, potentially leaving the hardware interrupt permanently enabled after the IRQ handler is freed.

Pre-existing issues:
- [High] Permanent denial of service (DoS) on the RX channel due to missing interrupt unmasking on error paths.
--

commit 7ab3fb54b60e0decf23b64d1d78df0e2b4409f2a
Author: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

mailbox: imx: Move the RX part of the mailbox into the threaded handler

This commit moves the RX mailbox callback handling into the threaded
interrupt handler. It updates the driver to mask the RX interrupt in the
primary handler and unmask it in the threaded handler upon success.

> diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
> --- a/drivers/mailbox/imx-mailbox.c
> +++ b/drivers/mailbox/imx-mailbox.c

[ ... ]

> @@ -524,6 +522,12 @@ static irqreturn_t imx_mu_isr_th(int irq, void *p)
>  		mbox_chan_txdone(chan, 0);
>  		return IRQ_HANDLED;
>  
> +	case IMX_MU_TYPE_RX:
> +		cp->pending = 0;
> +		if (!priv->dcfg->rx(priv, cp))
> +			imx_mu_xcr_rmw(priv, IMX_MU_RCR, IMX_MU_xCR_RIEn(priv->dcfg->type, cp->idx), 0);

[Severity: High]
This is a pre-existing issue, but does bypassing the unmasking step on
error permanently disable the channel?

If imx_mu_specific_rx() encounters an error, such as a read timeout or
receiving a message that exceeds the maximum size, it returns a negative
error code. When this happens, the condition evaluates to false and the
interrupt remains masked.

Since there does not seem to be a mechanism to automatically reset the
channel after these errors, could a buggy remote processor trigger this
to cause a permanent denial of service for the RX channel?

[Severity: High]
Can this unmasking step also race with channel shutdown?

If mbox_free_channel() is called concurrently, imx_mu_shutdown() masks the
interrupt and invokes free_irq(). The free_irq() function synchronizes
with the threaded handler and will block until imx_mu_isr_th() finishes
processing.

Because imx_mu_isr_th() proceeds to unmask the interrupt here after
processing, the hardware interrupt will be left enabled after the kernel
IRQ handler is fully unregistered. If a new message arrives afterward,
won't this cause an interrupt storm and prompt the genirq core to disable
the shared IRQ line entirely?

> +		return IRQ_HANDLED;
> +
>  	default:
>  		dev_warn_ratelimited(priv->dev, "Unhandled channel type %d\n",

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529-imx_mbox_rproc-v1-0-b8ffc36e11e5@linutronix.de?part=2