[Devel] [PATCH hci-8.1 0/3] drivers/vhost/blk: fix virtio-blk request header and payload parsing

[Devel] [PATCH hci-8.1 0/3] drivers/vhost/blk: fix virtio-blk request header and payload parsing

[Alexander Ivanov]

vhost-blk parsed guest requests from the whole descriptor chain without
telling the device-readable part from the device-writable one, and used
the header fields without byte order conversion.

Patch 1 converts hdr->type and hdr->sector to CPU byte order.

Patch 2 takes the request header only from the readable part and drops
requests whose readable part is shorter than the header.

Patch 3 takes the write payload from the readable part and the payload
of other requests from the writable part, reads the status byte from
the end of the writable part, and fails reads and writes with an empty,
oversized or misaligned payload with VIRTIO_BLK_S_IOERR instead of
leaving them hanging.

https://virtuozzo.atlassian.net/browse/VSTOR-142755

Alexander Ivanov (3):
  drivers/vhost/blk: convert virtio_blk_outhdr fields to CPU byte order
  drivers/vhost/blk: take virtio_blk_outhdr only from the readable part
  drivers/vhost/blk: take req->len from one side of the descriptor chain

 drivers/vhost/blk.c | 56 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 40 insertions(+), 16 deletions(-)

-- 
2.53.0

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

[Devel] [PATCH hci-8.1 1/3] drivers/vhost/blk: convert virtio_blk_outhdr fields to CPU byte order

[Alexander Ivanov]

hdr->type and hdr->sector are __virtio32 and __virtio64 but were used
without conversion. Since VIRTIO_F_VERSION_1 is offered, on a big-endian
host with a little-endian guest the request header was misinterpreted.

Convert both with vhost32_to_cpu()/vhost64_to_cpu() once, when the
request is set up.

https://virtuozzo.atlassian.net/browse/VSTOR-142755
Fixes: 1629db092dd7 ("drivers/vhost: vhost-blk accelerator for virtio-blk guests")
Feature: vhost-blk: in-kernel accelerator for virtio-blk guests
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 drivers/vhost/blk.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/vhost/blk.c b/drivers/vhost/blk.c
index b170439a1806..7234213924d5 100644
--- a/drivers/vhost/blk.c
+++ b/drivers/vhost/blk.c
@@ -454,16 +454,18 @@ static int vhost_blk_req_handle(struct vhost_virtqueue *vq,
 	struct iov_iter iter;
 	size_t in_len;
 	int ret, len;
+	u32 type;
 	u8 status;
 
 	req		= &blk_vq->req[head];
 	req->blk_vq	= blk_vq;
 	req->head	= head;
 	req->blk	= blk;
-	req->sector	= hdr->sector;
+	req->sector	= vhost64_to_cpu(vq, hdr->sector);
 	req->iov	= blk_vq->iov;
 	req->bio_err	= 0;
 	in_len		= iov_length(vq->iov + out, in);
+	type		= vhost32_to_cpu(vq, hdr->type);
 
 	if (in_len < sizeof(status) || in_len > INT_MAX)
 		return -EINVAL;
@@ -478,7 +480,7 @@ static int vhost_blk_req_handle(struct vhost_virtqueue *vq,
 	if (ret < 0 || req->iov_nr < 0)
 		return -EINVAL;
 
-	switch (hdr->type) {
+	switch (type) {
 	case VIRTIO_BLK_T_OUT:
 		req->bi_opf = REQ_OP_WRITE;
 		ret = vhost_blk_req_submit(req);
-- 
2.53.0

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

[Devel] [PATCH hci-8.1 2/3] drivers/vhost/blk: take virtio_blk_outhdr only from the readable part

[Alexander Ivanov]

The request header was moved from the whole descriptor chain, so a
chain with no readable part, or one shorter than the header, had header
bytes read from buffers the guest marked device-writable. Those bytes
were also trimmed in place from the writable segments, shifting the
status byte and the READ/GET_ID payload taken from them.

Take the header only from the readable part. When that part is shorter
than struct virtio_blk_outhdr, drop the request and report it through
error_ctx, like the other malformed-header paths do.

https://virtuozzo.atlassian.net/browse/VSTOR-142755
Fixes: 1629db092dd7 ("drivers/vhost: vhost-blk accelerator for virtio-blk guests")
Feature: vhost-blk: in-kernel accelerator for virtio-blk guests
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 drivers/vhost/blk.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/vhost/blk.c b/drivers/vhost/blk.c
index 7234213924d5..94317a5f2cb6 100644
--- a/drivers/vhost/blk.c
+++ b/drivers/vhost/blk.c
@@ -554,7 +554,12 @@ static void vhost_blk_handle_guest_kick(struct vhost_work *work)
 			break;
 		}
 
-		ret = move_iovec(vq->iov, hdr_iovec, sizeof(hdr), in + out, ARRAY_SIZE(hdr_iovec));
+		if (iov_length(vq->iov, out) < sizeof(hdr)) {
+			vhostblk_vq_err(blk, vq, "virtio_blk_hdr (head %d) is missing!", head);
+			break;
+		}
+
+		ret = move_iovec(vq->iov, hdr_iovec, sizeof(hdr), out, ARRAY_SIZE(hdr_iovec));
 		if (ret < 0) {
 			vhostblk_vq_err(blk, vq, "virtio_blk_hdr (head %d) is too split!", head);
 			break;
-- 
2.53.0

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

[Devel] [PATCH hci-8.1 3/3] drivers/vhost/blk: take req->len from one side of the descriptor chain

[Alexander Ivanov]

req->len covered the whole descriptor chain, so both readable and
writable data buffers became the payload. A read wrote disk data into
device-readable buffers and reported them as used, a write sent the
guest's writable buffers to disk.

Take write data only from the readable part and the payload of other
requests only from the writable part, as QEMU does. Read the status
byte from the end of the writable part.

Complete a read or write with VIRTIO_BLK_S_IOERR if its payload is
empty, above ALIGN_DOWN(INT_MAX, SECTOR_SIZE) or, for a write, not a
multiple of SECTOR_SIZE. Such requests used to fail in bio setup or
hit a WARN_ONCE() and leave the guest request hanging. Drop the now
unreachable WARN_ONCE().

https://virtuozzo.atlassian.net/browse/VSTOR-142755
Fixes: 1629db092dd7 ("drivers/vhost: vhost-blk accelerator for virtio-blk guests")
Fixes: 02ce37f62707 ("vhost-blk: add bounce-buffer for non-aligned requests")
Feature: vhost-blk: in-kernel accelerator for virtio-blk guests
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 drivers/vhost/blk.c | 43 ++++++++++++++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/drivers/vhost/blk.c b/drivers/vhost/blk.c
index 94317a5f2cb6..3448910f9046 100644
--- a/drivers/vhost/blk.c
+++ b/drivers/vhost/blk.c
@@ -277,12 +277,6 @@ 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 && !IS_ALIGNED(req->len, SECTOR_SIZE)) {
-		WARN_ONCE(1, "vhost-blk: write requests with unaligned len"
-			  " are not supported, len = %zu", req->len);
-		return -EINVAL;
-	}
-
 	/*
 	 * Requests with unaligned io vectors are not supported by underlined
 	 * block device, let's use an aligned intermediary "bounce buffer" to
@@ -450,10 +444,10 @@ static int vhost_blk_req_handle(struct vhost_virtqueue *vq,
 	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 iovec *data_iov, *st;
 	struct iov_iter iter;
 	size_t in_len;
-	int ret, len;
+	int ret, len, data_nr;
 	u32 type;
 	u8 status;
 
@@ -470,14 +464,37 @@ static int vhost_blk_req_handle(struct vhost_virtqueue *vq,
 	if (in_len < sizeof(status) || in_len > INT_MAX)
 		return -EINVAL;
 
-	req->len	= iov_length(vq->iov, total_iov_nr) - sizeof(status);
-	req->iov_nr	= move_iovec(vq->iov, req->iov, req->len, total_iov_nr,
+	/* The status is the last byte of the device-writable part */
+	for (st = vq->iov + out + in - 1; !st->iov_len; st--)
+		;
+	st->iov_len -= sizeof(status);
+	req->status[0].iov_base = st->iov_base + st->iov_len;
+	req->status[0].iov_len = sizeof(status);
+
+	if (type == VIRTIO_BLK_T_OUT) {
+		data_iov = vq->iov;
+		data_nr = out;
+	} else {
+		data_iov = vq->iov + out;
+		data_nr = in;
+	}
+
+	req->len = iov_length(data_iov, data_nr);
+
+	if ((type == VIRTIO_BLK_T_IN || type == VIRTIO_BLK_T_OUT) &&
+	    (!req->len || req->len > ALIGN_DOWN(INT_MAX, SECTOR_SIZE) ||
+	     (type == VIRTIO_BLK_T_OUT && !IS_ALIGNED(req->len, SECTOR_SIZE)))) {
+		ret = vhost_blk_set_status(req, VIRTIO_BLK_S_IOERR);
+		if (!ret)
+			vhost_add_used_and_signal(&blk->dev, vq, head, sizeof(status));
+		return ret;
+	}
+
+	req->iov_nr	= move_iovec(data_iov, req->iov, req->len, data_nr,
 				     ARRAY_SIZE(blk_vq->iov));
 	req->bb		= NULL;
 
-	ret = move_iovec(vq->iov, req->status, sizeof(status), total_iov_nr,
-			 ARRAY_SIZE(req->status));
-	if (ret < 0 || req->iov_nr < 0)
+	if (req->iov_nr < 0)
 		return -EINVAL;
 
 	switch (type) {
-- 
2.53.0

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

Re: [Devel] [PATCH hci-8.1 2/3] drivers/vhost/blk: take virtio_blk_outhdr only from the readable part

[no-reply]

> The request header was moved from the whole descriptor chain, so a
> chain with no readable part, or one shorter than the header, had header
> bytes read from buffers the guest marked device-writable. Those bytes
> were also trimmed in place from the writable segments, shifting the
> status byte and the READ/GET_ID payload taken from them.
> 
> Take the header only from the readable part. When that part is shorter
> than struct virtio_blk_outhdr, drop the request and report it through
> error_ctx, like the other malformed-header paths do.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-142755
> Fixes: 1629db092dd7 ("drivers/vhost: vhost-blk accelerator for virtio-blk guests")
> Feature: vhost-blk: in-kernel accelerator for virtio-blk guests
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261021084610.1476738-1-alexander.ivanov@virtuozzo.com?part=2


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

Re: [Devel] [PATCH hci-8.1 1/3] drivers/vhost/blk: convert virtio_blk_outhdr fields to CPU byte order

[no-reply]

> hdr->type and hdr->sector are __virtio32 and __virtio64 but were used
> without conversion. Since VIRTIO_F_VERSION_1 is offered, on a big-endian
> host with a little-endian guest the request header was misinterpreted.
> 
> Convert both with vhost32_to_cpu()/vhost64_to_cpu() once, when the
> request is set up.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-142755
> Fixes: 1629db092dd7 ("drivers/vhost: vhost-blk accelerator for virtio-blk guests")
> Feature: vhost-blk: in-kernel accelerator for virtio-blk guests
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261021084610.1476738-1-alexander.ivanov@virtuozzo.com?part=1


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

Re: [Devel] [PATCH hci-8.1 3/3] drivers/vhost/blk: take req->len from one side of the descriptor chain

[no-reply]

> req->len covered the whole descriptor chain, so both readable and
> writable data buffers became the payload. A read wrote disk data into
> device-readable buffers and reported them as used, a write sent the
> guest's writable buffers to disk.
> 
> Take write data only from the readable part and the payload of other
> requests only from the writable part, as QEMU does. Read the status
> byte from the end of the writable part.
> 
> Complete a read or write with VIRTIO_BLK_S_IOERR if its payload is
> empty, above ALIGN_DOWN(INT_MAX, SECTOR_SIZE) or, for a write, not a
> multiple of SECTOR_SIZE. Such requests used to fail in bio setup or
> hit a WARN_ONCE() and leave the guest request hanging. Drop the now
> unreachable WARN_ONCE().
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-142755
> Fixes: 1629db092dd7 ("drivers/vhost: vhost-blk accelerator for virtio-blk guests")
> Fixes: 02ce37f62707 ("vhost-blk: add bounce-buffer for non-aligned requests")
> Feature: vhost-blk: in-kernel accelerator for virtio-blk guests
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261021084610.1476738-1-alexander.ivanov@virtuozzo.com?part=3


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

Re: [Devel] [PATCH hci-8.1 0/3] drivers/vhost/blk: fix virtio-blk request header and payload parsing

[Andrey Zhadchenko]

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

On 10/21/26 10:46, Alexander Ivanov wrote:
> vhost-blk parsed guest requests from the whole descriptor chain without
> telling the device-readable part from the device-writable one, and used
> the header fields without byte order conversion.
> 
> Patch 1 converts hdr->type and hdr->sector to CPU byte order.
> 
> Patch 2 takes the request header only from the readable part and drops
> requests whose readable part is shorter than the header.
> 
> Patch 3 takes the write payload from the readable part and the payload
> of other requests from the writable part, reads the status byte from
> the end of the writable part, and fails reads and writes with an empty,
> oversized or misaligned payload with VIRTIO_BLK_S_IOERR instead of
> leaving them hanging.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-142755
> 
> Alexander Ivanov (3):
>    drivers/vhost/blk: convert virtio_blk_outhdr fields to CPU byte order
>    drivers/vhost/blk: take virtio_blk_outhdr only from the readable part
>    drivers/vhost/blk: take req->len from one side of the descriptor chain
> 
>   drivers/vhost/blk.c | 56 ++++++++++++++++++++++++++++++++-------------
>   1 file changed, 40 insertions(+), 16 deletions(-)
> 

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

Re: [Devel] [PATCH hci-8.1 0/3] drivers/vhost/blk: fix virtio-blk request header and payload parsing

[Konstantin Khorenko]

Hi Alexander,

The code in all three patches is fine, I have nothing to change there.
Two commit messages describe the old behaviour inaccurately, please fix
them in place for v2.

Patch 2, "take virtio_blk_outhdr only from the readable part":

> The request header was moved from the whole descriptor chain, so a
> chain with no readable part, or one shorter than the header, had header
> bytes read from buffers the guest marked device-writable.

The "shorter than the header" case was already rejected before the
patch: VHOST_MAX_METADATA_IOV is 1, so move_iovec() fills one target
segment and returns -1 when the header does not fit into the first
readable iovec, and the kick handler drops the chain as "too split".
Only a chain with no readable part at all had the header taken from a
writable buffer.

> Those bytes
> were also trimmed in place from the writable segments, shifting the
> status byte and the READ/GET_ID payload taken from them.

The status byte did not move: it was taken as the last byte of the
chain after req->len was computed. What shifted was the start of the
READ/GET_ID payload, by sizeof(hdr).

Suggested paragraph:

  The request header was moved from the whole descriptor chain, so a
  chain with no readable part had header bytes read from buffers the
  guest marked device-writable. Those bytes were also trimmed in place
  from the writable segments, shifting the start of the READ/GET_ID
  payload taken from them.

Patch 3, "take req->len from one side of the descriptor chain":

> Complete a read or write with VIRTIO_BLK_S_IOERR if its payload is
> empty, above ALIGN_DOWN(INT_MAX, SECTOR_SIZE) or, for a write, not a
> multiple of SECTOR_SIZE. Such requests used to fail in bio setup or
> hit a WARN_ONCE() and leave the guest request hanging. Drop the now
> unreachable WARN_ONCE().

Three things here.

"Such requests used to fail" is true only for the empty payload and the
unaligned write. A write above ALIGN_DOWN(INT_MAX, SECTOR_SIZE) was
never rejected before: req->len is size_t, the bios are split per
BIO_MAX_VECS and used_len for a write is one byte, so it was submitted
as is. The bound is a new restriction for writes and the message should
say so. For reads, the in_len > INT_MAX check still runs first, so a
read above INT_MAX keeps getting -EINVAL; the new bound only covers the
range below it.

"Leave the guest request hanging" understates it. On -EINVAL the kick
handler breaks out of the loop with the descriptor consumed and
notifications still disabled, so with EVENT_IDX the guest never kicks
again and the whole virtqueue stalls until QEMU resets it through
error_ctx.

The WARN_ONCE() was reachable from the guest: any write with an
unaligned length triggered it, and with panic_on_warn that panics the
host. That is the reason for the Fixes: 02ce37f62707 trailer and worth
one sentence, "now unreachable" is a side effect.

Suggested paragraphs:

  Complete a read or write with VIRTIO_BLK_S_IOERR if its payload is
  empty, above ALIGN_DOWN(INT_MAX, SECTOR_SIZE) or, for a write, not a
  multiple of SECTOR_SIZE. An empty payload used to fail in bio setup
  and an unaligned write hit a WARN_ONCE() reachable from the guest.
  In both cases the kick handler broke out with the descriptor consumed
  and notifications disabled, so the virtqueue stalled. Drop the
  WARN_ONCE().

  The size bound is new for writes: the readable side had no limit and
  a write of any size was submitted. Reads above INT_MAX still get
  -EINVAL from the in_len check, the bound only covers the range below
  it.

With the two messages fixed the series is good to go, no code changes
needed.

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team


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