All of lore.kernel.org
 help / color / mirror / Atom feed
From: Martin Radev <martin.b.radev@gmail.com>
To: kvm@vger.kernel.org, will@kernel.org,
	julien.thierry.kdev@gmail.com, andre.przywara@arm.com,
	alexandru.elisei@arm.com
Cc: Martin Radev <martin.b.radev@gmail.com>
Subject: [PATCH kvmtool 3/5] virtio: Check for overflows in QUEUE_NOTIFY and QUEUE_SEL
Date: Fri,  4 Mar 2022 01:10:48 +0200	[thread overview]
Message-ID: <20220303231050.2146621-4-martin.b.radev@gmail.com> (raw)
In-Reply-To: <20220303231050.2146621-1-martin.b.radev@gmail.com>

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++)
 		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;
 }
-- 
2.25.1


  parent reply	other threads:[~2022-03-03 23:11 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-03 23:10 [PATCH v2 kvmtool 0/5] Fix few small issues in virtio code Martin Radev
2022-03-03 23:10 ` [PATCH kvmtool 1/5] kvmtool: Add WARN_ONCE macro Martin Radev
2022-03-03 23:10 ` [PATCH kvmtool 2/5] virtio: Sanitize config accesses Martin Radev
2022-03-16 13:04   ` Alexandru Elisei
2022-03-27 20:37     ` Martin Radev
2022-04-22 10:12       ` Alexandru Elisei
2022-03-03 23:10 ` Martin Radev [this message]
2022-03-16 15:38   ` [PATCH kvmtool 3/5] virtio: Check for overflows in QUEUE_NOTIFY and QUEUE_SEL Alexandru Elisei
2022-03-27 20:45     ` Martin Radev
2022-04-22 10:35       ` Alexandru Elisei
2022-03-03 23:10 ` [PATCH kvmtool 4/5] Makefile: Mark stack as not executable Martin Radev
2022-03-03 23:10 ` [PATCH kvmtool 5/5] mmio: Sanitize addr and len Martin Radev
2022-03-16 15:39   ` Alexandru Elisei
2022-03-27 21:00     ` Martin Radev
2022-04-22 10:36       ` Alexandru Elisei
2022-03-10 14:56 ` [PATCH v2 kvmtool 0/5] Fix few small issues in virtio code Alexandru Elisei
2022-03-11 11:23   ` Andre Przywara
2022-03-14 17:11     ` Alexandru Elisei
2022-03-27 12:46       ` Martin Radev
2022-04-22 10:37         ` Alexandru Elisei
2022-05-06 13:20 ` Will Deacon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220303231050.2146621-4-martin.b.radev@gmail.com \
    --to=martin.b.radev@gmail.com \
    --cc=alexandru.elisei@arm.com \
    --cc=andre.przywara@arm.com \
    --cc=julien.thierry.kdev@gmail.com \
    --cc=kvm@vger.kernel.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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.