From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43076) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X0UM7-0001XM-L1 for qemu-devel@nongnu.org; Fri, 27 Jun 2014 07:30:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X0ULv-0000GW-VI for qemu-devel@nongnu.org; Fri, 27 Jun 2014 07:30:11 -0400 Received: from mail-wg0-x22f.google.com ([2a00:1450:400c:c00::22f]:43304) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X0ULv-0000GS-P7 for qemu-devel@nongnu.org; Fri, 27 Jun 2014 07:29:59 -0400 Received: by mail-wg0-f47.google.com with SMTP id k14so4982351wgh.6 for ; Fri, 27 Jun 2014 04:29:59 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <53AD55B2.2070000@redhat.com> Date: Fri, 27 Jun 2014 13:29:54 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1403858078-3402-1-git-send-email-dgilbert@redhat.com> In-Reply-To: <1403858078-3402-1-git-send-email-dgilbert@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] Allow mismatched virtio config-len List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert (git)" , qemu-devel@nongnu.org Cc: mst@redhat.com, quintela@redhat.com Il 27/06/2014 10:34, Dr. David Alan Gilbert (git) ha scritto: > From: "Dr. David Alan Gilbert" > > 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. Indeed, the alternative here is to break migration. As a follow up, it would be nice to let the bus detect whether the config_len change is valid or not. For virtio-mmio and s390, mst said that config length must always match (luckily, these machines aren't versioned so they are not affected by the wce change). For virtio-pci, it is okay as long as the old_length + VIRTIO_PCI_REGION_SIZE(vdev) and new_length + VIRTIO_PCI_REGION_SIZE(vdev) do not cross a power of two. Paolo > 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 > --- > hw/virtio/virtio.c | 30 ++++++++++++++++++++++++++---- > 1 file changed, 26 insertions(+), 4 deletions(-) > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index a3082d5..2b11142 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -927,11 +927,33 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f) > } > config_len = qemu_get_be32(f); > if (config_len != vdev->config_len) { > - error_report("Unexpected config length 0x%x. Expected 0x%zx", > - config_len, vdev->config_len); > - return -1; > + /* > + * Unfortunately the reality is that there are cases where we > + * see mismatched config lengths, so we have to deal with them > + * rather than rejecting them. > + */ > + > + if (config_len < vdev->config_len) { > + /* This is normal in some devices when they add a new option */ > + memset(vdev->config, 0xff, vdev->config_len); > + qemu_get_buffer(f, vdev->config, config_len); > + } else { > + int32_t diff; > + /* config_len > vdev->config_len > + * This is rarer, but is here to allow us to fix the case above > + */ > + qemu_get_buffer(f, vdev->config, vdev->config_len); > + /* > + * Even though we expect the diff to be small, we can't use > + * qemu_file_skip because it's not safe for a large skip. > + */ > + for (diff = config_len - vdev->config_len; diff > 0; diff--) { > + qemu_get_byte(f); > + } > + } > + } else { > + qemu_get_buffer(f, vdev->config, vdev->config_len); > } > - qemu_get_buffer(f, vdev->config, vdev->config_len); > > num = qemu_get_be32(f); > >