[Qemu-devel] Re: [PATCH] virtio-serial: don't crash on invalid input

[Michael S. Tsirkin]

On Wed, Mar 23, 2011 at 09:39:46PM +0530, Amit Shah wrote:
> On (Wed) 23 Mar 2011 [11:56:57], Michael S. Tsirkin wrote:
> > On Tue, Mar 22, 2011 at 10:25:06PM +0530, Amit Shah wrote:
> > > On (Tue) 22 Mar 2011 [18:32:50], Michael S. Tsirkin wrote:
> > > > Fix crash on invalid input in virtio-serial.
> > > > Discovered by code review, untested.
> 
> > > > @@ -654,6 +654,9 @@ static int virtio_serial_load(QEMUFile *f, void 
> > > > *opaque, int version_id)
> > > >  
> > > >          id = qemu_get_be32(f);
> > > >          port = find_port_by_id(s, id);
> > > > +        if (!port) {
> > > > +            return -EINVAL;
> > > > +        }
> > > 
> > > Just before this, we matched the ports_map which would bail out if the
> > > corresponding port isn't avl. in the destination, so this check is
> > > made redundant.
> > 
> > You are trusting the remote here, this is a security problem.
> > A malicious remote will always be able to create arbitrary guest state,
> > but it should not be able to corrupt the host.
> 
> I'm still unsure if we'll be able to achieve much if our primary
> defence goes down:  we currently primarily rely on libvirt and selinux
> to ensure we're in a sane state and any incoming migration is from a
> properly-initialised qemu instance.
> 
> If we're receiving data from an untrusted qemu instance or some random
> sender, we're doomed anyway.
> 
>               Amit

I think we need defence in depth: qemu must validate all input
to the point where remote can not crash qemu/host,
selinux must restrict what qemu can do if that fails.


-- 
MST