virtio: verify features on load

migrating between hosts which have different features
might break silently, if the migration destination
does not support some features supported by source.

Prevent this from happening by comparing acked feature
bits with the mask supported by the device.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/hw/syborg_virtio.c b/hw/syborg_virtio.c
index 6cf5a15c2ce8abe226841b1b621ee2c8067995a4..a84206a11c40fd69b5e2527d4ba3fbad1dbef852 100644 (file)
--- a/hw/syborg_virtio.c
+++ b/hw/syborg_virtio.c
@@ -87,7 +87,7 @@ static uint32_t syborg_virtio_readl(void *opaque, target_phys_addr_t offset)
         break;
     case SYBORG_VIRTIO_HOST_FEATURES:
         ret = vdev->get_features(vdev);
-        ret |= (1 << VIRTIO_F_NOTIFY_ON_EMPTY);
+        ret |= vdev->binding->get_features(s);
         break;
     case SYBORG_VIRTIO_GUEST_FEATURES:
         ret = vdev->features;
@@ -242,8 +242,16 @@ static void syborg_virtio_update_irq(void *opaque, uint16_t vector)
     qemu_set_irq(proxy->irq, level != 0);
 }
 
+static unsigned syborg_virtio_get_features(void *opaque)
+{
+    unsigned ret = 0;
+    ret |= (1 << VIRTIO_F_NOTIFY_ON_EMPTY);
+    return ret;
+}
+
 static VirtIOBindings syborg_virtio_bindings = {
-    .notify = syborg_virtio_update_irq
+    .notify = syborg_virtio_update_irq,
+    .get_features = syborg_virtio_get_features,
 };
 
 static int syborg_virtio_init(SyborgVirtIOProxy *proxy, VirtIODevice *vdev)

diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index d222ce03fe1c4001ec1c4b636d60c62131bd3d29..450013091c522d8713a2d47e843837a6c3acead4 100644 (file)
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -236,9 +236,7 @@ static uint32_t virtio_ioport_read(VirtIOPCIProxy *proxy, uint32_t addr)
     switch (addr) {
     case VIRTIO_PCI_HOST_FEATURES:
         ret = vdev->get_features(vdev);
-        ret |= (1 << VIRTIO_F_NOTIFY_ON_EMPTY);
-        ret |= (1 << VIRTIO_RING_F_INDIRECT_DESC);
-        ret |= (1 << VIRTIO_F_BAD_FEATURE);
+        ret |= vdev->binding->get_features(proxy);
         break;
     case VIRTIO_PCI_GUEST_FEATURES:
         ret = vdev->features;
@@ -382,12 +380,22 @@ static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
     msix_write_config(pci_dev, address, val, len);
 }
 
+static unsigned virtio_pci_get_features(void *opaque)
+{
+    unsigned ret = 0;
+    ret |= (1 << VIRTIO_F_NOTIFY_ON_EMPTY);
+    ret |= (1 << VIRTIO_RING_F_INDIRECT_DESC);
+    ret |= (1 << VIRTIO_F_BAD_FEATURE);
+    return ret;
+}
+
 static const VirtIOBindings virtio_pci_bindings = {
     .notify = virtio_pci_notify,
     .save_config = virtio_pci_save_config,
     .load_config = virtio_pci_load_config,
     .save_queue = virtio_pci_save_queue,
     .load_queue = virtio_pci_load_queue,
+    .get_features = virtio_pci_get_features,
 };
 
 static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,

diff --git a/hw/virtio.c b/hw/virtio.c
index 1f92171f688545b38bbe549dc3658b6fe86bc49c..cecd0dc04258314ab77b85db58b252c681587d02 100644 (file)
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -651,6 +651,9 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
 int virtio_load(VirtIODevice *vdev, QEMUFile *f)
 {
     int num, i, ret;
+    uint32_t features;
+    uint32_t supported_features = vdev->get_features(vdev) |
+        vdev->binding->get_features(vdev->binding_opaque);
 
     if (vdev->binding->load_config) {
         ret = vdev->binding->load_config(vdev->binding_opaque, f);
@@ -661,7 +664,13 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
     qemu_get_8s(f, &vdev->status);
     qemu_get_8s(f, &vdev->isr);
     qemu_get_be16s(f, &vdev->queue_sel);
-    qemu_get_be32s(f, &vdev->features);
+    qemu_get_be32s(f, &features);
+    if (features & ~supported_features) {
+        fprintf(stderr, "Features 0x%x unsupported. Allowed features: 0x%x\n",
+                features, supported_features);
+        return -1;
+    }
+    vdev->features = features;
     vdev->config_len = qemu_get_be32(f);
     qemu_get_buffer(f, vdev->config, vdev->config_len);
 

diff --git a/hw/virtio.h b/hw/virtio.h
index 15ad910768f6e84c54d5ee10aeccfa7e65176ac1..35532a6f25ff847915f3495825d33ab93dccbc35 100644 (file)
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -31,6 +31,11 @@
 /* We've given up on this device. */
 #define VIRTIO_CONFIG_S_FAILED          0x80
 
+/* Some virtio feature bits (currently bits 28 through 31) are reserved for the
+ * transport being used (eg. virtio_ring), the rest are per-device feature bits. */
+#define VIRTIO_TRANSPORT_F_START        28
+#define VIRTIO_TRANSPORT_F_END          32
+
 /* We notify when the ring is completely used, even if the guest is suppressing
  * callbacks */
 #define VIRTIO_F_NOTIFY_ON_EMPTY        24
@@ -82,6 +87,7 @@ typedef struct {
     void (*save_queue)(void * opaque, int n, QEMUFile *f);
     int (*load_config)(void * opaque, QEMUFile *f);
     int (*load_queue)(void * opaque, int n, QEMUFile *f);
+    unsigned (*get_features)(void * opaque);
 } VirtIOBindings;
 
 #define VIRTIO_PCI_QUEUE_MAX 16