Re: [PATCH 5/5] remoteproc: imx_rproc: Invoke the callback directly

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unprotected access to `rproc->notifyids` in `imx_rproc_rx_callback` via `idr_for_each` without `rcu_read_lock()` can race with `idr_remove` during `rproc-virtio` unbind, leading to Use-After-Free.
--

commit 54541ad2eee46e430bd395ed2d5711955d46ef22
Author: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

remoteproc: imx_rproc: Invoke the callback directly

The imx-mailbox driver moved callback invocation into the threaded IRQ
handler, making it run in a preemptible context. This patch removes the
workqueue and invokes the imx_rproc_notified_idr_cb() callback directly
from the RX callback.

> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 0dd80e688b0ea..c97bc1c401655 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c

[ ... ]

>  static void imx_rproc_rx_callback(struct mbox_client *cl, void *msg)
>  {
>  	struct rproc *rproc = dev_get_drvdata(cl->dev);
> -	struct imx_rproc *priv = rproc->priv;
>  
> -	queue_work(priv->workqueue, &priv->rproc_work);
> +	idr_for_each(&rproc->notifyids, imx_rproc_notified_idr_cb, rproc);

[Severity: High]
This is a pre-existing issue, but does this idr_for_each() call require
rcu_read_lock() protection?

Looking at the call chain, it also performs an idr_find() in
rproc_vq_interrupt() without rcu_read_lock():

imx_rproc_rx_callback()
  -> idr_for_each()
     -> imx_rproc_notified_idr_cb()
        -> rproc_vq_interrupt()
           -> idr_find()

If a user unbinds the rproc-virtio driver via sysfs concurrently,
rproc_virtio_remove() calls rproc_free_vring() which executes idr_remove().

Since idr_for_each() and idr_find() rely on RCU internally to protect radix
tree nodes, could running this in a preemptible context without
rcu_read_lock() race with idr_remove() and lead to a use-after-free of the
IDR nodes or the rvring structure?

>  }
>  
>  static int imx_rproc_xtr_mbox_init(struct rproc *rproc, bool tx_block)

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