Re: [Devel] [PATCH vz10] drivers/vhost/blk: take vq->mutex in the kick handlers

[Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>]

LGTM
Reviewed-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>

On 9/28/26 17:25, Konstantin Khorenko wrote:
> vhost.h documents vhost_vq_set_backend()/vhost_vq_get_backend() as
> "need to call with vq->mutex acquired": vq->private_data is a plain
> field with no barriers.
>    * vhost-net, vsock and scsi all take vq->mutex in their kick handlers
>    * vhost-blk reads the backend and works with the ring without it
> 
> On a weakly ordered CPU (arm64) the two stores can become visible in the
> opposite order:
> 
>      CPU0: VHOST_BLK_SET_BACKEND           CPU1: vhost worker
>      -----------------------------------   -------------------------------
>      blk_vq->req = kvmalloc_array(...);
>      mutex_lock(&vq->mutex);
>      vhost_vq_set_backend(vq, backend);
>      mutex_unlock(&vq->mutex);
> 
>      backend store becomes visible
>                                            f = vhost_vq_get_backend(vq);
>                                            /* sees the new backend */
>                                            head = vhost_get_vq_desc(...);
>                                            req = &blk_vq->req[head];
>                                            /* sees the old req: NULL on
>                                               the first SET_BACKEND, a
>                                               freed pointer after a -1/fd
>                                               cycle - a NULL-offset oops
>                                               or a use-after-free */
>      req store becomes visible - too late
> 
> x86-64 TSO forbids both the store-store reordering on CPU0 and the
> load-load reordering on CPU1, which is the only reason the current code
> works there.
> 
> Fixes: d8722ff88c5d ("drivers/vhost: vhost-blk accelerator for virtio-blk guests")
> https://virtuozzo.atlassian.net/browse/VSTOR-138640
> Feature: vhost-blk: in-kernel accelerator for virtio-blk guests
> 
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
> ---
>   drivers/vhost/blk.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vhost/blk.c b/drivers/vhost/blk.c
> index fbdd04f5a7e28..b170439a1806d 100644
> --- a/drivers/vhost/blk.c
> +++ b/drivers/vhost/blk.c
> @@ -530,9 +530,11 @@ static void vhost_blk_handle_guest_kick(struct vhost_work *work)
>   	blk = container_of(vq->dev, struct vhost_blk, dev);
>   	blk_vq = container_of(vq, struct vhost_blk_vq, vq);
>   
> +	mutex_lock(&vq->mutex);
> +
>   	f = vhost_vq_get_backend(vq);
>   	if (!f)
> -		return;
> +		goto out;
>   
>   	vhost_disable_notify(&blk->dev, vq);
>   	for (;;) {
> @@ -580,6 +582,8 @@ static void vhost_blk_handle_guest_kick(struct vhost_work *work)
>   			break;
>   		}
>   	}
> +out:
> +	mutex_unlock(&vq->mutex);
>   }
>   
>   static void vhost_blk_handle_host_kick(struct vhost_work *work)
> @@ -594,6 +598,8 @@ static void vhost_blk_handle_host_kick(struct vhost_work *work)
>   
>   	blk_vq = container_of(work, struct vhost_blk_vq, work);
>   	vq = &blk_vq->vq;
> +
> +	mutex_lock(&vq->mutex);
>   	llnode = llist_del_all(&blk_vq->llhead);
>   	added = false;
>   	while (llnode) {
> @@ -627,6 +633,7 @@ static void vhost_blk_handle_host_kick(struct vhost_work *work)
>   
>   	if (likely(added))
>   		vhost_signal(&blk->dev, vq);
> +	mutex_unlock(&vq->mutex);
>   }
>   
>   /* It is forbidden to call more than one vhost_blk_flush() simultaneously */
> 
> ---
> base-commit: 0679f0d70523972eec3a0643565521d93e26eb9c
> change-id: 20260928-prep-kh-vstor-138640-vhost-blk-vq-mutex-84a1a46032ae
> 
> Best regards,
> --
> Konstantin Khorenko <khorenko@virtuozzo.com>
> 

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel