Re: [Qemu-devel] [PATCH 0/3] virtio: Eliminate "exit(1)" upon invalid request in virtio-blk and virtio-scsi

[Michael S. Tsirkin]

On Tue, Apr 22, 2014 at 04:55:14PM +0800, Fam Zheng wrote:
> Today, buggy or malicous guests that submit invalid requests can cause QEMU's
> exit with an error message, which is not friendly to neither user/admin nor
> guest. When passing through a virtio device to a nested vm, there is also an
> D.O.S. vulnerability.
> 
> This series adds "broken" flag to VirtIODevice and allows device emulation 
> code
> to set it if invalid data from guest is seen, and then decide what to do with
> the (invalid and/or further) requests, by checking the status of the flag.
> 
> Upon device reset, "broken" is cleared and the device comes back to normal
> again.
> 
> In the patch 2 and 3, virtio-blk and virtio-scsi will just set the broken 
> flag,
> and stop poping requests from virt queue. In other words, the guest will find
> the device inresponsive, the only way it can do is resetting the device.
> 
> Other virtio device types, as well as virtqueue core code, have more exit(1)'s
> to be converted, but could be done on top of this.
> 
> Thanks,
> Fam
> 


It still seems trivially easy for a buggy guest to cause qemu to
abort, e.g. by supplying an invalid physical address to write to.

OTOH it seems possible that killing the malicious guest early reduces our
security attack surface.

Given that, what is the value of this patchset?


> Fam Zheng (3):
>   virtio: Introduce VirtIODevice.broken
>   virtio-blk: Don't exit on invalid VQ data
>   virtio-scsi: Don't exit on bad request
> 
>  hw/block/virtio-blk.c      | 20 +++++++++++++++-----
>  hw/scsi/virtio-scsi.c      | 40 +++++++++++++++++++++++++++++-----------
>  hw/virtio/virtio.c         | 12 ++++++++++++
>  include/hw/virtio/virtio.h |  3 +++
>  4 files changed, 59 insertions(+), 16 deletions(-)
> 
> -- 
> 1.9.2