Good day,
On Tue, Sep 22, 2026 at 03:01:40PM -0400, Yuho Choi wrote:
> rproc_vq_interrupt() loads rvring->vq twice: once to test it for NULL and
> once to pass it to vring_interrupt(). __rproc_virtio_del_vqs() clears the
> same field, so the second load can return NULL after the first one has
> passed the test, and vring_interrupt() dereferences its argument without
> checking it.
>
> Load the pointer once into a local variable and test that, and pair the
> read with WRITE_ONCE() on the store that clears the field.
>
> Fixes: 7a186941626d ("remoteproc: remove the single rpmsg vdev limitation")
> Signed-off-by: Yuho Choi <[email protected]>
> ---
> Found by code review; compile-tested only.
>
> drivers/remoteproc/remoteproc_virtio.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/remoteproc/remoteproc_virtio.c
> b/drivers/remoteproc/remoteproc_virtio.c
> index d5e9ff045a28a..fafd73e1368a0 100644
> --- a/drivers/remoteproc/remoteproc_virtio.c
> +++ b/drivers/remoteproc/remoteproc_virtio.c
> @@ -89,14 +89,19 @@ static bool rproc_virtio_notify(struct virtqueue *vq)
> irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int notifyid)
> {
> struct rproc_vring *rvring;
> + struct virtqueue *vq;
>
> dev_dbg(&rproc->dev, "vq index %d is interrupted\n", notifyid);
>
> rvring = idr_find(&rproc->notifyids, notifyid);
> - if (!rvring || !rvring->vq)
> + if (!rvring)
> + return IRQ_NONE;
> +
> + vq = READ_ONCE(rvring->vq);
> + if (!vq)
> return IRQ_NONE;
>
We could lose the CPU right here and meanwhile, __rproc_virtio_del_vqs() deletes
@vq. I don't see how READ_ONCE/WRITE_ONCE would change that.
Thanks,
Mathieu
> - return vring_interrupt(0, rvring->vq);
> + return vring_interrupt(0, vq);
> }
> EXPORT_SYMBOL(rproc_vq_interrupt);
>
> @@ -170,7 +175,7 @@ static void __rproc_virtio_del_vqs(struct virtio_device
> *vdev)
>
> list_for_each_entry_safe(vq, n, &vdev->vqs, list) {
> rvring = vq->priv;
> - rvring->vq = NULL;
> + WRITE_ONCE(rvring->vq, NULL);
> vring_del_virtqueue(vq);
> }
> }
>
> base-commit: f0100363d8c374bd8e9ea7c9ba02744f0b802ca4
> --
> 2.43.0
>
