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 <alx...@bu.edu> > Signed-off-by: Li Qiang <liq...@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 <pbonz...@redhat.com>
