All Virtuozzo development lists (kernel + QEMU)
 help / color / mirror / Atom feed
* [Devel] [PATCH hci-8.1 v2] drivers/vhost/blk: reject a head that is still in flight
@ 2026-10-29 13:48 Alexander Ivanov
  2026-10-29 13:52 ` no-reply
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Alexander Ivanov @ 2026-10-29 13:48 UTC (permalink / raw)
  To: andrey.zhadchenko; +Cc: devel

vhost_blk_req_handle() takes the request slot by head and overwrites it
without checking that it is free. A guest that puts a head into the
avail ring again while its request is in flight makes the slot get
submitted twice: req_inflight is raised for each submission, but the
completion path sees one slot and calls forget_request() once.
req_inflight never drops to zero, and vhost_blk_flush() on backend drop
leaves QEMU in D state forever with the block device pinned. The second
submission also overwrites bio[], bio_nr and the iovecs of bios that
are still in flight, and frees the bounce buffer and bio[] of the first
submission under them, which is a use-after-free.

Mark a slot in flight from bio submission until its completion is
handled, both under vq->mutex, and fail the request if the guest posts
an in-flight head. Log the head and let the caller report the error on
the vq error eventfd and stop processing the ring. Allocate the slots
zeroed so the flag starts clear.

Fixes: d8722ff88c5d ("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>
---
v2:
- note that the second submission frees buffers under in-flight bios
  (use-after-free)
- log the reused head with vhostblk_vq_err()

 drivers/vhost/blk.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/vhost/blk.c b/drivers/vhost/blk.c
index b170439a1806..aa646a598586 100644
--- a/drivers/vhost/blk.c
+++ b/drivers/vhost/blk.c
@@ -79,6 +79,7 @@ struct vhost_blk_req {
 	u16 head;
 	size_t len;
 	int bio_err;
+	bool inflight;
 
 	struct vhost_blk *blk;
 	struct vhost_blk_vq *blk_vq;
@@ -434,6 +435,7 @@ static int vhost_blk_req_submit(struct vhost_blk_req *req)
 	if (ret < 0)
 		goto out;
 
+	req->inflight = true;
 	vhost_blk_bio_send(req);
 	return ret;
 
@@ -457,6 +459,11 @@ static int vhost_blk_req_handle(struct vhost_virtqueue *vq,
 	u8 status;
 
 	req		= &blk_vq->req[head];
+	if (unlikely(req->inflight)) {
+		vhostblk_vq_err(blk, vq, "Guest reused in-flight head %d", head);
+		return -EBUSY;
+	}
+
 	req->blk_vq	= blk_vq;
 	req->head	= head;
 	req->blk	= blk;
@@ -628,6 +635,7 @@ static void vhost_blk_handle_host_kick(struct vhost_work *work)
 			added = true;
 		}
 
+		req->inflight = false;
 		forget_request(req);
 	}
 
@@ -703,8 +711,8 @@ static int vhost_blk_setup_vqs(struct vhost_blk *blk)
 		if (!vhost_vq_is_setup(vq))
 			continue;
 
-		blk->vqs[i].req = kvmalloc_array(vq->num, sizeof(struct vhost_blk_req),
-						 GFP_KERNEL);
+		blk->vqs[i].req = kvcalloc(vq->num, sizeof(struct vhost_blk_req),
+					   GFP_KERNEL);
 		if (!blk->vqs[i].req)
 			return -ENOMEM;
 
-- 
2.53.0

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Devel] [PATCH hci-8.1 v2] drivers/vhost/blk: reject a head that is still in flight
  2026-10-29 13:48 [Devel] [PATCH hci-8.1 v2] drivers/vhost/blk: reject a head that is still in flight Alexander Ivanov
@ 2026-10-29 13:52 ` no-reply
  2026-10-29 13:59   ` Alexander Ivanov
  2026-11-02 10:36 ` Konstantin Khorenko
  2026-11-02 10:36 ` Konstantin Khorenko
  2 siblings, 1 reply; 5+ messages in thread
From: no-reply @ 2026-10-29 13:52 UTC (permalink / raw)
  To: andrey.zhadchenko, Alexander Ivanov; +Cc: devel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] vhost-blk: in-flight head error is logged and signalled twice
--

commit 50430ed7a617303d49fca9a87eb2e315f023f9f7
Author: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

drivers/vhost/blk: reject a head that is still in flight

This patch adds an inflight flag to each vhost_blk_req slot. The flag is set
at bio submission and cleared when completion is handled. If the guest
posts a head whose slot is still in flight, vhost_blk_req_handle() now
returns -EBUSY. The slots are also allocated zeroed.

> diff --git a/drivers/vhost/blk.c b/drivers/vhost/blk.c
> --- a/drivers/vhost/blk.c
> +++ b/drivers/vhost/blk.c

[ ... ]

> @@ -457,6 +459,11 @@ static int vhost_blk_req_handle(struct vhost_virtqueue *vq,
>  	u8 status;
>  
>  	req		= &blk_vq->req[head];
> +	if (unlikely(req->inflight)) {
> +		vhostblk_vq_err(blk, vq, "Guest reused in-flight head %d", head);
> +		return -EBUSY;
> +	}
> +

[Severity: Low]
Is this error reported twice? vhostblk_vq_err() here already calls
pr_warn_ratelimited() and eventfd_signal() on vq->error_ctx, and then
-EBUSY is returned.

In vhost_blk_handle_guest_kick(), only -EAGAIN and -ENOMEM are special-cased:

	ret = vhost_blk_req_handle(vq, &hdr, head, out, in);
	if (ret == -EAGAIN || ret == -ENOMEM) {
		...
	} else if (ret < 0) {
		vhostblk_vq_err(blk, vq, "Failed to process guest request (head %d) with %d",
				head, ret);
		break;
	}

So -EBUSY reaches the second branch, which calls vhostblk_vq_err() again.
That gives a second warning and a second eventfd_signal() for one guest
misbehaviour.

The commit message says "Log the head and let the caller report the error on
the vq error eventfd". Should the vhostblk_vq_err() in vhost_blk_req_handle()
be a plain log message, such as pr_warn_ratelimited(), so that only the
caller signals the eventfd? Other early errors in this function, such as
-EINVAL, are reported only by the caller.

> [ ... ]

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

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Devel] [PATCH hci-8.1 v2] drivers/vhost/blk: reject a head that is still in flight
  2026-10-29 13:52 ` no-reply
