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

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

[Konstantin Khorenko]

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

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

[Andrey Zhadchenko]

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

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

[Konstantin Khorenko]

On Mon, 28 Sep 2026 17:25:08 +0200, Konstantin Khorenko wrote:
> drivers/vhost/blk: take vq->mutex in the kick handlers

Applied to branch-rh10-6.12.0-211.52.1.22.x.vz10-ovz, will be released in rh10-6.12.0-211.52.1.22.5.vz10, thanks!

[1/1] drivers/vhost/blk: take vq->mutex in the kick handlers
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/b9922ee12fcf

-- 
Konstantin Khorenko <khorenko@virtuozzo.com>

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