[PATCH v1] virtio: stregthen virtqueue size invariants

Denis Plotnikov posted 1 patch 4 years, 4 months ago
Test asan failed
Test checkpatch failed
Test FreeBSD failed
Test docker-mingw@fedora failed
Test docker-clang@ubuntu failed
Test docker-quick@centos7 failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20191223131820.19300-1-dplotnikov@virtuozzo.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>
hw/virtio/virtio.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH v1] virtio: stregthen virtqueue size invariants
Posted by Denis Plotnikov 4 years, 4 months ago
1. virtqueue_size is a power of 2
2. virtqueue_size > 2, since seg_max is virtqueue_size - 2

Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
---
 hw/virtio/virtio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 04716b5f6c..e3ab69061e 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2166,7 +2166,8 @@ void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
      */
     if (!!num != !!vdev->vq[n].vring.num ||
         num > VIRTQUEUE_MAX_SIZE ||
-        num < 0) {
+        num < 2 ||
+        !is_power_of_2(num)) {
         return;
     }
     vdev->vq[n].vring.num = num;
-- 
2.17.0


Re: [PATCH v1] virtio: stregthen virtqueue size invariants
Posted by Stefan Hajnoczi 4 years, 3 months ago
On Mon, Dec 23, 2019 at 04:18:20PM +0300, Denis Plotnikov wrote:
> 1. virtqueue_size is a power of 2
> 2. virtqueue_size > 2, since seg_max is virtqueue_size - 2
> 
> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
> ---
>  hw/virtio/virtio.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 04716b5f6c..e3ab69061e 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -2166,7 +2166,8 @@ void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
>       */
>      if (!!num != !!vdev->vq[n].vring.num ||
>          num > VIRTQUEUE_MAX_SIZE ||
> -        num < 0) {
> +        num < 2 ||

This is generic VIRTIO code so it's not possible to make assumptions
about seg_max, which is a virtio-blk/scsi concept.  Other VIRTIO devices
may tolerate num < 2 just fine.  Please drop this.

> +        !is_power_of_2(num)) {

This constraint only applies to the Split Virtqueue layout.  Please see
VIRTIO 1.1 "2.7.10.1 Structure Size and Alignment" for the Packed
Virtqueue layout:

  The Queue Size value does not have to be a power of 2.

The condition should be:

  !(virtio_has_features(vdev, VIRTIO_F_RING_PACKED) || is_power_of_2(num))