diff mbox series

[kvmtool,3/5] virtio: Check for overflows in QUEUE_NOTIFY and QUEUE_SEL

Message ID 20220303231050.2146621-4-martin.b.radev@gmail.com (mailing list archive)
State New, archived
Headers show
Series Fix few small issues in virtio code | expand

Commit Message

Martin Radev March 3, 2022, 11:10 p.m. UTC
This patch checks for overflows in QUEUE_NOTIFY and QUEUE_SEL in
the PCI and MMIO operation handling paths. Further, the return
value type of get_vq_count is changed from int to uint since negative
doesn't carry any semantic meaning.

Signed-off-by: Martin Radev <martin.b.radev@gmail.com>
---
 include/kvm/virtio.h |  2 +-
 virtio/9p.c          |  2 +-
 virtio/balloon.c     |  2 +-
 virtio/blk.c         |  2 +-
 virtio/console.c     |  2 +-
 virtio/mmio.c        | 20 ++++++++++++++++++--
 virtio/net.c         |  4 ++--
 virtio/pci.c         | 21 ++++++++++++++++++---
 virtio/rng.c         |  2 +-
 virtio/scsi.c        |  2 +-
 virtio/vsock.c       |  2 +-
 11 files changed, 46 insertions(+), 15 deletions(-)

Comments

Alexandru Elisei March 16, 2022, 3:38 p.m. UTC | #1
Hi,

