On Wed, 17 Aug 2022 08:13:59 +0200, Dmitry Vyukov [off-list ref] wrote:
On Mon, 15 Aug 2022 17:32:06 -0400, Michael wrote:
quoted
So if you pass the size parameter for a legacy device it will
try to make the ring smaller and that is not legal with
legacy at all. But the driver treats legacy and modern
the same, it allocates a smaller queue anyway.
Lo and behold, I pass disable-modern=on to qemu and it happily
corrupts memory exactly the same as GCP does.
Ouch!
I understand that the host does the actual corruption,
but could you think of any additional debug checking in the guest
that would caught this in future? Potentially only when KASAN
is enabled which can verify validity of memory ranges.
Some kind of additional layer of sanity checking.
This caused a bit of a havoc for syzbot with almost 100 unique
crash signatures, so would be useful to catch such issues more
reliably in future.
We can add a check to vring size before calling vp_legacy_set_queue_address().
Checking the memory range directly is a bit cumbersome.
Thanks.
diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c
index 2257f1b3d8ae..0673831f45b6 100644
--- a/drivers/virtio/virtio_pci_legacy.c
+++ b/drivers/virtio/virtio_pci_legacy.c
@@ -146,6 +146,8 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
goto out_del_vq;
}
+ BUG_ON(num != virtqueue_get_vring_size(vq));
+
/* activate the queue */
vp_legacy_set_queue_address(&vp_dev->ldev, index, q_pfn);
Thanks