Re: [PATCH v3 6/6] virtio_vdpa: implement synchronize_cbs()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Possible Use-After-Free of virtqueue memory in callback during find_vqs error handling
--

commit 05a7e5947030bc4c942e05c96bf1d657ed426d56
Author: Karl Mehltretter <kmehltretter@gmail.com>

virtio_vdpa: implement synchronize_cbs()

virtio_vdpa relies on the core's synchronize_rcu() fallback for callback
synchronization. This commit adds an SRCU domain per device and implements
synchronize_cbs() with synchronize_srcu() to cover the callbacks regardless
of the calling context.

> diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
> index 6bcf4567a9292..f9559b6b2e0b6 100644
> --- a/drivers/virtio/virtio_vdpa.c
> +++ b/drivers/virtio/virtio_vdpa.c
[ ... ]
> @@ -123,8 +128,22 @@ static irqreturn_t virtio_vdpa_config_cb(void *private)
>  static irqreturn_t virtio_vdpa_virtqueue_cb(void *private)
>  {
>  	struct virtqueue *vq = private;
> +	struct virtio_vdpa_device *vd_dev = to_virtio_vdpa_device(vq->vdev);

[Severity: High]
This is a pre-existing issue, but could this callback be invoked
concurrently on a freed vq, leading to a use-after-free when
dereferencing vq->vdev?

During driver probe, if virtio_vdpa_find_vqs() successfully creates some
virtqueues but fails on a subsequent one, it jumps to err_setup_vq and
calls virtio_vdpa_del_vqs():

drivers/virtio/virtio_vdpa.c:virtio_vdpa_find_vqs() {
    ...
err_setup_vq:
	virtio_vdpa_del_vqs(vdev);
    ...
}

This tears down the previously created virtqueues. However, the callback is
not cleared with set_vq_cb(..., NULL), nor does it synchronize with pending
or concurrently running interrupt handlers (such as vp_vdpa_vq_handler).

Could a spurious or device-initiated interrupt occurring right before or
during vring_del_virtqueue(vq) race and cause virtio_vdpa_virtqueue_cb()
to dereference the freed vq structure when accessing vq->vdev?

> +	irqreturn_t ret;
> +	int idx;
> +
> +	idx = srcu_read_lock(&vd_dev->cb_srcu);
> +	ret = vring_interrupt(0, vq);
> +	srcu_read_unlock(&vd_dev->cb_srcu, idx);
> +
> +	return ret;
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908053817.26065-1-kmehltretter@gmail.com?part=6