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

[Konstantin Khorenko <khorenko@virtuozzo.com>]

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