hw/virtio/virtio.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
QEMU must be careful when loading device state off migration streams to
prevent a malicious source from exploiting the emulator. Overdoing these
checks has the side effect of allowing a guest to "pin itself" in cloud
environments by messing with state which is entirely in its control.
Similarly to what f3081539 achieved in usb_device_post_load(), this
commit removes such a check from virtio_load(). Worth noting, the result
of a load without this check is the same as if a guest enables a VQ with
invalid indexes to begin with. That is, the virtual device is set in a
broken state (by the datapath handler) and must be reset.
Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
---
v2: Keep the check around for logging.
hw/virtio/virtio.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 6f8f865aff..ceb58fda6c 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -17,6 +17,7 @@
#include "trace.h"
#include "exec/address-spaces.h"
#include "qemu/error-report.h"
+#include "qemu/log.h"
#include "qemu/main-loop.h"
#include "qemu/module.h"
#include "hw/virtio/virtio.h"
@@ -3160,12 +3161,12 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
nheads = vring_avail_idx(&vdev->vq[i]) - vdev->vq[i].last_avail_idx;
/* Check it isn't doing strange things with descriptor numbers. */
if (nheads > vdev->vq[i].vring.num) {
- error_report("VQ %d size 0x%x Guest index 0x%x "
- "inconsistent with Host index 0x%x: delta 0x%x",
- i, vdev->vq[i].vring.num,
- vring_avail_idx(&vdev->vq[i]),
- vdev->vq[i].last_avail_idx, nheads);
- return -1;
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "VQ %d size 0x%x Guest index 0x%x "
+ "inconsistent with Host index 0x%x: delta 0x%x",
+ i, vdev->vq[i].vring.num,
+ vring_avail_idx(&vdev->vq[i]),
+ vdev->vq[i].last_avail_idx, nheads);
}
vdev->vq[i].used_idx = vring_used_idx(&vdev->vq[i]);
vdev->vq[i].shadow_avail_idx = vring_avail_idx(&vdev->vq[i]);
--
2.18.4
On 2020/10/28 下午9:47, Felipe Franciosi wrote: > QEMU must be careful when loading device state off migration streams to > prevent a malicious source from exploiting the emulator. Overdoing these > checks has the side effect of allowing a guest to "pin itself" in cloud > environments by messing with state which is entirely in its control. > > Similarly to what f3081539 achieved in usb_device_post_load(), this > commit removes such a check from virtio_load(). Worth noting, the result > of a load without this check is the same as if a guest enables a VQ with > invalid indexes to begin with. That is, the virtual device is set in a > broken state (by the datapath handler) and must be reset. > > Signed-off-by: Felipe Franciosi <felipe@nutanix.com> > --- > v2: Keep the check around for logging. > > hw/virtio/virtio.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 6f8f865aff..ceb58fda6c 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -17,6 +17,7 @@ > #include "trace.h" > #include "exec/address-spaces.h" > #include "qemu/error-report.h" > +#include "qemu/log.h" > #include "qemu/main-loop.h" > #include "qemu/module.h" > #include "hw/virtio/virtio.h" > @@ -3160,12 +3161,12 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) > nheads = vring_avail_idx(&vdev->vq[i]) - vdev->vq[i].last_avail_idx; > /* Check it isn't doing strange things with descriptor numbers. */ > if (nheads > vdev->vq[i].vring.num) { > - error_report("VQ %d size 0x%x Guest index 0x%x " > - "inconsistent with Host index 0x%x: delta 0x%x", > - i, vdev->vq[i].vring.num, > - vring_avail_idx(&vdev->vq[i]), > - vdev->vq[i].last_avail_idx, nheads); > - return -1; > + qemu_log_mask(LOG_GUEST_ERROR, > + "VQ %d size 0x%x Guest index 0x%x " > + "inconsistent with Host index 0x%x: delta 0x%x", > + i, vdev->vq[i].vring.num, > + vring_avail_idx(&vdev->vq[i]), > + vdev->vq[i].last_avail_idx, nheads); > } > vdev->vq[i].used_idx = vring_used_idx(&vdev->vq[i]); > vdev->vq[i].shadow_avail_idx = vring_avail_idx(&vdev->vq[i]); Acked-by: Jason Wang <jasowang@redhat.com>
© 2016 - 2024 Red Hat, Inc.