1. Patchew
  2. linux
  3. View series

[PATCH v3] virtio-blk: clamp max_segments to virtqueue ring size

Sergii Ushakov posted 1 patch 6 days, 12 hours ago
There is a newer version of this series
drivers/block/virtio_blk.c | 35 ++++++++++++++++++++++++++++++++++-
1 file changed, 34 insertions(+), 1 deletion(-)
[PATCH v3] virtio-blk: clamp max_segments to virtqueue ring size
Posted by Sergii Ushakov 6 days, 12 hours ago 
In virtblk_add_req(), each request consumes 2 extra descriptors (out_hdr
and in_hdr) in addition to the data scatter-gather segments.

When a hypervisor (e.g. QNX Hypervisor) advertises VIRTIO_BLK_F_SEG_MAX
with seg_max = 1024 alongside a 1024-entry split ring (vring.num = 1024)
and VIRTIO_RING_F_INDIRECT_DESC disabled, lim->max_segments is set to
1024. When the block layer submits requests with 1023 or 1024 data
segments, total_sg reaches 1025 or 1026. This exceeds vring.num (1024),
causing virtqueue_add_split() to return -ENOSPC and permanently wedge
the blk-mq queue.

Furthermore, the Virtio specification (2.7.5.3.1) requires that a
descriptor chain never exceed the Queue Size, and virtqueue_add_split()
falls back to direct descriptors if indirect table allocation fails.

Fix this by:
1. Rejecting queues with ring_size < 3 at probe, or
   ring_size < (queue_max_segments + 2) during resume/reset recovery in
   init_vq().
2. Unconditionally clamping sg_elems to (ring_size - 2) across all
   virtqueues in virtblk_read_limits().

Signed-off-by: Sergii Ushakov <sergiiushakov@google.com>
---
Hi Michael,

Thank you for the detailed review!

I logged the values in virtblk_read_limits() on the QNX Hypervisor guest:
  virtio_blk virtio0: 1/0/0 default/read/poll queues
  virtio_blk virtio0: F_SEG_MAX=1 seg_max=1024 F_INDIRECT=0 vring_size=1024 max_segments=1024

  dev_info(&vdev->dev,
           "virtio_blk debug: F_SEG_MAX=%d seg_max=%u F_INDIRECT=%d vring_size=%u max_segments=%u\n",
           virtio_has_feature(vdev, VIRTIO_BLK_F_SEG_MAX),
           sg_elems,
           virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC),
           virtqueue_get_vring_size(vblk->vqs[0].vq),
           lim->max_segments);

So what actually happens:
1. The host *does* advertise VIRTIO_BLK_F_SEG_MAX=1 with seg_max=1024 (setting
   seg_max equal to the virtqueue size vring_size=1024), and does *not*
   offer VIRTIO_RING_F_INDIRECT_DESC (F_INDIRECT=0).
2. Because seg_max (1024) <= VIRTIO_BLK_MAX_SG_ELEMS - 2 (32766),
   virtblk_read_limits() sets lim->max_segments = 1024.
3. The block layer submits requests with 1023 or 1024 data segments.
   virtblk_add_req() adds 2 extra descriptors (out_hdr and in_hdr),
   resulting in total_sg > 1024.
4. Since total_sg > vq->split.vring.num and !vq->indirect,
   virtqueue_add_split() triggers:
     WARN_ON_ONCE(total_sg > vq->split.vring.num && !vq->indirect);

In v3, I drop the !virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC)
condition and clamp sg_elems to (ring_size - 2) unconditionally across all
virtqueues.

Also, init_vq() checks all virtqueues:
- At initial probe (!vblk->disk), it enforces ring_size >= 3.
- On resume/recovery, it enforces
  ring_size >= queue_max_segments(vblk->disk->queue) + 2.

Logs after the v3 patch:
  virtio_blk virtio0: F_SEG_MAX=1 seg_max=1022 F_INDIRECT=0 vring_size=1024 max_segments=1022

 drivers/block/virtio_blk.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 32bf3ba07a9d..c263d0e3db95 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -1021,6 +1021,21 @@ static int init_vq(struct virtio_blk *vblk)
 		goto out;
 
 	for (i = 0; i < num_vqs; i++) {
+		unsigned int ring_size = virtqueue_get_vring_size(vqs[i]);
+		unsigned int min_ring_size = 3;
+
+		if (vblk->disk)
+			min_ring_size = queue_max_segments(vblk->disk->queue) + 2;
+
+		if (ring_size < min_ring_size) {
+			dev_err(&vdev->dev,
+				"virtqueue %u ring size %u is smaller than minimum %u\n",
+				i, ring_size, min_ring_size);
+			vdev->config->del_vqs(vdev);
+			err = -EINVAL;
+			goto out;
+		}
+
 		spin_lock_init(&vblk->vqs[i].lock);
 		vblk->vqs[i].vq = vqs[i];
 	}
