[Devel] [PATCH hci-8.1] drivers/vhost/blk: reject a head that is still in flight

[Devel] [PATCH hci-8.1] drivers/vhost/blk: reject a head that is still in flight

[Alexander Ivanov]

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.

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. The caller reports the error on the vq error eventfd
and stops processing the ring. Allocate the slots zeroed so the flag
starts clear.

https://virtuozzo.atlassian.net/browse/VSTOR-152123
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>
---
 drivers/vhost/blk.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/vhost/blk.c b/drivers/vhost/blk.c
index b170439a1806..668986449373 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,9 @@ static int vhost_blk_req_handle(struct vhost_virtqueue *vq,
 	u8 status;
 
 	req		= &blk_vq->req[head];
+	if (unlikely(req->inflight))
+		return -EBUSY;
+
 	req->blk_vq	= blk_vq;
 	req->head	= head;
 	req->blk	= blk;
@@ -628,6 +633,7 @@ static void vhost_blk_handle_host_kick(struct vhost_work *work)
 			added = true;
 		}
 
+		req->inflight = false;
 		forget_request(req);
 	}
 
@@ -703,8 +709,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

Re: [Devel] [PATCH hci-8.1] drivers/vhost/blk: reject a head that is still in flight

[Andrey Zhadchenko]

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

On 10/28/26 09:14, Alexander Ivanov wrote:
> 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.
> 
> 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. The caller reports the error on the vq error eventfd
> and stops processing the ring. Allocate the slots zeroed so the flag
> starts clear.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-152123
> 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>
> ---
>   drivers/vhost/blk.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/vhost/blk.c b/drivers/vhost/blk.c
> index b170439a1806..668986449373 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,9 @@ static int vhost_blk_req_handle(struct vhost_virtqueue *vq,
>   	u8 status;
>   
>   	req		= &blk_vq->req[head];
> +	if (unlikely(req->inflight))
> +		return -EBUSY;
> +
>   	req->blk_vq	= blk_vq;
>   	req->head	= head;
>   	req->blk	= blk;
> @@ -628,6 +633,7 @@ static void vhost_blk_handle_host_kick(struct vhost_work *work)
>   			added = true;
>   		}
>   
> +		req->inflight = false;
>   		forget_request(req);
>   	}
>   
> @@ -703,8 +709,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;
>   

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

Re: [Devel] [PATCH hci-8.1] drivers/vhost/blk: reject a head that is still in flight

[Konstantin Khorenko]

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team

On 10/28/26 09:14, Alexander Ivanov wrote:
  > The second
  > submission also overwrites bio[], bio_nr and the iovecs of bios that
  > are still in flight.

  It also frees the bounce buffer and bio[] of the first submission under
  bios that are still in flight, so this is a use-after-free, not only a
  counter leak. Worth a sentence here.

  > +     if (unlikely(req->inflight))
  > +             return -EBUSY;

  Please add a vhostblk_vq_err() here naming the head. Now the host log
  only shows the caller's generic "Failed to process guest request" with
  -16.


  A separate ticket, not for this patch: vhost_blk_req_done() ignores a
  failed vhost_vq_work_queue(). After vhost_worker_killed() (QEMU
  SIGKILLed with bios in flight) vq->worker is NULL, the completion is
  lost, forget_request() never runs and release hangs in
  vhost_blk_flush() the same way.

  Please file a ticket an post it here for the reference.

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

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1] drivers/vhost/blk: reject a head that is still in flight

[Alexander Ivanov]

On 10/29/26 13:05, Konstantin Khorenko wrote:
>
> --
> Best regards,
>
> Konstantin Khorenko,
> Virtuozzo Linux Kernel Team
>
> On 10/28/26 09:14, Alexander Ivanov wrote:
>    > The second
>    > submission also overwrites bio[], bio_nr and the iovecs of bios that
>    > are still in flight.
>
>    It also frees the bounce buffer and bio[] of the first submission under
>    bios that are still in flight, so this is a use-after-free, not only a
>    counter leak. Worth a sentence here.
>
>    > +     if (unlikely(req->inflight))
>    > +             return -EBUSY;
>
>    Please add a vhostblk_vq_err() here naming the head. Now the host log
>    only shows the caller's generic "Failed to process guest request" with
>    -16.
Fixed in the next version.
>
>    A separate ticket, not for this patch: vhost_blk_req_done() ignores a
>    failed vhost_vq_work_queue(). After vhost_worker_killed() (QEMU
>    SIGKILLed with bios in flight) vq->worker is NULL, the completion is
>    lost, forget_request() never runs and release hangs in
>    vhost_blk_flush() the same way.
>
>    Please file a ticket an post it here for the reference.
>
>    Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>
https://virtuozzo.atlassian.net/browse/VSTOR-152380
>
> --
> Best regards,
>
> Konstantin Khorenko,
> Virtuozzo Linux Kernel Team
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel