qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 0/3] virtio: Eliminate "exit(1)" upon invalid re


From: Michael S. Tsirkin
Subject: Re: [Qemu-devel] [PATCH 0/3] virtio: Eliminate "exit(1)" upon invalid request in virtio-blk and virtio-scsi
Date: Wed, 23 Apr 2014 08:41:16 +0300

On Wed, Apr 23, 2014 at 11:22:51AM +0800, Fam Zheng wrote:
> On Tue, 04/22 22:12, Michael S. Tsirkin wrote:
> > 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.
> > 
> 
> IMO, a buggy guest doing wrong operations is not necessarily the end of world
> for QEMU, we shouldn't tear down the whole process because of invalid input.

OK so we'd have to find a way to handle errors under memory APIs,
in such a way that doesn't break all users.
Maybe returning some dummy value on read and on ignoring writes?
Another tricky issue would be device triggering infinite recursion by
doing io on itself.

> This series is a step towards getting rid of such code,

Sure, incremental patches are good. At this point I think it's
a good idea to clearly mark this as RFC - I don't think we should yet merge
this upstream until the solution is a bit more complete.

Changing virtio is the easy part though, so I'm not sure it's a good
idea to start there.

> if there are trivial
> ways to abort qemu from a guest bug,

There's no "if" here :)

> then they may be candidates to improve as
> well.

I'd say let's see some progress on the harder parts of the problem first.


> Regarding the malicious guest, protecting D.O.S. attack is also valuable, 
> isn't
> it?
> 
> Thanks,
> Fam

Guest denying itself a service? I'm not sure why it's valuable.

If we can have a guarantee (even if that's only for some specific
machine types) that illegal input won't ever cause QEMU
to abort, that could be valuable e.g. to make fuzzing easier.
Just not having it abort on a specific path only serves to make
guest driver development easier. That also has value but only for
things that are hard for drivers to get right.
E.g. what's the chance driver gets the header size wrong? Probably nil.

-- 
MST



reply via email to

[Prev in Thread] Current Thread [Next in Thread]