qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: pbonzini@redhat.com, qemu-devel@nongnu.org, quintela@redhat.com
Subject: Re: [Qemu-devel] [PATCH] Allow mismatched virtio config-len
Date: Fri, 27 Jun 2014 20:04:29 +0100	[thread overview]
Message-ID: <20140627190429.GI2408@work-vm> (raw)
In-Reply-To: <20140627150447.GA4617@redhat.com>

* Michael S. Tsirkin (mst@redhat.com) wrote:
> On Fri, Jun 27, 2014 at 03:42:10PM +0100, Dr. David Alan Gilbert wrote:
> > * Michael S. Tsirkin (mst@redhat.com) wrote:
> > > On Fri, Jun 27, 2014 at 09:34:38AM +0100, Dr. David Alan Gilbert (git) wrote:
> > > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > > > 
> > > > 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 <dgilbert@redhat.com>
> > > 
> > > 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 / dgilbert@redhat.com / Manchester, UK

      reply	other threads:[~2014-06-27 19:04 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-27  8:34 [Qemu-devel] [PATCH] Allow mismatched virtio config-len Dr. David Alan Gilbert (git)
2014-06-27 11:29 ` Paolo Bonzini
2014-06-27 14:32 ` Michael S. Tsirkin
2014-06-27 14:42   ` Dr. David Alan Gilbert
2014-06-27 15:04     ` Michael S. Tsirkin
2014-06-27 19:04       ` Dr. David Alan Gilbert [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140627190429.GI2408@work-vm \
    --to=dgilbert@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).