Re: [PATCH v1] mailbox: bcm-pdc: Free the IRQ before cancelling receive work

[Jassi Brar]

On Tue, Sep 15, 2026 at 9:45 AM Yibo Tan <lhfff@xxxxxxxxxx> wrote:
>
> pdc_irq_handler() queues rx_work to process received data.  Because the
> driver requests a managed IRQ, the handler remains registered until
> after probe or remove returns unless the driver frees it explicitly.
>
> The pdc_mb_init() error path and pdc_remove() currently cancel the work
> while the IRQ handler is still registered.  A later interrupt can queue
> the work again.  Cleanup then destroys the DMA pools and frees struct
> pdc_state, so pdc_work_cb() can access freed memory and registers.
>
> KASAN reported an invalid access in pdc_work_cb() when an interrupt
> queued the work during removal.  After this change, removal waited for
> the work to finish and the same test completed without a kernel
> diagnostic.
>
> Add pdc_stop() to disable the hardware, free the managed IRQ and wait for
> rx_work.  Call it in both paths before destroying either DMA pool.
>
> Fixes: 8aef00f090bc ("mailbox: bcm-pdc: Convert from threaded IRQ to tasklet")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: Codex:GPT-5
> Signed-off-by: Yibo Tan <lhfff@xxxxxxxxxx>
> ---
>  drivers/mailbox/bcm-pdc-mailbox.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mailbox/bcm-pdc-mailbox.c b/drivers/mailbox/bcm-pdc-mailbox.c
> index 6fcc4002ad4f..aea2e5273bf0 100644
> --- a/drivers/mailbox/bcm-pdc-mailbox.c
> +++ b/drivers/mailbox/bcm-pdc-mailbox.c
> @@ -1335,6 +1335,15 @@ static void pdc_hw_disable(struct pdc_state *pdcs)
>                   &dma_reg->dmarcv.control);
>  }
>
> +static void pdc_stop(struct pdc_state *pdcs)
> +{
> +       struct device *dev = &pdcs->pdev->dev;
> +
> +       pdc_hw_disable(pdcs);
> +       devm_free_irq(dev, pdcs->pdc_irq, dev);
> +       cancel_work_sync(&pdcs->rx_work);
> +}
> +
>  /**
>   * pdc_rx_buf_pool_create() - Pool of receive buffers used to catch the metadata
>   * header returned with each response message.
> @@ -1581,15 +1590,16 @@ static int pdc_probe(struct platform_device *pdev)
>         /* Initialize mailbox controller */
>         err = pdc_mb_init(pdcs);
>         if (err)
> -               goto cleanup_buf_pool;
> +               goto cleanup_irq;
>
>         pdc_setup_debugfs(pdcs);
>
>         dev_dbg(dev, "pdc_probe() successful");
>         return PDC_SUCCESS;
>
> +cleanup_irq:
> +       pdc_stop(pdcs);
>  cleanup_buf_pool:
> -       cancel_work_sync(&pdcs->rx_work);
>         dma_pool_destroy(pdcs->rx_buf_pool);
>
>  cleanup_ring_pool:
> @@ -1605,9 +1615,7 @@ static void pdc_remove(struct platform_device *pdev)
>
>         pdc_free_debugfs();
>
> -       cancel_work_sync(&pdcs->rx_work);
> -
> -       pdc_hw_disable(pdcs);
> +       pdc_stop(pdcs);
>
>         dma_pool_destroy(pdcs->rx_buf_pool);
>         dma_pool_destroy(pdcs->ring_pool);
>
I am not comfortable picking this patch without ack from someone from
BCM, especially since it seems found and fixed by AI and it changes
the order of teardown.

Regards,
Jasis