Re: [Qemu-devel] [PATCH] Allow mismatched virtio config-len

[Dr. David Alan Gilbert]

* Michael S. Tsirkin (address@hidden) wrote:
> On Fri, Jun 27, 2014 at 03:42:10PM +0100, Dr. David Alan Gilbert wrote:
> > * Michael S. Tsirkin (address@hidden) wrote:
> > > On Fri, Jun 27, 2014 at 09:34:38AM +0100, Dr. David Alan Gilbert (git) 
> > > wrote:
> > > > From: "Dr. David Alan Gilbert" <address@hidden>
> > > > 
> > > > Commit 'virtio: validate config_len on load' restricted config_len
> > > > loaded from the wire to match the config_len that the device had.
> > > > 
> > > > Unfortunately, there are cases where this isn't true, the one
> > > > we found it on was the wqe addition in virtio-blk.
> > > 
> > > I think you mean wce.
> > 
> > Oops - yes.
> > 
> > > > Allow mismatched config-lengths:
> > > >    *) If the version on the wire is shorter then ensure that the
> > > >       remainder is 0xff filled (as virtio_config_read does on
> > > >       out of range reads)
> > > >    *) If the version on the wire is longer, load what we have space
> > > >       for and skip the rest.
> > > > 
> > > > Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> > > 
> > > Looks good overall, but I am having thoughts about the
> > > padding with 0xff.
> > > We previously didn't do this (before virtio: validate config_len on
> > > load) so it seems safest (at least for 2.1) not to do it now either.
> > 
> > Who allocates that memory? If it's known to be a value then I agree; however
> > if it's uninitialised then I think it's best to pick a value rather than
> > have behaviour that depends on random junk in the memory.
> 
> It's initialized: e.g. for net it includes the mac,
> for block the wce value.

OK, I've just posted a v2 that is the simplified code you suggested.
(Not had quite as much testing as my previous version yet).

Dave

--
Dr. David Alan Gilbert / address@hidden / Manchester, UK