rproc_vq_interrupt() looks up rvring->vq and passes it to vring_interrupt() without synchronizing against __rproc_virtio_del_vqs(), which clears rvring->vq and frees the virtqueue. rproc_stop() and __rproc_detach() stop the subdevices, and so delete the virtqueues, before the remote processor is stopped or detached, so the remote can still signal a vring meanwhile. The interrupt path can then pass a NULL or already freed virtqueue to vring_interrupt(). Protect rvring->vq with SRCU: rproc_vq_interrupt() uses the virtqueue inside a read-side section, and __rproc_virtio_del_vqs() clears the pointers and waits for readers before freeing the virtqueues. Plain RCU is not usable because rproc_vq_interrupt() is also called from process context, where vring callbacks may sleep. Fixes: 7a186941626d ("remoteproc: remove the single rpmsg vdev limitation") Cc: stable@vger.kernel.org Signed-off-by: Yuho Choi --- Changes in v2: - Rework the fix: v1 only loaded rvring->vq once, which closes the NULL window but not the use-after-free once the virtqueue is freed after the load (Mathieu Poirier). Use SRCU so __rproc_virtio_del_vqs() waits for rproc_vq_interrupt() before freeing the virtqueues. - Update the subject accordingly. v1: https://lore.kernel.org/all/20260922190159.509836-1-oss.patchbox@gmail.com/ Compile-tested only (arm64 and x86_64 defconfig + REMOTEPROC/RPMSG_VIRTIO, W=1, sparse). drivers/remoteproc/remoteproc_virtio.c | 35 +++++++++++++++++++++----- include/linux/remoteproc.h | 2 +- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c index d5e9ff045a28a..cd8f3a8b4870a 100644 --- a/drivers/remoteproc/remoteproc_virtio.c +++ b/drivers/remoteproc/remoteproc_virtio.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -26,6 +27,14 @@ #include "remoteproc_internal.h" +/* + * Protects rproc_vring->vq: rproc_vq_interrupt() uses the virtqueue inside a + * read-side section, and __rproc_virtio_del_vqs() waits for those sections to + * finish before freeing it. Sleepable RCU is used because vring callbacks may + * sleep when rproc_vq_interrupt() is called from process context. + */ +DEFINE_STATIC_SRCU(rproc_vq_srcu); + static int copy_dma_range_map(struct device *to, struct device *from) { const struct bus_dma_region *map = from->dma_range_map, *new_map, *r; @@ -88,15 +97,24 @@ static bool rproc_virtio_notify(struct virtqueue *vq) */ irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int notifyid) { + irqreturn_t ret = IRQ_NONE; struct rproc_vring *rvring; + struct virtqueue *vq; + int idx; 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; - return vring_interrupt(0, rvring->vq); + idx = srcu_read_lock(&rproc_vq_srcu); + vq = srcu_dereference(rvring->vq, &rproc_vq_srcu); + if (vq) + ret = vring_interrupt(0, vq); + srcu_read_unlock(&rproc_vq_srcu, idx); + + return ret; } EXPORT_SYMBOL(rproc_vq_interrupt); @@ -153,8 +171,8 @@ static struct virtqueue *rp_find_vq(struct virtio_device *vdev, vq->num_max = num; - rvring->vq = vq; vq->priv = rvring; + rcu_assign_pointer(rvring->vq, vq); /* Update vring in resource table */ rsc = (void *)rproc->table_ptr + rvdev->rsc_offset; @@ -168,11 +186,16 @@ static void __rproc_virtio_del_vqs(struct virtio_device *vdev) struct virtqueue *vq, *n; struct rproc_vring *rvring; - list_for_each_entry_safe(vq, n, &vdev->vqs, list) { + list_for_each_entry(vq, &vdev->vqs, list) { rvring = vq->priv; - rvring->vq = NULL; - vring_del_virtqueue(vq); + RCU_INIT_POINTER(rvring->vq, NULL); } + + /* Wait for rproc_vq_interrupt() callers still using the virtqueues */ + synchronize_srcu(&rproc_vq_srcu); + + list_for_each_entry_safe(vq, n, &vdev->vqs, list) + vring_del_virtqueue(vq); } static void rproc_virtio_del_vqs(struct virtio_device *vdev) diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h index a44368737b39a..919d19fe99198 100644 --- a/include/linux/remoteproc.h +++ b/include/linux/remoteproc.h @@ -340,7 +340,7 @@ struct rproc_vring { u32 align; int notifyid; struct rproc_vdev *rvdev; - struct virtqueue *vq; + struct virtqueue __rcu *vq; }; /** base-commit: aa98230e410f0ed212b6788c46b1e4d49e0ff7ca -- 2.43.0