diff mbox series

[v3,03/12] virtio_ring: Maintain a shadow copy of descriptors

Message ID 20220516104140.1047229-4-ascull@google.com
State Accepted
Commit 10a14536366350fdd2d14af1981d9e3d8cb3c524
Delegated to: Tom Rini
Headers show
Series virtio: Harden and test vring | expand

Commit Message

Andrew Scull May 16, 2022, 10:41 a.m. UTC
The shared descriptors should only be written by the guest driver,
however, the device is still able to overwrite and corrupt them.
Maintain a private shadow copy of the descriptors for the driver to
use for state tracking, removing the need to read from the shared
descriptors.

Signed-off-by: Andrew Scull <ascull@google.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 drivers/virtio/virtio_ring.c | 49 ++++++++++++++++++++++++------------
 include/virtio_ring.h        | 10 ++++++++
 2 files changed, 43 insertions(+), 16 deletions(-)
diff mbox series

Patch

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index d3fc842f30..73671d79da 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -19,13 +19,21 @@ 
 static unsigned int virtqueue_attach_desc(struct virtqueue *vq, unsigned int i,
 					  struct virtio_sg *sg, u16 flags)
 {
+	struct vring_desc_shadow *desc_shadow = &vq->vring_desc_shadow[i];
 	struct vring_desc *desc = &vq->vring.desc[i];
 
-	desc->addr = cpu_to_virtio64(vq->vdev, (u64)(uintptr_t)sg->addr);
-	desc->len = cpu_to_virtio32(vq->vdev, sg->length);
-	desc->flags = cpu_to_virtio16(vq->vdev, flags);
+	/* Update the shadow descriptor. */
+	desc_shadow->addr = (u64)(uintptr_t)sg->addr;
+	desc_shadow->len = sg->length;
+	desc_shadow->flags = flags;
 
-	return virtio16_to_cpu(vq->vdev, desc->next);
+	/* Update the shared descriptor to match the shadow. */
+	desc->addr = cpu_to_virtio64(vq->vdev, desc_shadow->addr);
+	desc->len = cpu_to_virtio32(vq->vdev, desc_shadow->len);
+	desc->flags = cpu_to_virtio16(vq->vdev, desc_shadow->flags);
+	desc->next = cpu_to_virtio16(vq->vdev, desc_shadow->next);
+
+	return desc_shadow->next;
 }
 
 int virtqueue_add(struct virtqueue *vq, struct virtio_sg *sgs[],
@@ -65,7 +73,8 @@  int virtqueue_add(struct virtqueue *vq, struct virtio_sg *sgs[],
 		i = virtqueue_attach_desc(vq, i, sgs[n], flags);
 	}
 	/* Last one doesn't continue */
-	desc[prev].flags &= cpu_to_virtio16(vq->vdev, ~VRING_DESC_F_NEXT);
+	vq->vring_desc_shadow[prev].flags &= ~VRING_DESC_F_NEXT;
+	desc[prev].flags = cpu_to_virtio16(vq->vdev, vq->vring_desc_shadow[prev].flags);
 
 	/* We're using some buffers from the free list. */
 	vq->num_free -= descs_used;
@@ -134,17 +143,16 @@  void virtqueue_kick(struct virtqueue *vq)
 static void detach_buf(struct virtqueue *vq, unsigned int head)
 {
 	unsigned int i;
-	__virtio16 nextflag = cpu_to_virtio16(vq->vdev, VRING_DESC_F_NEXT);
 
 	/* Put back on free list: unmap first-level descriptors and find end */
 	i = head;
 
-	while (vq->vring.desc[i].flags & nextflag) {
-		i = virtio16_to_cpu(vq->vdev, vq->vring.desc[i].next);
+	while (vq->vring_desc_shadow[i].flags & VRING_DESC_F_NEXT) {
+		i = vq->vring_desc_shadow[i].next;
 		vq->num_free++;
 	}
 
-	vq->vring.desc[i].next = cpu_to_virtio16(vq->vdev, vq->free_head);
+	vq->vring_desc_shadow[i].next = vq->free_head;
 	vq->free_head = head;
 
 	/* Plus final descriptor */
@@ -197,8 +205,7 @@  void *virtqueue_get_buf(struct virtqueue *vq, unsigned int *len)
 		virtio_store_mb(&vring_used_event(&vq->vring),
 				cpu_to_virtio16(vq->vdev, vq->last_used_idx));
 
-	return (void *)(uintptr_t)virtio64_to_cpu(vq->vdev,
-						  vq->vring.desc[i].addr);
+	return (void *)(uintptr_t)vq->vring_desc_shadow[i].addr;
 }
 
 static struct virtqueue *__vring_new_virtqueue(unsigned int index,
@@ -207,6 +214,7 @@  static struct virtqueue *__vring_new_virtqueue(unsigned int index,
 {
 	unsigned int i;
 	struct virtqueue *vq;
+	struct vring_desc_shadow *vring_desc_shadow;
 	struct virtio_dev_priv *uc_priv = dev_get_uclass_priv(udev);
 	struct udevice *vdev = uc_priv->vdev;
 
@@ -214,10 +222,17 @@  static struct virtqueue *__vring_new_virtqueue(unsigned int index,
 	if (!vq)
 		return NULL;
 
+	vring_desc_shadow = calloc(vring.num, sizeof(struct vring_desc_shadow));
+	if (!vring_desc_shadow) {
+		free(vq);
+		return NULL;
+	}
+
 	vq->vdev = vdev;
 	vq->index = index;
 	vq->num_free = vring.num;
 	vq->vring = vring;
+	vq->vring_desc_shadow = vring_desc_shadow;
 	vq->last_used_idx = 0;
 	vq->avail_flags_shadow = 0;
 	vq->avail_idx_shadow = 0;
@@ -235,7 +250,7 @@  static struct virtqueue *__vring_new_virtqueue(unsigned int index,
 	/* Put everything in free lists */
 	vq->free_head = 0;
 	for (i = 0; i < vring.num - 1; i++)
-		vq->vring.desc[i].next = cpu_to_virtio16(vdev, i + 1);
+		vq->vring_desc_shadow[i].next = i + 1;
 
 	return vq;
 }
@@ -288,6 +303,7 @@  struct virtqueue *vring_create_virtqueue(unsigned int index, unsigned int num,
 void vring_del_virtqueue(struct virtqueue *vq)
 {
 	free(vq->vring.desc);
+	free(vq->vring_desc_shadow);
 	list_del(&vq->list);
 	free(vq);
 }
@@ -333,11 +349,12 @@  void virtqueue_dump(struct virtqueue *vq)
 	printf("\tlast_used_idx %u, avail_flags_shadow %u, avail_idx_shadow %u\n",
 	       vq->last_used_idx, vq->avail_flags_shadow, vq->avail_idx_shadow);
 
-	printf("Descriptor dump:\n");
+	printf("Shadow descriptor dump:\n");
 	for (i = 0; i < vq->vring.num; i++) {
-		printf("\tdesc[%u] = { 0x%llx, len %u, flags %u, next %u }\n",
-		       i, vq->vring.desc[i].addr, vq->vring.desc[i].len,
-		       vq->vring.desc[i].flags, vq->vring.desc[i].next);
+		struct vring_desc_shadow *desc = &vq->vring_desc_shadow[i];
+
+		printf("\tdesc_shadow[%u] = { 0x%llx, len %u, flags %u, next %u }\n",
+		       i, desc->addr, desc->len, desc->flags, desc->next);
 	}
 
 	printf("Avail ring dump:\n");
diff --git a/include/virtio_ring.h b/include/virtio_ring.h
index 6fc0593b14..52cbe77c0a 100644
--- a/include/virtio_ring.h
+++ b/include/virtio_ring.h
@@ -55,6 +55,14 @@  struct vring_desc {
 	__virtio16 next;
 };
 
+/* Shadow of struct vring_desc in guest byte order. */
+struct vring_desc_shadow {
+	u64 addr;
+	u32 len;
+	u16 flags;
+	u16 next;
+};
+
 struct vring_avail {
 	__virtio16 flags;
 	__virtio16 idx;
@@ -89,6 +97,7 @@  struct vring {
  * @index: the zero-based ordinal number for this queue
  * @num_free: number of elements we expect to be able to fit
  * @vring: actual memory layout for this queue
+ * @vring_desc_shadow: guest-only copy of descriptors
  * @event: host publishes avail event idx
  * @free_head: head of free buffer list
  * @num_added: number we've added since last sync
@@ -102,6 +111,7 @@  struct virtqueue {
 	unsigned int index;
 	unsigned int num_free;
 	struct vring vring;
+	struct vring_desc_shadow *vring_desc_shadow;
 	bool event;
 	unsigned int free_head;
 	unsigned int num_added;