[Devel] [PATCH VZ10 v2 0/5] vhost-blk: fix protocol handling and backend setup

[Devel] [PATCH VZ10 v2 0/5] vhost-blk: fix protocol handling and backend setup

[Andrey Zhadchenko]

This series fixes several vhost-blk bugs and harden some userspace
interactions.

First patch hardens VIRTIO_BLK_T_GET_ID. Leave the serial empty and
copy the null terminator, just as QEMU.

The second patch reports the correct used-ring length: the number of
bytes written to the guest, including status.

The third patch fixes flush. REQ_OP_FLUSH is not a valid bio op; use
REQ_OP_WRITE | REQ_PREFLUSH as blkdev_issue_flush() does.

The fourth patch fixes sector alignment checks. SECTOR_MASK is 7, not
a bitmask, so the code was checking 8-byte alignment. Use
(SECTOR_SIZE - 1).

The last patch reworks queue/backend setup. vhost_blk_setup() silently
ignored a changed vq->num once requests were allocated and
double-fetched user input. Request allocation is now tied to backend
existence, and drop_backend/flush/fput are consolidated.

v2:
 - Fix patch fixing used-ring length: only include req->len in read
requests.

Andrey Zhadchenko (5):
  drivers/vhost/blk: harden get_id command
  drivers/vhost/blk: report correct used-ring lengths
  drivers/vhost/blk: fix flush support
  drivers/vhost/blk: fix sector alignment calculation
  drivers/vhost/blk: rework queue/backend setup

 drivers/vhost/blk.c | 167 +++++++++++++++++++++-----------------------
 1 file changed, 78 insertions(+), 89 deletions(-)

-- 
2.43.5

[Devel] [PATCH VZ10 v2 1/5] drivers/vhost/blk: harden get_id command

[Andrey Zhadchenko]

A defensive patch.
QEMU is fine with reporting empty serial and including null
terminator. So we will do the same.

https://virtuozzo.atlassian.net/browse/VSTOR-138640
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
---
 drivers/vhost/blk.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/vhost/blk.c b/drivers/vhost/blk.c
index dac03566bfca5..edd3e75873ff5 100644
--- a/drivers/vhost/blk.c
+++ b/drivers/vhost/blk.c
@@ -105,8 +105,6 @@ struct vhost_blk {
 	pid_t owner_pid;
 };
 