@@ -1253,7 +1268,7 @@ static int virtblk_read_limits(struct virtio_blk *vblk,
 	u16 min_io_size;
 	u8 physical_block_exp, alignment_offset;
 	size_t max_dma_size;
-	int err;
+	int err, i;
 
 	/* We need to know how many segments before we allocate. */
 	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SEG_MAX,
@@ -1267,6 +1282,23 @@ static int virtblk_read_limits(struct virtio_blk *vblk,
 	/* Prevent integer overflows and honor max vq size */
 	sg_elems = min_t(u32, sg_elems, VIRTIO_BLK_MAX_SG_ELEMS - 2);
 
+	/*
+	 * virtblk_add_req() uses separate outgoing and incoming header
+	 * descriptors (out_hdr and in_hdr), consuming 2 extra descriptors
+	 * per request in addition to the data segments.
+	 *
+	 * Per virtio specification (2.7.5.3.1), a driver MUST NOT create a
+	 * descriptor chain longer than the Queue Size of the device.
+	 *
+	 * Clamp max_segments to (ring_size - 2) across all virtqueues so
+	 * that a request never exceeds the ring size of any queue.
+	 */
+	for (i = 0; i < vblk->num_vqs; i++) {
+		u32 ring_size = virtqueue_get_vring_size(vblk->vqs[i].vq);
+
+		sg_elems = min_t(u32, sg_elems, ring_size - 2);
+	}
+
 	/* We can handle whatever the host told us to handle. */
 	lim->max_segments = sg_elems;
 
@@ -1466,6 +1498,7 @@ static int virtblk_probe(struct virtio_device *vdev)
 	mutex_init(&vblk->vdev_mutex);
 
 	vblk->vdev = vdev;
+	vblk->disk = NULL;
 
 	INIT_WORK(&vblk->config_work, virtblk_config_changed_work);
 
-- 
2.55.0.1082.g2b9226bbc0-goog
Re: [PATCH v3] virtio-blk: clamp max_segments to virtqueue ring size
Posted by Michael S. Tsirkin 6 days, 11 hours ago 
Love the patch! yet something to improve:

On Fri, Sep 18, 2026 at 01:34:12PM +0200, Sergii Ushakov wrote:
> In virtblk_add_req(), each request consumes 2 extra descriptors (out_hdr
> and in_hdr) in addition to the data scatter-gather segments.
> 
> When a hypervisor (e.g. QNX Hypervisor) advertises VIRTIO_BLK_F_SEG_MAX
> with seg_max = 1024 alongside a 1024-entry split ring (vring.num = 1024)
> and VIRTIO_RING_F_INDIRECT_DESC disabled, lim->max_segments is set to
> 1024. When the block layer submits requests with 1023 or 1024 data
> segments, total_sg reaches 1025 or 1026. This exceeds vring.num (1024),
> causing virtqueue_add_split() to return -ENOSPC and permanently wedge
> the blk-mq queue.
> 
> Furthermore, the Virtio specification (2.7.5.3.1) requires that a
> descriptor chain never exceed the Queue Size, and virtqueue_add_split()
> falls back to direct descriptors if indirect table allocation fails.
> 
> Fix this by:
> 1. Rejecting queues with ring_size < 3 at probe, or
>    ring_size < (queue_max_segments + 2) during resume/reset recovery in
>    init_vq().
> 2. Unconditionally clamping sg_elems to (ring_size - 2) across all
>    virtqueues in virtblk_read_limits().
> 


Fixes tag please?

> Signed-off-by: Sergii Ushakov <sergiiushakov@google.com>
> ---
> Hi Michael,
> 
> Thank you for the detailed review!
> 
> I logged the values in virtblk_read_limits() on the QNX Hypervisor guest:
>   virtio_blk virtio0: 1/0/0 default/read/poll queues
>   virtio_blk virtio0: F_SEG_MAX=1 seg_max=1024 F_INDIRECT=0 vring_size=1024 max_segments=1024
> 
>   dev_info(&vdev->dev,
>            "virtio_blk debug: F_SEG_MAX=%d seg_max=%u F_INDIRECT=%d vring_size=%u max_segments=%u\n",
>            virtio_has_feature(vdev, VIRTIO_BLK_F_SEG_MAX),
>            sg_elems,
>            virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC),
>            virtqueue_get_vring_size(vblk->vqs[0].vq),
>            lim->max_segments);
> 
> So what actually happens:
> 1. The host *does* advertise VIRTIO_BLK_F_SEG_MAX=1 with seg_max=1024 (setting
>    seg_max equal to the virtqueue size vring_size=1024), and does *not*
>    offer VIRTIO_RING_F_INDIRECT_DESC (F_INDIRECT=0).
> 2. Because seg_max (1024) <= VIRTIO_BLK_MAX_SG_ELEMS - 2 (32766),
>    virtblk_read_limits() sets lim->max_segments = 1024.
> 3. The block layer submits requests with 1023 or 1024 data segments.
>    virtblk_add_req() adds 2 extra descriptors (out_hdr and in_hdr),
>    resulting in total_sg > 1024.
> 4. Since total_sg > vq->split.vring.num and !vq->indirect,
>    virtqueue_add_split() triggers:
>      WARN_ON_ONCE(total_sg > vq->split.vring.num && !vq->indirect);
> 
> In v3, I drop the !virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC)
> condition and clamp sg_elems to (ring_size - 2) unconditionally across all
> virtqueues.
> 
> Also, init_vq() checks all virtqueues:
> - At initial probe (!vblk->disk), it enforces ring_size >= 3.
> - On resume/recovery, it enforces
>   ring_size >= queue_max_segments(vblk->disk->queue) + 2.
> 
> Logs after the v3 patch:
>   virtio_blk virtio0: F_SEG_MAX=1 seg_max=1022 F_INDIRECT=0 vring_size=1024 max_segments=1022
> 
>  drivers/block/virtio_blk.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 32bf3ba07a9d..c263d0e3db95 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -1021,6 +1021,21 @@ static int init_vq(struct virtio_blk *vblk)
>  		goto out;
>  
>  	for (i = 0; i < num_vqs; i++) {
> +		unsigned int ring_size = virtqueue_get_vring_size(vqs[i]);
> +		unsigned int min_ring_size = 3;
> +
> +		if (vblk->disk)
> +			min_ring_size = queue_max_segments(vblk->disk->queue) + 2;

could you pls add code comments documenting the constants here?


> +
> +		if (ring_size < min_ring_size) {
> +			dev_err(&vdev->dev,
> +				"virtqueue %u ring size %u is smaller than minimum %u\n",
> +				i, ring_size, min_ring_size);
> +			vdev->config->del_vqs(vdev);
> +			err = -EINVAL;
> +			goto out;
> +		}
> +
>  		spin_lock_init(&vblk->vqs[i].lock);
>  		vblk->vqs[i].vq = vqs[i];
>  	}
> @@ -1253,7 +1268,7 @@ static int virtblk_read_limits(struct virtio_blk *vblk,
>  	u16 min_io_size;
>  	u8 physical_block_exp, alignment_offset;
>  	size_t max_dma_size;
> -	int err;
> +	int err, i;
>  
>  	/* We need to know how many segments before we allocate. */
>  	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SEG_MAX,
> @@ -1267,6 +1282,23 @@ static int virtblk_read_limits(struct virtio_blk *vblk,
>  	/* Prevent integer overflows and honor max vq size */
>  	sg_elems = min_t(u32, sg_elems, VIRTIO_BLK_MAX_SG_ELEMS - 2);
>  
> +	/*
> +	 * virtblk_add_req() uses separate outgoing and incoming header
> +	 * descriptors (out_hdr and in_hdr), consuming 2 extra descriptors
> +	 * per request in addition to the data segments.
> +	 *
> +	 * Per virtio specification (2.7.5.3.1), a driver MUST NOT create a
> +	 * descriptor chain longer than the Queue Size of the device.
> +	 *
> +	 * Clamp max_segments to (ring_size - 2) across all virtqueues so
> +	 * that a request never exceeds the ring size of any queue.
> +	 */
> +	for (i = 0; i < vblk->num_vqs; i++) {
> +		u32 ring_size = virtqueue_get_vring_size(vblk->vqs[i].vq);
> +
> +		sg_elems = min_t(u32, sg_elems, ring_size - 2);
> +	}
> +
>  	/* We can handle whatever the host told us to handle. */
>  	lim->max_segments = sg_elems;
>  
> @@ -1466,6 +1498,7 @@ static int virtblk_probe(struct virtio_device *vdev)
>  	mutex_init(&vblk->vdev_mutex);
>  
>  	vblk->vdev = vdev;
> +	vblk->disk = NULL;
>  
>  	INIT_WORK(&vblk->config_work, virtblk_config_changed_work);
>  
> -- 
> 2.55.0.1082.g2b9226bbc0-goog
[PATCH v4] virtio-blk: clamp max_segments to virtqueue ring size
Posted by Sergii Ushakov 6 days, 10 hours ago 
In virtblk_add_req(), each request consumes 2 extra descriptors (out_hdr
and in_hdr) in addition to the data scatter-gather segments.

When a hypervisor (e.g. QNX Hypervisor) advertises VIRTIO_BLK_F_SEG_MAX
with seg_max = 1024 alongside a 1024-entry split ring (vring.num = 1024)
and VIRTIO_RING_F_INDIRECT_DESC disabled, lim->max_segments is set to
1024. When the block layer submits requests with 1023 or 1024 data
segments, total_sg reaches 1025 or 1026. This exceeds vring.num (1024),
causing virtqueue_add_split() to return -ENOSPC and permanently wedge
the blk-mq queue.

Furthermore, the Virtio specification (2.7.5.3.1) requires that a
descriptor chain never exceed the Queue Size, and virtqueue_add_split()
falls back to direct descriptors if indirect table allocation fails.

Fix this by:
1. Rejecting queues with ring_size < 3 at probe, or
   ring_size < (queue_max_segments + 2) during resume/reset recovery in
   init_vq().
2. Unconditionally clamping sg_elems to (ring_size - 2) across all
   virtqueues in virtblk_read_limits().

Fixes: 0864b79a1533 ("virtio: block: dynamic maximum segments")
Signed-off-by: Sergii Ushakov <sergiiushakov@google.com>
---
Changes in v4:
- Add Fixes tag.
- Add code comments in init_vq() documenting the constants 3 and + 2.

 drivers/block/virtio_blk.c | 44 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 32bf3ba07a9d..cafca309b4d0 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -1021,6 +1021,30 @@ static int init_vq(struct virtio_blk *vblk)
 		goto out;
 
 	for (i = 0; i < num_vqs; i++) {
+		unsigned int ring_size = virtqueue_get_vring_size(vqs[i]);
+		/*
+		 * Each request consumes 2 extra descriptors (out_hdr and
+		 * in_hdr) in addition to the data segments.
+		 *
+		 * At initial probe (!vblk->disk), each virtqueue must fit at
+		 * least 1 data segment + 2 header descriptors (3).
+		 * On resume/recovery (vblk->disk), each virtqueue must fit the
+		 * existing queue_max_segments() + 2 header descriptors.
+		 */
+		unsigned int min_ring_size = 3;
+
+		if (vblk->disk)
+			min_ring_size = queue_max_segments(vblk->disk->queue) + 2;
+
+		if (ring_size < min_ring_size) {
+			dev_err(&vdev->dev,
+				"virtqueue %u ring size %u is smaller than minimum %u\n",
+				i, ring_size, min_ring_size);
+			vdev->config->del_vqs(vdev);
+			err = -EINVAL;
+			goto out;
+		}
+
 		spin_lock_init(&vblk->vqs[i].lock);
 		vblk->vqs[i].vq = vqs[i];
 	}
@@ -1253,7 +1277,7 @@ static int virtblk_read_limits(struct virtio_blk *vblk,
 	u16 min_io_size;
 	u8 physical_block_exp, alignment_offset;
 	size_t max_dma_size;
-	int err;
+	int err, i;
 
 	/* We need to know how many segments before we allocate. */
 	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SEG_MAX,
@@ -1267,6 +1291,23 @@ static int virtblk_read_limits(struct virtio_blk *vblk,
 	/* Prevent integer overflows and honor max vq size */
 	sg_elems = min_t(u32, sg_elems, VIRTIO_BLK_MAX_SG_ELEMS - 2);
 
+	/*
+	 * virtblk_add_req() uses separate outgoing and incoming header
+	 * descriptors (out_hdr and in_hdr), consuming 2 extra descriptors
+	 * per request in addition to the data segments.
+	 *
+	 * Per virtio specification (2.7.5.3.1), a driver MUST NOT create a
+	 * descriptor chain longer than the Queue Size of the device.
+	 *
+	 * Clamp max_segments to (ring_size - 2) across all virtqueues so
+	 * that a request never exceeds the ring size of any queue.
+	 */
+	for (i = 0; i < vblk->num_vqs; i++) {
+		u32 ring_size = virtqueue_get_vring_size(vblk->vqs[i].vq);
+
+		sg_elems = min_t(u32, sg_elems, ring_size - 2);
+	}
+
 	/* We can handle whatever the host told us to handle. */
 	lim->max_segments = sg_elems;
 
@@ -1466,6 +1507,7 @@ static int virtblk_probe(struct virtio_device *vdev)
 	mutex_init(&vblk->vdev_mutex);
 
 	vblk->vdev = vdev;
+	vblk->disk = NULL;
 
 	INIT_WORK(&vblk->config_work, virtblk_config_changed_work);
 
-- 
2.55.0.1082.g2b9226bbc0-goog

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.