On Fri, Mar 04, 2022 at 01:10:48AM +0200, Martin Radev wrote:
> This patch checks for overflows in QUEUE_NOTIFY and QUEUE_SEL in
> the PCI and MMIO operation handling paths. Further, the return
> value type of get_vq_count is changed from int to uint since negative
> doesn't carry any semantic meaning.
> 
> Signed-off-by: Martin Radev <martin.b.radev@gmail.com>
> ---
>  include/kvm/virtio.h |  2 +-
>  virtio/9p.c          |  2 +-
>  virtio/balloon.c     |  2 +-
>  virtio/blk.c         |  2 +-
>  virtio/console.c     |  2 +-
>  virtio/mmio.c        | 20 ++++++++++++++++++--
>  virtio/net.c         |  4 ++--
>  virtio/pci.c         | 21 ++++++++++++++++++---
>  virtio/rng.c         |  2 +-
>  virtio/scsi.c        |  2 +-
>  virtio/vsock.c       |  2 +-
>  11 files changed, 46 insertions(+), 15 deletions(-)
> 
> diff --git a/include/kvm/virtio.h b/include/kvm/virtio.h
> index 3880e74..ad274ac 100644
> --- a/include/kvm/virtio.h
> +++ b/include/kvm/virtio.h
> @@ -187,7 +187,7 @@ struct virtio_ops {
>  	size_t (*get_config_size)(struct kvm *kvm, void *dev);
>  	u32 (*get_host_features)(struct kvm *kvm, void *dev);
>  	void (*set_guest_features)(struct kvm *kvm, void *dev, u32 features);
> -	int (*get_vq_count)(struct kvm *kvm, void *dev);
> +	unsigned int (*get_vq_count)(struct kvm *kvm, void *dev);
>  	int (*init_vq)(struct kvm *kvm, void *dev, u32 vq, u32 page_size,
>  		       u32 align, u32 pfn);
>  	void (*exit_vq)(struct kvm *kvm, void *dev, u32 vq);
> diff --git a/virtio/9p.c b/virtio/9p.c
> index 6074f3a..7374f1e 100644
> --- a/virtio/9p.c
> +++ b/virtio/9p.c
> @@ -1469,7 +1469,7 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
>  	return size;
>  }
>  
> -static int get_vq_count(struct kvm *kvm, void *dev)
> +static unsigned int get_vq_count(struct kvm *kvm, void *dev)
>  {
>  	return NUM_VIRT_QUEUES;
>  }
> diff --git a/virtio/balloon.c b/virtio/balloon.c
> index 5bcd6ab..450b36a 100644
> --- a/virtio/balloon.c
> +++ b/virtio/balloon.c
> @@ -251,7 +251,7 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
>  	return size;
>  }
>  
> -static int get_vq_count(struct kvm *kvm, void *dev)
> +static unsigned int get_vq_count(struct kvm *kvm, void *dev)
>  {
>  	return NUM_VIRT_QUEUES;
>  }
> diff --git a/virtio/blk.c b/virtio/blk.c
> index af71c0c..46ee028 100644
> --- a/virtio/blk.c
> +++ b/virtio/blk.c
> @@ -291,7 +291,7 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
>  	return size;
>  }
>  
> -static int get_vq_count(struct kvm *kvm, void *dev)
> +static unsigned int get_vq_count(struct kvm *kvm, void *dev)
>  {
>  	return NUM_VIRT_QUEUES;
>  }
> diff --git a/virtio/console.c b/virtio/console.c
> index dae6034..8315808 100644
> --- a/virtio/console.c
> +++ b/virtio/console.c
> @@ -216,7 +216,7 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
>  	return size;
>  }
>  
> -static int get_vq_count(struct kvm *kvm, void *dev)
> +static unsigned int get_vq_count(struct kvm *kvm, void *dev)
>  {
>  	return VIRTIO_CONSOLE_NUM_QUEUES;
>  }
> diff --git a/virtio/mmio.c b/virtio/mmio.c
> index 0094856..d3555b4 100644
> --- a/virtio/mmio.c
> +++ b/virtio/mmio.c
> @@ -175,13 +175,22 @@ static void virtio_mmio_config_out(struct kvm_cpu *vcpu,
>  {
>  	struct virtio_mmio *vmmio = vdev->virtio;
>  	struct kvm *kvm = vmmio->kvm;
> +	unsigned int vq_count = vdev->ops->get_vq_count(kvm, vmmio->dev);
>  	u32 val = 0;
>  
>  	switch (addr) {
>  	case VIRTIO_MMIO_HOST_FEATURES_SEL:
>  	case VIRTIO_MMIO_GUEST_FEATURES_SEL:
> +		val = ioport__read32(data);
> +		*(u32 *)(((void *)&vmmio->hdr) + addr) = val;
> +		break;
>  	case VIRTIO_MMIO_QUEUE_SEL:
>  		val = ioport__read32(data);
> +		if (val >= vq_count) {
> +			WARN_ONCE(1, "QUEUE_SEL value (%u) is larger than VQ count (%u)\n",
> +				val, vq_count);
> +			break;
> +		}
>  		*(u32 *)(((void *)&vmmio->hdr) + addr) = val;
>  		break;
>  	case VIRTIO_MMIO_STATUS:
> @@ -227,6 +236,11 @@ static void virtio_mmio_config_out(struct kvm_cpu *vcpu,
>  		break;
>  	case VIRTIO_MMIO_QUEUE_NOTIFY:
>  		val = ioport__read32(data);
> +		if (val >= vq_count) {
> +			WARN_ONCE(1, "QUEUE_NOTIFY value (%u) is larger than VQ count (%u)\n",
> +				val, vq_count);
> +			break;
> +		}
>  		vdev->ops->notify_vq(vmmio->kvm, vmmio->dev, val);
>  		break;
>  	case VIRTIO_MMIO_INTERRUPT_ACK:
> @@ -346,10 +360,12 @@ int virtio_mmio_init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
>  
>  int virtio_mmio_reset(struct kvm *kvm, struct virtio_device *vdev)
>  {
> -	int vq;
> +	unsigned int vq;
>  	struct virtio_mmio *vmmio = vdev->virtio;
> +	unsigned int vq_count;
>  
> -	for (vq = 0; vq < vdev->ops->get_vq_count(kvm, vmmio->dev); vq++)
> +	vq_count = vdev->ops->get_vq_count(kvm, vmmio->dev);
> +	for (vq = 0; vq < vq_count; vq++)

Nitpick: this change is unnecessary and pollutes the git history for this
file. Same for virtio_pci_reset() below.

>  		virtio_mmio_exit_vq(kvm, vdev, vq);
>  
>  	return 0;
> diff --git a/virtio/net.c b/virtio/net.c
> index ec5dc1f..8dd523f 100644
> --- a/virtio/net.c
> +++ b/virtio/net.c
> @@ -755,11 +755,11 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
>  	return size;
>  }
>  
> -static int get_vq_count(struct kvm *kvm, void *dev)
> +static unsigned int get_vq_count(struct kvm *kvm, void *dev)
>  {
>  	struct net_dev *ndev = dev;
>  
> -	return ndev->queue_pairs * 2 + 1;
> +	return ndev->queue_pairs * 2U + 1U;

I don't think the cast is needed, as far as I know signed integers are
converted to unsigned integers as far back as C89 (and probably even
before that).

Other than the nitpicks above, the patch looks good.

Thanks,
Alex

>  }
>  
>  static struct virtio_ops net_dev_virtio_ops = {
> diff --git a/virtio/pci.c b/virtio/pci.c
> index 0b5cccd..9a6cbf3 100644
> --- a/virtio/pci.c
> +++ b/virtio/pci.c
> @@ -329,9 +329,11 @@ static bool virtio_pci__data_out(struct kvm_cpu *vcpu, struct virtio_device *vde
>  	struct virtio_pci *vpci;
>  	struct kvm *kvm;
>  	u32 val;
> +	unsigned int vq_count;
>  
>  	kvm = vcpu->kvm;
>  	vpci = vdev->virtio;
> +	vq_count = vdev->ops->get_vq_count(kvm, vpci->dev);
>  
>  	switch (offset) {
>  	case VIRTIO_PCI_GUEST_FEATURES:
> @@ -351,10 +353,21 @@ static bool virtio_pci__data_out(struct kvm_cpu *vcpu, struct virtio_device *vde
>  		}
>  		break;
>  	case VIRTIO_PCI_QUEUE_SEL:
> -		vpci->queue_selector = ioport__read16(data);
> +		val = ioport__read16(data);
> +		if (val >= vq_count) {
> +			WARN_ONCE(1, "QUEUE_SEL value (%u) is larger than VQ count (%u)\n",
> +				val, vq_count);
> +			return false;
> +		}
> +		vpci->queue_selector = val;
>  		break;
>  	case VIRTIO_PCI_QUEUE_NOTIFY:
>  		val = ioport__read16(data);
> +		if (val >= vq_count) {
> +			WARN_ONCE(1, "QUEUE_SEL value (%u) is larger than VQ count (%u)\n",
> +				val, vq_count);
> +			return false;
> +		}
>  		vdev->ops->notify_vq(kvm, vpci->dev, val);
>  		break;
>  	case VIRTIO_PCI_STATUS:
> @@ -647,10 +660,12 @@ int virtio_pci__init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
>  
>  int virtio_pci__reset(struct kvm *kvm, struct virtio_device *vdev)
>  {
> -	int vq;
> +	unsigned int vq;
> +	unsigned int vq_count;
>  	struct virtio_pci *vpci = vdev->virtio;
>  
> -	for (vq = 0; vq < vdev->ops->get_vq_count(kvm, vpci->dev); vq++)
> +	vq_count = vdev->ops->get_vq_count(kvm, vpci->dev);
> +	for (vq = 0; vq < vq_count; vq++)
>  		virtio_pci_exit_vq(kvm, vdev, vq);
>  
>  	return 0;
> diff --git a/virtio/rng.c b/virtio/rng.c
> index c7835a0..75b682e 100644
> --- a/virtio/rng.c
> +++ b/virtio/rng.c
> @@ -147,7 +147,7 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
>  	return size;
>  }
>  
> -static int get_vq_count(struct kvm *kvm, void *dev)
> +static unsigned int get_vq_count(struct kvm *kvm, void *dev)
>  {
>  	return NUM_VIRT_QUEUES;
>  }
> diff --git a/virtio/scsi.c b/virtio/scsi.c
> index 8f1c348..60432cc 100644
> --- a/virtio/scsi.c
> +++ b/virtio/scsi.c
> @@ -176,7 +176,7 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
>  	return size;
>  }
>  
> -static int get_vq_count(struct kvm *kvm, void *dev)
> +static unsigned int get_vq_count(struct kvm *kvm, void *dev)
>  {
>  	return NUM_VIRT_QUEUES;
>  }
> diff --git a/virtio/vsock.c b/virtio/vsock.c
> index 34397b6..64b4e95 100644
> --- a/virtio/vsock.c
> +++ b/virtio/vsock.c
> @@ -204,7 +204,7 @@ static void notify_vq_gsi(struct kvm *kvm, void *dev, u32 vq, u32 gsi)
>  		die_perror("VHOST_SET_VRING_CALL failed");
>  }
>  
> -static int get_vq_count(struct kvm *kvm, void *dev)
> +static unsigned int get_vq_count(struct kvm *kvm, void *dev)
>  {
>  	return VSOCK_VQ_MAX;
>  }
> -- 
> 2.25.1
>
Martin Radev March 27, 2022, 8:45 p.m. UTC | #2
Thanks for the review.
Comments are inline.
Here is the patch:


From dc2b54f3368bd012eaa2f55bb8b98278f6278df1 Mon Sep 17 00:00:00 2001
From: Martin Radev <martin.b.radev@gmail.com>
Date: Sun, 16 Jan 2022 18:50:44 +0200
Subject: [PATCH kvmtool 5/6] virtio: Check for overflows in QUEUE_NOTIFY and
 QUEUE_SEL

This patch checks for overflows in QUEUE_NOTIFY and QUEUE_SEL in
the PCI and MMIO operation handling paths. Further, the return
value type of get_vq_count is changed from int to uint since negative
doesn't carry any semantic meaning.

Signed-off-by: Martin Radev <martin.b.radev@gmail.com>
---
 include/kvm/virtio.h |  2 +-
 virtio/9p.c          |  2 +-
 virtio/balloon.c     |  2 +-
 virtio/blk.c         |  2 +-
 virtio/console.c     |  2 +-
 virtio/mmio.c        | 16 +++++++++++++++-
 virtio/net.c         |  2 +-
 virtio/pci.c         | 17 +++++++++++++++--
 virtio/rng.c         |  2 +-
 virtio/scsi.c        |  2 +-
 virtio/vsock.c       |  2 +-
 11 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/include/kvm/virtio.h b/include/kvm/virtio.h
index 3880e74..ad274ac 100644
--- a/include/kvm/virtio.h
+++ b/include/kvm/virtio.h
@@ -187,7 +187,7 @@ struct virtio_ops {
 	size_t (*get_config_size)(struct kvm *kvm, void *dev);
 	u32 (*get_host_features)(struct kvm *kvm, void *dev);
 	void (*set_guest_features)(struct kvm *kvm, void *dev, u32 features);
-	int (*get_vq_count)(struct kvm *kvm, void *dev);
+	unsigned int (*get_vq_count)(struct kvm *kvm, void *dev);
 	int (*init_vq)(struct kvm *kvm, void *dev, u32 vq, u32 page_size,
 		       u32 align, u32 pfn);
 	void (*exit_vq)(struct kvm *kvm, void *dev, u32 vq);
diff --git a/virtio/9p.c b/virtio/9p.c
index 57cd6d0..7c9d792 100644
--- a/virtio/9p.c
+++ b/virtio/9p.c
@@ -1469,7 +1469,7 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
-static int get_vq_count(struct kvm *kvm, void *dev)
+static unsigned int get_vq_count(struct kvm *kvm, void *dev)
 {
 	return NUM_VIRT_QUEUES;
 }
diff --git a/virtio/balloon.c b/virtio/balloon.c
index 5bcd6ab..450b36a 100644
--- a/virtio/balloon.c
+++ b/virtio/balloon.c
@@ -251,7 +251,7 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
-static int get_vq_count(struct kvm *kvm, void *dev)
+static unsigned int get_vq_count(struct kvm *kvm, void *dev)
 {
 	return NUM_VIRT_QUEUES;
 }
diff --git a/virtio/blk.c b/virtio/blk.c
index af71c0c..46ee028 100644
--- a/virtio/blk.c
+++ b/virtio/blk.c
@@ -291,7 +291,7 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
-static int get_vq_count(struct kvm *kvm, void *dev)
+static unsigned int get_vq_count(struct kvm *kvm, void *dev)
 {
 	return NUM_VIRT_QUEUES;
 }
diff --git a/virtio/console.c b/virtio/console.c
index dae6034..8315808 100644
--- a/virtio/console.c
+++ b/virtio/console.c
@@ -216,7 +216,7 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
-static int get_vq_count(struct kvm *kvm, void *dev)
+static unsigned int get_vq_count(struct kvm *kvm, void *dev)
 {
 	return VIRTIO_CONSOLE_NUM_QUEUES;
 }
diff --git a/virtio/mmio.c b/virtio/mmio.c
index c14f08a..4c16359 100644
--- a/virtio/mmio.c
+++ b/virtio/mmio.c
@@ -175,13 +175,22 @@ static void virtio_mmio_config_out(struct kvm_cpu *vcpu,
 {
 	struct virtio_mmio *vmmio = vdev->virtio;
 	struct kvm *kvm = vmmio->kvm;
+	unsigned int vq_count = vdev->ops->get_vq_count(kvm, vmmio->dev);
 	u32 val = 0;
 
 	switch (addr) {
 	case VIRTIO_MMIO_HOST_FEATURES_SEL:
 	case VIRTIO_MMIO_GUEST_FEATURES_SEL:
+		val = ioport__read32(data);
+		*(u32 *)(((void *)&vmmio->hdr) + addr) = val;
+		break;
 	case VIRTIO_MMIO_QUEUE_SEL:
 		val = ioport__read32(data);
+		if (val >= vq_count) {
+			WARN_ONCE(1, "QUEUE_SEL value (%u) is larger than VQ count (%u)\n",
+				val, vq_count);
+			break;
+		}
 		*(u32 *)(((void *)&vmmio->hdr) + addr) = val;
 		break;
 	case VIRTIO_MMIO_STATUS:
@@ -227,6 +236,11 @@ static void virtio_mmio_config_out(struct kvm_cpu *vcpu,
 		break;
 	case VIRTIO_MMIO_QUEUE_NOTIFY:
 		val = ioport__read32(data);
+		if (val >= vq_count) {
+			WARN_ONCE(1, "QUEUE_NOTIFY value (%u) is larger than VQ count (%u)\n",
+				val, vq_count);
+			break;
+		}
 		vdev->ops->notify_vq(vmmio->kvm, vmmio->dev, val);
 		break;
 	case VIRTIO_MMIO_INTERRUPT_ACK:
@@ -346,7 +360,7 @@ int virtio_mmio_init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
 
 int virtio_mmio_reset(struct kvm *kvm, struct virtio_device *vdev)
 {
-	int vq;
+	unsigned int vq;
 	struct virtio_mmio *vmmio = vdev->virtio;
 
 	for (vq = 0; vq < vdev->ops->get_vq_count(kvm, vmmio->dev); vq++)
diff --git a/virtio/net.c b/virtio/net.c
index ec5dc1f..67070d6 100644
--- a/virtio/net.c
+++ b/virtio/net.c
@@ -755,7 +755,7 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
-static int get_vq_count(struct kvm *kvm, void *dev)
+static unsigned int get_vq_count(struct kvm *kvm, void *dev)
 {
 	struct net_dev *ndev = dev;
 
diff --git a/virtio/pci.c b/virtio/pci.c
index 13f2b76..eb7af32 100644
--- a/virtio/pci.c
+++ b/virtio/pci.c
@@ -320,9 +320,11 @@ static bool virtio_pci__data_out(struct kvm_cpu *vcpu, struct virtio_device *vde
 	struct virtio_pci *vpci;
 	struct kvm *kvm;
 	u32 val;
+	unsigned int vq_count;
 
 	kvm = vcpu->kvm;
 	vpci = vdev->virtio;
+	vq_count = vdev->ops->get_vq_count(kvm, vpci->dev);
 
 	switch (offset) {
 	case VIRTIO_PCI_GUEST_FEATURES:
@@ -342,10 +344,21 @@ static bool virtio_pci__data_out(struct kvm_cpu *vcpu, struct virtio_device *vde
 		}
 		break;
 	case VIRTIO_PCI_QUEUE_SEL:
-		vpci->queue_selector = ioport__read16(data);
+		val = ioport__read16(data);
+		if (val >= vq_count) {
+			WARN_ONCE(1, "QUEUE_SEL value (%u) is larger than VQ count (%u)\n",
+				val, vq_count);
+			return false;
+		}
+		vpci->queue_selector = val;
 		break;
 	case VIRTIO_PCI_QUEUE_NOTIFY:
 		val = ioport__read16(data);
+		if (val >= vq_count) {
+			WARN_ONCE(1, "QUEUE_SEL value (%u) is larger than VQ count (%u)\n",
+				val, vq_count);
+			return false;
+		}
 		vdev->ops->notify_vq(kvm, vpci->dev, val);
 		break;
 	case VIRTIO_PCI_STATUS:
@@ -638,7 +651,7 @@ int virtio_pci__init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
 
 int virtio_pci__reset(struct kvm *kvm, struct virtio_device *vdev)
 {
-	int vq;
+	unsigned int vq;
 	struct virtio_pci *vpci = vdev->virtio;
 
 	for (vq = 0; vq < vdev->ops->get_vq_count(kvm, vpci->dev); vq++)
diff --git a/virtio/rng.c b/virtio/rng.c
index c7835a0..75b682e 100644
--- a/virtio/rng.c
+++ b/virtio/rng.c
@@ -147,7 +147,7 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
-static int get_vq_count(struct kvm *kvm, void *dev)
+static unsigned int get_vq_count(struct kvm *kvm, void *dev)
 {
 	return NUM_VIRT_QUEUES;
 }
diff --git a/virtio/scsi.c b/virtio/scsi.c
index 8f1c348..60432cc 100644
--- a/virtio/scsi.c
+++ b/virtio/scsi.c
@@ -176,7 +176,7 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
-static int get_vq_count(struct kvm *kvm, void *dev)
+static unsigned int get_vq_count(struct kvm *kvm, void *dev)
 {
 	return NUM_VIRT_QUEUES;
 }
diff --git a/virtio/vsock.c b/virtio/vsock.c
index 34397b6..64b4e95 100644
--- a/virtio/vsock.c
+++ b/virtio/vsock.c
@@ -204,7 +204,7 @@ static void notify_vq_gsi(struct kvm *kvm, void *dev, u32 vq, u32 gsi)
 		die_perror("VHOST_SET_VRING_CALL failed");
 }
 
-static int get_vq_count(struct kvm *kvm, void *dev)
+static unsigned int get_vq_count(struct kvm *kvm, void *dev)
 {
 	return VSOCK_VQ_MAX;
 }
Alexandru Elisei April 22, 2022, 10:35 a.m. UTC | #3
Hi,

On Sun, Mar 27, 2022 at 11:45:13PM +0300, Martin Radev wrote:
> 
> Thanks for the review.
> Comments are inline.
> Here is the patch:

Would you mind making a new version of the series to send this patch?

Thanks,
Alex
diff mbox series

Patch

diff --git a/include/kvm/virtio.h b/include/kvm/virtio.h
index 3880e74..ad274ac 100644
--- a/include/kvm/virtio.h
+++ b/include/kvm/virtio.h
@@ -187,7 +187,7 @@  struct virtio_ops {
 	size_t (*get_config_size)(struct kvm *kvm, void *dev);
 	u32 (*get_host_features)(struct kvm *kvm, void *dev);
 	void (*set_guest_features)(struct kvm *kvm, void *dev, u32 features);
-	int (*get_vq_count)(struct kvm *kvm, void *dev);
+	unsigned int (*get_vq_count)(struct kvm *kvm, void *dev);
 	int (*init_vq)(struct kvm *kvm, void *dev, u32 vq, u32 page_size,
 		       u32 align, u32 pfn);
 	void (*exit_vq)(struct kvm *kvm, void *dev, u32 vq);
diff --git a/virtio/9p.c b/virtio/9p.c
index 6074f3a..7374f1e 100644
--- a/virtio/9p.c
+++ b/virtio/9p.c
@@ -1469,7 +1469,7 @@  static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
-static int get_vq_count(struct kvm *kvm, void *dev)
+static unsigned int get_vq_count(struct kvm *kvm, void *dev)
 {
 	return NUM_VIRT_QUEUES;
 }
diff --git a/virtio/balloon.c b/virtio/balloon.c
index 5bcd6ab..450b36a 100644
--- a/virtio/balloon.c
+++ b/virtio/balloon.c
@@ -251,7 +251,7 @@  static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
-static int get_vq_count(struct kvm *kvm, void *dev)
+static unsigned int get_vq_count(struct kvm *kvm, void *dev)
 {
 	return NUM_VIRT_QUEUES;
 }
diff --git a/virtio/blk.c b/virtio/blk.c
index af71c0c..46ee028 100644
--- a/virtio/blk.c
+++ b/virtio/blk.c
@@ -291,7 +291,7 @@  static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
-static int get_vq_count(struct kvm *kvm, void *dev)
+static unsigned int get_vq_count(struct kvm *kvm, void *dev)
 {
 	return NUM_VIRT_QUEUES;
 }
diff --git a/virtio/console.c b/virtio/console.c
index dae6034..8315808 100644
--- a/virtio/console.c
+++ b/virtio/console.c
@@ -216,7 +216,7 @@  static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
-static int get_vq_count(struct kvm *kvm, void *dev)
+static unsigned int get_vq_count(struct kvm *kvm, void *dev)
 {
 	return VIRTIO_CONSOLE_NUM_QUEUES;
 }
diff --git a/virtio/mmio.c b/virtio/mmio.c
index 0094856..d3555b4 100644
--- a/virtio/mmio.c
+++ b/virtio/mmio.c
@@ -175,13 +175,22 @@  static void virtio_mmio_config_out(struct kvm_cpu *vcpu,
 {
 	struct virtio_mmio *vmmio = vdev->virtio;
 	struct kvm *kvm = vmmio->kvm;
+	unsigned int vq_count = vdev->ops->get_vq_count(kvm, vmmio->dev);
 	u32 val = 0;
 
 	switch (addr) {
 	case VIRTIO_MMIO_HOST_FEATURES_SEL:
 	case VIRTIO_MMIO_GUEST_FEATURES_SEL:
+		val = ioport__read32(data);
+		*(u32 *)(((void *)&vmmio->hdr) + addr) = val;
+		break;
 	case VIRTIO_MMIO_QUEUE_SEL:
 		val = ioport__read32(data);
+		if (val >= vq_count) {
+			WARN_ONCE(1, "QUEUE_SEL value (%u) is larger than VQ count (%u)\n",
+				val, vq_count);
+			break;
+		}
 		*(u32 *)(((void *)&vmmio->hdr) + addr) = val;
 		break;
 	case VIRTIO_MMIO_STATUS:
@@ -227,6 +236,11 @@  static void virtio_mmio_config_out(struct kvm_cpu *vcpu,
 		break;
 	case VIRTIO_MMIO_QUEUE_NOTIFY:
 		val = ioport__read32(data);
+		if (val >= vq_count) {
+			WARN_ONCE(1, "QUEUE_NOTIFY value (%u) is larger than VQ count (%u)\n",
+				val, vq_count);
+			break;
+		}
 		vdev->ops->notify_vq(vmmio->kvm, vmmio->dev, val);
 		break;
 	case VIRTIO_MMIO_INTERRUPT_ACK:
@@ -346,10 +360,12 @@  int virtio_mmio_init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
 
 int virtio_mmio_reset(struct kvm *kvm, struct virtio_device *vdev)
 {
-	int vq;
+	unsigned int vq;
 	struct virtio_mmio *vmmio = vdev->virtio;
+	unsigned int vq_count;
 
-	for (vq = 0; vq < vdev->ops->get_vq_count(kvm, vmmio->dev); vq++)
+	vq_count = vdev->ops->get_vq_count(kvm, vmmio->dev);
+	for (vq = 0; vq < vq_count; vq++)
 		virtio_mmio_exit_vq(kvm, vdev, vq);
 
 	return 0;
diff --git a/virtio/net.c b/virtio/net.c
index ec5dc1f..8dd523f 100644
--- a/virtio/net.c
+++ b/virtio/net.c
@@ -755,11 +755,11 @@  static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
-static int get_vq_count(struct kvm *kvm, void *dev)
+static unsigned int get_vq_count(struct kvm *kvm, void *dev)
 {
 	struct net_dev *ndev = dev;
 
-	return ndev->queue_pairs * 2 + 1;
+	return ndev->queue_pairs * 2U + 1U;
 }
 
 static struct virtio_ops net_dev_virtio_ops = {
diff --git a/virtio/pci.c b/virtio/pci.c
index 0b5cccd..9a6cbf3 100644
--- a/virtio/pci.c
+++ b/virtio/pci.c
@@ -329,9 +329,11 @@  static bool virtio_pci__data_out(struct kvm_cpu *vcpu, struct virtio_device *vde
 	struct virtio_pci *vpci;
 	struct kvm *kvm;
 	u32 val;
+	unsigned int vq_count;
 
 	kvm = vcpu->kvm;
 	vpci = vdev->virtio;
+	vq_count = vdev->ops->get_vq_count(kvm, vpci->dev);
 
 	switch (offset) {
 	case VIRTIO_PCI_GUEST_FEATURES:
@@ -351,10 +353,21 @@  static bool virtio_pci__data_out(struct kvm_cpu *vcpu, struct virtio_device *vde
 		}
 		break;
 	case VIRTIO_PCI_QUEUE_SEL:
-		vpci->queue_selector = ioport__read16(data);
+		val = ioport__read16(data);
+		if (val >= vq_count) {
+			WARN_ONCE(1, "QUEUE_SEL value (%u) is larger than VQ count (%u)\n",
+				val, vq_count);
+			return false;
+		}
+		vpci->queue_selector = val;
 		break;
 	case VIRTIO_PCI_QUEUE_NOTIFY:
 		val = ioport__read16(data);
+		if (val >= vq_count) {
+			WARN_ONCE(1, "QUEUE_SEL value (%u) is larger than VQ count (%u)\n",
+				val, vq_count);
+			return false;
+		}
 		vdev->ops->notify_vq(kvm, vpci->dev, val);
 		break;
 	case VIRTIO_PCI_STATUS:
@@ -647,10 +660,12 @@  int virtio_pci__init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
 
 int virtio_pci__reset(struct kvm *kvm, struct virtio_device *vdev)
 {
-	int vq;
+	unsigned int vq;
+	unsigned int vq_count;
 	struct virtio_pci *vpci = vdev->virtio;
 
-	for (vq = 0; vq < vdev->ops->get_vq_count(kvm, vpci->dev); vq++)
+	vq_count = vdev->ops->get_vq_count(kvm, vpci->dev);
+	for (vq = 0; vq < vq_count; vq++)
 		virtio_pci_exit_vq(kvm, vdev, vq);
 
 	return 0;
diff --git a/virtio/rng.c b/virtio/rng.c
index c7835a0..75b682e 100644
--- a/virtio/rng.c
+++ b/virtio/rng.c
@@ -147,7 +147,7 @@  static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
-static int get_vq_count(struct kvm *kvm, void *dev)
+static unsigned int get_vq_count(struct kvm *kvm, void *dev)
 {
 	return NUM_VIRT_QUEUES;
 }
diff --git a/virtio/scsi.c b/virtio/scsi.c
index 8f1c348..60432cc 100644
--- a/virtio/scsi.c
+++ b/virtio/scsi.c
@@ -176,7 +176,7 @@  static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
-static int get_vq_count(struct kvm *kvm, void *dev)
+static unsigned int get_vq_count(struct kvm *kvm, void *dev)
 {
 	return NUM_VIRT_QUEUES;
 }
diff --git a/virtio/vsock.c b/virtio/vsock.c
index 34397b6..64b4e95 100644
--- a/virtio/vsock.c
+++ b/virtio/vsock.c
@@ -204,7 +204,7 @@  static void notify_vq_gsi(struct kvm *kvm, void *dev, u32 vq, u32 gsi)
 		die_perror("VHOST_SET_VRING_CALL failed");
 }
 
-static int get_vq_count(struct kvm *kvm, void *dev)
+static unsigned int get_vq_count(struct kvm *kvm, void *dev)
 {
 	return VSOCK_VQ_MAX;
 }