[v4] virtio,pc,acpi: fixes, tests

[PULL v4 20/48] virtio: update MemoryRegionCaches when guest set bad features

Michael S. Tsirkin posted 48 patches 5 years ago

Diff against v1 v2 v3
Download mbox

Maintainers: Christian Borntraeger <borntraeger@de.ibm.com>, Paolo Bonzini <pbonzini@redhat.com>, David Hildenbrand <david@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Raphael Norwitz <raphael.norwitz@nutanix.com>, Thomas Huth <thuth@redhat.com>, Halil Pasic <pasic@linux.ibm.com>, Eric Blake <eblake@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>, Eduardo Habkost <ehabkost@redhat.com>, Laurent Vivier <lvivier@redhat.com>, Cornelia Huck <cohuck@redhat.com>, Richard Henderson <rth@twiddle.net>, Kevin Wolf <kwolf@redhat.com>, Aurelien Jarno <aurelien@aurel32.net>, Eric Auger <eric.auger@redhat.com>, Markus Armbruster <armbru@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Max Reitz <mreitz@redhat.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>

Expand all Fold all

[PULL v4 20/48] virtio: update MemoryRegionCaches when guest set bad features

Posted by Michael S. Tsirkin 5 years ago

From: Li Qiang <liq3ea@163.com>

Current the 'virtio_set_features' only update the 'MemorRegionCaches'
when the 'virtio_set_features_nocheck' return '0' which means it is
not bad features. However the guest can still trigger the access of the
used vring after set bad features. In this situation it will cause assert
failure in 'ADDRESS_SPACE_ST_CACHED'.

Buglink: https://bugs.launchpad.net/qemu/+bug/1890333
Fixes: db812c4073c7 ("virtio: update MemoryRegionCaches when guest negotiates features")
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Signed-off-by: Li Qiang <liq3ea@163.com>
Message-Id: <20200919082706.6703-1-liq3ea@163.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/virtio.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index a2edb4f386..6f8f865aff 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2963,17 +2963,16 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val)
         return -EINVAL;
     }
     ret = virtio_set_features_nocheck(vdev, val);
-    if (!ret) {
-        if (virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
-            /* VIRTIO_RING_F_EVENT_IDX changes the size of the caches.  */
-            int i;
-            for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
-                if (vdev->vq[i].vring.num != 0) {
-                    virtio_init_region_cache(vdev, i);
-                }
+    if (virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
+        /* VIRTIO_RING_F_EVENT_IDX changes the size of the caches.  */
+        int i;
+        for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
+            if (vdev->vq[i].vring.num != 0) {
+                virtio_init_region_cache(vdev, i);
             }
         }
-
+    }
+    if (!ret) {
         if (!virtio_device_started(vdev, vdev->status) &&
             !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
             vdev->start_on_kick = true;
-- 
MST