On Wed, May 29, 2013 at 10:00:33AM +0100, Peter Maydell wrote:
> On 29 May 2013 09:24, Michael S. Tsirkin <mst@xxxxxxxxxx> wrote:
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > index f4db224..fd09ea7 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -467,51 +467,70 @@ static uint64_t virtio_pci_config_common_read(void *opaque, hwaddr addr,
> > {
> > VirtIOPCIProxy *proxy = opaque;
> > VirtIODevice *vdev = proxy->vdev;
> > + struct virtio_pci_common_cfg cfg;
> >
> > uint64_t low = 0xffffffffull;
> >
> > switch (addr) {
> > case offsetof(struct virtio_pci_common_cfg, device_feature_select):
> > + assert(size == sizeof cfg.device_feature_select);
> > return proxy->device_feature_select;
>
> Asserting is definitely the wrong thing here, since the
> guest can trigger it.
So?
It's a driver bug. It can reset or crash guest with the same effect,
and it likely will if we let it continue.
assert makes sure we don't let it escalate into some
hard to debug security problem.
> If you really want to use offsetof like this you're
> going to need to decorate the structs with QEMU_PACKED.
>
> thanks
> -- PMM
Nope.
These structs are carefully designed not to have any padding.
And if there was a bug and there was some padding, we still
can't fix it with PACKED because this structure
is used to interact with the guest code which does not
have the packed attribute.
--
MST
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