-static int gen;
-
 static int move_iovec(struct iovec *from, struct iovec *to,
 		      size_t len, int iov_count_from, int iov_count_to)
 {
@@ -491,7 +489,9 @@ static int vhost_blk_req_handle(struct vhost_virtqueue *vq,
 		ret = vhost_blk_req_submit(req);
 		break;
 	case VIRTIO_BLK_T_GET_ID:
-		len = strnlen(blk->serial, VIRTIO_BLK_ID_BYTES);
+		len = min_t(size_t,
+			    strnlen(blk->serial, VIRTIO_BLK_ID_BYTES) + 1,
+			    min_t(size_t, req->len, VIRTIO_BLK_ID_BYTES));
 		iov_iter_init(&iter, ITER_DEST, req->iov, req->iov_nr, req->len);
 		ret = copy_to_iter(blk->serial, len, &iter);
 		status = ret != len ? VIRTIO_BLK_S_IOERR : VIRTIO_BLK_S_OK;
@@ -688,7 +688,6 @@ static int vhost_blk_open(struct inode *inode, struct file *file)
 	}
 
 	memset(blk->serial, 0, sizeof(blk->serial));
-	snprintf(blk->serial, VIRTIO_BLK_ID_BYTES, "vhost-blk%d", gen++);
 
 	atomic_set(&blk->req_inflight[0], 0);
 	atomic_set(&blk->req_inflight[1], 0);
-- 
2.43.5

[Devel] [PATCH VZ10 v2 2/5] drivers/vhost/blk: report correct used-ring lengths

[Andrey Zhadchenko]

We are expected to return amount of bytes written to the guest,
which also includes status.

https://virtuozzo.atlassian.net/browse/VSTOR-138640
Fixes: 40a5928ec730 ("drivers/vhost: vhost-blk accelerator for virtio-blk guests")
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
---
v2:
 - only add req->len to used length for read requests in
vhost_blk_handle_host_kick()

 drivers/vhost/blk.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/vhost/blk.c b/drivers/vhost/blk.c
index edd3e75873ff5..2c41a073004bc 100644
--- a/drivers/vhost/blk.c
+++ b/drivers/vhost/blk.c
@@ -445,12 +445,14 @@ static int vhost_blk_req_submit(struct vhost_blk_req *req)
 
 static int vhost_blk_req_handle(struct vhost_virtqueue *vq,
 				struct virtio_blk_outhdr *hdr,
-				u16 head, u16 total_iov_nr)
+				u16 head, u16 out, u16 in)
 {
 	struct vhost_blk *blk = container_of(vq->dev, struct vhost_blk, dev);
 	struct vhost_blk_vq *blk_vq = container_of(vq, struct vhost_blk_vq, vq);
 	struct vhost_blk_req *req;
+	u16 total_iov_nr = out + in;
 	struct iov_iter iter;
+	size_t in_len;
 	int ret, len;
 	u8 status;
 
@@ -461,8 +463,9 @@ static int vhost_blk_req_handle(struct vhost_virtqueue *vq,
 	req->sector	= hdr->sector;
 	req->iov	= blk_vq->iov;
 	req->bio_err	= 0;
+	in_len		= iov_length(vq->iov + out, in);
 
-	if (iov_length(vq->iov, total_iov_nr) < sizeof(status))
+	if (in_len < sizeof(status) || in_len > INT_MAX)
 		return -EINVAL;
 
 	req->len	= iov_length(vq->iov, total_iov_nr) - sizeof(status);
@@ -498,14 +501,14 @@ static int vhost_blk_req_handle(struct vhost_virtqueue *vq,
 		ret = vhost_blk_set_status(req, status);
 		if (ret)
 			break;
-		vhost_add_used_and_signal(&blk->dev, vq, head, len);
+		vhost_add_used_and_signal(&blk->dev, vq, head, in_len);
 		break;
 	default:
 		status = VIRTIO_BLK_S_UNSUPP;
 		ret = vhost_blk_set_status(req, status);
 		if (ret)
 			break;
-		vhost_add_used_and_signal(&blk->dev, vq, head, 0);
+		vhost_add_used_and_signal(&blk->dev, vq, head, sizeof(status));
 	}
 
 	return ret;
@@ -561,7 +564,7 @@ static void vhost_blk_handle_guest_kick(struct vhost_work *work)
 			break;
 		}
 
-		ret = vhost_blk_req_handle(vq, &hdr, head, out + in);
+		ret = vhost_blk_req_handle(vq, &hdr, head, out, in);
 		if (ret == -EAGAIN || ret == -ENOMEM) {
 			vhost_discard_vq_desc(vq, 1);
 			vhost_poll_queue(&vq->poll);
@@ -610,7 +613,12 @@ static void vhost_blk_handle_host_kick(struct vhost_work *work)
 		if (vhost_blk_set_status(req, status)) {
 			vhostblk_vq_err(blk, vq, "Failed to write status");
 		} else {
-			vhost_add_used(vq, req->head, req->len);
+			int used_len = sizeof(status);
+
+			if (req->bi_opf == REQ_OP_READ)
+				used_len += req->len;
+
+			vhost_add_used(vq, req->head, used_len);
 			added = true;
 		}
 
-- 
2.43.5

[Devel] [PATCH VZ10 v2 3/5] drivers/vhost/blk: fix flush support

[Andrey Zhadchenko]

REQ_OP_FLUSH is a technical flag for requests. For bios the
correct combination would be REQ_OP_WRITE | REQ_PREFLUSH.
See an example in blkdev_issue_flush().

https://virtuozzo.atlassian.net/browse/VSTOR-138640
Fixes: 40a5928ec730 ("drivers/vhost: vhost-blk accelerator for virtio-blk guests")
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
---
 drivers/vhost/blk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/vhost/blk.c b/drivers/vhost/blk.c
index 2c41a073004bc..5c8d5b210c315 100644
--- a/drivers/vhost/blk.c
+++ b/drivers/vhost/blk.c
@@ -274,7 +274,7 @@ static int vhost_blk_bio_make(struct vhost_blk_req *req,
 	sector_t sector = req->sector;
 	unsigned long pos = 0;
 
-	if (unlikely(req->bi_opf == REQ_OP_FLUSH))
+	if (unlikely(req->bi_opf & REQ_PREFLUSH))
 		return vhost_blk_bio_make_simple(req, bdev);
 
 	if (req->bi_opf == REQ_OP_WRITE && req->len & SECTOR_MASK) {
@@ -488,7 +488,7 @@ static int vhost_blk_req_handle(struct vhost_virtqueue *vq,
 		ret = vhost_blk_req_submit(req);
 		break;
 	case VIRTIO_BLK_T_FLUSH:
-		req->bi_opf = REQ_OP_FLUSH;
+		req->bi_opf = REQ_OP_WRITE | REQ_PREFLUSH;
 		ret = vhost_blk_req_submit(req);
 		break;
 	case VIRTIO_BLK_T_GET_ID:
-- 
2.43.5

[Devel] [PATCH VZ10 v2 4/5] drivers/vhost/blk: fix sector alignment calculation

[Andrey Zhadchenko]

SECTOR_MASK is actually a number 7, not some bitmask. Hence
the code actually checked 8 byte alignment.
Use (SECTOR_SIZE - 1) to correctly check sector alignment.

https://virtuozzo.atlassian.net/browse/VSTOR-138640
Fixes: a0d3b8956fb0 ("vhost-blk: rework iov and bio handling")
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
---
 drivers/vhost/blk.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/vhost/blk.c b/drivers/vhost/blk.c
index 5c8d5b210c315..6a483e527990e 100644
--- a/drivers/vhost/blk.c
+++ b/drivers/vhost/blk.c
@@ -209,13 +209,13 @@ inline static bool vhost_blk_iov_need_bb(struct vhost_blk_req *req)
 {
 	int i;
 
-	if (req->len & SECTOR_MASK)
+	if (req->len & (SECTOR_SIZE - 1))
 		return true;
 
 	for (i = 0; i < req->iov_nr; i++) {
-		if (((size_t)req->iov[i].iov_base) & SECTOR_MASK)
+		if (((size_t)req->iov[i].iov_base) & (SECTOR_SIZE - 1))
 			return true;
-		if (req->iov[i].iov_len & SECTOR_MASK)
+		if (req->iov[i].iov_len & (SECTOR_SIZE - 1))
 			return true;
 	}
 
@@ -277,7 +277,7 @@ static int vhost_blk_bio_make(struct vhost_blk_req *req,
 	if (unlikely(req->bi_opf & REQ_PREFLUSH))
 		return vhost_blk_bio_make_simple(req, bdev);
 
-	if (req->bi_opf == REQ_OP_WRITE && req->len & SECTOR_MASK) {
+	if (req->bi_opf == REQ_OP_WRITE && req->len & (SECTOR_SIZE - 1)) {
 		WARN_ONCE(1, "vhost-blk: write requests with unaligned len"
 			  " are not supported, len = %zu", req->len);
 		return -EINVAL;
@@ -339,7 +339,7 @@ static int vhost_blk_bio_make(struct vhost_blk_req *req,
 	nr_pages = bio_iov_vecs_to_alloc(&iter, BIO_MAX_VECS);
 	do {
 		/* We can't handle next bio if it's start is not sector aligned */
-		if (pos & SECTOR_MASK) {
+		if (pos & (SECTOR_SIZE - 1)) {
 			WARN_ONCE(1, "vhost-blk: guest provided unaligned buffers");
 			ret = -EINVAL;
 			goto err_bio;
-- 
2.43.5

[Devel] [PATCH VZ10 v2 5/5] drivers/vhost/blk: rework queue/backend setup

[Andrey Zhadchenko]

vhost_blk_setup() is pretty bad: silently refusing changed vq->num
if requests are already allocated, fetching user input second time
(double-fetch vulnerability).
To handle this, tie request allocation to backend existence. After
all, if there is no backend, there is no point in having requests.
Also expand it to get rid of boilerplate drop_backend, flush,
fput sequence in a few places.

https://virtuozzo.atlassian.net/browse/VSTOR-138640
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
---
 drivers/vhost/blk.c | 126 +++++++++++++++++++-------------------------
 1 file changed, 54 insertions(+), 72 deletions(-)

diff --git a/drivers/vhost/blk.c b/drivers/vhost/blk.c
index 6a483e527990e..456ccfbe369f0 100644
--- a/drivers/vhost/blk.c
+++ b/drivers/vhost/blk.c
@@ -658,11 +658,14 @@ static void vhost_blk_flush(struct vhost_blk *blk)
 	spin_unlock(&blk->flush_lock);
 }
 
-static inline void vhost_blk_drop_backends(struct vhost_blk *blk)
+static void vhost_blk_drop_backend(struct vhost_blk *blk)
 {
 	struct vhost_virtqueue *vq;
 	int i;
 
+	if (!blk->backend)
+		return;
+
 	for (i = 0; i < VHOST_BLK_VQ_MAX; i++) {
 		vq = &blk->vqs[i].vq;
 
@@ -670,6 +673,44 @@ static inline void vhost_blk_drop_backends(struct vhost_blk *blk)
 		vhost_vq_set_backend(vq, NULL);
 		mutex_unlock(&vq->mutex);
 	}
+
+	vhost_blk_flush(blk);
+
+	for (i = 0; i < VHOST_BLK_VQ_MAX; i++) {
+		kvfree(blk->vqs[i].req);
+		blk->vqs[i].req = NULL;
+	}
+
+	fput(blk->backend);
+	blk->backend = NULL;
+}
+
+static int vhost_blk_setup_vqs(struct vhost_blk *blk)
+{
+	struct vhost_virtqueue *vq;
+	int i;
+
+	for (i = 0; i < VHOST_BLK_VQ_MAX; i++) {
+		vq = &blk->vqs[i].vq;
+
+		if (!vhost_vq_is_setup(vq))
+			continue;
+
+		blk->vqs[i].req = kvmalloc_array(vq->num, sizeof(struct vhost_blk_req),
+						 GFP_KERNEL);
+		if (!blk->vqs[i].req)
+			return -ENOMEM;
+
+		mutex_lock(&vq->mutex);
+		vhost_vq_set_backend(vq, blk->backend);
+		if (vhost_vq_init_access(vq)) {
+			mutex_unlock(&vq->mutex);
+			return -EFAULT;
+		}
+		mutex_unlock(&vq->mutex);
+	}
+
+	return 0;
 }
 
 static int vhost_blk_open(struct inode *inode, struct file *file)
@@ -722,16 +763,10 @@ static int vhost_blk_open(struct inode *inode, struct file *file)
 static int vhost_blk_release(struct inode *inode, struct file *f)
 {
 	struct vhost_blk *blk = f->private_data;
-	int i;
 
-	vhost_blk_drop_backends(blk);
-	vhost_blk_flush(blk);
+	vhost_blk_drop_backend(blk);
 	vhost_dev_stop(&blk->dev);
-	if (blk->backend)
-		fput(blk->backend);
 	vhost_dev_cleanup(&blk->dev);
-	for (i = 0; i < VHOST_BLK_VQ_MAX; i++)
-		kvfree(blk->vqs[i].req);
 	kfree(blk->dev.vqs);
 	kvfree(blk);
 
@@ -765,32 +800,19 @@ static int vhost_blk_set_features(struct vhost_blk *blk, u64 features)
 
 static long vhost_blk_set_backend(struct vhost_blk *blk, int fd)
 {
-	struct vhost_virtqueue *vq;
 	struct file *file;
 	struct inode *inode;
-	int ret, i;
+	int ret;
 
 	mutex_lock(&blk->dev.mutex);
 	ret = vhost_dev_check_owner(&blk->dev);
 	if (ret)
 		goto out_dev;
 
-	/*
-	 * fd < 0 means "stop the device".  Detach the backend from every vq so
-	 * vhost_blk_handle_guest_kick() stops fetching descriptors, drain the
-	 * in-flight requests, and release the backing file.
-	 */
+	/* fd < 0 means "stop the device" */
 	if (fd < 0) {
-		if (!blk->backend) {
-			ret = 0;		/* already stopped */
-			goto out_dev;
-		}
-		vhost_blk_drop_backends(blk);
-		vhost_blk_flush(blk);
-		fput(blk->backend);
-		blk->backend = NULL;
 		ret = 0;
-		goto out_dev;
+		goto out_drop;
 	}
 
 	if (blk->backend) {
@@ -807,31 +829,20 @@ static long vhost_blk_set_backend(struct vhost_blk *blk, int fd)
 	inode = file->f_mapping->host;
 	if (!S_ISBLK(inode->i_mode)) {
 		ret = -EFAULT;
-		goto out_file;
-	}
-
-	for (i = 0; i < VHOST_BLK_VQ_MAX; i++) {
-		vq = &blk->vqs[i].vq;
-		if (!vhost_vq_access_ok(vq)) {
-			ret = -EFAULT;
-			goto out_drop;
-		}
-
-		mutex_lock(&vq->mutex);
-		vhost_vq_set_backend(vq, file);
-		ret = vhost_vq_init_access(vq);
-		mutex_unlock(&vq->mutex);
+		fput(file);
+		goto out_dev;
 	}
 
 	blk->backend = file;
+	ret = vhost_blk_setup_vqs(blk);
+	if (ret)
+		goto out_drop;
 
 	mutex_unlock(&blk->dev.mutex);
 	return 0;
 
 out_drop:
-	vhost_blk_drop_backends(blk);
-out_file:
-	fput(file);
+	vhost_blk_drop_backend(blk);
 out_dev:
 	mutex_unlock(&blk->dev.mutex);
 	return ret;
@@ -840,7 +851,7 @@ static long vhost_blk_set_backend(struct vhost_blk *blk, int fd)
 static long vhost_blk_reset_owner(struct vhost_blk *blk)
 {
 	struct vhost_iotlb *umem;
-	int err, i;
+	int err;
 
 	mutex_lock(&blk->dev.mutex);
 	err = vhost_dev_check_owner(&blk->dev);
@@ -851,42 +862,15 @@ static long vhost_blk_reset_owner(struct vhost_blk *blk)
 		err = -ENOMEM;
 		goto done;
 	}
-	vhost_blk_drop_backends(blk);
-	if (blk->backend) {
-		fput(blk->backend);
-		blk->backend = NULL;
-	}
-	vhost_blk_flush(blk);
+	vhost_blk_drop_backend(blk);
 	vhost_dev_stop(&blk->dev);
 	vhost_dev_reset_owner(&blk->dev, umem);
 
-	for (i = 0; i < VHOST_BLK_VQ_MAX; i++) {
-		kvfree(blk->vqs[i].req);
-		blk->vqs[i].req = NULL;
-	}
-
 done:
 	mutex_unlock(&blk->dev.mutex);
 	return err;
 }
 
-static int vhost_blk_setup(struct vhost_blk *blk, void __user *argp)
-{
-	struct vhost_vring_state s;
-
-	if (copy_from_user(&s, argp, sizeof(s)))
-		return -EFAULT;
-
-	if (blk->vqs[s.index].req)
-		return 0;
-
-	blk->vqs[s.index].req = kvmalloc(sizeof(struct vhost_blk_req) * s.num, GFP_KERNEL);
-	if (!blk->vqs[s.index].req)
-		return -ENOMEM;
-
-	return 0;
-}
-
 static long vhost_blk_ioctl(struct file *f, unsigned int ioctl,
 			    unsigned long arg)
 {
@@ -924,8 +908,6 @@ static long vhost_blk_ioctl(struct file *f, unsigned int ioctl,
 		ret = vhost_dev_ioctl(&blk->dev, ioctl, argp);
 		if (ret == -ENOIOCTLCMD)
 			ret = vhost_vring_ioctl(&blk->dev, ioctl, argp);
-		if (!ret && ioctl == VHOST_SET_VRING_NUM)
-			ret = vhost_blk_setup(blk, argp);
 		vhost_blk_flush(blk);
 		mutex_unlock(&blk->dev.mutex);
 		return ret;
-- 
2.43.5

Re: [Devel] [PATCH VZ10 v2 0/5] vhost-blk: fix protocol handling and backend setup

[Vasileios Almpanis]

On Mon, 24 Aug 2026 18:59:19 +0300, Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com> wrote:
> vhost-blk: fix protocol handling and backend setup
> 
> This series fixes several vhost-blk bugs and harden some userspace
> interactions.
> 
> First patch hardens VIRTIO_BLK_T_GET_ID. Leave the serial empty and
> copy the null terminator, just as QEMU.
> 
> [...]

Reviewed-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>

-- 
Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>