| Message ID | 20200919082706.6703-1-liq3ea@163.com |
|---|---|
| State | New |
| Headers | show |
| Series | virtio: update MemoryRegionCaches when guest set bad features | expand |
On 19/09/20 10:27, Li Qiang wrote: > 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> > --- > 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 e983025217..4441ae5ed4 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; > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index e983025217..4441ae5ed4 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;
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> --- hw/virtio/virtio.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)