@ 2026-10-29 13:59   ` Alexander Ivanov
  0 siblings, 0 replies; 5+ messages in thread
From: Alexander Ivanov @ 2026-10-29 13:59 UTC (permalink / raw)
  To: no-reply, andrey.zhadchenko; +Cc: devel

>> @@ -457,6 +459,11 @@ static int vhost_blk_req_handle(struct vhost_virtqueue *vq,
>>   	u8 status;
>>   
>>   	req		= &blk_vq->req[head];
>> +	if (unlikely(req->inflight)) {
>> +		vhostblk_vq_err(blk, vq, "Guest reused in-flight head %d", head);
>> +		return -EBUSY;
>> +	}
>> +
> [Severity: Low]
> Is this error reported twice? vhostblk_vq_err() here already calls
> pr_warn_ratelimited() and eventfd_signal() on vq->error_ctx, and then
> -EBUSY is returned.
>
> In vhost_blk_handle_guest_kick(), only -EAGAIN and -ENOMEM are special-cased:
>
> 	ret = vhost_blk_req_handle(vq, &hdr, head, out, in);
> 	if (ret == -EAGAIN || ret == -ENOMEM) {
> 		...
> 	} else if (ret < 0) {
> 		vhostblk_vq_err(blk, vq, "Failed to process guest request (head %d) with %d",
> 				head, ret);
> 		break;
> 	}
>
> So -EBUSY reaches the second branch, which calls vhostblk_vq_err() again.
> That gives a second warning and a second eventfd_signal() for one guest
> misbehaviour.
>
> The commit message says "Log the head and let the caller report the error on
> the vq error eventfd". Should the vhostblk_vq_err() in vhost_blk_req_handle()
> be a plain log message, such as pr_warn_ratelimited(), so that only the
> caller signals the eventfd? Other early errors in this function, such as
> -EINVAL, are reported only by the caller.
The vhostblk_vq_err() was added at an earlier reviewer's request.
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Devel] [PATCH hci-8.1 v2] drivers/vhost/blk: reject a head that is still in flight
  2026-10-29 13:48 [Devel] [PATCH hci-8.1 v2] drivers/vhost/blk: reject a head that is still in flight Alexander Ivanov
  2026-10-29 13:52 ` no-reply
@ 2026-11-02 10:36 ` Konstantin Khorenko
  2026-11-02 10:36 ` Konstantin Khorenko
  2 siblings, 0 replies; 5+ messages in thread
From: Konstantin Khorenko @ 2026-11-02 10:36 UTC (permalink / raw)
  To: Alexander Ivanov; +Cc: devel

On 10/29/26 14:48, Alexander Ivanov wrote:
> Fixes: d8722ff88c5d ("drivers/vhost: vhost-blk accelerator for virtio-blk guests")

Applied with two changes to the message, no code changes:

- the Jira URL line https://virtuozzo.atlassian.net/browse/VSTOR-152123
  is back, v2 dropped it
- Fixes: now names 1629db092dd7, the hash of that commit in this branch;
  d8722ff88c5d is its 16.x copy and does not resolve here

Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Devel] [PATCH hci-8.1 v2] drivers/vhost/blk: reject a head that is still in flight
  2026-10-29 13:48 [Devel] [PATCH hci-8.1 v2] drivers/vhost/blk: reject a head that is still in flight Alexander Ivanov
  2026-10-29 13:52 ` no-reply
  2026-11-02 10:36 ` Konstantin Khorenko
@ 2026-11-02 10:36 ` Konstantin Khorenko
  2 siblings, 0 replies; 5+ messages in thread
From: Konstantin Khorenko @ 2026-11-02 10:36 UTC (permalink / raw)
  To: andrey.zhadchenko, Alexander Ivanov; +Cc: devel

On Thu, 29 Oct 2026 14:48:22 +0100, Alexander Ivanov wrote:
> drivers/vhost/blk: reject a head that is still in flight

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.15.vz10, thanks!

[1/1] drivers/vhost/blk: reject a head that is still in flight
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/49a47b9982ee

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team


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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-02 10:38 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-29 13:48 [Devel] [PATCH hci-8.1 v2] drivers/vhost/blk: reject a head that is still in flight Alexander Ivanov
2026-10-29 13:52 ` no-reply
2026-10-29 13:59   ` Alexander Ivanov
2026-11-02 10:36 ` Konstantin Khorenko
2026-11-02 10:36 ` Konstantin Khorenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.